[Backport release-26.05] ci/eval: Allow disallowed attrs to depend on other disallowed attrs (#537612)

Author: MattSturgeon

ci/eval/default.nix

@@ -100,6 +100,7 @@ let
         myChunk=$2
         system=$3
         outputDir=$4
+        preEvalFile=$5
 
         # Default is 5, higher values effectively disable the warning.
         # This randomly breaks Eval.
@@ -121,12 +122,12 @@ let
           --show-trace \
           --arg chunkSize "$chunkSize" \
           --arg myChunk "$myChunk" \
-          --arg preEvalFile "${preEvalFile}" \
+          --arg preEvalFile "$preEvalFile" \
           --arg systems "[ \"$system\" ]" \
           --arg includeBroken ${lib.boolToString includeBroken} \
           --argstr extraNixpkgsConfigJson ${lib.escapeShellArg (builtins.toJSON extraNixpkgsConfig)} \
           -I ${nixpkgs} \
-          -I ${preEvalFile} \
+          -I "$preEvalFile" \
           > "$outputDir/result/$myChunk" \
           2> "$outputDir/stderr/$myChunk"
         exitCode=$?
@@ -164,12 +165,6 @@ let
         echo "System: $evalSystem"
         cores=$NIX_BUILD_CORES
         echo "Cores: $cores"
-        attrCount=$(jq '.paths | length' "${preEvalFile}")
-        echo "Attribute count: $attrCount"
-        echo "Chunk size: $chunkSize"
-        # Same as `attrCount / chunkSize` but rounded up
-        chunkCount=$(( (attrCount - 1) / chunkSize + 1 ))
-        echo "Chunk count: $chunkCount"
 
         mkdir -p $out/${evalSystem}
 
@@ -190,29 +185,61 @@ let
           done
         ) &
 
-        seq_end=$(( chunkCount - 1 ))
+        chunkedEval() {
+          local chunkOutputDir=$1
+          local preEvalFile=$2
 
-        ${lib.optionalString quickTest ''
-          seq_end=0
-        ''}
+          local attrCount=$(jq '.paths | length' "$preEvalFile")
+          echo "Attribute count: $attrCount"
+          echo "Chunk size: $chunkSize"
+          # Same as `attrCount / chunkSize` but rounded up
+          local chunkCount=$(( (attrCount - 1) / chunkSize + 1 ))
+          echo "Chunk count: $chunkCount"
 
-        chunkOutputDir=$(mktemp -d)
-        mkdir "$chunkOutputDir"/{result,stats,timestats,stderr}
+          local seq_end=$(( chunkCount - 1 ))
+          ${lib.optionalString quickTest ''
+            seq_end=0
+          ''}
 
-        seq -w 0 "$seq_end" |
-          command time -f "%e" -o "$out/${evalSystem}/total-time" \
-          xargs -I{} -P"$cores" \
-          ${singleChunk} "$chunkSize" {} "$evalSystem" "$chunkOutputDir"
+          mkdir -p "$chunkOutputDir"/{result,stats,timestats,stderr}
 
-        cp -r "$chunkOutputDir"/stats $out/${evalSystem}/stats-by-chunk
+          seq -w 0 "$seq_end" |
+            xargs -I{} -P"$cores" \
+            ${singleChunk} "$chunkSize" {} "$evalSystem" "$chunkOutputDir" "$preEvalFile"
 
-        if (( chunkSize * chunkCount != attrCount )); then
-          # A final incomplete chunk would mess up the stats, don't include it
-          rm "$chunkOutputDir"/stats/"$seq_end"
-        fi
+          if (( chunkSize * chunkCount != attrCount )); then
+            # A final incomplete chunk would mess up the stats, don't include it
+            rm "$chunkOutputDir"/stats/"$seq_end"
+          fi
+        }
+
+        chunkOutputDirs=$(mktemp -d)
+
+        # Preparation for the second eval
+        disallowedAttributesPreEvalFile=$(mktemp)
+        jq '{
+          paths: (.attrPathsDisallowedForInternalUse | map(.attrPath)),
+          attrPathsDisallowedForInternalUse: []
+        }' ${preEvalFile} > "$disallowedAttributesPreEvalFile"
+
+        startEpoch=$(date +%s)
+
+        # The first eval evaluates only attributes that are not disallowed for internal Nixpkgs use, ensuring that they don't depend on disallowed attributes
+        # Because the first eval doesn't evaluate the disallowed attributes themselves, but we still want to check that they don't fail evaluation, we evaluate them separately in a second eval
+        # The reason we need two evals is because we want disallowed attributes to be able to depend on other disallowed attributes, which inherently needs a separate Nixpkgs instantiation
+        # And while we could interleave that instantiation into a single eval, that would ~double memory usage for all chunks, while doing it separately doesn't
+        echo "Evaluating the internally allowed attributes"
+        chunkedEval "$chunkOutputDirs"/allowed ${preEvalFile}
+        echo "Evaluating the internally disallowed attributes"
+        chunkedEval "$chunkOutputDirs"/disallowed "$disallowedAttributesPreEvalFile"
+
+        echo $(( $(date +%s) - startEpoch )) > "$out/${evalSystem}/total-time"
+
+        # We only use the stats from the allowed attrs eval, because the disallowed attrs are generally not even a full chunk
+        cp -r "$chunkOutputDirs"/allowed/stats $out/${evalSystem}/stats-by-chunk
 
