-
-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
darwin.stdenv: allow patchShebangs
during the bootstrap
#242519
Conversation
Creating this as a draft PR while x86_64-darwin builds. I feel pretty confident that it will, but I don’t want to waste time on Hydra if it doesn’t. Once the stdenv is built, I’ll set the PR to ready. |
This fixes pyicu (and any other package that uses `icu-config` instead of the CMake or some other module to get the build flags). What happened here is the bootstrap disables `patchShebangs` to avoid propagating the bootstrap tools to the final stdenv (due to `sh` and `bash` being on the `PATH` from the bootstrap tools). Because of that, the `#!/bin/sh` line in `icu-config` was not updated, causing it to invoke the system bash on Darwin. While that is undesirable in its own right, when the system bash is invoked as `sh`, `echo -n` will print `-n`, resulting in the breakage see in NixOS#241951 (comment). The fix is to build bash earlier in the bootstrap while making sure it is picked up over the one in the bootstrap tools. That allows `patchShebangs` to be enabled during the bootstrap. Any package with scripts that is included in the final stdenv should now have its scripts’ shebang lines properly patched.
aad246c
to
856ebe6
Compare
Setting this to review. I had to build link bash against the system CoreFoundation to break a dependency cycle on x86_64-darwin. I was able to successfully build a stdenv on both x86_64-darwin and aarch64-darwin, and I was able to build pyicu on aarch64-darwin. I feel pretty confident pyicu will build on x86_64-darwin, but I don’t want to delay longer than necessary. |
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.
LGTM. These cycles are crazy!
The failure in |
To be clear, Hydra showed the failure the same on both |
Ah, I now caught up the confirmation for those questions in another thread: #242202 (comment) |
OK, the |
Sorry about the delayed response. I was asleep at the time these comments were posted.
If it’s due to icu, then I expect it to be fixed. The
I’m building it now and will report back. |
@vcunat Both texlive and R build for me with this commit. |
Description of changes
This fixes pyicu (and any other package that uses
icu-config
instead of the CMake or some other module to get the build flags).What happened here is the bootstrap disables
patchShebangs
to avoid propagating the bootstrap tools to the final stdenv (due tosh
andbash
being on thePATH
from the bootstrap tools). Because of that, the#!/bin/sh
line inicu-config
was not updated, causing it to invoke the system bash on Darwin. While that is undesirable in its own right, when the system bash is invoked assh
,echo -n
will print-n
, resulting in the breakage see in #241951 (comment).The fix is to build bash earlier in the bootstrap while making sure it is picked up over the one in the bootstrap tools. That allows
patchShebangs
to be enabled during the bootstrap. Any package with scripts that is included in the final stdenv should now have its scripts’ shebang lines properly patched.I tested that pyicu builds, and this is the contents of
icu-config
now:Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)