-
Notifications
You must be signed in to change notification settings - Fork 74
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
Add 'no confirmation' flag to providers, write provider info to STDERR #311
base: master
Are you sure you want to change the base?
Conversation
src/dependencies.jl
Outdated
@@ -425,8 +425,8 @@ function generate_steps(dep::LibraryDependency,h::AptGet,opts) | |||
end | |||
sudo = get(opts, :sudo, has_sudo[]) ? `sudo` : `` | |||
@build_steps begin | |||
println("Installing dependency $(h.package) via `$(sudoname(sudo))apt-get install $(h.package)`:") | |||
`$sudo apt-get install $(h.package)` | |||
info("Installing dependency $(h.package) via `$(sudoname(sudo))apt-get install $(h.package)`:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot to add the -y
here and for pacman.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, I guess it should say that in the informational message too
@@ -461,8 +461,8 @@ function generate_steps(dep::LibraryDependency,h::Zypper,opts) | |||
end | |||
sudo = get(opts, :sudo, has_sudo[]) ? `sudo` : `` | |||
@build_steps begin | |||
println("Installing dependency $(h.package) via `$(sudoname(sudo))zypper install $(h.package)`:") | |||
`$sudo zypper install $(h.package)` | |||
info("Installing dependency $(h.package) via `$(sudoname(sudo))zypper install -n $(h.package)`:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be -y
too? At least that's why https://en.opensuse.org/SDB:Zypper_manual_%28plain%29 says.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I read that it was -n
for --no-confirmation
or something like that. I can make it -y
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-n
seems to be an obscure option about names.
Adding this flag is a really dangerous thing to do when sudo is involved. |
We're only installing packages, so nothing terrible should happen. I guess we could check what happens in case of conflicts, i.e. do some package managers remove conflicting packages to be able to install the new one? I would expect something like The alternative is to find out why |
Conflict resolution would be an especially harmful case. I've seen apt-get conflicts try to remove build-essential and everything depending on it before. That would be really nasty to do without giving a chance to confirm. |
If this isn't a good way to go and we can't figure out why Yum fails without input, should I just omit the Yum provider for FFTW so that CentOS et al. always have to build it from source? |
@tkelman I think we need to check this. The manpage for apt-get says:
There's also On the contrary, regarding
I've checked that CentOS 7 has the same behavior. So a solution would be to make the change only for yum, which is where the problem (is known to) happen anyway. Of course we could also fix the underlying problem which blocks user interaction, if somebody has ideas about that.
@ararslan Unfortunately this will likely fail on many systems too, since you need to have the compiler toolchain installed, which probably isn't the case of most users. |
@ararslan perhaps this can be updated to just change |
Pacman will not auto-confirm removals when given |
STDOUT
in theprintln
calls ingenerate_steps
is getting swallowed, so instead we should useinfo
, which prints toSTDERR
. This gets through and prints as expected.Each package manager has a "no confirmation" flag, but for whatever reason we weren't using it for anything but FreeBSD. Apparently Yum actually fails when it expects interactive input but doesn't get it, which seems to be what's causing JuliaMath/FFTW.jl#20.
Thanks to @nalimilan for figuring this all out!