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

Don't require sudo for installing package #156

Closed
GlenHertz opened this issue Aug 15, 2018 · 12 comments
Closed

Don't require sudo for installing package #156

GlenHertz opened this issue Aug 15, 2018 · 12 comments

Comments

@GlenHertz
Copy link

Hi,

I don't have sudo access and the installer wants to expand the maximum number of watched files for which it requires sudo. The installer fails and I have to run it again. Please provide a way to bypass sudo so that the installer will complete with success.

Glen

@timholy
Copy link
Owner

timholy commented Aug 16, 2018

I'm sorry about the inconvenience. This is a change I'm a bit uncertain about: on one hand it's annoying to have this happen, OTOH it's also bad that Revise mysteriously failed for some people. It's possible that recent design changes make this less of a problem, but really it's not uncommon to load ~500 Julia packages, and that's a significant fraction of 8192 which is the default on many systems.

Unfortunately, I'm unaware of a way to increase the maximum without sudo. I've already built in a way to disable the warning, see the isfile check in the deps/build.jl script. Perhaps I should advertise this in the message? I thought I had, but I don't see any evidence of that.

@GlenHertz
Copy link
Author

During install when the issue is printed out to the terminal, can you ask the user if they want to do it or not and allow them to bypass it without the error?

@timholy
Copy link
Owner

timholy commented Aug 16, 2018

No, unfortunately build.jl is non-interactive except for the ability to enter a password. Heck, just to print the explanation of why it was asking for your password, I had to circumvent our ordinary print methods and send it straight to the tty via a shell command.

I'd love it if it were more interactive: I'd like to ask users whether they want to add the automatic startup to their .julia/config/startup.jl as well, and one could conceivably even ask about some of the other settings. Then once configured, one could write a file so that the user wouldn't be bothered again. CC @KristofferC.

@timholy
Copy link
Owner

timholy commented Aug 16, 2018

I can turn this off. I was just annoyed by bug reports about it not working. And I did set up a mechanism so you can prevent yourself from being annoyed.

@tkf
Copy link
Contributor

tkf commented Aug 18, 2018

I had the same problem. It would be nice if I can set

ENV["REVISE_ASK_USER_WATCHES"] = "no"

to turn it off. Finding ~/.julia/packages/Revise/.../deps/ to create user_watches is a bit tedious with Pkg3.

@timholy
Copy link
Owner

timholy commented Aug 18, 2018

Sure, if that's easier. I've sometimes wondered if it would be better to have a config file, though, than setting all the ENV variables. Thoughts?

@GlenHertz
Copy link
Author

I don't think it is appropriate for a package to go ahead and change your OS settings with no option to opt out as it will change the settings for all applications and the number of watchers might be a low value to prevent apps from degrading the filesystem performance. I would like the installer to give the warning and explain the potential issue and print out the command to run to increase the max watchers.

I think adding env vars or config files is the wrong place for this as a user would likely not do that ahead of time. OS changes should be 'opt-in'.

Is it possible to trap the error when Revise is running and then output a better error message (with remedy)?

@tkf
Copy link
Contributor

tkf commented Aug 19, 2018

@timholy I totally agree that configuration via ENV is not great. It would be really nice if "package option" is supported by Pkg3 JuliaLang/Juleps#38

@tkf
Copy link
Contributor

tkf commented Aug 19, 2018

How about @warning in __init__ and running sudo in async_steal_repl_backend?

I think it makes sense to do the check at run-time since it looks like max_user_watches can be reset after reboot in some environments: https://askubuntu.com/questions/716431/inotify-max-user-watches-value-resets-on-reboot-how-to-change-it-permanently

@timholy
Copy link
Owner

timholy commented Aug 19, 2018

I don't think it is appropriate for a package to go ahead and change your OS settings with no option to opt out

You have two options, (1) (not advertised) to create that empty file or (2) enter a wrong password. But I get that it's not ideal now.

👍 to the idea of trapping the error, that seems like the best solution.

@GlenHertz
Copy link
Author

Great. This will even cover the case where the user is running on a different machine than the installer. Thanks Tim!

@timholy
Copy link
Owner

timholy commented Aug 19, 2018

I just hope I wrote the check for the error correctly! This is a very hard one to catch.

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

No branches or pull requests

3 participants