stdenv, binutils: Build cctools targeting macOS on Linux without pointless rebuilds #40933
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
GrahamcOfBorg
added
6.topic: darwin
LnL7
reviewed
May 22, 2018
| ''; | ||
| postInstall = '' | ||
| cat >$out/bin/dsymutil << EOF | ||
| #!${stdenv.shell} |
| # think the best solution would just be to fixup linux RPATHs so we don't | ||
| # need to set `-rpath` anywhere. | ||
| # + lib.optionalString targetPlatform.isDarwin '' | ||
| # export NIX_TARGET_DONT_SET_RPATH=1 |
There was a problem hiding this comment.
We don't really want rpaths on darwin, can we keep this for native darwin builds?
There was a problem hiding this comment.
Yeah due to the way cc-wrapper works, mapping these to the infix salt, it will also do depsTargetTarget (intended effect of NIX_TARGET_DONT_SET_RPATH) for native builds. Yay impurities!
Ericson2314
force-pushed
the
linux-to-darwin
branch
2 times, most recently
from
9a074d3
to
752fb0a
Compare
We want `buildPackages` to be almost the same as `buildPackages.buildPackges`, but that is only true if most packages don't care about the target platform. The commented code however made them all care about whether the target platform was Darwin.
Unfortunately this is a crude hack that we use the same binutils source everywhere in the bootstrap chain.
Ericson2314
force-pushed
the
linux-to-darwin
branch
from
752fb0a
to
4ffa8b6
Compare
GrahamcOfBorg
added
10.rebuild-linux: 0
Ericson2314
commented
May 23, 2018
| @@ -19,10 +19,11 @@ in | |||
| stdenv.mkDerivation rec { | |||
| name = targetPrefix + basename; | |||
|
|
|||
| src = fetchurl { | |||
| # HACK to ensure that we preserve source from bootstrap binutils to not rebuild LLVM | |||
| src = stdenv.__bootPackages.binutils-unwrapped.src or (fetchurl { | |||
There was a problem hiding this comment.
This is really gross. Maybe we should just builtins.fetchurl everywhere? IMO with fetchurl, builtins.fetchurl, fetchurlBoot and <nix/fetchurl.nix>, there are too many ways to fetch in nixpkgs.
There was a problem hiding this comment.
It's good enough for now though. At least we avoid the mass rebuild. Any improvement can be another mass rebuild.
Ericson2314
changed the title
Ericson2314
changed the title
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
Most things that used to be in here could get merged straight to master (yay!). What's left is these two ugly things. Contrary to the branch name, this does not give full linux->darwin cross, as that requires #36867, but it does allow doing:
And not waiting far too long for pointless rebuilds.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)CC @alexfmpe @ElvishJerricco