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
libav: remove /bin/sh dependencies #36583
Conversation
that resulted in some Hydra build fails
cc @shlevy please review |
|
One of the outputs installed in bin/ is a shell script, so I considered |
@GrahamcOfBorg build libav |
Failure on x86_64-darwin (full log) Partial log (click to expand)
|
Success on aarch64-linux (full log) Partial log (click to expand)
|
Success on x86_64-linux (full log) Partial log (click to expand)
|
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.
This looks good to me. We will eventually have a notion of patchBuildtimeShebangs
that will be more appropriate in postPatch
, but since it doesn't exist yet this will just fail on cross. We can fix that later.
@@ -45,7 +45,11 @@ let | |||
++ optional (vpxSupport && hasPrefix "0.8." version) ./vpxenc-0.8.17-libvpx-1.5.patch | |||
; | |||
|
|||
preConfigure = "patchShebangs doc/texi2pod.pl"; | |||
postPatch = '' | |||
patchShebangs . |
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.
@Ericson2314 FYI another point that needs patchBuildShebangs
@shlevy Do you know what the motivation behind |
@Mic92 I think the main motivation is specifically static linking... bringing in a binary and all of its dependent libraries is sometimes a pain and sometimes impossible to do right, as @7c6f434c points out. And to be honest, if it were up to me there'd be no /bin/sh in the sandbox at all (or it would just immediately error out with a "nice" error message). Many years ago I tried to argue that |
@shlevy yes taking |
@shlevy could we at least agree on something like |
My answer is always going to be in favor of things failing with /bin/sh, sorry 😄 I see the short-term value in a sane common base (though why |
Motivation for this change
impurity resulted in a Hydra build fail on 18.03.
/cc ZHF #36453
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)