From 686120ee4a34f658b2f19dcac9f9dc44dbc98b93 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 22 Aug 2024 17:07:14 -0700 Subject: [PATCH] fix: good errors for failures caused by allowSubstitutes This caused an absolute saga which I would not like anyone else to have to experience. Let's put in a laser targeted error message that diagnoses this exact problem. Fixes: https://git.lix.systems/lix-project/lix/issues/484 Change-Id: I2a79f04aeb4a1b67c10115e5e39501d958836298 --- doc/manual/rl-next/allowsubstitutes-errors.md | 21 ++++++++++ src/libstore/build/derivation-goal.cc | 37 +++++++++-------- src/libstore/build/local-derivation-goal.cc | 24 +++++++---- tests/functional/meson.build | 1 + tests/functional/regression-484.sh | 41 +++++++++++++++++++ 5 files changed, 101 insertions(+), 23 deletions(-) create mode 100644 doc/manual/rl-next/allowsubstitutes-errors.md create mode 100644 tests/functional/regression-484.sh diff --git a/doc/manual/rl-next/allowsubstitutes-errors.md b/doc/manual/rl-next/allowsubstitutes-errors.md new file mode 100644 index 000000000..47b0555b1 --- /dev/null +++ b/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. +``` diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index f31c3acd5..0f082b193 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -261,24 +261,29 @@ Goal::WorkResult DerivationGoal::haveDerivation(bool inBuildSlot) through substitutes. If that doesn't work, we'll build them. */ WaitForGoals result; - if (settings.useSubstitutes && parsedDrv->substitutesAllowed()) - for (auto & [outputName, status] : initialOutputs) { - if (!status.wanted) continue; - if (!status.known) - result.goals.insert( - worker.makeDrvOutputSubstitutionGoal( - DrvOutput{status.outputHash, outputName}, - buildMode == bmRepair ? Repair : NoRepair - ) - ); - else { - auto * cap = getDerivationCA(*drv); - result.goals.insert(worker.makePathSubstitutionGoal( - status.known->path, - buildMode == bmRepair ? Repair : NoRepair, - cap ? std::optional { *cap } : std::nullopt)); + if (settings.useSubstitutes) { + if (parsedDrv->substitutesAllowed()) { + for (auto & [outputName, status] : initialOutputs) { + if (!status.wanted) continue; + if (!status.known) + result.goals.insert( + worker.makeDrvOutputSubstitutionGoal( + DrvOutput{status.outputHash, outputName}, + buildMode == bmRepair ? Repair : NoRepair + ) + ); + else { + auto * cap = getDerivationCA(*drv); + result.goals.insert(worker.makePathSubstitutionGoal( + status.known->path, + buildMode == bmRepair ? Repair : NoRepair, + 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); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 1e3c4109d..dadda4de2 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -436,13 +436,23 @@ std::set LocalDerivationGoal::startBuilder() killSandbox(false); /* Right platform? */ - 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}", - drv->platform, - concatStringsSep(", ", parsedDrv->getRequiredSystemFeatures()), - worker.store.printStorePath(drvPath), - settings.thisSystem, - concatStringsSep(", ", worker.store.systemFeatures)); + if (!parsedDrv->canBuildLocally(worker.store)) { + 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(", ", worker.store.systemFeatures), + Uncolored(addendum)) + }); + } /* Create a temporary directory where the build will take place. */ diff --git a/tests/functional/meson.build b/tests/functional/meson.build index fb8d77a57..bb706ce9e 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -189,6 +189,7 @@ functional_tests_scripts = [ 'test-libstoreconsumer.sh', 'extra-sandbox-profile.sh', 'substitute-truncated-nar.sh', + 'regression-484.sh', ] # Plugin tests require shared libraries support. diff --git a/tests/functional/regression-484.sh b/tests/functional/regression-484.sh new file mode 100644 index 000000000..1aa039ac2 --- /dev/null +++ b/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" < 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