From 28d27c271ddcdaf407a95458cd29b9109b94030c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 22 Mar 2020 11:30:49 -0400 Subject: [PATCH] Separate derivation resolving from derivation hashing Use the newly parameterized derivation types to enforce the invariants needed to split these. --- src/libexpr/primops.cc | 81 ++++++++++++++++++++------------- src/libstore/derivations.cc | 90 +++++++++++++++++++++++-------------- src/libstore/derivations.hh | 14 ++++-- src/libstore/local-store.cc | 3 +- 4 files changed, 118 insertions(+), 70 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 8de23495187d..2215da68c643 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -557,7 +557,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * ignoreNulls = state.forceBool(*attr->value, pos); /* Build the derivation expression by processing the attributes. */ - Derivation drv; + DerivationT drv; PathSet context; @@ -715,45 +715,53 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * throw EvalError("derivation names are not allowed to end in '%s', at %s", drvExtension, posDrvName); if (outputHash) { - /* Handle fixed-output derivations. */ + /* Set "out" output with hash algo and hash for fixed-output derivations. */ if (outputs.size() != 1 || *(outputs.begin()) != "out") throw Error(format("multiple outputs are not supported in fixed-output derivations, at %1%") % posDrvName); HashType ht = outputHashAlgo.empty() ? htUnknown : parseHashType(outputHashAlgo); Hash h(*outputHash, ht); - - auto outPath = state.store->makeFixedOutputPath(outputHashRecursive, h, drvName); - if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign("out", DerivationOutput(std::move(outPath), - (outputHashRecursive ? "r:" : "") + printHashType(h.type), + drv.outputs.insert_or_assign("out", DerivationOutputT( + NoPath(), + (outputHashRecursive ? "r:" : "") + printHashType(ht), h.to_string(Base16, false))); } - else { - /* Compute a hash over the "masked" store derivation, which is - the final one except that in the list of outputs, the - output paths are empty strings, and the corresponding - environment variables have an empty value. This ensures - that changes in the set of output names do get reflected in - the hash. */ - for (auto & i : outputs) { - if (!jsonObject) drv.env[i] = ""; - drv.outputs.insert_or_assign(i, - DerivationOutput(StorePath::dummy.clone(), "", "")); - } - - Hash h = hashDerivationModulo(*state.store, Derivation(drv), true); - - for (auto & i : outputs) { - auto outPath = state.store->makeOutputPath(i, h, drvName); - if (!jsonObject) drv.env[i] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign(i, - DerivationOutput(std::move(outPath), "", "")); + /* Compute the final derivation, which additionally contains the outputs + paths created from the hash of the initial one. */ + Derivation drvFinal; + //drvFinal.inputSrcs = drv.inputSrcs; + for (auto & i : drv.inputSrcs) + drvFinal.inputSrcs.insert(i.clone()); + drvFinal.platform = drv.platform; + drvFinal.builder = drv.builder; + drvFinal.args = drv.args; + drvFinal.env = drv.env; + { + const auto drvOrPseudo = derivationModulo(*state.store, drv); + if (const DerivationT *pval = std::get_if<0>(&drvOrPseudo)) { + Hash hash = hashDerivation(*state.store, *pval); + for (auto & i : outputs) { + auto outPath = state.store->makeOutputPath(i, hash, drvName); + if (!jsonObject) drvFinal.env[i] = state.store->printStorePath(outPath); + drvFinal.outputs.insert_or_assign(i, + DerivationOutput(std::move(outPath), "", "")); + } + } else if (std::get_if<1>(&drvOrPseudo)) { + HashType ht = outputHashAlgo.empty() ? htUnknown : parseHashType(outputHashAlgo); + Hash h(*outputHash, ht); + auto outPath = state.store->makeFixedOutputPath(outputHashRecursive, h, drvName); + auto & output = drv.outputs.find("out")->second; + drvFinal.outputs.insert_or_assign("out", DerivationOutput( + std::move(outPath), + std::string(output.hashAlgo), + std::string(output.hash))); + if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath); } } /* Write the resulting term into the Nix store directory. */ - auto drvPath = writeDerivation(state.store, drv, drvName, state.repair); + auto drvPath = writeDerivation(state.store, drvFinal, drvName, state.repair); auto drvPathS = state.store->printStorePath(drvPath); printMsg(lvlChatty, "instantiated '%1%' -> '%2%'", drvName, drvPathS); @@ -761,12 +769,21 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * /* Optimisation, but required in read-only mode! because in that case we don't actually write store derivations, so we can't read them later. */ - drvHashes.insert_or_assign(drvPath.clone(), - hashDerivationModulo(*state.store, Derivation(drv), false)); + { + const auto drvOrPseudo = derivationModulo(*state.store, drvFinal); + Hash hash; + if (const DerivationT *pval = std::get_if<0>(&drvOrPseudo)) { + hash = hashDerivation(*state.store, *pval); + } else if (const std::string *pval = std::get_if<1>(&drvOrPseudo)) { + hash = hashString(htSHA256, *pval); + } + // Cache it + drvHashes.insert_or_assign(drvPath.clone(), std::move(hash)); + } - state.mkAttrs(v, 1 + drv.outputs.size()); + state.mkAttrs(v, 1 + drvFinal.outputs.size()); mkString(*state.allocAttr(v, state.sDrvPath), drvPathS, {"=" + drvPathS}); - for (auto & i : drv.outputs) { + for (auto & i : drvFinal.outputs) { mkString(*state.allocAttr(v, state.symbols.create(i.first)), state.store->printStorePath(i.second.path), {"!" + i.first + "!" + drvPathS}); } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index ed0ace9520ff..2f58cecb1e51 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -42,6 +42,12 @@ BasicDerivationT::BasicDerivationT(const BasicDerivationT & other) } +template +DerivationT::DerivationT(const BasicDerivationT & other) + : BasicDerivationT(other) +{ +} + template DerivationT::DerivationT(const DerivationT & other) : BasicDerivationT(other) @@ -78,7 +84,7 @@ StorePath writeDerivation(ref store, (that can be missing (of course) and should not necessarily be held during a garbage collection). */ auto suffix = std::string(name) + drvExtension; - auto contents = drv.unparse(*store, false); + auto contents = drv.unparse(*store); return settings.readOnlyMode ? store->computeStorePathForText(suffix, contents, references) : store->addTextToStore(suffix, contents, references, repair); @@ -283,8 +289,7 @@ static string printStoreDrvPath(const Store & store, const Hash & hash) { } template -string DerivationT::unparse(const Store & store, bool maskOutputs, - std::map * actualInputs) const +string DerivationT::unparse(const Store & store) const { string s; s.reserve(65536); @@ -294,7 +299,7 @@ string DerivationT::unparse(const Store & store, bool for (auto & i : this->outputs) { if (first) first = false; else s += ','; s += '('; printUnquotedString(s, i.first); - s += ','; printUnquotedString(s, maskOutputs ? "" : printStorePath(store, i.second.path)); + s += ','; printUnquotedString(s, printStorePath(store, i.second.path)); s += ','; printUnquotedString(s, i.second.hashAlgo); s += ','; printUnquotedString(s, i.second.hash); s += ')'; @@ -302,20 +307,11 @@ string DerivationT::unparse(const Store & store, bool s += "],["; first = true; - if (actualInputs) { - for (auto & i : *actualInputs) { - if (first) first = false; else s += ','; - s += '('; printUnquotedString(s, i.first); - s += ','; printUnquotedStrings(s, i.second.begin(), i.second.end()); - s += ')'; - } - } else { - for (auto & i : inputDrvs) { - if (first) first = false; else s += ','; - s += '('; printUnquotedString(s, printStoreDrvPath(store, i.first)); - s += ','; printUnquotedStrings(s, i.second.begin(), i.second.end()); - s += ')'; - } + for (auto & i : inputDrvs) { + if (first) first = false; else s += ','; + s += '('; printUnquotedString(s, printStoreDrvPath(store, i.first)); + s += ','; printUnquotedStrings(s, i.second.begin(), i.second.end()); + s += ')'; } s += "],"; @@ -330,8 +326,10 @@ string DerivationT::unparse(const Store & store, bool first = true; for (auto & i : this->env) { if (first) first = false; else s += ','; + auto o = this->outputs.find(i.first); s += '('; printString(s, i.first); - s += ','; printString(s, maskOutputs && this->outputs.count(i.first) ? "" : i.second); + // TODO use proper placeholders + s += ','; printString(s, o != this->outputs.end() ? "" : i.second); s += ')'; } @@ -340,6 +338,10 @@ string DerivationT::unparse(const Store & store, bool return s; } +template +Hash hashDerivation(Store & store, const DerivationT & drv) { + return hashString(htSHA256, drv.unparse(store)); +} // FIXME: remove bool isDerivation(const string & fileName) @@ -370,15 +372,19 @@ static const Hash & pathDerivationModulo(Store & store, const StorePath & drvPat auto h = drvHashes.find(drvPath); if (h == drvHashes.end()) { assert(store.isValidPath(drvPath)); - // Cache it - h = drvHashes.insert_or_assign( - drvPath.clone(), - hashDerivationModulo( + const std::variant, std::string> drvOrPseudo = derivationModulo( + store, + readDerivation( store, - readDerivation( - store, - store.toRealPath(store.printStorePath(drvPath))), - false)).first; + store.toRealPath(store.printStorePath(drvPath)))); + Hash hash; + if (const DerivationT *pval = std::get_if<0>(&drvOrPseudo)) { + hash = hashDerivation(store, *pval); + } else if (const std::string *pval = std::get_if<1>(&drvOrPseudo)) { + hash = hashString(htSHA256, *pval); + } + // Cache it + h = drvHashes.insert_or_assign(drvPath.clone(), hash).first; } return h->second; } @@ -403,26 +409,29 @@ static const Hash & pathDerivationModulo(Store & store, const StorePath & drvPat paths have been replaced by the result of a recursive call to this function, and that for fixed-output derivations we return a hash of its output path. */ -Hash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) +template +std::variant, string> derivationModulo( + Store & store, + const DerivationT & drv) { /* Return a fixed hash for fixed-output derivations. */ if (drv.isFixedOutput()) { - DerivationOutputs::const_iterator i = drv.outputs.begin(); - return hashString(htSHA256, "fixed:out:" + typename DerivationOutputsT::const_iterator i = drv.outputs.begin(); + return "fixed:out:" + i->second.hashAlgo + ":" + i->second.hash + ":" - + store.printStorePath(i->second.path)); + + printStorePath(store, i->second.path); } /* For other derivations, replace the inputs paths with recursive calls to this function. */ - std::map inputs2; + DerivationT drvNorm((BasicDerivationT)drv); for (auto & i : drv.inputDrvs) { const auto h = pathDerivationModulo(store, i.first); - inputs2.insert_or_assign(h.to_string(Base16, false), i.second); + drvNorm.inputDrvs.insert_or_assign(h, i.second); } - return hashString(htSHA256, drv.unparse(store, maskOutputs, &inputs2)); + return drvNorm; } @@ -502,6 +511,19 @@ template struct BasicDerivationT; template struct BasicDerivationT; template struct DerivationT; +template struct DerivationT; template struct DerivationT; +template +std::variant, std::string> derivationModulo( + Store & store, + const DerivationT & drv); +template +std::variant, std::string> derivationModulo( + Store & store, + const DerivationT & drv); + +template Hash hashDerivation(Store & store, const DerivationT & drv); +template Hash hashDerivation(Store & store, const DerivationT & drv); + } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 279f99e93614..e6bef0f712ca 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -87,11 +87,11 @@ struct DerivationT : BasicDerivationT DerivationInputsT inputDrvs; /* inputs that are sub-derivations */ /* Print a derivation. */ - std::string unparse(const Store & store, bool maskOutputs, - std::map * actualInputs = nullptr) const; + std::string unparse(const Store & store) const; DerivationT() { } DerivationT(DerivationT && other) = default; + DerivationT(const BasicDerivationT & other); explicit DerivationT(const DerivationT & other); }; @@ -111,7 +111,15 @@ Derivation readDerivation(const Store & store, const Path & drvPath); // FIXME: remove bool isDerivation(const string & fileName); -Hash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); +template +std::variant, string> derivationModulo( + Store & store, + const DerivationT & drv); + +template +Hash hashDerivation( + Store & store, + const DerivationT & drv); /* Memoisation of hashDerivationModulo(). */ typedef std::map DrvHashes; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index ae7513ad8154..408e0b27c4aa 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -564,7 +564,8 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat } else { - Hash h = hashDerivationModulo(*this, drv, true); + const Hash h = hashDerivation(*this, + std::get<0>(derivationModulo(*this, drv))); for (auto & i : drv.outputs) check(makeOutputPath(i.first, h, drvName), i.second.path, i.first); }