Merge "fix: good errors for failures caused by allowSubstitutes" into main

doc/manual/rl-next/allowsubstitutes-errors.md

@@ -0,0 +1,21 @@
+---
+synopsis: "Build failures caused by `allowSubstitutes = false` while being the wrong system now produce a decent error"
+issues: [fj#484]
+cls: [1841]
+category: Fixes
+credits: jade
+---
+
+Nix allows derivations to set `allowSubstitutes = false` in order to force them to be built locally without querying substituters for them.
+This is useful for derivations that are very fast to build (especially if they produce large output).
+However, this can shoot you in the foot if the derivation *has* to be substituted such as if the derivation is for another architecture, which is what `--always-allow-substitutes` is for.
+
+Perhaps such derivations that are known to be impossible to build locally should ignore `allowSubstitutes` (irrespective of remote builders) in the future, but this at least reports the failure and solution directly.
+
+```
+$ nix build -f fail.nix
+error: a 'unicornsandrainbows-linux' with features {} is required to build '/nix/store/...-meow.drv', but I am a 'x86_64-linux' with features {...}
+
+       Hint: the failing derivation has allowSubstitutes set to false, forcing it to be built rather than substituted.
+       Passing --always-allow-substitutes to force substitution may resolve this failure if the path is available in a substituter.
+```

src/libstore/build/derivation-goal.cc

@@ -261,7 +261,8 @@ Goal::WorkResult DerivationGoal::haveDerivation(bool inBuildSlot)
        through substitutes.  If that doesn't work, we'll build
        them. */
     WaitForGoals result;
-    if (settings.useSubstitutes && parsedDrv->substitutesAllowed())
+    if (settings.useSubstitutes) {
+        if (parsedDrv->substitutesAllowed()) {
             for (auto & [outputName, status] : initialOutputs) {
                 if (!status.wanted) continue;
                 if (!status.known)
@@ -279,6 +280,10 @@ Goal::WorkResult DerivationGoal::haveDerivation(bool inBuildSlot)
                         cap ? std::optional { *cap } : std::nullopt));
                 }
             }
+        } else {
+            trace("skipping substitute because allowSubstitutes is false");
+        }
+    }
 
     if (result.goals.empty()) { /* to prevent hang (no wake-up event) */
         return outputsSubstitutionTried(inBuildSlot);

src/libstore/build/local-derivation-goal.cc

@@ -451,13 +451,23 @@ std::set<int> LocalDerivationGoal::startBuilder()
     killSandbox(false);
 
     /* Right platform? */
-    if (!parsedDrv->canBuildLocally(worker.store))
+    if (!parsedDrv->canBuildLocally(worker.store)) {
-        throw Error("a '%s' with features {%s} is required to build '%s', but I am a '%s' with features {%s}",
+        HintFmt addendum{""};
+        if (settings.useSubstitutes && !parsedDrv->substitutesAllowed()) {
+            addendum = HintFmt("\n\nHint: the failing derivation has %s set to %s, forcing it to be built rather than substituted.\n"
+                "Passing %s to force substitution may resolve this failure if the path is available in a substituter.",
+                "allowSubstitutes", "false", "--always-allow-substitutes");
+        }
+        throw Error({
+            .msg = HintFmt("a '%s' with features {%s} is required to build '%s', but I am a '%s' with features {%s}%s",
                 drv->platform,
                 concatStringsSep(", ", parsedDrv->getRequiredSystemFeatures()),
                 worker.store.printStorePath(drvPath),
                 settings.thisSystem,
-            concatStringsSep<StringSet>(", ", worker.store.systemFeatures));
+                concatStringsSep<StringSet>(", ", worker.store.systemFeatures),
+                Uncolored(addendum))
+        });
+    }
 
     /* Create a temporary directory where the build will take
        place. */

tests/functional/meson.build

@@ -190,6 +190,7 @@ functional_tests_scripts = [
   'test-libstoreconsumer.sh',
   'extra-sandbox-profile.sh',
   'substitute-truncated-nar.sh',
+  'regression-484.sh',
 ]
 
 # Plugin tests require shared libraries support.

tests/functional/regression-484.sh

@@ -0,0 +1,41 @@
+source common.sh
+
+# Ensures that nix build will deliver a usable error message when it encounters
+# a build failure potentially caused by allowSubstitutes.
+
+clearStore
+
+cd $TEST_HOME
+
+putDrv() {
+    cat > "$1" <<EOF
+builtins.derivation {
+  name = "meow";
+  builder = "/bin/sh";
+  args = [];
+  system = "unicornsandrainbows-linux";
+  allowSubstitutes = ${2};
+  preferLocalBuild = true;
+}
+EOF
+}
+
+putDrv drv-disallow-substitutes.nix false
+putDrv drv-allow-substitutes.nix true
+
+expect 1 nix build --substitute --substituters '' --offline -f drv-disallow-substitutes.nix 2> output.txt
+
+# error: a 'unicornsandrainbows-linux' with features {} is required to build '$TMPDIR/regression-484/store/...-meow.drv', but I am a 'x86_64-linux' with features {benchmark, big-parallel, kvm, nixos-test, uid-range}
+#
+#        Hint: the failing derivation has allowSubstitutes set to false, forcing it to be built rather than substituted.
+#        Passing --always-allow-substitutes to force substitution may resolve this failure if the path is available in a substituter.
+cat output.txt
+grepQuiet unicornsandrainbows-linux output.txt
+grepQuiet always-allow-substitutes output.txt
+grepQuiet allowSubstitutes output.txt
+
+expect 1 nix build --substitute --substituters '' --offline -f drv-allow-substitutes.nix 2> output.txt
+
+cat output.txt
+grepQuiet unicornsandrainbows-linux output.txt
+grepQuiet -v allowSubstitutes output.txt