Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid dumping files in working dir #105

Closed
jzaefferer opened this issue Jan 17, 2015 · 10 comments · Fixed by #117
Closed

Avoid dumping files in working dir #105

jzaefferer opened this issue Jan 17, 2015 · 10 comments · Fixed by #117

Comments

@jzaefferer
Copy link
Contributor

When running browserstack-runner, currently it seems to dump two files into the working directory, browserstack-run.pid and BrowserStackLocal. I don't think either should be there.

@JamesMGreene
Copy link

Seems like a good case to leverage require("os").tmpdir() to get the system's temp directory base path and then create a randomly named directory under that for each runner instance.

@jzaefferer
Copy link
Contributor Author

@dhimil is there any reason for those files to end up in the working directory? If not, I can try to create a PR that stores them in the OS tmpdir, as James suggests above.

@dhimil
Copy link
Contributor

dhimil commented Jan 20, 2015

Concerns with temp dir:

  • CI environments
  • tmp dirs are deleted on reboot. Will end up downloading huge binaries again.

@jzaefferer
Copy link
Contributor Author

CI environments

They should support tmp dirs as well. We can test that with Travis quickly enough.

tmp dirs are deleted on reboot. Will end up downloading huge binaries again.

That applies to Travis already. I don't see this as a problem for local development either.

As an alternative, can we put the files into the module's folder, e.g. node_modules/browserstack-runner/? Usually __dirname can be used to access that.

@dhimil
Copy link
Contributor

dhimil commented Jan 20, 2015

I guess we had __dirname but we switch to this because of permission issues for some users.

@jzaefferer
Copy link
Contributor Author

That would be this commit, right? c27a2ad

Which addressed #91. Over there I wrote:

Maybe this could be addressed by downloading the binary into a folder that the same process creates, which should give it permission to write the file?

If node_modules isn't writable, we can't create a directory in there either. Using tmp might work, but then it might download the binary too often.

I wonder if there's some way around the nobody: ownership so that we could go back to using __dirname.

@jzaefferer
Copy link
Contributor Author

I found this discussion: npm/npm#3849

In short, @rxaviers original problem in #91 might get addressed (locally, per user) by running npm -g config set user [user], where "[user]" is replaced with a local user with enough permissions.

@rxaviers I think the problem existed in 0.2.0 - could you install/run that again, after changing the npm config as above, to see if it helps? If so, we could revert the above commit, going back to using __dirname and documenting this.

@rxaviers
Copy link

@jzaefferer I had a new browserstack-runner 3.0 global installation and it worked alright without using your suggested config set user.

@jzaefferer
Copy link
Contributor Author

@rxaviers as discussed elsewhere, the point is to test a version that still stored BrowserStackTunnel inside the module's directory, not in the project root. 0.2.0 did that, so we can use it to test if setting npm config ... helps.

@rxaviers
Copy link

Tested using 0.2.0 and config set user and it worked.

jzaefferer added a commit to jzaefferer/browserstack-runner that referenced this issue Apr 24, 2015
Revert "On linux, EACCES open ... BrowserStackLocal error. Fixes browserstack#91"

This reverts commit c27a2ad

Documents how to avoid the issue described in browserstack#91 by configuring npm
properly.

Ref browserstack#91
Fixes browserstack#105
jzaefferer added a commit to jzaefferer/browserstack-runner that referenced this issue Jul 22, 2015
Revert "On linux, EACCES open ... BrowserStackLocal error. Fixes browserstack#91"

This reverts commit c27a2ad

Documents how to avoid the issue described in browserstack#91 by configuring npm
properly.

Ref browserstack#91
Fixes browserstack#105
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants