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
chromium: Add missing dependency on gnugrep #86662
chromium: Add missing dependency on gnugrep #86662
Conversation
This change doesn't look like it should fix the issue. Did it actually fix it in practice? The reason I don't think it fixes the issue is because the grep issue is a runtime dependency (in the wrapper script), and you added it as a buildInput only... but a buildInput doesn't end up as a runtime dependency unless there's a reference to it in one of the output artifacts. Because of that, I think you'll also want to add an explicit reference to the gnugrep derivation. Specifically, the following line could be changed to add the runtime reference:
If we change that to |
The bin script that runs chromium calls out to gnugrep - but gnugrep is missing as a runtime dependency of the chromium package. I found this out when I was trying to put it in a docker image.
f8d6e28
to
25e338a
Compare
@euank yeah, I had sorta assumed that |
ran |
Huh, I don’t see how that changes anything at runtime, since that build script is run in the nix sandbox (inside of |
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.
The diff LGTM.
@Profpatsch this isn't clear from the limited diff context but that line is actually part of the wrapper ($out/bin/chromium
) and as a result it is actually executed at runtime instead of in the build sandbox.
The bin script that runs chromium calls out to gnugrep:
but gnugrep ismissing as a runtime dependency of the chromium package. I found thisout when I was trying to put it in a docker image and was getting errors that grep didn't exist.
I'm a little unfamiliar with how makeWrapper works, so this may not actually be the right way to include this - I'm mostly submitting this as a call for help, because it felt broken to me.
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)