From 49b25ea85c9695a0668f65bff5839aa3feccd263 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 15 Jan 2024 08:17:42 +0100 Subject: [PATCH 1/2] refactor: Impure derivation type isPure -> isImpure To quote the method doc: Non-impure derivations can still behave impurely, to the degree permitted by the sandbox. Hence why this method isn't `isPure`: impure derivations are not the negation of pure derivations. Purity can not be ascertained except by rather heavy tools. --- src/libstore/build/derivation-goal.cc | 18 +++++++++--------- src/libstore/build/local-derivation-goal.cc | 2 +- src/libstore/derivations.cc | 10 +++++----- src/libstore/derivations.hh | 13 +++++++++---- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index f8728ed4a..00cbf4228 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -223,7 +223,7 @@ void DerivationGoal::haveDerivation() if (!drv->type().hasKnownOutputPaths()) experimentalFeatureSettings.require(Xp::CaDerivations); - if (!drv->type().isPure()) { + if (drv->type().isImpure()) { experimentalFeatureSettings.require(Xp::ImpureDerivations); for (auto & [outputName, output] : drv->outputs) { @@ -304,7 +304,7 @@ void DerivationGoal::outputsSubstitutionTried() { trace("all outputs substituted (maybe)"); - assert(drv->type().isPure()); + assert(!drv->type().isImpure()); if (nrFailed > 0 && nrFailed > nrNoSubstituters + nrIncompleteClosure && !settings.tryFallback) { done(BuildResult::TransientFailure, {}, @@ -397,9 +397,9 @@ void DerivationGoal::gaveUpOnSubstitution() for (const auto & [inputDrvPath, inputNode] : dynamic_cast(drv.get())->inputDrvs.map) { /* Ensure that pure, non-fixed-output derivations don't depend on impure derivations. */ - if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && drv->type().isPure() && !drv->type().isFixed()) { + if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && !drv->type().isImpure() && !drv->type().isFixed()) { auto inputDrv = worker.evalStore.readDerivation(inputDrvPath); - if (!inputDrv.type().isPure()) + if (inputDrv.type().isImpure()) throw Error("pure derivation '%s' depends on impure derivation '%s'", worker.store.printStorePath(drvPath), worker.store.printStorePath(inputDrvPath)); @@ -439,7 +439,7 @@ void DerivationGoal::gaveUpOnSubstitution() void DerivationGoal::repairClosure() { - assert(drv->type().isPure()); + assert(!drv->type().isImpure()); /* If we're repairing, we now know that our own outputs are valid. Now check whether the other paths in the outputs closure are @@ -1100,7 +1100,7 @@ void DerivationGoal::resolvedFinished() worker.store.printStorePath(resolvedDrvGoal->drvPath), outputName); }(); - if (drv->type().isPure()) { + if (!drv->type().isImpure()) { auto newRealisation = realisation; newRealisation.id = DrvOutput { initialOutput->outputHash, outputName }; newRealisation.signatures.clear(); @@ -1395,7 +1395,7 @@ void DerivationGoal::flushLine() std::map> DerivationGoal::queryPartialDerivationOutputMap() { - assert(drv->type().isPure()); + assert(!drv->type().isImpure()); if (!useDerivation || drv->type().hasKnownOutputPaths()) { std::map> res; for (auto & [name, output] : drv->outputs) @@ -1411,7 +1411,7 @@ std::map> DerivationGoal::queryPartialDeri OutputPathMap DerivationGoal::queryDerivationOutputMap() { - assert(drv->type().isPure()); + assert(!drv->type().isImpure()); if (!useDerivation || drv->type().hasKnownOutputPaths()) { OutputPathMap res; for (auto & [name, output] : drv->outputsAndOptPaths(worker.store)) @@ -1428,7 +1428,7 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap() std::pair DerivationGoal::checkPathValidity() { - if (!drv->type().isPure()) return { false, {} }; + if (drv->type().isImpure()) return { false, {} }; bool checkHash = buildMode == bmRepair; auto wantedOutputsLeft = std::visit(overloaded { diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index f85301950..2ba8be7d6 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2724,7 +2724,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() .outPath = newInfo.path }; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) - && drv->type().isPure()) + && !drv->type().isImpure()) { signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 2fafcb8e7..393806652 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -110,17 +110,17 @@ bool DerivationType::isSandboxed() const } -bool DerivationType::isPure() const +bool DerivationType::isImpure() const { return std::visit(overloaded { [](const InputAddressed & ia) { - return true; + return false; }, [](const ContentAddressed & ca) { - return true; + return false; }, [](const Impure &) { - return false; + return true; }, }, raw); } @@ -840,7 +840,7 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut }; } - if (!type.isPure()) { + if (type.isImpure()) { std::map outputHashes; for (const auto & [outputName, _] : drv.outputs) outputHashes.insert_or_assign(outputName, impureOutputHash); diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 2a326b578..522523e45 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -253,12 +253,17 @@ struct DerivationType { bool isSandboxed() const; /** - * Whether the derivation is expected to produce the same result - * every time, and therefore it only needs to be built once. This is - * only false for derivations that have the attribute '__impure = + * Whether the derivation is expected to produce a different result + * every time, and therefore it needs to be rebuilt every time. This is + * only true for derivations that have the attribute '__impure = * true'. + * + * Non-impure derivations can still behave impurely, to the degree permitted + * by the sandbox. Hence why this method isn't `isPure`: impure derivations + * are not the negation of pure derivations. Purity can not be ascertained + * except by rather heavy tools. */ - bool isPure() const; + bool isImpure() const; /** * Does the derivation knows its own output paths? From 6a99c18c304cd199950bf32d9b9cb07c0276f0b7 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 15 Jan 2024 08:18:53 +0100 Subject: [PATCH 2/2] doc/glossary: Define impure derivation --- doc/manual/src/glossary.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/manual/src/glossary.md b/doc/manual/src/glossary.md index 4507d8bf3..46cc5926c 100644 --- a/doc/manual/src/glossary.md +++ b/doc/manual/src/glossary.md @@ -156,6 +156,11 @@ builder can rely on external inputs such as the network or the system time) but the Nix model assumes it. +- [impure derivation]{#gloss-impure-derivation} + + [An experimental feature](#@docroot@/contributing/experimental-features.md#xp-feature-impure-derivations) that allows derivations to be explicitly marked as impure, + so that they are always rebuilt, and their outputs not reused by subsequent calls to realise them. + - [Nix database]{#gloss-nix-database} An SQlite database to track [reference]s between [store object]s.