-        cat "$chunkOutputDir"/result/* | jq -s 'add | map_values(.outputs)' > $out/${evalSystem}/paths.json
-        cat "$chunkOutputDir"/result/* | jq -s 'add | map_values(.meta)' > $out/${evalSystem}/meta.json
+        cat "$chunkOutputDirs"/*/result/* | jq -s 'add | map_values(.outputs)' > $out/${evalSystem}/paths.json
+        cat "$chunkOutputDirs"/*/result/* | jq -s 'add | map_values(.meta)' > $out/${evalSystem}/meta.json
       '';
 
   diff = callPackage ./diff.nix { };

pkgs/top-level/default.nix

@@ -216,27 +216,37 @@ let
 
   fixedPoint = boot stages;
 
+  removeInternallyDisallowedAttrPaths =
+    let
+      # Same as `lib.removeAttrs`, but can remove nested attributes (and order of arguments is fixed)
+      # TODO: Consider moving to lib.attrpaths.removeAttrPaths
+      removeAttrPaths =
+        attrPathsToRemove: set:
+        let
+          split = lib.partition (
+            attrPath:
+            assert attrPath != [ ];
+            lib.length attrPath == 1
+          ) attrPathsToRemove;
+          nestedApplied =
+            set
+            // lib.mapAttrs (name: attrPaths: removeAttrPaths (lib.map lib.tail attrPaths) set.${name}) (
+              lib.groupBy (attrPath: lib.head attrPath) split.wrong
+            );
+        in
+        lib.removeAttrs nestedApplied (lib.map lib.head split.right);
+    in
+    removeAttrPaths (map (x: x.attrPath) config.attrPathsDisallowedForInternalUse);
+
   pkgs =
     # Generally only set by CI, don't want to cause a performance hit for users
     if config.attrPathsDisallowedForInternalUse == [ ] then
       fixedPoint
     else
       # See ./stage.nix, which replaced config.attrPathsDisallowedForInternalUse with aborts.
-      # We replace these attribute paths with their original derivations again,
-      # because CI would just error out from the aborting attributes themselves.
-      # Internally all packages still see the aborting attributes if used as dependencies,
-      # because we do this here after the fixed-point is calculated.
-      # Note that we don't want to remove the attributes entirely like what aliases.nix does,
-      # because unlike aliases, CI still needs to check the packages to evaluate at all,
-      # which it wouldn't if they're removed entirely.
-      lib.updateManyAttrsByPath
-        (map (attrs: {
-          path = attrs.attrPath;
-          update =
-            _:
-            lib.getAttrFromPath attrs.attrPath fixedPoint.__internalBeforeInternallyDisallowedAttrPathsOverlay;
-        }) config.attrPathsDisallowedForInternalUse)
-        (removeAttrs fixedPoint [ "__internalBeforeInternallyDisallowedAttrPathsOverlay" ]);
+      # To prevent these attributes from causing CI failures we remove them entirely.
+      # These attrs are still evaluated but in a different way, see ci/eval/default.nix
+      removeInternallyDisallowedAttrPaths fixedPoint;
 
 in
 checked pkgs

pkgs/top-level/stage.nix

@@ -317,20 +317,16 @@ let
   };
 
   # Replaces the attributes in config.attrPathsDisallowedForInternalUse with aborts.
+  # Not warnings because those wouldn't give a backtrace, which is important for debugging
   # Not throws because those would be ignored by nix-env, which is what CI uses to evaluate everything
-  # See also ./default.nix, where these attributes are added back again so they're still checked by CI
+  # See also ./default.nix, which removes these attributes entirely from the end result
   internallyDisallowedAttrPathsOverlay =
     final: prev:
     # Generally only set by CI, don't want to cause a performance hit for users
     if config.attrPathsDisallowedForInternalUse == [ ] then
       { }
     else
-      {
-        # So that ./default.nix can add them back again outside the fixed point
-        # Don't use this in packages!
-        __internalBeforeInternallyDisallowedAttrPathsOverlay = prev;
-      }
-      // lib.updateManyAttrsByPath (map (
+      lib.updateManyAttrsByPath (map (
         { attrPath, reason }:
         {
           path = attrPath;