Improve check for sudo availability #330
Conversation
| @@ -572,7 +572,23 @@ include("show.jl") | |||
|
|
|||
| const has_sudo = Ref{Bool}(false) | |||
| function __init__() | |||
| has_sudo[] = try success(`sudo -V`) catch err false end | |||
| has_sudo[] = try | |||
| success(`command -v sudo`) | |||
There was a problem hiding this comment.
What system have this command and what problem does it solve?
There was a problem hiding this comment.
command is POSIX and it avoids calling sudo directly, since people have been complaining that BinDeps is making sudo calls and angering their sysadmins.
There was a problem hiding this comment.
it's not a command on any of the system I see. It also doesn't disable sudo.
There was a problem hiding this comment.
That's odd... It's available on Linux (tested in my Arch VM), macOS, and FreeBSD. It should avoid starting a sudo process, which is the goal.
There was a problem hiding this comment.
Did you actually run the julia function call?
Also, it'll still make sudo available. I highly doubt sudo -V is causing the warning.
There was a problem hiding this comment.
Did you actually run the julia function call?
Yes?
Also, it'll still make sudo available.
The goal of has_sudo is not checking whether the user has sudo privileges, it's to check whether sudo is installed on the system. command -v sudo does that without actually starting a sudo process, unlike sudo -V. Someone was saying that their system administrator didn't like that BinDeps was always invoking sudo on startup, so this avoids doing that.
There was a problem hiding this comment.
@staticfloat recommended success(`sh -c "command -v sudo"`). Can you confirm that works for you, @yuyichao?
There was a problem hiding this comment.
Did you actually run the julia function call?
Yes?
Well that's very strange. I can't find any system (3 arch, 1 debian) with command as a command.
julia> success(`command -v sudo`)
ERROR: could not spawn `command -v sudo`: no such file or directory (ENOENT)
Stacktrace:
[1] _jl_spawn(::String, ::Array{String,1}, ::Ptr{Void}, ::Base.Process, ::Base.DevNullStream, ::Base.DevNullStream, ::Base.DevNullStream) at ./process.jl:363
[2] (::getfield(Base, Symbol("##398#399")){Cmd})(::Base.DevNullStream, ::Base.DevNullStream, ::Base.DevNullStream) at ./process.jl:515
[3] setup_stdio at ./process.jl:502 [inlined]
[4] #spawn#397(::Nullable{Base.ProcessChain}, ::Function, ::Cmd, ::Tuple{Base.DevNullStream,Base.DevNullStream,Base.DevNullStream}) at ./process.jl:514
[5] (::getfield(Base, Symbol("#kw##spawn")))(::Array{Any,1}, ::typeof(spawn), ::Cmd, ::Tuple{Base.DevNullStream,Base.DevNullStream,Base.DevNullStream}) at ./<missing>:0
[6] #spawn#403(::Nullable{Base.ProcessChain}, ::Function, ::Cmd) at ./process.jl:558
[7] success(::Cmd) at ./process.jl:700Someone was saying that their system administrator didn't like that BinDeps was always invoking sudo on startup, so this avoids doing that.
In that case this should just be done lazily. And that's not what #325 is about so this should definitely not replace that since that is about "when installing packages".
success(`sh -c "command -v sudo"`)
Sure, though if someone asked if sudo exist and got an yes he would presumably want to run it, in which case it'll certainly cause whatever warning that was there before. I'm actually not sure how sudo can be configured to generate a warning on -V
There was a problem hiding this comment.
We check whether sudo exists when BinDeps is loaded since the check is in __init__. We do it so that we know that we can use package managers to install with sudo, not necessarily that we're going to use sudo. The sudo check happens no matter what you do with BinDeps, even if you just load it to use a single function, which is what I'm trying to avoid with this.
My understanding of #325 was that the goal is the same: avoid calling sudo at startup.
There was a problem hiding this comment.
even if you just load it to use a single function, which is what I'm trying to avoid with this.
Lazily initialize this can also avoid that.
My understanding of #325 was that the goal is the same: avoid calling sudo at startup.
The results (value of has_sudo) are totally different though.
It'd be nice if there was a way to check whether the current user has
sudoprivileges but I'm not sure how to do that reliably. Unrelated to #325.