From eda7de0027869433a4966b1e4204d2d415bd7c4f Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Thu, 24 Aug 2023 21:21:55 -0700 Subject: [PATCH 1/4] stdenv: pass actuallySplice explicitly Previously, stdenv used `adjacentPackages==null` to signal that `actuallySplice` should be `false`. Let's pass it explicitly for greater clarity. --- pkgs/stdenv/booter.nix | 3 ++- pkgs/top-level/stage.nix | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkgs/stdenv/booter.nix b/pkgs/stdenv/booter.nix index 7fc1fa42b965c..64797dc2d4153 100644 --- a/pkgs/stdenv/booter.nix +++ b/pkgs/stdenv/booter.nix @@ -99,7 +99,8 @@ stageFuns: let if args.__raw or false then args' else allPackages ((builtins.removeAttrs args' ["selfBuild"]) // { - adjacentPackages = if args.selfBuild or true then null else rec { + actuallySplice = !(args.selfBuild or true); + adjacentPackages = rec { pkgsBuildBuild = prevStage.buildPackages; pkgsBuildHost = prevStage; pkgsBuildTarget = diff --git a/pkgs/top-level/stage.nix b/pkgs/top-level/stage.nix index 1f37bbb70bda6..064337208304e 100644 --- a/pkgs/top-level/stage.nix +++ b/pkgs/top-level/stage.nix @@ -45,6 +45,7 @@ in # avoid expensive splicing. `pkgsHostTarget` is skipped because it is always # defined as the current stage. adjacentPackages +, actuallySplice , # The standard environment to use for building packages. stdenv @@ -117,7 +118,7 @@ let stdenvBootstappingAndPlatforms = self: super: let withFallback = thisPkgs: - (if adjacentPackages == null then self else thisPkgs) + (if actuallySplice then thisPkgs else self) // { recurseForDerivations = false; }; in { # Here are package sets of from related stages. They are all in the form @@ -145,7 +146,7 @@ let inherit stdenv; }; - splice = self: super: import ./splice.nix lib self (adjacentPackages != null); + splice = self: super: import ./splice.nix lib self actuallySplice; allPackages = self: super: let res = import ./all-packages.nix From 7e6bbdef7fe607bdfca401eff805a4659f88b68a Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Thu, 24 Aug 2023 22:35:43 -0700 Subject: [PATCH 2/4] stdenv: remove selfBuild parameter 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. --- pkgs/stdenv/booter.nix | 4 ++-- pkgs/stdenv/cross/default.nix | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pkgs/stdenv/booter.nix b/pkgs/stdenv/booter.nix index 64797dc2d4153..5fa481b019342 100644 --- a/pkgs/stdenv/booter.nix +++ b/pkgs/stdenv/booter.nix @@ -98,8 +98,8 @@ stageFuns: let thisStage = if args.__raw or false then args' - else allPackages ((builtins.removeAttrs args' ["selfBuild"]) // { - actuallySplice = !(args.selfBuild or true); + else allPackages (args' // { + actuallySplice = thisStage.stdenv.buildPlatform != thisStage.stdenv.targetPlatform; adjacentPackages = rec { pkgsBuildBuild = prevStage.buildPackages; pkgsBuildHost = prevStage; diff --git a/pkgs/stdenv/cross/default.nix b/pkgs/stdenv/cross/default.nix index cf6a55fec208c..dac6d1241b363 100644 --- a/pkgs/stdenv/cross/default.nix +++ b/pkgs/stdenv/cross/default.nix @@ -24,7 +24,6 @@ in lib.init bootStages ++ [ # Build tool Packages (vanillaPackages: { inherit config overlays; - selfBuild = false; stdenv = assert vanillaPackages.stdenv.buildPlatform == localSystem; assert vanillaPackages.stdenv.hostPlatform == localSystem; @@ -43,7 +42,6 @@ in lib.init bootStages ++ [ in { inherit config; overlays = overlays ++ crossOverlays; - selfBuild = false; stdenv = adaptStdenv (buildPackages.stdenv.override (old: rec { buildPlatform = localSystem; hostPlatform = crossSystem; From b4ca0a924bb06121234d02448a1e615dbe7d09ba Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Thu, 24 Aug 2023 22:37:46 -0700 Subject: [PATCH 3/4] stdenv: remove assumption that there are exactly two non-native stages 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 --- pkgs/stdenv/booter.nix | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/pkgs/stdenv/booter.nix b/pkgs/stdenv/booter.nix index 5fa481b019342..d21277c814752 100644 --- a/pkgs/stdenv/booter.nix +++ b/pkgs/stdenv/booter.nix @@ -99,20 +99,25 @@ stageFuns: let if args.__raw or false then args' else allPackages (args' // { - actuallySplice = thisStage.stdenv.buildPlatform != thisStage.stdenv.targetPlatform; - adjacentPackages = rec { - pkgsBuildBuild = prevStage.buildPackages; - pkgsBuildHost = prevStage; - pkgsBuildTarget = - if args.stdenv.targetPlatform == args.stdenv.hostPlatform - then pkgsBuildHost - else assert args.stdenv.hostPlatform == args.stdenv.buildPlatform; thisStage; - pkgsHostHost = - if args.stdenv.hostPlatform == args.stdenv.targetPlatform - then thisStage - else assert args.stdenv.buildPlatform == args.stdenv.hostPlatform; pkgsBuildHost; - pkgsTargetTarget = nextStage; - }; + # Technically we only need to check `build!=target` since nixpkgs currently doesn't allow + # both `build!=host` and `host!=target` at the same time, but we check both in order to be + # future-proof. + actuallySplice = with thisStage.stdenv; buildPlatform != hostPlatform || hostPlatform != targetPlatform; + adjacentPackages = + let + # search backwards from `stage` for the most recent stage containing packages which execute on `on` and target `for` + seek = stage: { on, for }@args: + if on == stage.stdenv.hostPlatform && + for == stage.stdenv.targetPlatform + then stage + else seek stage.stdenv.__bootPackages or (throw "nixpkgs internal error: Bootstrapping stage with requested host and target platform not found.") args; + in rec { + pkgsBuildBuild = seek thisStage { on = thisStage.stdenv.buildPlatform; for = thisStage.stdenv.buildPlatform; }; + pkgsBuildHost = seek thisStage { on = thisStage.stdenv.buildPlatform; for = thisStage.stdenv.hostPlatform; }; + pkgsBuildTarget = seek thisStage { on = thisStage.stdenv.buildPlatform; for = thisStage.stdenv.targetPlatform; }; + pkgsHostHost = seek thisStage { on = thisStage.stdenv.hostPlatform; for = thisStage.stdenv.hostPlatform; }; + pkgsTargetTarget = nextStage; + }; }); in thisStage; From 232fdd8f3ae3e97b4146ad84b507bfdc508c17a3 Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Thu, 24 Aug 2023 22:43:57 -0700 Subject: [PATCH 4/4] stdenv: cross: insert no-op stages 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 --- pkgs/stdenv/cross/default.nix | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pkgs/stdenv/cross/default.nix b/pkgs/stdenv/cross/default.nix index dac6d1241b363..9ff2c1b683e05 100644 --- a/pkgs/stdenv/cross/default.nix +++ b/pkgs/stdenv/cross/default.nix @@ -33,6 +33,27 @@ in lib.init bootStages ++ [ allowCustomOverrides = true; }) + # Prevent (some) unwanted assumptions about the number of stages + # from creeping in. Previously, nixpkgs contained code which + # assumed that `pkgs.stdenv.__bootPackages == pkgs.stdenv.pkgsBuildHost`. + # + # We add this no-op stage here, between pkgsBuildHost and + # pkgsHostHost, in order to deliberately break any code which + # makes that assumption -- such code will likely fail since + # `pkgs.stdenv.__bootPackages.hostPlatform != + # pkgs.stdenv.pkgsBuildHost.hostPlatform`, and will end up trying + # to run `hostPlatform` binaries during the build. + # + # We previously had two additional no-op stages: one before + # `pkgsBuildHost`, and one after `pkgsHostHost`. However these + # appeared to increase eval-time CPU usage by 1%, which was deemed + # undesirable. See https://github.com/NixOS/nixpkgs/pull/251299 + # + (prevStage: { + inherit config overlays; + inherit (prevStage) stdenv; + }) + # Run Packages (buildPackages: let adaptStdenv =