From c092fa4702215fdb61611c5dd28194401d056170 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 23 Sep 2020 16:30:42 +0200 Subject: Allow non-CA derivations to depend on CA derivations --- src/libstore/build/derivation-goal.cc | 12 ++++++- src/libstore/derivations.cc | 65 +++++++++++++++++++++++++++++++---- src/libstore/derivations.hh | 14 ++++++-- src/libstore/local-store.cc | 4 ++- 4 files changed, 84 insertions(+), 11 deletions(-) (limited to 'src/libstore') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index db0c2bb6c..cc8737fd5 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -493,7 +493,8 @@ void DerivationGoal::inputsRealised() if (useDerivation) { auto & fullDrv = *dynamic_cast(drv.get()); - if (!fullDrv.inputDrvs.empty() && fullDrv.type() == DerivationType::CAFloating) { + if ((!fullDrv.inputDrvs.empty() && + fullDrv.type() == DerivationType::CAFloating) || fullDrv.type() == DerivationType::DeferredInputAddressed) { /* We are be able to resolve this derivation based on the now-known results of dependencies. If so, we become a stub goal aliasing that resolved derivation goal */ @@ -3166,6 +3167,15 @@ void DerivationGoal::registerOutputs() [&](DerivationOutputCAFloating dof) { return newInfoFromCA(dof); }, + [&](DerivationOutputDeferred) { + // No derivation should reach that point without having been + // rewritten first + assert(false); + // Ugly, but the compiler insists on having this return a value + // of type `ValidPathInfo` despite the `assert(false)`, so + // let's provide it + return *(ValidPathInfo*)0; + }, }, output.output); /* Calculate where we'll move the output files. In the checking case we diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 07b4e772b..3b3a25391 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -21,6 +21,9 @@ std::optional DerivationOutput::path(const Store & store, std::string [](DerivationOutputCAFloating dof) -> std::optional { return std::nullopt; }, + [](DerivationOutputDeferred) -> std::optional { + return std::nullopt; + }, }, output); } @@ -37,6 +40,7 @@ bool derivationIsCA(DerivationType dt) { case DerivationType::InputAddressed: return false; case DerivationType::CAFixed: return true; case DerivationType::CAFloating: return true; + case DerivationType::DeferredInputAddressed: return false; }; // Since enums can have non-variant values, but making a `default:` would // disable exhaustiveness warnings. @@ -48,6 +52,7 @@ bool derivationIsFixed(DerivationType dt) { case DerivationType::InputAddressed: return false; case DerivationType::CAFixed: return true; case DerivationType::CAFloating: return false; + case DerivationType::DeferredInputAddressed: return false; }; assert(false); } @@ -57,6 +62,7 @@ bool derivationIsImpure(DerivationType dt) { case DerivationType::InputAddressed: return false; case DerivationType::CAFixed: return true; case DerivationType::CAFloating: return false; + case DerivationType::DeferredInputAddressed: return false; }; assert(false); } @@ -180,6 +186,11 @@ static DerivationOutput parseDerivationOutput(const Store & store, }; } } else { + if (pathS == "") { + return DerivationOutput { + .output = DerivationOutputDeferred { } + }; + } validatePath(pathS); return DerivationOutput { .output = DerivationOutputInputAddressed { @@ -325,6 +336,11 @@ string Derivation::unparse(const Store & store, bool maskOutputs, s += ','; printUnquotedString(s, makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); s += ','; printUnquotedString(s, ""); }, + [&](DerivationOutputDeferred) { + s += ','; printUnquotedString(s, ""); + s += ','; printUnquotedString(s, ""); + s += ','; printUnquotedString(s, ""); + } }, i.second.output); s += ')'; } @@ -389,7 +405,7 @@ std::string outputPathName(std::string_view drvName, std::string_view outputName DerivationType BasicDerivation::type() const { - std::set inputAddressedOutputs, fixedCAOutputs, floatingCAOutputs; + std::set inputAddressedOutputs, fixedCAOutputs, floatingCAOutputs, deferredIAOutputs; std::optional floatingHashType; for (auto & i : outputs) { std::visit(overloaded { @@ -408,22 +424,27 @@ DerivationType BasicDerivation::type() const throw Error("All floating outputs must use the same hash type"); } }, + [&](DerivationOutputDeferred _) { + deferredIAOutputs.insert(i.first); + }, }, i.second.output); } - if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty()) { + if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { throw Error("Must have at least one output"); - } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty()) { + } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { return DerivationType::InputAddressed; - } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.empty() && floatingCAOutputs.empty()) { + } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { if (fixedCAOutputs.size() > 1) // FIXME: Experimental feature? throw Error("Only one fixed output is allowed for now"); if (*fixedCAOutputs.begin() != "out") throw Error("Single fixed output must be named \"out\""); return DerivationType::CAFixed; - } else if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && ! floatingCAOutputs.empty()) { + } else if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && ! floatingCAOutputs.empty() && deferredIAOutputs.empty()) { return DerivationType::CAFloating; + } else if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && !deferredIAOutputs.empty()) { + return DerivationType::DeferredInputAddressed; } else { throw Error("Can't mix derivation output types"); } @@ -454,6 +475,8 @@ static const DrvHashModulo & pathDerivationModulo(Store & store, const StorePath return h->second; } +UnknownHashes unknownHashes; + /* See the header for interface details. These are the implementation details. For fixed-output derivations, each hash in the map is not the @@ -476,7 +499,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m /* Return a fixed hash for fixed-output derivations. */ switch (drv.type()) { case DerivationType::CAFloating: - throw Error("Regular input-addressed derivations are not yet allowed to depend on CA derivations"); + return unknownHashes; case DerivationType::CAFixed: { std::map outputHashes; for (const auto & i : drv.outputs) { @@ -491,12 +514,15 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m } case DerivationType::InputAddressed: break; + case DerivationType::DeferredInputAddressed: + break; } /* For other derivations, replace the inputs paths with recursive calls to this function. */ std::map inputs2; for (auto & i : drv.inputDrvs) { + bool hasUnknownHash = false; const auto & res = pathDerivationModulo(store, i.first); std::visit(overloaded { // Regular non-CA derivation, replace derivation @@ -514,7 +540,13 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m justOut); } }, + [&](UnknownHashes) { + hasUnknownHash = true; + }, }, res); + if (hasUnknownHash) { + return unknownHashes; + } } return hashString(htSHA256, drv.unparse(store, maskOutputs, &inputs2)); @@ -620,6 +652,11 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr << (makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)) << ""; }, + [&](DerivationOutputDeferred) { + out << "" + << "" + << ""; + }, }, i.second.output); } worker_proto::write(store, out, drv.inputSrcs); @@ -645,7 +682,6 @@ std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath } -// N.B. Outputs are left unchanged static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) { debug("Rewriting the derivation"); @@ -666,6 +702,21 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String newEnv.emplace(envName, envValue); } drv.env = newEnv; + + auto hashModulo = hashDerivationModulo(store, Derivation(drv), true); + for (auto & [outputName, output] : drv.outputs) { + if (std::holds_alternative(output.output)) { + Hash h = std::get(hashModulo); + auto outPath = store.makeOutputPath(outputName, h, drv.name); + drv.env[outputName] = store.printStorePath(outPath); + output = DerivationOutput { + .output = DerivationOutputInputAddressed { + .path = std::move(outPath), + }, + }; + } + } + } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 6d292b2e5..f9ba935e6 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -41,12 +41,18 @@ struct DerivationOutputCAFloating HashType hashType; }; +/* Input-addressed output which depends on a (CA) derivation whose hash isn't + * known atm + */ +struct DerivationOutputDeferred {}; + struct DerivationOutput { std::variant< DerivationOutputInputAddressed, DerivationOutputCAFixed, - DerivationOutputCAFloating + DerivationOutputCAFloating, + DerivationOutputDeferred > output; std::optional hashAlgoOpt(const Store & store) const; /* Note, when you use this function you should make sure that you're passing @@ -72,6 +78,7 @@ typedef std::map StringPairs; enum struct DerivationType : uint8_t { InputAddressed, + DeferredInputAddressed, CAFixed, CAFloating, }; @@ -167,9 +174,12 @@ std::string outputPathName(std::string_view drvName, std::string_view outputName // whose output hashes are always known since they are fixed up-front. typedef std::map CaOutputHashes; +struct UnknownHashes {}; + typedef std::variant< Hash, // regular DRV normalized hash - CaOutputHashes + CaOutputHashes, // Fixed-output derivation hashes + UnknownHashes // Deferred hashes for floating outputs drvs and their dependencies > DrvHashModulo; /* Returns hashes with the details of fixed-output subderivations diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index d29236a9c..d29a68179 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -573,6 +573,8 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat [&](DerivationOutputCAFloating _) { /* Nothing to check */ }, + [&](DerivationOutputDeferred) { + }, }, i.second.output); } } @@ -817,7 +819,7 @@ std::map> LocalStore::queryPartialDerivati } /* can't just use else-if instead of `!haveCached` because we need to unlock `drvPathResolutions` before it is locked in `Derivation::resolve`. */ - if (!haveCached && drv.type() == DerivationType::CAFloating) { + if (!haveCached && (drv.type() == DerivationType::CAFloating || drv.type() == DerivationType::DeferredInputAddressed)) { /* Try resolve drv and use that path instead. */ auto attempt = drv.tryResolve(*this); if (!attempt) -- cgit v1.2.3 From bc081bcd816542d66f1578788b93df4d7e07b135 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 24 Sep 2020 10:11:58 +0200 Subject: Inline `unkownHashes` See https://github.com/NixOS/nix/pull/4056#discussion_r493661632 --- src/libstore/derivations.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src/libstore') diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 3b3a25391..517ecfaa2 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -475,8 +475,6 @@ static const DrvHashModulo & pathDerivationModulo(Store & store, const StorePath return h->second; } -UnknownHashes unknownHashes; - /* See the header for interface details. These are the implementation details. For fixed-output derivations, each hash in the map is not the @@ -499,7 +497,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m /* Return a fixed hash for fixed-output derivations. */ switch (drv.type()) { case DerivationType::CAFloating: - return unknownHashes; + return UnknownHashes {}; case DerivationType::CAFixed: { std::map outputHashes; for (const auto & i : drv.outputs) { @@ -545,7 +543,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m }, }, res); if (hasUnknownHash) { - return unknownHashes; + return UnknownHashes {}; } } -- cgit v1.2.3