cc-wrapper: use a temporary file for reponse file#245282
cc-wrapper: use a temporary file for reponse file#245282wegank merged 1 commit intoNixOS:stagingfrom
Conversation
ofborg
bot
added
10.rebuild-darwin-stdenv
|
TIL what a "response file" is. Thanks! For the benefit of others.
Do we need to remove it? Debugging failed sandboxed builds in Nix is notoriously difficult. Really the only reliable tool is LGTM other than this, but somebody with Darwin hardware should review this. |
When I was exploring solutions to the clang 16 issue, I tried leaving the temporary files around (with the idea they would be cleaned up when the whole build is cleaned up). Unfortunately, not every build is well-behaved. I found temporary response files in
Given my work recently on the Darwin stdenv and having to fix the fallout of updating clang, I’m sympathetic to that. However, I don’t think the trade-offs are worth it. It’s usually been possible to debug interactively (e.g., source |
|
I’m going to open a PR to revert the |
reckenrode
force-pushed
the
gccStdenv-fix
branch
from
7f0d812 to
ede83e0
Compare
To work around intermitent build failures with clang 16, the stdenv attempted to pass arguments on the command-line on newer versions of macOS. Unfortunately, the larger `ARG_MAX` is still not large enough to build qtwebengine. This commit reverts the `NIX_CC_NO_RESPONSE_FILE` logic in the stdenv. The changes to cc-wrapper in NixOS#245282 are needed for clang 16 to prevent the above-mentioned build failures.
The Darwin stdenv rework conditionally sets `NIX_CC_USE_RESPONSE_FILE`
depending on the `ARG_MAX` of the build system. If it is at least 1 MiB,
the stdenv passes the arguments on the command-line (like Linux).
Otherwise, it falls back to the response file. This was done to prevent
intermitent failures with clang 16 being unable to read the response
file. Unfortunately, this breaks `gccStdenv` on older Darwin platforms.
Note: While the stdenv logic will also be reverted, this change is
needed for compatibility with clang 16.
GCC is capable of using a response file, but it does not work correctly
when the response file is a file descriptor. This can be reproduced
using the following sequence of commands:
$ nix shell nixpkgs#gcc; NIX_CC_USE_RESPONSE_FILE=1 gcc
# Linux
/nix/store/9n9gjvzci75gp2sh1c4rh626dhizqynl-binutils-2.39/bin/ld: unrecognized option '-B/nix/store/vnwdak3n1w2jjil119j65k8mw1z23p84-glibc-2.35-224/lib/'
/nix/store/9n9gjvzci75gp2sh1c4rh626dhizqynl-binutils-2.39/bin/ld: use the --help option for usage information
collect2: error: ld returned 1 exit status
# Darwin
ld: unknown option: -mmacosx-version-min=11.0
collect2: error: ld returned 1 exit status
Instead of using process substitution, create a temporary file and
remove it in a trap. This should also prevent the intermitent build
failures with clang 16 on older Darwin systems.
Fixes NixOS#245167
|
I was going to update the PR when I got home tonight, but thanks for pushing an updated commit. |
This changes ld-wrapper to use a temporary file for the response file passed to ld instead of using process substitution. ld64 does not handle long command-lines when reading from the response file, which defeats the point of using a response file to handle long command-lines. cctools-port was patched to work around this, but nixpkgs is now using Apple’s source release directly instead of the port. Since it’s preferable not to patch Apple’s release heavily (to reduce the difficulty of updating to new versions and to match upstream’s behavior), use the approach that was adopted in cc-wrapper to work around issues with response files in newer versions of clang. Related PRs (cctools-port): - NixOS#213831 - tpoechtrager/cctools-port#132 Related PRs (cc-wrapper): - NixOS#245282 - NixOS#258608
This changes ld-wrapper to use a temporary file for the response file passed to ld instead of using process substitution. ld64 does not handle long command-lines when reading from the response file, which defeats the point of using a response file to handle long command-lines. cctools-port was patched to work around this, but nixpkgs is now using Apple’s source release directly instead of the port. Since it’s preferable not to patch Apple’s release heavily (to reduce the difficulty of updating to new versions and to match upstream’s behavior), use the approach that was adopted in cc-wrapper to work around issues with response files in newer versions of clang. Related PRs (cctools-port): - NixOS#213831 - tpoechtrager/cctools-port#132 Related PRs (cc-wrapper): - NixOS#245282 - NixOS#258608
This changes ld-wrapper to use a temporary file for the response file passed to ld instead of using process substitution. ld64 does not handle long command-lines when reading from the response file, which defeats the point of using a response file to handle long command-lines. cctools-port was patched to work around this, but nixpkgs is now using Apple’s source release directly instead of the port. Since it’s preferable not to patch Apple’s release heavily (to reduce the difficulty of updating to new versions and to match upstream’s behavior), use the approach that was adopted in cc-wrapper to work around issues with response files in newer versions of clang. Related PRs (cctools-port): - NixOS#213831 - tpoechtrager/cctools-port#132 Related PRs (cc-wrapper): - NixOS#245282 - NixOS#258608
This changes ld-wrapper to use a temporary file for the response file passed to ld instead of using process substitution. ld64 does not handle long command-lines when reading from the response file, which defeats the point of using a response file to handle long command-lines. cctools-port was patched to work around this, but nixpkgs is now using Apple’s source release directly instead of the port. Since it’s preferable not to patch Apple’s release heavily (to reduce the difficulty of updating to new versions and to match upstream’s behavior), use the approach that was adopted in cc-wrapper to work around issues with response files in newer versions of clang. Related PRs (cctools-port): - NixOS#213831 - tpoechtrager/cctools-port#132 Related PRs (cc-wrapper): - NixOS#245282 - NixOS#258608
3 participants
The Darwin stdenv rework conditionally sets
NIX_CC_USE_RESPONSE_FILEdepending on theARG_MAXof the build system. If it is at least 1 MiB, the stdenv passes the arguments on the command-line (like Linux). Otherwise, it falls back to the response file. This was done to prevent intermitent failures with clang 16 being unable to read the response file. Unfortunately, this breaksgccStdenvon older Darwin platforms.Note: While the stdenv logic will also be reverted, this change is needed for compatibility with clang 16.
GCC is capable of using a response file, but it does not work correctly when the response file is a file descriptor. This can be reproduced using the following sequence of commands:
The fix is to create a temporary file instead and remove it in a trap. This should also prevent intermitent failures with clang 16 on older Darwin systems.
Tested by building acme-client with NIX_CC_USE_RESPONSE_FILE=1 set.
$TMPwas reviewed afterward to confirm no files, andfswatchwas used to confirm that temporary response files were used.Fixes #245167
Description of changes
Things done
sandbox = trueset 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/)