stdenv: inline and remove actuallySplice (no effect on eval) #263278
Closed
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
Previously, stdenv used `adjacentPackages==null` to signal that `actuallySplice` should be `false`. Let's pass it explicitly for greater clarity.
It's quite difficult to understand what this `selfBuild` parameter is signaling, but it turns out to simply indicate which stages have any kind of cross compilation (i.e. either build!=host or host!=target). Let's check that condition directly for greater clarity.
The native stdenv bootstrap (e.g. `pkgs/stdenv/linux/default.nix`) is very elegantly structured as a sequence of arbitrarily-many stages. This makes it easy to refactor and modularize and reuse, because if a stage gets too complicated we can just split it up into two separate sequential stages. We can also insert optional stages. Unfortunately we can't do the same thing with cross-compiled stdenvs, because `adjacentPackages` includes the hardwired assumption that there are exactly two stages after the end of the native bootstrap stage sequence: - The (build==host)!=target stage - The build!=(host==target) stage This commit eliminates that assumption. Instead of resolving things like `pkgsBuildHost` to `prevStage`, we instead *search backwards* for the most recent stage which creates packages on our buildPlatform and for our hostPlatform. This commit should not affect eval. Note that `pkgsTargetTarget`, which becomes `targetPackages` still has hardwired assumptions. I think setting `pkgsTargetTarget` to `nextStage` is just flat-out wrong for every stage except the "main" stage (the `build!=(host==target)` stage), but I fear that there may be code that relies on the particular way that it is wrong. It looks like `pkgsTargetTarget` and `targetPackages` exist exclusively in order to provide access to the `postStage` hack; I intend to submit a separate PR that `throw`s if it is used in any other way, to see what breaks. Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
This commit inserts extra stages into the cross-compilation stdenv bootstrap in order to prevent assumptions about the number of stages from creeping back in (see previous commit). If this commit is used without the previous commit, eval will fail badly. The no-op stages added by this commit serve as an example of what wasn't possible before the PR which includes them. Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
This commit inlines the `actuallySplice` variable, which is simply `stdenv.buildPlatform!=stdenv.targetPlatform`. Since it isn't anything mysterious, the code becomes much easier to understand when written that way. This commit also restructures `stdenvBootstappingAndPlatforms`, but in a way that makes absolutely no change to its behavior.
ghost
marked this pull request as ready for review
ghost
closed this
This pull request was closed.
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
Split out from
Includes
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/)This commit inlines the
actuallySplicevariable, which is simplystdenv.buildPlatform!=stdenv.targetPlatform.Since it isn't anything mysterious, the code becomes much easier to
understand when written that way.
This commit also restructures
stdenvBootstappingAndPlatforms, butin a way that makes absolutely no change to its behavior.