{cc,bintools}-wrapper, ghc, libgcc: Define wrapper env vars as full paths #44767
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ericson2314
changed the title
matthewbauer
reviewed
Aug 8, 2018
| export CC="${targetCC}/bin/${targetCC.targetPrefix}cc" | ||
| export CXX="${targetCC}/bin/${targetCC.targetPrefix}cxx" | ||
| export CC="$CC_FOR_TARGET" | ||
| export CXX="$CXX_FOR_TARGET" | ||
| # Use gold to work around https://sourceware.org/bugzilla/show_bug.cgi?id=16177 | ||
| export LD="${targetCC.bintools}/bin/${targetCC.bintools.targetPrefix}ld${stdenv.lib.optionalString targetPlatform.isAarch32 ".gold"}" |
There was a problem hiding this comment.
Why is this one not changed?
There was a problem hiding this comment.
Because we are forcing gold to be used
|
Failure on x86_64-darwin (full log) Attempted: libgcc Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: libgcc Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: libgcc Partial log (click to expand)
|
|
@GrahamcOfBorg build stdenv |
|
Timed out, unknown build status on x86_64-darwin (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
I cannot test on Darwin at the moment, but it seems unlikely that this would cause any problems. |
|
I see a disadvantage: some packages tend to save these variables somewhere in the outputs, so this PR blows runtime closures for those. Example where it results into a failure due to closure check: https://hydra.nixos.org/build/79797093 Most of these cases probably can be handled, e.g. put those files into |
vcunat
added a commit
that referenced
this pull request
Aug 19, 2018
vcunat
added a commit
that referenced
this pull request
Aug 19, 2018
|
Agh, nothing comes free. Thank you very much for catching, @vcunat. |
|
Several packages started failing with some libtool error that might be due to this – any ideas? https://hydra.nixos.org/build/79808675 |
|
|
|
No, my bad, I don't know a workaround. (I confused two packages together.) |
vcunat
added a commit
that referenced
this pull request
Aug 21, 2018
Some packages just can't handle them #44767. It was tempting to try to abstract this in some way, but I didn't do that ATM.
|
Well, this is in master now, but we would better fixup at least the most common closure offenders, e.g. python. EDIT: or
|
|
This appears to break windows builds too: https://hydra.nixos.org/build/79957713/ Error is: |
matthewbauer
added a commit
to matthewbauer/nixpkgs
that referenced
this pull request
Aug 21, 2018
…nv-var-path" This reverts commit 89efc27, reversing changes made to d0f1102. Fixes NixOS#44936
matthewbauer
added a commit
that referenced
this pull request
Aug 21, 2018
This reverts commit 38eea80.
dtzWill
pushed a commit
to dtzWill/nixpkgs
that referenced
this pull request
Aug 23, 2018
This reverts commit 38eea80.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
This makes builds more robust by having the env vars not rely on the PATH. That, for example, helps keep separate toolchains untangled even when on native builds neither are prefixed.
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)CC @bgamari we talked about this once