wrapCC, wrapBintools: correctly cross-compile if buildPlatform != hostPlatform #304861
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
ofborg
bot
added
the
6.topic: cross-compilation
ofborg
bot
added
8.has: package (new)
sternenseemann
force-pushed
the
cc-bintools-wrappers-correct-cross
branch
from
0105c54 to
97716b6
Compare
|
I think the |
sternenseemann
force-pushed
the
cc-bintools-wrappers-correct-cross
branch
from
97716b6 to
3c4b8fd
Compare
We don't need to artificially make sure that we can execute the wrapper scripts on the build platform by using stdenv's shell (which comes from buildPackages) since our cross infrastructure will get us the wrapper from buildPackages. The upside of this change is that cross-compiled wrappers (e.g. pkgsCross.aarch64-multiplatform.gcc) will actually work when executed! For bootstrapping this is also not a problem, since we have a long build->build platform chain so runtimeShell is just as good as stdenvNoCC.shell. We do fall back to old ways, though, by explicitly using the bootstrap-tools shell in stage2, so the adjacent bash is only used from stage4 onwards. This is unnecessary in principle (I'll try removing this hack in the future), but ensures this change causes zero rebuilds.
The cc and bintools wrapper contained ad hoc bootstrapping logic for expand-response-params (which was callPackage-ed in a let binding). This lead to the strange situation that the bootstrapping logic related to expand-response-params is split between the wrapper derivations (where it is duplicated) and the actual stdenv bootstrapping. To clean this up, the wrappers simply should take expand-response-params as an ordinary input: They need an adjacent expand-response-params (i.e. one that runs on their host platform), but don't care about the how. Providing this is only problematic during stdenv bootstrapping where we have to pull it from the previous stage at times.
Since the tool is exposed more prominently now, we should clear up what it is and note that it is to be considered unstable, i.e. we may change it if the necessity arises. (In practice it is probably going to be fairly stable though, as compiler interfaces tend to be quite stable.) Should we add a version?
sternenseemann
force-pushed
the
cc-bintools-wrappers-correct-cross
branch
from
3c4b8fd to
b544fd9
Compare
13 tasks
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-darwin: 1
and removed
10.rebuild-darwin: 0
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.
Description of changes
The wrapper scripts have two main dependencies (aside from
ccandbintoolswhich are a given):shellthus far.expand-response-params, a small C program which requires a wrapped compiler to be built.Previously, these programs would be essentially taken from/built with the package set used to build the package set that the wrapper itself is a part of (usually
buildPackages). This is incorrect, as it only “works” ifhostPlatform == buildPlatform(relative to the wrapper) or it will be impossible to execute the dependencies.This has the consequence that if you e.g. build
pkgsCross.aarch64-multiplatform.gcc, it will be impossible to execute the resulting (wrapped) compiler due to exec format errors.This weird situation stems from the stdenv bootstrapping process where we have the concrete problem that building a shell and C program requires a wrapped C compiler, but we also require these to build the wrapper. The solution is to either do without the dependencies or take them from the previous bootstrapping stage and, ultimately,
bootstrapTools. These is sound in this situation since during (initial) stdenv bootstrappingbuildPlatform == hostPlatform == targetPlatform. It is, however, incorrect to have this leak into the main package set where cross compilation may happen.The solution to this problem is to make the wrappers behave correctly in the main package set (taking cross into account) and worry about bootstrapping later. This is exactly what I've done:
shellwhich defaults tostdenvNoCC.shell(which is usually equivalent tobuildPackages.runtimeShell), we useruntimeShellwhich is an attribute that exposes the default shell we use on the host platform.expand-response-paramsinline in the wrapper derivation by trying to pull astdenvout ofbuildPackages(so that we are effectively usingbuildPackages.buildPackages.gcc/clang), we just take it as an ordinary input (which works perfectly in the main package set) and let the caller worry about bootstrapping.In a second step I've reflected these changes in the darwin and linux stdenvs, making sure that no derivation hash changes by inserting extra bootstrapping instructions where necessary (the single rebuild is caused by the newly added
expand-response-paramsattribute). Some of these may actually be unnecessary (i.e. we may be able to build a properruntimeShellearlier etc.), but this will have to be investigated separately as that would cause a stdenv rebuild.This change is best reviewed commit by commit.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.