From 2be64efb02e9eeb0fdacfec5d3314bf02ab13395 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 15 Mar 2020 02:23:17 -0400 Subject: Generalize `isFixedOutput` in preparation for CA drvs Today's fixed output derivations and regular derivations differ in a few ways which are largely orthogonal. This replaces `isFixedOutput` with a `type` that returns an enum of possible combinations. --- src/libstore/build.cc | 47 ++++++++++++++++++++++++++++----------------- src/libstore/derivations.cc | 45 ++++++++++++++++++++++++++++++++++++------- src/libstore/derivations.hh | 18 ++++++++++++++++- src/libstore/local-store.cc | 38 +++++++++++++++++++++--------------- 4 files changed, 107 insertions(+), 41 deletions(-) (limited to 'src') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 9c6aedfa5..914e888b7 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -795,8 +795,8 @@ private: /* RAII object to delete the chroot directory. */ std::shared_ptr autoDelChroot; - /* Whether this is a fixed-output derivation. */ - bool fixedOutput; + /* The sort of derivation we are building. */ + DerivationType derivationType; /* Whether to run the build in a private network namespace. */ bool privateNetwork = false; @@ -1369,12 +1369,12 @@ void DerivationGoal::inputsRealised() debug("added input paths %s", worker.store.showPaths(inputPaths)); - /* Is this a fixed-output derivation? */ - fixedOutput = drv->isFixedOutput(); + /* What type of derivation are we building? */ + derivationType = drv->type(); /* Don't repeat fixed-output derivations since they're already verified by their output hash.*/ - nrRounds = fixedOutput ? 1 : settings.buildRepeat + 1; + nrRounds = DtAxisFixed & derivationType ? 1 : settings.buildRepeat + 1; /* Okay, try to build. Note that here we don't wait for a build slot to become available, since we don't need one if there is a @@ -1724,7 +1724,7 @@ void DerivationGoal::buildDone() st = dynamic_cast(&e) ? BuildResult::NotDeterministic : statusOk(status) ? BuildResult::OutputRejected : - fixedOutput || diskFull ? BuildResult::TransientFailure : + DtAxisImpure & derivationType || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure; } @@ -1930,7 +1930,7 @@ void DerivationGoal::startBuilder() else if (settings.sandboxMode == smDisabled) useChroot = false; else if (settings.sandboxMode == smRelaxed) - useChroot = !fixedOutput && !noChroot; + useChroot = !(DtAxisImpure & derivationType) && !noChroot; } if (worker.store.storeDir != worker.store.realStoreDir) { @@ -2112,7 +2112,7 @@ void DerivationGoal::startBuilder() "nogroup:x:65534:\n") % sandboxGid).str()); /* Create /etc/hosts with localhost entry. */ - if (!fixedOutput) + if (!(DtAxisImpure & derivationType)) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); /* Make the closure of the inputs available in the chroot, @@ -2318,7 +2318,7 @@ void DerivationGoal::startBuilder() us. */ - if (!fixedOutput) + if (!(DtAxisImpure & derivationType)) privateNetwork = true; userNamespaceSync.create(); @@ -2519,7 +2519,7 @@ void DerivationGoal::initEnv() derivation, tell the builder, so that for instance `fetchurl' can skip checking the output. On older Nixes, this environment variable won't be set, so `fetchurl' will do the check. */ - if (fixedOutput) env["NIX_OUTPUT_CHECKED"] = "1"; + if (DtAxisFixed & derivationType) env["NIX_OUTPUT_CHECKED"] = "1"; /* *Only* if this is a fixed-output derivation, propagate the values of the environment variables specified in the @@ -2530,7 +2530,7 @@ void DerivationGoal::initEnv() to the builder is generally impure, but the output of fixed-output derivations is by definition pure (since we already know the cryptographic hash of the output). */ - if (fixedOutput) { + if (derivationType & DtAxisImpure) { for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) env[i] = getEnv(i).value_or(""); } @@ -3144,7 +3144,7 @@ void DerivationGoal::runChild() /* Fixed-output derivations typically need to access the network, so give them access to /etc/resolv.conf and so on. */ - if (fixedOutput) { + if (DtAxisImpure & derivationType) { ss.push_back("/etc/resolv.conf"); // Only use nss functions to resolve hosts and @@ -3385,7 +3385,7 @@ void DerivationGoal::runChild() sandboxProfile += "(import \"sandbox-defaults.sb\")\n"; - if (fixedOutput) + if (DtAxisImpure & derivationType) sandboxProfile += "(import \"sandbox-network.sb\")\n"; /* Our rwx outputs */ @@ -3644,10 +3644,10 @@ void DerivationGoal::registerOutputs() hash). */ std::string ca; - if (fixedOutput) { + if (i.second.hashAlgo != "") { - bool recursive; Hash h; - i.second.parseHashInfo(recursive, h); + bool recursive; HashType ht; + i.second.parseHashType(recursive, ht); if (!recursive) { /* The output path should be a regular file without execute permission. */ @@ -3658,11 +3658,16 @@ void DerivationGoal::registerOutputs() /* Check the hash. In hash mode, move the path produced by the derivation to its content-addressed location. */ - Hash h2 = recursive ? hashPath(h.type, actualPath).first : hashFile(h.type, actualPath); + Hash h2 = recursive ? hashPath(ht, actualPath).first : hashFile(ht, actualPath); auto dest = worker.store.makeFixedOutputPath(recursive, h2, i.second.path.name()); - if (h != h2) { + // true if ither floating CA, or incorrect fixed hash. + bool needsMove = true; + + if (i.second.hash != "") { + Hash h = Hash(i.second.hash, ht); + if (h != h2) { /* Throw an error after registering the path as valid. */ @@ -3670,7 +3675,13 @@ void DerivationGoal::registerOutputs() delayedException = std::make_exception_ptr( BuildError("hash mismatch in fixed-output derivation '%s':\n wanted: %s\n got: %s", worker.store.printStorePath(dest), h.to_string(SRI), h2.to_string(SRI))); + } else { + // matched the fixed hash, so no move needed. + needsMove = false; + } + } + if (needsMove) { Path actualDest = worker.store.toRealPath(worker.store.printStorePath(dest)); if (worker.store.isValidPath(dest)) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 205b90e55..4b72573bf 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -8,8 +8,12 @@ namespace nix { +// Avoid shadow +HashType parseHashAlgo(const string & s) { + return parseHashType(s); +} -void DerivationOutput::parseHashInfo(bool & recursive, Hash & hash) const +void DerivationOutput::parseHashType(bool & recursive, HashType & hashType) const { recursive = false; string algo = hashAlgo; @@ -19,10 +23,16 @@ void DerivationOutput::parseHashInfo(bool & recursive, Hash & hash) const algo = string(algo, 2); } - HashType hashType = parseHashType(algo); - if (hashType == htUnknown) + HashType hashType_loc = parseHashAlgo(algo); + if (hashType_loc == htUnknown) throw Error("unknown hash algorithm '%s'", algo); + hashType = hashType_loc; +} +void DerivationOutput::parseHashInfo(bool & recursive, Hash & hash) const +{ + HashType hashType; + parseHashType(recursive, hashType); hash = Hash(this->hash, hashType); } @@ -328,11 +338,28 @@ bool isDerivation(const string & fileName) } -bool BasicDerivation::isFixedOutput() const +DerivationType BasicDerivation::type() const { - return outputs.size() == 1 && + if (outputs.size() == 1 && outputs.begin()->first == "out" && - outputs.begin()->second.hash != ""; + outputs.begin()->second.hash != "") + { + return DtCAFixed; + } + + auto const algo = outputs.begin()->second.hashAlgo; + if (algo != "") { + throw Error("Invalid mix of CA and regular outputs"); + } + for (auto & i : outputs) { + if (i.second.hash != "") { + throw Error("Non-fixed-output derivation has fixed output"); + } + if (i.second.hashAlgo != "") { + throw Error("Invalid mix of CA and regular outputs"); + } + } + return DtRegular; } @@ -362,13 +389,17 @@ DrvHashes drvHashes; Hash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) { /* Return a fixed hash for fixed-output derivations. */ - if (drv.isFixedOutput()) { + switch (drv.type()) { + case DtCAFixed: { DerivationOutputs::const_iterator i = drv.outputs.begin(); return hashString(htSHA256, "fixed:out:" + i->second.hashAlgo + ":" + i->second.hash + ":" + store.printStorePath(i->second.path)); } + default: + break; + } /* For other derivations, replace the inputs paths with recursive calls to this function.*/ diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index c2df66229..85f52ff4c 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -22,6 +22,7 @@ struct DerivationOutput , hashAlgo(std::move(hashAlgo)) , hash(std::move(hash)) { } + void parseHashType(bool & recursive, HashType & hashType) const; void parseHashInfo(bool & recursive, Hash & hash) const; }; @@ -33,6 +34,21 @@ typedef std::map DerivationInputs; typedef std::map StringPairs; +// Bit: +// 7: regular vs ca +// 6: floating vs fixed hash if ca, regular always floating +// 5: pure vs impure if ca, regular always pure +// _: Unassigned +enum DerivationTypeAxis : uint8_t { + DtAxisCA = 0b10000000, + DtAxisFixed = 0b01000000, + DtAxisImpure = 0b00100000, +}; +enum DerivationType : uint8_t { + DtRegular = 0b0000000, + DtCAFixed = 0b11100000, +}; + struct BasicDerivation { DerivationOutputs outputs; /* keyed on symbolic IDs */ @@ -53,7 +69,7 @@ struct BasicDerivation bool isBuiltin() const; /* Return true iff this is a fixed-output derivation. */ - bool isFixedOutput() const; + DerivationType type() const; /* Return the output paths of a derivation. */ StorePathSet outputPaths() const; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index cd2e86f29..e048560bc 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -559,21 +559,29 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat }; - if (drv.isFixedOutput()) { - DerivationOutputs::const_iterator out = drv.outputs.find("out"); - if (out == drv.outputs.end()) - throw Error("derivation '%s' does not have an output named 'out'", printStorePath(drvPath)); - - bool recursive; Hash h; - out->second.parseHashInfo(recursive, h); - - check(makeFixedOutputPath(recursive, h, drvName), out->second.path, "out"); - } - - else { - Hash h = hashDerivationModulo(*this, drv, true); - for (auto & i : drv.outputs) - check(makeOutputPath(i.first, h, drvName), i.second.path, i.first); + // Don't need the answer, but do this anways to assert is proper + // combination. The code below is more general and naturally allows + // combinations that currently prohibited. + drv.type(); + + std::optional h; + for (auto & i : drv.outputs) { + if (i.second.hashAlgo == "") { + if (!h) { + // somewhat expensive so we do lazily + h = hashDerivationModulo(*this, drv, true); + } + StorePath path = makeOutputPath(i.first, *h, drvName); + check(path, i.second.path, i.first); + } else { + if (i.second.hash == "") { + throw Error("Fixed output derivation needs hash"); + } + bool recursive; Hash h; + i.second.parseHashInfo(recursive, h); + StorePath path = makeFixedOutputPath(recursive, h, drvName); + check(path, i.second.path, i.first); + } } } -- cgit v1.2.3 From e5178fd22d35d58be00902b5425359b8b33019a0 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 16 Mar 2020 16:40:13 -0400 Subject: Fix typos Thanks @asymmetric! Co-Authored-By: asymmetric --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index e048560bc..743c56cf3 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -561,7 +561,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat // Don't need the answer, but do this anways to assert is proper // combination. The code below is more general and naturally allows - // combinations that currently prohibited. + // combinations that are currently prohibited. drv.type(); std::optional h; -- cgit v1.2.3 From 049179ba0776e293cd478cbb86ce7a167b64cdb0 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 18 Mar 2020 19:07:05 -0400 Subject: Fix typos Thanks @asymmetric I failed to do them all in one batch Co-Authored-By: asymmetric --- src/libstore/build.cc | 2 +- src/libstore/local-store.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 914e888b7..1c95038cf 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3662,7 +3662,7 @@ void DerivationGoal::registerOutputs() auto dest = worker.store.makeFixedOutputPath(recursive, h2, i.second.path.name()); - // true if ither floating CA, or incorrect fixed hash. + // true if either floating CA, or incorrect fixed hash. bool needsMove = true; if (i.second.hash != "") { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 743c56cf3..774027a51 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -559,7 +559,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat }; - // Don't need the answer, but do this anways to assert is proper + // Don't need the answer, but do this anyways to assert is proper // combination. The code below is more general and naturally allows // combinations that are currently prohibited. drv.type(); -- cgit v1.2.3 From f1cf3ab870343a6894c08e2bb893ea69badfc397 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 19 Mar 2020 00:37:57 -0400 Subject: hashDerivationModulo: Generalize for multiple fixed ouputs per drv See documentattion in header and comments in implementation for details. This is actually done in preparation for floating ca derivations, not multi-output fixed ca derivations, but the distinction doesn't yet mattter. Thanks @cole-h for finding and fixing a bunch of typos. --- src/libexpr/primops.cc | 4 +- src/libstore/derivations.cc | 98 ++++++++++++++++++++++++++++++--------------- src/libstore/derivations.hh | 33 ++++++++++++++- src/libstore/local-store.cc | 4 +- 4 files changed, 102 insertions(+), 37 deletions(-) (limited to 'src') diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 8de234951..c9a16784e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -742,7 +742,9 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * DerivationOutput(StorePath::dummy.clone(), "", "")); } - Hash h = hashDerivationModulo(*state.store, Derivation(drv), true); + // Regular, non-CA derivation should always return a single hash and not + // hash per output. + Hash h = std::get<0>(hashDerivationModulo(*state.store, Derivation(drv), true)); for (auto & i : outputs) { auto outPath = state.store->makeOutputPath(i, h, drvName); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 205b90e55..13f2b4770 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -338,49 +338,81 @@ bool BasicDerivation::isFixedOutput() const DrvHashes drvHashes; +/* pathDerivationModulo and hashDerivationModulo are mutually recursive + */ -/* Returns the hash of a derivation modulo fixed-output - subderivations. A fixed-output derivation is a derivation with one - output (`out') for which an expected hash and hash algorithm are - specified (using the `outputHash' and `outputHashAlgo' - attributes). We don't want changes to such derivations to - propagate upwards through the dependency graph, changing output - paths everywhere. - - For instance, if we change the url in a call to the `fetchurl' - function, we do not want to rebuild everything depending on it - (after all, (the hash of) the file being downloaded is unchanged). - So the *output paths* should not change. On the other hand, the - *derivation paths* should change to reflect the new dependency - graph. - - That's what this function does: it returns a hash which is just the - hash of the derivation ATerm, except that any input derivation - 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) +/* Look up the derivation by value and memoize the + `hashDerivationModulo` call. + */ +static DrvHashModulo & pathDerivationModulo(Store & store, const StorePath & drvPath) +{ + auto h = drvHashes.find(drvPath); + if (h == drvHashes.end()) { + assert(store.isValidPath(drvPath)); + // Cache it + h = drvHashes.insert_or_assign( + drvPath.clone(), + hashDerivationModulo( + store, + readDerivation( + store, + store.toRealPath(store.printStorePath(drvPath))), + false)).first; + } + return h->second; +} + +/* See the header for interface details. These are the implementation details. + + For fixed ouput derivations, each hash in the map is not the + corresponding output's content hash, but a hash of that hash along + with other constant data. The key point is that the value is a pure + function of the output's contents, and there are no preimage attacks + spoofing an either an output's contents for a derivation, or + derivation for an output's contents. + + For regular derivations, it looks up each subderivation from its hash + and recurs. If the subderivation is also regular, it simply + substitutes the derivation path with its hash. If the subderivation + is fixed-output, however, it takes each output hash and pretends it + is a derivation hash producing a single "out" output. This is so we + don't leak the provenance of fixed outputs, reducing pointless cache + misses as the build itself won't know this. + */ +DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) { /* Return a fixed hash for fixed-output derivations. */ if (drv.isFixedOutput()) { - DerivationOutputs::const_iterator i = drv.outputs.begin(); - return hashString(htSHA256, "fixed:out:" - + i->second.hashAlgo + ":" - + i->second.hash + ":" - + store.printStorePath(i->second.path)); + std::map outputHashes; + for (const auto & i : drv.outputs) { + const Hash h = hashString(htSHA256, "fixed:out:" + + i.second.hashAlgo + ":" + + i.second.hash + ":" + + store.printStorePath(i.second.path)); + outputHashes.insert_or_assign(std::string(i.first), std::move(h)); + } + return outputHashes; } /* For other derivations, replace the inputs paths with recursive - calls to this function.*/ + calls to this function. */ std::map inputs2; for (auto & i : drv.inputDrvs) { - auto h = drvHashes.find(i.first); - if (h == drvHashes.end()) { - assert(store.isValidPath(i.first)); - h = drvHashes.insert_or_assign(i.first.clone(), hashDerivationModulo(store, - readDerivation(store, store.toRealPath(store.printStorePath(i.first))), false)).first; + const auto res = pathDerivationModulo(store, i.first); + if (const Hash *pval = std::get_if<0>(&res)) { + // regular non-CA derivation, replace derivation + inputs2.insert_or_assign(pval->to_string(Base16, false), i.second); + } else if (const std::map *pval = std::get_if<1>(&res)) { + // CA derivation's output hashes + std::set justOut = { std::string("out") }; + for (auto & output : i.second) { + /* Put each one in with a single "out" output.. */ + const auto h = pval->at(output); + inputs2.insert_or_assign( + h.to_string(Base16, false), + justOut); + } } - inputs2.insert_or_assign(h->second.to_string(Base16, false), i.second); } return hashString(htSHA256, drv.unparse(store, maskOutputs, &inputs2)); diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index c2df66229..c021bf907 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -5,6 +5,7 @@ #include "store-api.hh" #include +#include namespace nix { @@ -87,10 +88,38 @@ Derivation readDerivation(const Store & store, const Path & drvPath); // FIXME: remove bool isDerivation(const string & fileName); -Hash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); +typedef std::variant< + Hash, // regular DRV normalized hash + std::map // known CA drv's output hashes +> DrvHashModulo; + +/* Returns hashes with the details of fixed-output subderivations + expunged. + + A fixed-output derivation is a derivation whose outputs have a + specified content hash and hash algorithm. (Currently they must have + exactly one output (`out'), which is specified using the `outputHash' + and `outputHashAlgo' attributes, but the algorithm doesn't assume + this). We don't want changes to such derivations to propagate upwards + through the dependency graph, changing output paths everywhere. + + For instance, if we change the url in a call to the `fetchurl' + function, we do not want to rebuild everything depending on it (after + all, (the hash of) the file being downloaded is unchanged). So the + *output paths* should not change. On the other hand, the *derivation + paths* should change to reflect the new dependency graph. + + For fixed output derivations, this returns a map from the names of + each output to hashes unique up to the outputs' contents. + + For regular derivations, it returns a single hash of the derivation + ATerm, after subderivations have been likewise expunged from that + derivation. + */ +DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); /* Memoisation of hashDerivationModulo(). */ -typedef std::map DrvHashes; +typedef std::map DrvHashes; extern DrvHashes drvHashes; // FIXME: global, not thread-safe diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index cd2e86f29..8639cbf20 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -571,7 +571,9 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat } else { - Hash h = hashDerivationModulo(*this, drv, true); + // Regular, non-CA derivation should always return a single hash and not + // hash per output. + Hash h = std::get<0>(hashDerivationModulo(*this, drv, true)); for (auto & i : drv.outputs) check(makeOutputPath(i.first, h, drvName), i.second.path, i.first); } -- cgit v1.2.3 From d5b3328dd1e3de8910b237d54fb9dbf36629870f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 19 Mar 2020 23:37:52 -0400 Subject: Apply suggestions from code review Co-Authored-By: Cole Helbling --- src/libstore/derivations.cc | 6 +++--- src/libstore/derivations.hh | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 13f2b4770..4c51bdef3 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -364,12 +364,12 @@ static DrvHashModulo & pathDerivationModulo(Store & store, const StorePath & drv /* See the header for interface details. These are the implementation details. - For fixed ouput derivations, each hash in the map is not the + For fixed-output derivations, each hash in the map is not the corresponding output's content hash, but a hash of that hash along with other constant data. The key point is that the value is a pure function of the output's contents, and there are no preimage attacks - spoofing an either an output's contents for a derivation, or - derivation for an output's contents. + either spoofing an output's contents for a derivation, or + spoofing a derivation for an output's contents. For regular derivations, it looks up each subderivation from its hash and recurs. If the subderivation is also regular, it simply diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index c021bf907..9f8b7a23e 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -100,14 +100,14 @@ typedef std::variant< specified content hash and hash algorithm. (Currently they must have exactly one output (`out'), which is specified using the `outputHash' and `outputHashAlgo' attributes, but the algorithm doesn't assume - this). We don't want changes to such derivations to propagate upwards + this.) We don't want changes to such derivations to propagate upwards through the dependency graph, changing output paths everywhere. For instance, if we change the url in a call to the `fetchurl' - function, we do not want to rebuild everything depending on it (after - all, (the hash of) the file being downloaded is unchanged). So the - *output paths* should not change. On the other hand, the *derivation - paths* should change to reflect the new dependency graph. + function, we do not want to rebuild everything depending on it---after + all, (the hash of) the file being downloaded is unchanged. So the + *output paths* should not change. On the other hand, the *derivation + paths* should change to reflect the new dependency graph. For fixed output derivations, this returns a map from the names of each output to hashes unique up to the outputs' contents. -- cgit v1.2.3 From e3173242362c8421429633c0e9f64ab0211760bd Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 19 Mar 2020 23:38:51 -0400 Subject: Apply suggestions from code review --- src/libstore/derivations.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 9f8b7a23e..5e708642e 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -109,8 +109,8 @@ typedef std::variant< *output paths* should not change. On the other hand, the *derivation paths* should change to reflect the new dependency graph. - For fixed output derivations, this returns a map from the names of - each output to hashes unique up to the outputs' contents. + For fixed-output derivations, this returns a map from the name of + each output to its hash, unique up to the output's contents. For regular derivations, it returns a single hash of the derivation ATerm, after subderivations have been likewise expunged from that -- cgit v1.2.3 From 25004030591b3d5a6ce5219f36309ac05344c92b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 3 Jun 2020 17:38:54 +0000 Subject: Use enum and predicates rather than bitfile for derivation type --- src/libstore/build.cc | 18 +++++++++--------- src/libstore/derivations.cc | 32 +++++++++++++++++++++++++++++--- src/libstore/derivations.hh | 29 ++++++++++++++++------------- 3 files changed, 54 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 3c9c973d3..9dc824ecb 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1382,7 +1382,7 @@ void DerivationGoal::inputsRealised() /* Don't repeat fixed-output derivations since they're already verified by their output hash.*/ - nrRounds = DtAxisFixed & derivationType ? 1 : settings.buildRepeat + 1; + nrRounds = derivationIsFixed(derivationType) ? 1 : settings.buildRepeat + 1; /* Okay, try to build. Note that here we don't wait for a build slot to become available, since we don't need one if there is a @@ -1760,7 +1760,7 @@ void DerivationGoal::buildDone() st = dynamic_cast(&e) ? BuildResult::NotDeterministic : statusOk(status) ? BuildResult::OutputRejected : - DtAxisImpure & derivationType || diskFull ? BuildResult::TransientFailure : + derivationIsImpure(derivationType) || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure; } @@ -1966,7 +1966,7 @@ void DerivationGoal::startBuilder() else if (settings.sandboxMode == smDisabled) useChroot = false; else if (settings.sandboxMode == smRelaxed) - useChroot = !(DtAxisImpure & derivationType) && !noChroot; + useChroot = !(derivationIsImpure(derivationType)) && !noChroot; } if (worker.store.storeDir != worker.store.realStoreDir) { @@ -2132,7 +2132,7 @@ void DerivationGoal::startBuilder() "nogroup:x:65534:\n") % sandboxGid).str()); /* Create /etc/hosts with localhost entry. */ - if (!(DtAxisImpure & derivationType)) + if (!(derivationIsImpure(derivationType))) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); /* Make the closure of the inputs available in the chroot, @@ -2341,7 +2341,7 @@ void DerivationGoal::startBuilder() us. */ - if (!(DtAxisImpure & derivationType)) + if (!(derivationIsImpure(derivationType))) privateNetwork = true; userNamespaceSync.create(); @@ -2542,7 +2542,7 @@ void DerivationGoal::initEnv() derivation, tell the builder, so that for instance `fetchurl' can skip checking the output. On older Nixes, this environment variable won't be set, so `fetchurl' will do the check. */ - if (DtAxisFixed & derivationType) env["NIX_OUTPUT_CHECKED"] = "1"; + if (derivationIsFixed(derivationType)) env["NIX_OUTPUT_CHECKED"] = "1"; /* *Only* if this is a fixed-output derivation, propagate the values of the environment variables specified in the @@ -2553,7 +2553,7 @@ void DerivationGoal::initEnv() to the builder is generally impure, but the output of fixed-output derivations is by definition pure (since we already know the cryptographic hash of the output). */ - if (derivationType & DtAxisImpure) { + if (derivationIsImpure(derivationType)) { for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) env[i] = getEnv(i).value_or(""); } @@ -3167,7 +3167,7 @@ void DerivationGoal::runChild() /* Fixed-output derivations typically need to access the network, so give them access to /etc/resolv.conf and so on. */ - if (DtAxisImpure & derivationType) { + if (derivationIsImpure(derivationType)) { ss.push_back("/etc/resolv.conf"); // Only use nss functions to resolve hosts and @@ -3408,7 +3408,7 @@ void DerivationGoal::runChild() sandboxProfile += "(import \"sandbox-defaults.sb\")\n"; - if (DtAxisImpure & derivationType) + if (derivationIsImpure(derivationType)) sandboxProfile += "(import \"sandbox-network.sb\")\n"; /* Our rwx outputs */ diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index e99515bb5..224637e64 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -37,6 +37,32 @@ void DerivationOutput::parseHashInfo(FileIngestionMethod & recursive, Hash & has } +bool derivationIsCA(DerivationType dt) { + switch (dt) { + case DerivationType::Regular: return false; + case DerivationType::CAFixed: return true; + }; + // Since enums can have non-variant values, but making a `default:` would + // disable exhaustiveness warnings. + abort(); +} + +bool derivationIsFixed(DerivationType dt) { + switch (dt) { + case DerivationType::Regular: return false; + case DerivationType::CAFixed: return true; + }; + abort(); +} + +bool derivationIsImpure(DerivationType dt) { + switch (dt) { + case DerivationType::Regular: return false; + case DerivationType::CAFixed: return true; + }; + abort(); +} + BasicDerivation::BasicDerivation(const BasicDerivation & other) : platform(other.platform) , builder(other.builder) @@ -344,7 +370,7 @@ DerivationType BasicDerivation::type() const outputs.begin()->first == "out" && outputs.begin()->second.hash != "") { - return DtCAFixed; + return DerivationType::CAFixed; } auto const algo = outputs.begin()->second.hashAlgo; @@ -359,7 +385,7 @@ DerivationType BasicDerivation::type() const throw Error("Invalid mix of CA and regular outputs"); } } - return DtRegular; + return DerivationType::Regular; } @@ -390,7 +416,7 @@ Hash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutput { /* Return a fixed hash for fixed-output derivations. */ switch (drv.type()) { - case DtCAFixed: { + case DerivationType::CAFixed: { DerivationOutputs::const_iterator i = drv.outputs.begin(); return hashString(htSHA256, "fixed:out:" + i->second.hashAlgo + ":" diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 5a8d0d69c..1e0ee719d 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -34,21 +34,24 @@ typedef std::map DerivationInputs; typedef std::map StringPairs; -// Bit: -// 7: regular vs ca -// 6: floating vs fixed hash if ca, regular always floating -// 5: pure vs impure if ca, regular always pure -// _: Unassigned -enum DerivationTypeAxis : uint8_t { - DtAxisCA = 0b10000000, - DtAxisFixed = 0b01000000, - DtAxisImpure = 0b00100000, -}; -enum DerivationType : uint8_t { - DtRegular = 0b0000000, - DtCAFixed = 0b11100000, +enum struct DerivationType : uint8_t { + Regular, + CAFixed, }; +/* Do the outputs of the derivation have paths calculated from their content, + or from the derivation itself? */ +bool derivationIsCA(DerivationType); + +/* Is the content of the outputs fixed a-priori via a hash? Never true for + non-CA derivations. */ +bool derivationIsFixed(DerivationType); + +/* Is the derivation impure and needs to access non-deterministic resources, or + pure and can be sandboxed? Note that whether or not we actually sandbox the + derivation is controlled separately. Never true for non-CA derivations. */ +bool derivationIsImpure(DerivationType); + struct BasicDerivation { DerivationOutputs outputs; /* keyed on symbolic IDs */ -- cgit v1.2.3 From 3a9e4c32624b36b70cf8d553fd76a85ee97773ab Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 3 Jun 2020 18:50:45 +0000 Subject: Don't anticipate CA but not fixed outputs for now --- src/libstore/build.cc | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 9dc824ecb..0b9a022df 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3690,10 +3690,10 @@ void DerivationGoal::registerOutputs() hash). */ std::string ca; - if (i.second.hashAlgo != "") { + if (derivationIsFixed(derivationType)) { - FileIngestionMethod outputHashMode; HashType ht; - i.second.parseHashType(outputHashMode, ht); + FileIngestionMethod outputHashMode; Hash h; + i.second.parseHashInfo(outputHashMode, h); if (outputHashMode == FileIngestionMethod::Flat) { /* The output path should be a regular file without execute permission. */ @@ -3706,17 +3706,12 @@ void DerivationGoal::registerOutputs() /* Check the hash. In hash mode, move the path produced by the derivation to its content-addressed location. */ Hash h2 = outputHashMode == FileIngestionMethod::Recursive - ? hashPath(ht, actualPath).first - : hashFile(ht, actualPath); + ? hashPath(h.type, actualPath).first + : hashFile(h.type, actualPath); auto dest = worker.store.makeFixedOutputPath(outputHashMode, h2, i.second.path.name()); - // true if either floating CA, or incorrect fixed hash. - bool needsMove = true; - - if (i.second.hash != "") { - Hash h = Hash(i.second.hash, ht); - if (h != h2) { + if (h != h2) { /* Throw an error after registering the path as valid. */ @@ -3724,13 +3719,7 @@ void DerivationGoal::registerOutputs() delayedException = std::make_exception_ptr( BuildError("hash mismatch in fixed-output derivation '%s':\n wanted: %s\n got: %s", worker.store.printStorePath(dest), h.to_string(SRI), h2.to_string(SRI))); - } else { - // matched the fixed hash, so no move needed. - needsMove = false; - } - } - if (needsMove) { Path actualDest = worker.store.Store::toRealPath(dest); if (worker.store.isValidPath(dest)) -- cgit v1.2.3 From 74b251b2f3d6414de051c8523011c0ee3c5ea154 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 3 Jun 2020 18:53:04 +0000 Subject: Don't anticipate multiple CA outputs for now --- src/libstore/local-store.cc | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 1db450ee8..80b48d308 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -552,29 +552,21 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat }; - // Don't need the answer, but do this anyways to assert is proper - // combination. The code below is more general and naturally allows - // combinations that are currently prohibited. - drv.type(); - - std::optional h; - for (auto & i : drv.outputs) { - if (i.second.hashAlgo == "") { - if (!h) { - // somewhat expensive so we do lazily - h = hashDerivationModulo(*this, drv, true); - } - StorePath path = makeOutputPath(i.first, *h, drvName); - check(path, i.second.path, i.first); - } else { - if (i.second.hash == "") { - throw Error("Fixed output derivation needs hash"); - } - FileIngestionMethod recursive; Hash h; - i.second.parseHashInfo(recursive, h); - StorePath path = makeFixedOutputPath(recursive, h, drvName); - check(path, i.second.path, i.first); - } + if (derivationIsFixed(drv.type())) { + DerivationOutputs::const_iterator out = drv.outputs.find("out"); + if (out == drv.outputs.end()) + throw Error("derivation '%s' does not have an output named 'out'", printStorePath(drvPath)); + + FileIngestionMethod method; Hash h; + out->second.parseHashInfo(method, h); + + check(makeFixedOutputPath(method, h, drvName), out->second.path, "out"); + } + + else { + Hash h = hashDerivationModulo(*this, drv, true); + for (auto & i : drv.outputs) + check(makeOutputPath(i.first, h, drvName), i.second.path, i.first); } } -- cgit v1.2.3 From bf9f040112c33213910faef40077122f1932d462 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 21 Jun 2020 16:51:39 +0000 Subject: Tweak declaration I think this is clearer --- src/libstore/derivations.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 1f64086d7..ea57be0aa 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -404,7 +404,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m }, // CA derivation's output hashes [&](CaOutputHashes outputHashes) { - std::set justOut = { std::string("out") }; + std::set justOut = { "out" }; for (auto & output : i.second) { /* Put each one in with a single "out" output.. */ const auto h = outputHashes.at(output); -- cgit v1.2.3 From 3804e3df9bb479fe1d399f29d16a1aabaf352c19 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 21 Jun 2020 21:05:37 +0000 Subject: Don't anticipate hash algo without hash in derivation for now When we merge with master, the new lack of string types make this case impossible (after parsing). Later, when we actually implemenent CA-derivations, we'll change the types to allow that. --- src/libstore/derivations.cc | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 9864cf63e..f985e7ae5 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -359,21 +359,9 @@ DerivationType BasicDerivation::type() const outputs.begin()->second.hash != "") { return DerivationType::CAFixed; + } else { + return DerivationType::Regular; } - - auto const algo = outputs.begin()->second.hashAlgo; - if (algo != "") { - throw Error("Invalid mix of CA and regular outputs"); - } - for (auto & i : outputs) { - if (i.second.hash != "") { - throw Error("Non-fixed-output derivation has fixed output"); - } - if (i.second.hashAlgo != "") { - throw Error("Invalid mix of CA and regular outputs"); - } - } - return DerivationType::Regular; } -- cgit v1.2.3 From ffc18583b1a086849ac0efd17da40ff510299b52 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 22:15:14 +0000 Subject: Move C++17 "pattern matching" boilerplat to utils.hh --- src/libstore/content-address.cc | 4 ---- src/libstore/derivations.cc | 4 ---- src/libstore/store-api.cc | 4 ---- src/libutil/util.hh | 5 +++++ 4 files changed, 5 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 6cb69d0a9..f45f77d5c 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -24,10 +24,6 @@ std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) + hash.to_string(Base32, true); } -// FIXME Put this somewhere? -template struct overloaded : Ts... { using Ts::operator()...; }; -template overloaded(Ts...) -> overloaded; - std::string renderContentAddress(ContentAddress ca) { return std::visit(overloaded { [](TextHash th) { diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index d267468af..ce2025933 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -8,10 +8,6 @@ namespace nix { -// FIXME Put this somewhere? -template struct overloaded : Ts... { using Ts::operator()...; }; -template overloaded(Ts...) -> overloaded; - StorePath DerivationOutput::path(const Store & store, std::string_view drvName) const { return std::visit(overloaded { diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 8d46bb436..62514d3be 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -802,10 +802,6 @@ void ValidPathInfo::sign(const Store & store, const SecretKey & secretKey) sigs.insert(secretKey.signDetached(fingerprint(store))); } -// FIXME Put this somewhere? -template struct overloaded : Ts... { using Ts::operator()...; }; -template overloaded(Ts...) -> overloaded; - bool ValidPathInfo::isContentAddressed(const Store & store) const { if (! ca) return false; diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 3641daaec..d38657843 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -597,4 +597,9 @@ constexpr auto enumerate(T && iterable) } +// C++17 std::visit boilerplate +template struct overloaded : Ts... { using Ts::operator()...; }; +template overloaded(Ts...) -> overloaded; + + } -- cgit v1.2.3 From fedfc913ad75984b476e7838d6254c547f9134d5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 16:12:21 +0000 Subject: Use more std::visit to prepare for new variant N.B. not using `std::visit` for fetchurl because there is no attempt to handle all the cases (e.g. no `else`) and lambda complicates early return. --- src/libstore/derivations.cc | 32 +++++++++++++++++++------------- src/libstore/derivations.hh | 5 ++++- src/nix/show-derivation.cc | 13 +++++++++---- 3 files changed, 32 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index ce2025933..09683a005 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -283,13 +283,16 @@ string Derivation::unparse(const Store & store, bool maskOutputs, if (first) first = false; else s += ','; s += '('; printUnquotedString(s, i.first); s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(i.second.path(store, name))); - if (auto hash = std::get_if(&i.second.output)) { - s += ','; printUnquotedString(s, hash->hash.printMethodAlgo()); - s += ','; printUnquotedString(s, hash->hash.hash.to_string(Base16, false)); - } else { - s += ','; printUnquotedString(s, ""); - s += ','; printUnquotedString(s, ""); - } + std::visit(overloaded { + [&](DerivationOutputInputAddressed doi) { + s += ','; printUnquotedString(s, ""); + s += ','; printUnquotedString(s, ""); + }, + [&](DerivationOutputFixed dof) { + s += ','; printUnquotedString(s, dof.hash.printMethodAlgo()); + s += ','; printUnquotedString(s, dof.hash.hash.to_string(Base16, false)); + }, + }, i.second.output); s += ')'; } @@ -503,12 +506,15 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr for (auto & i : drv.outputs) { out << i.first << store.printStorePath(i.second.path(store, drv.name)); - if (auto hash = std::get_if(&i.second.output)) { - out << hash->hash.printMethodAlgo() - << hash->hash.hash.to_string(Base16, false); - } else { - out << "" << ""; - } + std::visit(overloaded { + [&](DerivationOutputInputAddressed doi) { + out << "" << ""; + }, + [&](DerivationOutputFixed dof) { + out << dof.hash.printMethodAlgo() + << dof.hash.hash.to_string(Base16, false); + }, + }, i.second.output); } writeStorePaths(store, out, drv.inputSrcs); out << drv.platform << drv.builder << drv.args; diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index fd8828373..4dc542536 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -25,7 +25,10 @@ struct DerivationOutputFixed struct DerivationOutput { - std::variant output; + std::variant< + DerivationOutputInputAddressed, + DerivationOutputFixed + > output; StorePath path(const Store & store, std::string_view drvName) const; }; diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index a868023d4..f9952e177 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -70,10 +70,15 @@ struct CmdShowDerivation : InstallablesCommand for (auto & output : drv.outputs) { auto outputObj(outputsObj.object(output.first)); outputObj.attr("path", store->printStorePath(output.second.path(*store, drv.name))); - if (auto hash = std::get_if(&output.second.output)) { - outputObj.attr("hashAlgo", hash->hash.printMethodAlgo()); - outputObj.attr("hash", hash->hash.hash.to_string(Base16, false)); - } + + std::visit(overloaded { + [&](DerivationOutputInputAddressed doi) { + }, + [&](DerivationOutputFixed dof) { + outputObj.attr("hashAlgo", dof.hash.printMethodAlgo()); + outputObj.attr("hash", dof.hash.hash.to_string(Base16, false)); + }, + }, output.second.output); } } -- cgit v1.2.3 From 230c9b4329b3d285e57f4cce058c121256187da1 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 16:12:21 +0000 Subject: Change types to prepare the way for CA derivations We've added the variant to `DerivationOutput` to support them, but made `DerivationOutput::path` partial to avoid actually implementing them. With this chage, we can all collaborate on "just" removing `DerivationOutput::path` calls to implement CA derivations. --- src/libstore/derivations.cc | 71 +++++++++++++++++++++++++++++++-------------- src/libstore/derivations.hh | 21 ++++++++++++-- src/nix/show-derivation.cc | 3 ++ 3 files changed, 71 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 09683a005..375d089ec 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -8,15 +8,20 @@ namespace nix { -StorePath DerivationOutput::path(const Store & store, std::string_view drvName) const +std::optional DerivationOutput::pathOpt(const Store & store, std::string_view drvName) const { return std::visit(overloaded { - [](DerivationOutputInputAddressed doi) { - return doi.path; + [](DerivationOutputInputAddressed doi) -> std::optional { + return { doi.path }; + }, + [&](DerivationOutputFixed dof) -> std::optional { + return { + store.makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName) + }; + }, + [](DerivationOutputFloating dof) -> std::optional { + return std::nullopt; }, - [&](DerivationOutputFixed dof) { - return store.makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName); - } }, output); } @@ -128,14 +133,21 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream } const HashType hashType = parseHashType(hashAlgo); - return DerivationOutput { - .output = DerivationOutputFixed { - .hash = FixedOutputHash { - .method = std::move(method), - .hash = Hash(hash, hashType), - }, - } - }; + return hash != "" + ? DerivationOutput { + .output = DerivationOutputFixed { + .hash = FixedOutputHash { + .method = std::move(method), + .hash = Hash(hash, hashType), + }, + } + } + : DerivationOutput { + .output = DerivationOutputFloating { + .method = std::move(method), + .hashType = std::move(hashType), + }, + }; } else return DerivationOutput { .output = DerivationOutputInputAddressed { @@ -292,6 +304,10 @@ string Derivation::unparse(const Store & store, bool maskOutputs, s += ','; printUnquotedString(s, dof.hash.printMethodAlgo()); s += ','; printUnquotedString(s, dof.hash.hash.to_string(Base16, false)); }, + [&](DerivationOutputFloating dof) { + s += ','; printUnquotedString(s, makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); + s += ','; printUnquotedString(s, ""); + }, }, i.second.output); s += ')'; } @@ -439,14 +455,21 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) hashAlgo = string(hashAlgo, 2); } auto hashType = parseHashType(hashAlgo); - return DerivationOutput { - .output = DerivationOutputFixed { - .hash = FixedOutputHash { - .method = std::move(method), - .hash = Hash(hash, hashType), - }, - } - }; + return hash != "" + ? DerivationOutput { + .output = DerivationOutputFixed { + .hash = FixedOutputHash { + .method = std::move(method), + .hash = Hash(hash, hashType), + }, + } + } + : DerivationOutput { + .output = DerivationOutputFloating { + .method = std::move(method), + .hashType = std::move(hashType), + }, + }; } else return DerivationOutput { .output = DerivationOutputInputAddressed { @@ -514,6 +537,10 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr out << dof.hash.printMethodAlgo() << dof.hash.hash.to_string(Base16, false); }, + [&](DerivationOutputFloating dof) { + out << (makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)) + << ""; + }, }, i.second.output); } writeStorePaths(store, out, drv.inputSrcs); diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 4dc542536..36ac09210 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -15,6 +15,8 @@ namespace nix { struct DerivationOutputInputAddressed { + /* Will need to become `std::optional` once input-addressed + derivations are allowed to depend on cont-addressed derivations */ StorePath path; }; @@ -23,13 +25,28 @@ struct DerivationOutputFixed FixedOutputHash hash; /* hash used for expected hash computation */ }; +struct DerivationOutputFloating +{ + /* information used for expected hash computation */ + FileIngestionMethod method; + HashType hashType; +}; + struct DerivationOutput { std::variant< DerivationOutputInputAddressed, - DerivationOutputFixed + DerivationOutputFixed, + DerivationOutputFloating > output; - StorePath path(const Store & store, std::string_view drvName) const; + std::optional hashAlgoOpt(const Store & store) const; + std::optional pathOpt(const Store & store, std::string_view drvName) const; + /* DEPRECATED: Remove after CA drvs are fully implemented */ + StorePath path(const Store & store, std::string_view drvName) const { + auto p = pathOpt(store, drvName); + if (!p) throw Error("floating content-addressed derivations are not yet implemented"); + return *p; + } }; typedef std::map DerivationOutputs; diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index f9952e177..95898d566 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -78,6 +78,9 @@ struct CmdShowDerivation : InstallablesCommand outputObj.attr("hashAlgo", dof.hash.printMethodAlgo()); outputObj.attr("hash", dof.hash.hash.to_string(Base16, false)); }, + [&](DerivationOutputFloating dof) { + outputObj.attr("hashAlgo", makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); + }, }, output.second.output); } } -- cgit v1.2.3 From 1feb8981df6adf8519a1f2d31883eb12db11fcb5 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 17 Jul 2020 11:33:27 -0400 Subject: Revert "Don't anticipate hash algo without hash in derivation for now" This reverts commit 3804e3df9bb479fe1d399f29d16a1aabaf352c19. --- src/libstore/derivations.cc | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index bff228230..487ed47e9 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -387,13 +387,34 @@ bool isDerivation(const string & fileName) DerivationType BasicDerivation::type() const { - if (outputs.size() == 1 && - outputs.begin()->first == "out" && - std::holds_alternative(outputs.begin()->second.output)) - { + std::set inputAddressedOutputs, fixedCAOutputs; + for (auto & i : outputs) { + std::visit(overloaded { + [&](DerivationOutputInputAddressed _) { + inputAddressedOutputs.insert(i.first); + }, + [&](DerivationOutputFixed _) { + fixedCAOutputs.insert(i.first); + }, + [&](DerivationOutputFloating _) { + throw Error("Floating CA output derivations are not yet implemented"); + }, + }, i.second.output); + } + + if (inputAddressedOutputs.empty() && fixedCAOutputs.empty()) { + throw Error("Must have at least one output"); + } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty()) { + return DerivationType::Regular; + } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.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 { - return DerivationType::Regular; + throw Error("Can't mix derivation output types"); } } -- cgit v1.2.3 From 205dcd140d46db94481329578b4fee8275e6c534 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 17 Jul 2020 12:43:46 -0400 Subject: Revert "Don't anticipate multiple CA outputs for now" This reverts commit 74b251b2f3d6414de051c8523011c0ee3c5ea154. --- src/libstore/local-store.cc | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 19537b8e5..9117ff384 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -544,11 +544,8 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat std::string drvName(drvPath.name()); drvName = string(drvName, 0, drvName.size() - drvExtension.size()); - auto check = [&](const StorePath & expected, const StorePath & actual, const std::string & varName) + auto envHasRightPath = [&](const StorePath & actual, const std::string & varName) { - if (actual != expected) - throw Error("derivation '%s' has incorrect output '%s', should be '%s'", - printStorePath(drvPath), printStorePath(actual), printStorePath(expected)); auto j = drv.env.find(varName); if (j == drv.env.end() || parseStorePath(j->second) != actual) throw Error("derivation '%s' has incorrect environment variable '%s', should be '%s'", @@ -556,18 +553,34 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat }; - if (derivationIsFixed(drv.type())) { - DerivationOutputs::const_iterator out = drv.outputs.find("out"); - if (out == drv.outputs.end()) - throw Error("derivation '%s' does not have an output named 'out'", printStorePath(drvPath)); - } + // Don't need the answer, but do this anyways to assert is proper + // combination. The code below is more general and naturally allows + // combinations that are currently prohibited. + drv.type(); - else { - // Regular, non-CA derivation should always return a single hash and not - // hash per output. - Hash h = std::get<0>(hashDerivationModulo(*this, drv, true)); - for (auto & i : drv.outputs) - check(makeOutputPath(i.first, h, drvName), i.second.path(*this, drv.name), i.first); + std::optional h; + for (auto & i : drv.outputs) { + std::visit(overloaded { + [&](DerivationOutputInputAddressed doia) { + if (!h) { + // somewhat expensive so we do lazily + auto temp = hashDerivationModulo(*this, drv, true); + h = std::get(temp); + } + StorePath recomputed = makeOutputPath(i.first, *h, drvName); + if (doia.path != recomputed) + throw Error("derivation '%s' has incorrect output '%s', should be '%s'", + printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed)); + envHasRightPath(doia.path, i.first); + }, + [&](DerivationOutputFixed dof) { + StorePath path = makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName); + envHasRightPath(path, i.first); + }, + [&](DerivationOutputFloating _) { + throw Error("Floating CA output derivations are not yet implemented"); + }, + }, i.second.output); } } -- cgit v1.2.3 From bbc633c98ca2c2f11303efafe4d58edd6d9b1018 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 17 Jul 2020 13:10:32 -0400 Subject: Revert "Don't anticipate CA but not fixed outputs for now" This reverts commit 3a9e4c32624b36b70cf8d553fd76a85ee97773ab. --- src/libstore/build.cc | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 28f3ead75..b59bfa8bc 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3721,9 +3721,22 @@ void DerivationGoal::registerOutputs() hash). */ std::optional ca; - if (derivationIsFixed(derivationType)) { - - FixedOutputHash outputHash = std::get(i.second.output).hash; + if (! std::holds_alternative(i.second.output)) { + DerivationOutputFloating outputHash; + std::visit(overloaded { + [&](DerivationOutputInputAddressed doi) { + throw Error("No."); + }, + [&](DerivationOutputFixed dof) { + outputHash = DerivationOutputFloating { + .method = dof.hash.method, + .hashType = *dof.hash.hash.type, + }; + }, + [&](DerivationOutputFloating dof) { + outputHash = dof; + }, + }, i.second.output); if (outputHash.method == FileIngestionMethod::Flat) { /* The output path should be a regular file without execute permission. */ @@ -3737,12 +3750,17 @@ void DerivationGoal::registerOutputs() /* Check the hash. In hash mode, move the path produced by the derivation to its content-addressed location. */ Hash h2 = outputHash.method == FileIngestionMethod::Recursive - ? hashPath(*outputHash.hash.type, actualPath).first - : hashFile(*outputHash.hash.type, actualPath); + ? hashPath(outputHash.hashType, actualPath).first + : hashFile(outputHash.hashType, actualPath); auto dest = worker.store.makeFixedOutputPath(outputHash.method, h2, i.second.path(worker.store, drv->name).name()); - if (outputHash.hash != h2) { + // true if either floating CA, or incorrect fixed hash. + bool needsMove = true; + + if (auto p = std::get_if(& i.second.output)) { + Hash & h = p->hash.hash; + if (h != h2) { /* Throw an error after registering the path as valid. */ @@ -3750,9 +3768,15 @@ void DerivationGoal::registerOutputs() delayedException = std::make_exception_ptr( BuildError("hash mismatch in fixed-output derivation '%s':\n wanted: %s\n got: %s", worker.store.printStorePath(dest), - outputHash.hash.to_string(SRI, true), + h.to_string(SRI, true), h2.to_string(SRI, true))); + } else { + // matched the fixed hash, so no move needed. + needsMove = false; + } + } + if (needsMove) { Path actualDest = worker.store.Store::toRealPath(dest); if (worker.store.isValidPath(dest)) -- cgit v1.2.3 From 6756cecfcf266801219b1e2da7e79f14695ccecf Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 17 Jul 2020 19:55:41 +0000 Subject: Add `DerivationType::CAFloating` --- src/libstore/derivations.cc | 28 +++++++++++++++++++++------- src/libstore/derivations.hh | 1 + 2 files changed, 22 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 487ed47e9..2a95c7e69 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -28,6 +28,7 @@ bool derivationIsCA(DerivationType dt) { switch (dt) { case DerivationType::Regular: return false; case DerivationType::CAFixed: return true; + case DerivationType::CAFloating: return true; }; // Since enums can have non-variant values, but making a `default:` would // disable exhaustiveness warnings. @@ -38,6 +39,7 @@ bool derivationIsFixed(DerivationType dt) { switch (dt) { case DerivationType::Regular: return false; case DerivationType::CAFixed: return true; + case DerivationType::CAFloating: return false; }; abort(); } @@ -46,6 +48,7 @@ bool derivationIsImpure(DerivationType dt) { switch (dt) { case DerivationType::Regular: return false; case DerivationType::CAFixed: return true; + case DerivationType::CAFloating: return false; }; abort(); } @@ -387,7 +390,8 @@ bool isDerivation(const string & fileName) DerivationType BasicDerivation::type() const { - std::set inputAddressedOutputs, fixedCAOutputs; + std::set inputAddressedOutputs, fixedCAOutputs, floatingCAOutputs; + std::optional floatingHashType; for (auto & i : outputs) { std::visit(overloaded { [&](DerivationOutputInputAddressed _) { @@ -396,23 +400,31 @@ DerivationType BasicDerivation::type() const [&](DerivationOutputFixed _) { fixedCAOutputs.insert(i.first); }, - [&](DerivationOutputFloating _) { - throw Error("Floating CA output derivations are not yet implemented"); + [&](DerivationOutputFloating dof) { + floatingCAOutputs.insert(i.first); + if (!floatingHashType) { + floatingHashType = dof.hashType; + } else { + if (*floatingHashType != dof.hashType) + throw Error("All floating outputs must use the same hash type"); + } }, }, i.second.output); } - if (inputAddressedOutputs.empty() && fixedCAOutputs.empty()) { + if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty()) { throw Error("Must have at least one output"); - } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty()) { + } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty()) { return DerivationType::Regular; - } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.empty()) { + } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.empty() && floatingCAOutputs.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()) { + return DerivationType::CAFloating; } else { throw Error("Can't mix derivation output types"); } @@ -464,6 +476,8 @@ 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"); case DerivationType::CAFixed: { std::map outputHashes; for (const auto & i : drv.outputs) { @@ -476,7 +490,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m } return outputHashes; } - default: + case DerivationType::Regular: break; } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 45217b3d5..b8e2494a3 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -61,6 +61,7 @@ typedef std::map StringPairs; enum struct DerivationType : uint8_t { Regular, CAFixed, + CAFloating, }; /* Do the outputs of the derivation have paths calculated from their content, -- cgit v1.2.3 From 362ae93851830ecce6ade70462fe991cc522d27b Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Mon, 20 Jul 2020 14:13:37 -0400 Subject: Add UnimplementedError to ease grepping for these --- src/libfetchers/git.cc | 2 +- src/libstore/build.cc | 2 +- src/libstore/derivations.hh | 2 +- src/libutil/error.hh | 1 + 4 files changed, 4 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 5d38e0c2b..b1b47c45f 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -121,7 +121,7 @@ struct GitInputScheme : InputScheme args.push_back(*ref); } - if (input.getRev()) throw Error("cloning a specific revision is not implemented"); + if (input.getRev()) throw UnimplementedError("cloning a specific revision is not implemented"); args.push_back(destDir); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 1c88d91bc..3380dbdaf 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1197,7 +1197,7 @@ void DerivationGoal::haveDerivation() if (parsedDrv->contentAddressed()) { settings.requireExperimentalFeature("ca-derivations"); - throw Error("ca-derivations isn't implemented yet"); + throw UnimplementedError("ca-derivations isn't implemented yet"); } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 36ac09210..c8f8d10dc 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -44,7 +44,7 @@ struct DerivationOutput /* DEPRECATED: Remove after CA drvs are fully implemented */ StorePath path(const Store & store, std::string_view drvName) const { auto p = pathOpt(store, drvName); - if (!p) throw Error("floating content-addressed derivations are not yet implemented"); + if (!p) throw UnimplementedError("floating content-addressed derivations are not yet implemented"); return *p; } }; diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 0daaf3be2..f3babcbde 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -192,6 +192,7 @@ public: MakeError(Error, BaseError); MakeError(UsageError, Error); +MakeError(UnimplementedError, Error); class SysError : public Error { -- cgit v1.2.3 From 6357b1b0fb33bfa26f1d44326fc01d0c86d86f2c Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Mon, 20 Jul 2020 14:17:25 -0400 Subject: Add another Unimplemented case --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9117ff384..e07b33897 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -578,7 +578,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat envHasRightPath(path, i.first); }, [&](DerivationOutputFloating _) { - throw Error("Floating CA output derivations are not yet implemented"); + throw UnimplementedError("Floating CA output derivations are not yet implemented"); }, }, i.second.output); } -- cgit v1.2.3 From 5ce95b9529ad8c53b4395d98635d035d92913091 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 21 Jul 2020 09:47:40 -0400 Subject: Update src/libstore/build.cc --- src/libstore/build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 73e29390d..95350356b 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3725,7 +3725,7 @@ void DerivationGoal::registerOutputs() DerivationOutputFloating outputHash; std::visit(overloaded { [&](DerivationOutputInputAddressed doi) { - throw Error("No."); + assert(false); // Enclosing `if` handles this case in other branch }, [&](DerivationOutputFixed dof) { outputHash = DerivationOutputFloating { -- cgit v1.2.3 From 9423f64ee2b9fe84618e06654fb6b55766b0cf44 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 22 Jul 2020 23:59:25 +0000 Subject: Parse CA derivations using new output variants We no longer need `ParsedDerivation` because everything libstore needs to know about is in the `BasicDerivation` proper. --- src/libexpr/eval.cc | 1 + src/libexpr/eval.hh | 1 + src/libexpr/primops.cc | 24 ++++++++++++++++++++++-- src/libstore/build.cc | 2 +- src/libstore/parsed-derivations.cc | 5 ----- src/libstore/parsed-derivations.hh | 2 -- 6 files changed, 25 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 7a2f55504..32b09fb0d 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -345,6 +345,7 @@ EvalState::EvalState(const Strings & _searchPath, ref store) , sStructuredAttrs(symbols.create("__structuredAttrs")) , sBuilder(symbols.create("builder")) , sArgs(symbols.create("args")) + , sContentAddressed(symbols.create("__contentAddressed")) , sOutputHash(symbols.create("outputHash")) , sOutputHashAlgo(symbols.create("outputHashAlgo")) , sOutputHashMode(symbols.create("outputHashMode")) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 8986952e3..0382298b3 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -74,6 +74,7 @@ public: sSystem, sOverrides, sOutputs, sOutputName, sIgnoreNulls, sFile, sLine, sColumn, sFunctor, sToString, sRight, sWrong, sStructuredAttrs, sBuilder, sArgs, + sContentAddressed, sOutputHash, sOutputHashAlgo, sOutputHashMode, sRecurseForDerivations, sDescription, sSelf, sEpsilon; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index d12d571ad..a322a60ed 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -583,6 +583,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * PathSet context; + bool contentAddressed = false; std::optional outputHash; std::string outputHashAlgo; auto ingestionMethod = FileIngestionMethod::Flat; @@ -639,6 +640,9 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (i->value->type == tNull) continue; } + if (i->name == state.sContentAddressed) + contentAddressed = state.forceBool(*i->value, pos); + /* The `args' attribute is special: it supplies the command-line arguments to the builder. */ if (i->name == state.sArgs) { @@ -694,7 +698,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * } } catch (Error & e) { - e.addTrace(posDrvName, + e.addTrace(posDrvName, "while evaluating the attribute '%1%' of the derivation '%2%'", key, drvName); throw; @@ -761,7 +765,10 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * }); if (outputHash) { - /* Handle fixed-output derivations. */ + /* Handle fixed-output derivations. + + Ignore `__contentAddressed` because fixed output derivations are + already content addressed. */ if (outputs.size() != 1 || *(outputs.begin()) != "out") throw Error({ .hint = hintfmt("multiple outputs are not supported in fixed-output derivations"), @@ -783,6 +790,19 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * }); } + else if (contentAddressed) { + HashType ht = parseHashType(outputHashAlgo); + for (auto & i : outputs) { + if (!jsonObject) drv.env[i] = hashPlaceholder(i); + drv.outputs.insert_or_assign(i, DerivationOutput { + .output = DerivationOutputFloating { + .method = ingestionMethod, + .hashType = std::move(ht), + }, + }); + } + } + else { /* Compute a hash over the "masked" store derivation, which is the final one except that in the list of outputs, the diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 95350356b..188f50444 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1195,7 +1195,7 @@ void DerivationGoal::haveDerivation() parsedDrv = std::make_unique(drvPath, *drv); - if (parsedDrv->contentAddressed()) { + if (drv->type() == DerivationType::CAFloating) { settings.requireExperimentalFeature("ca-derivations"); throw UnimplementedError("ca-derivations isn't implemented yet"); } diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index c7797b730..24f848e46 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -117,9 +117,4 @@ bool ParsedDerivation::substitutesAllowed() const return getBoolAttr("allowSubstitutes", true); } -bool ParsedDerivation::contentAddressed() const -{ - return getBoolAttr("__contentAddressed", false); -} - } diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh index 0b8e8d031..6ee172d81 100644 --- a/src/libstore/parsed-derivations.hh +++ b/src/libstore/parsed-derivations.hh @@ -34,8 +34,6 @@ public: bool willBuildLocally() const; bool substitutesAllowed() const; - - bool contentAddressed() const; }; } -- cgit v1.2.3 From 951415b5685fe52d31770eadabd66d95ea75cfae Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 27 Jul 2020 17:56:36 +0000 Subject: Require `ca-derivations` everywhere we create a CA derivation "create" as in read one in from a serialized form, or build one from scratch in memory. --- src/libexpr/primops.cc | 4 +++- src/libstore/derivations.cc | 12 +++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index bc528140b..784c12b16 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -640,8 +640,10 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (i->value->type == tNull) continue; } - if (i->name == state.sContentAddressed) + if (i->name == state.sContentAddressed) { + settings.requireExperimentalFeature("ca-derivations"); contentAddressed = state.forceBool(*i->value, pos); + } /* The `args' attribute is special: it supplies the command-line arguments to the builder. */ diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index c88bb3c6d..6a12e8734 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -163,12 +163,13 @@ static DerivationOutput parseDerivationOutput(const Store & store, std::istrings }, } } - : DerivationOutput { - .output = DerivationOutputFloating { + : (settings.requireExperimentalFeature("ca-derivations"), + DerivationOutput { + .output = DerivationOutputFloating { .method = std::move(method), .hashType = std::move(hashType), }, - }; + }); } else return DerivationOutput { .output = DerivationOutputInputAddressed { @@ -559,12 +560,13 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) }, } } - : DerivationOutput { + : (settings.requireExperimentalFeature("ca-derivations"), + DerivationOutput { .output = DerivationOutputFloating { .method = std::move(method), .hashType = std::move(hashType), }, - }; + }); } else return DerivationOutput { .output = DerivationOutputInputAddressed { -- cgit v1.2.3 From 8065c6d1606402e936b048aa75ad98ffdd7c8d60 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 27 Jul 2020 20:45:34 +0000 Subject: Abstract out topo sorting logic --- src/libstore/misc.cc | 51 +++++++++++++++--------------------------------- src/libutil/topo-sort.hh | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 35 deletions(-) create mode 100644 src/libutil/topo-sort.hh (limited to 'src') diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index c4d22a634..34a14d30d 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -4,6 +4,7 @@ #include "local-store.hh" #include "store-api.hh" #include "thread-pool.hh" +#include "topo-sort.hh" namespace nix { @@ -246,41 +247,21 @@ void Store::queryMissing(const std::vector & targets, StorePaths Store::topoSortPaths(const StorePathSet & paths) { - StorePaths sorted; - StorePathSet visited, parents; - - std::function dfsVisit; - - dfsVisit = [&](const StorePath & path, const StorePath * parent) { - if (parents.count(path)) - throw BuildError("cycle detected in the references of '%s' from '%s'", - printStorePath(path), printStorePath(*parent)); - - if (!visited.insert(path).second) return; - parents.insert(path); - - StorePathSet references; - try { - references = queryPathInfo(path)->references; - } catch (InvalidPath &) { - } - - for (auto & i : references) - /* Don't traverse into paths that don't exist. That can - happen due to substitutes for non-existent paths. */ - if (i != path && paths.count(i)) - dfsVisit(i, &path); - - sorted.push_back(path); - parents.erase(path); - }; - - for (auto & i : paths) - dfsVisit(i, nullptr); - - std::reverse(sorted.begin(), sorted.end()); - - return sorted; + return topoSort(paths, + {[&](const StorePath & path) { + StorePathSet references; + try { + references = queryPathInfo(path)->references; + } catch (InvalidPath &) { + } + return references; + }}, + {[&](const StorePath & path, const StorePath & parent) { + return BuildError( + "cycle detected in the references of '%s' from '%s'", + printStorePath(path), + printStorePath(parent)); + }}); } diff --git a/src/libutil/topo-sort.hh b/src/libutil/topo-sort.hh new file mode 100644 index 000000000..7a68ff169 --- /dev/null +++ b/src/libutil/topo-sort.hh @@ -0,0 +1,40 @@ +#include "error.hh" + +namespace nix { + +template +std::vector topoSort(std::set items, + std::function(const T &)> getChildren, + std::function makeCycleError) +{ + std::vector sorted; + std::set visited, parents; + + std::function dfsVisit; + + dfsVisit = [&](const T & path, const T * parent) { + if (parents.count(path)) throw makeCycleError(path, *parent); + + if (!visited.insert(path).second) return; + parents.insert(path); + + std::set references = getChildren(path); + + for (auto & i : references) + /* Don't traverse into items that don't exist in our starting set. */ + if (i != path && items.count(i)) + dfsVisit(i, &path); + + sorted.push_back(path); + parents.erase(path); + }; + + for (auto & i : items) + dfsVisit(i, nullptr); + + std::reverse(sorted.begin(), sorted.end()); + + return sorted; +} + +} -- cgit v1.2.3 From 2980b244b7e5f1660c4a07d7589f4a2dd47f9acd Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 28 Jul 2020 15:39:45 -0400 Subject: Use assert(false) instead of abort() --- src/libstore/derivations.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index b708ecc57..ca77366bf 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -14,7 +14,7 @@ bool derivationIsCA(DerivationType dt) { }; // Since enums can have non-variant values, but making a `default:` would // disable exhaustiveness warnings. - abort(); + assert(false); } bool derivationIsFixed(DerivationType dt) { @@ -22,7 +22,7 @@ bool derivationIsFixed(DerivationType dt) { case DerivationType::Regular: return false; case DerivationType::CAFixed: return true; }; - abort(); + assert(false); } bool derivationIsImpure(DerivationType dt) { @@ -30,7 +30,7 @@ bool derivationIsImpure(DerivationType dt) { case DerivationType::Regular: return false; case DerivationType::CAFixed: return true; }; - abort(); + assert(false); } // FIXME Put this somewhere? -- cgit v1.2.3 From 088dcea0e80bf2861fd9d6b808e76a1669b7122a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 5 Aug 2020 15:41:51 +0200 Subject: Typo --- src/libexpr/eval.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 7a2f55504..ecac5d522 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1256,10 +1256,10 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v, const Pos & po try { lambda.body->eval(*this, env2, v); } catch (Error & e) { - addErrorTrace(e, lambda.pos, "while evaluating %s", - (lambda.name.set() - ? "'" + (string) lambda.name + "'" - : "anonymous lambdaction")); + addErrorTrace(e, lambda.pos, "while evaluating %s", + (lambda.name.set() + ? "'" + (string) lambda.name + "'" + : "anonymous lambda")); addErrorTrace(e, pos, "from call site%s", ""); throw; } -- cgit v1.2.3 From e7b0847f2d9674bc18532c86b2daf421347513e4 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 14:44:39 +0000 Subject: Make names more consistent --- src/libexpr/primops.cc | 2 +- src/libstore/build.cc | 10 +++++----- src/libstore/derivations.cc | 36 ++++++++++++++++++------------------ src/libstore/derivations.hh | 14 ++++++++++---- src/libstore/local-store.cc | 4 ++-- src/libstore/misc.cc | 2 +- src/nix/show-derivation.cc | 4 ++-- 7 files changed, 39 insertions(+), 33 deletions(-) (limited to 'src') diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index a9b5a10c9..6fec028be 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -774,7 +774,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * auto outPath = state.store->makeFixedOutputPath(ingestionMethod, h, drvName); if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign("out", DerivationOutput { - .output = DerivationOutputFixed { + .output = DerivationOutputCAFixed { .hash = FixedOutputHash { .method = ingestionMethod, .hash = std::move(h), diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0176e7f65..6c9f55a53 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3722,18 +3722,18 @@ void DerivationGoal::registerOutputs() std::optional ca; if (! std::holds_alternative(i.second.output)) { - DerivationOutputFloating outputHash; + DerivationOutputCAFloating outputHash; std::visit(overloaded { [&](DerivationOutputInputAddressed doi) { assert(false); // Enclosing `if` handles this case in other branch }, - [&](DerivationOutputFixed dof) { - outputHash = DerivationOutputFloating { + [&](DerivationOutputCAFixed dof) { + outputHash = DerivationOutputCAFloating { .method = dof.hash.method, .hashType = dof.hash.hash.type, }; }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { outputHash = dof; }, }, i.second.output); @@ -3758,7 +3758,7 @@ void DerivationGoal::registerOutputs() // true if either floating CA, or incorrect fixed hash. bool needsMove = true; - if (auto p = std::get_if(& i.second.output)) { + if (auto p = std::get_if(& i.second.output)) { Hash & h = p->hash.hash; if (h != h2) { diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 8f2339885..b17d0cf79 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -13,12 +13,12 @@ std::optional DerivationOutput::pathOpt(const Store & store, std::str [](DerivationOutputInputAddressed doi) -> std::optional { return { doi.path }; }, - [&](DerivationOutputFixed dof) -> std::optional { + [&](DerivationOutputCAFixed dof) -> std::optional { return { store.makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName) }; }, - [](DerivationOutputFloating dof) -> std::optional { + [](DerivationOutputCAFloating dof) -> std::optional { return std::nullopt; }, }, output); @@ -27,7 +27,7 @@ std::optional DerivationOutput::pathOpt(const Store & store, std::str bool derivationIsCA(DerivationType dt) { switch (dt) { - case DerivationType::Regular: return false; + case DerivationType::InputAddressed: return false; case DerivationType::CAFixed: return true; case DerivationType::CAFloating: return true; }; @@ -38,7 +38,7 @@ bool derivationIsCA(DerivationType dt) { bool derivationIsFixed(DerivationType dt) { switch (dt) { - case DerivationType::Regular: return false; + case DerivationType::InputAddressed: return false; case DerivationType::CAFixed: return true; case DerivationType::CAFloating: return false; }; @@ -47,7 +47,7 @@ bool derivationIsFixed(DerivationType dt) { bool derivationIsImpure(DerivationType dt) { switch (dt) { - case DerivationType::Regular: return false; + case DerivationType::InputAddressed: return false; case DerivationType::CAFixed: return true; case DerivationType::CAFloating: return false; }; @@ -156,7 +156,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, std::istrings return hash != "" ? DerivationOutput { - .output = DerivationOutputFixed { + .output = DerivationOutputCAFixed { .hash = FixedOutputHash { .method = std::move(method), .hash = Hash::parseNonSRIUnprefixed(hash, hashType), @@ -164,7 +164,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, std::istrings } } : DerivationOutput { - .output = DerivationOutputFloating { + .output = DerivationOutputCAFloating { .method = std::move(method), .hashType = std::move(hashType), }, @@ -321,11 +321,11 @@ string Derivation::unparse(const Store & store, bool maskOutputs, s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); }, - [&](DerivationOutputFixed dof) { + [&](DerivationOutputCAFixed dof) { s += ','; printUnquotedString(s, dof.hash.printMethodAlgo()); s += ','; printUnquotedString(s, dof.hash.hash.to_string(Base16, false)); }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { s += ','; printUnquotedString(s, makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); s += ','; printUnquotedString(s, ""); }, @@ -390,10 +390,10 @@ DerivationType BasicDerivation::type() const [&](DerivationOutputInputAddressed _) { inputAddressedOutputs.insert(i.first); }, - [&](DerivationOutputFixed _) { + [&](DerivationOutputCAFixed _) { fixedCAOutputs.insert(i.first); }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { floatingCAOutputs.insert(i.first); if (!floatingHashType) { floatingHashType = dof.hashType; @@ -408,7 +408,7 @@ DerivationType BasicDerivation::type() const if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty()) { throw Error("Must have at least one output"); } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty()) { - return DerivationType::Regular; + return DerivationType::InputAddressed; } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.empty() && floatingCAOutputs.empty()) { if (fixedCAOutputs.size() > 1) // FIXME: Experimental feature? @@ -474,7 +474,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m case DerivationType::CAFixed: { std::map outputHashes; for (const auto & i : drv.outputs) { - auto & dof = std::get(i.second.output); + auto & dof = std::get(i.second.output); auto hash = hashString(htSHA256, "fixed:out:" + dof.hash.printMethodAlgo() + ":" + dof.hash.hash.to_string(Base16, false) + ":" @@ -483,7 +483,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m } return outputHashes; } - case DerivationType::Regular: + case DerivationType::InputAddressed: break; } @@ -552,7 +552,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) auto hashType = parseHashType(hashAlgo); return hash != "" ? DerivationOutput { - .output = DerivationOutputFixed { + .output = DerivationOutputCAFixed { .hash = FixedOutputHash { .method = std::move(method), .hash = Hash::parseNonSRIUnprefixed(hash, hashType), @@ -560,7 +560,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) } } : DerivationOutput { - .output = DerivationOutputFloating { + .output = DerivationOutputCAFloating { .method = std::move(method), .hashType = std::move(hashType), }, @@ -628,11 +628,11 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr [&](DerivationOutputInputAddressed doi) { out << "" << ""; }, - [&](DerivationOutputFixed dof) { + [&](DerivationOutputCAFixed dof) { out << dof.hash.printMethodAlgo() << dof.hash.hash.to_string(Base16, false); }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { out << (makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)) << ""; }, diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index b1cda85cb..5a410a164 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -14,6 +14,7 @@ namespace nix { /* Abstract syntax of derivations. */ +/* The traditional non-fixed-output derivation type. */ struct DerivationOutputInputAddressed { /* Will need to become `std::optional` once input-addressed @@ -21,12 +22,17 @@ struct DerivationOutputInputAddressed StorePath path; }; -struct DerivationOutputFixed +/* Fixed-output derivations, whose output paths are content addressed + according to that fixed output. */ +struct DerivationOutputCAFixed { FixedOutputHash hash; /* hash used for expected hash computation */ }; -struct DerivationOutputFloating +/* Floating-output derivations, whose output paths are content addressed, but + not fixed, and so are dynamically calculated from whatever the output ends + up being. */ +struct DerivationOutputCAFloating { /* information used for expected hash computation */ FileIngestionMethod method; @@ -37,8 +43,8 @@ struct DerivationOutput { std::variant< DerivationOutputInputAddressed, - DerivationOutputFixed, - DerivationOutputFloating + DerivationOutputCAFixed, + DerivationOutputCAFloating > output; std::optional hashAlgoOpt(const Store & store) const; std::optional pathOpt(const Store & store, std::string_view drvName) const; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 7de065ba8..de7ddb84b 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -573,11 +573,11 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed)); envHasRightPath(doia.path, i.first); }, - [&](DerivationOutputFixed dof) { + [&](DerivationOutputCAFixed dof) { StorePath path = makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName); envHasRightPath(path, i.first); }, - [&](DerivationOutputFloating _) { + [&](DerivationOutputCAFloating _) { throw UnimplementedError("Floating CA output derivations are not yet implemented"); }, }, i.second.output); diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index ddba5d052..0ae1ceaad 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -113,7 +113,7 @@ std::optional getDerivationCA(const BasicDerivation & drv) { auto out = drv.outputs.find("out"); if (out != drv.outputs.end()) { - if (auto v = std::get_if(&out->second.output)) + if (auto v = std::get_if(&out->second.output)) return v->hash; } return std::nullopt; diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index 25ea19834..1b51d114f 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -74,11 +74,11 @@ struct CmdShowDerivation : InstallablesCommand std::visit(overloaded { [&](DerivationOutputInputAddressed doi) { }, - [&](DerivationOutputFixed dof) { + [&](DerivationOutputCAFixed dof) { outputObj.attr("hashAlgo", dof.hash.printMethodAlgo()); outputObj.attr("hash", dof.hash.hash.to_string(Base16, false)); }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { outputObj.attr("hashAlgo", makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); }, }, output.second.output); -- cgit v1.2.3 From e561a13a5863f25c81e8abc9d235a12925fd454e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 14:45:56 +0000 Subject: Reanme `DerivationType::Regular` defintion too This is the one non-prefixed occurence --- src/libstore/derivations.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 5a410a164..14e0e947a 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -65,7 +65,7 @@ typedef std::map DerivationInputs; typedef std::map StringPairs; enum struct DerivationType : uint8_t { - Regular, + InputAddressed, CAFixed, CAFloating, }; -- cgit v1.2.3 From 25f79121564b21ec7c84f33c9348b169f20d2bdc Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 5 Aug 2020 16:47:48 +0200 Subject: Style fix --- src/libstore/content-address.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 749551d1a..6428aa736 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -48,14 +48,14 @@ ContentAddress parseContentAddress(std::string_view rawCa) { { auto optPrefix = splitPrefixTo(rest, ':'); if (!optPrefix) - throw UsageError("not a content address because it is not in the form \":\": %s", rawCa); + throw UsageError("not a content address because it is not in the form ':': %s", rawCa); prefix = *optPrefix; } auto parseHashType_ = [&](){ auto hashTypeRaw = splitPrefixTo(rest, ':'); if (!hashTypeRaw) - throw UsageError("content address hash must be in form \":\", but found: %s", rawCa); + throw UsageError("content address hash must be in form ':', but found: %s", rawCa); HashType hashType = parseHashType(*hashTypeRaw); return std::move(hashType); }; @@ -81,7 +81,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { .hash = Hash::parseNonSRIUnprefixed(rest, std::move(hashType)), }; } else - throw UsageError("content address prefix \"%s\" is unrecognized. Recogonized prefixes are \"text\" or \"fixed\"", prefix); + throw UsageError("content address prefix '%s' is unrecognized. Recogonized prefixes are 'text' or 'fixed'", prefix); }; std::optional parseContentAddressOpt(std::string_view rawCaOpt) { -- cgit v1.2.3 From b9ebe373bbab6f19ee650ef9769ad76c32b7244d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 14:49:25 +0000 Subject: Sed some names to perhaps avoid conflicts --- src/libexpr/primops.cc | 4 ++-- src/libstore/build.cc | 10 +++++----- src/libstore/builtins/fetchurl.cc | 2 +- src/libstore/derivations.cc | 36 ++++++++++++++++++------------------ src/libstore/derivations.hh | 10 +++++----- src/libstore/local-store.cc | 4 ++-- src/nix/show-derivation.cc | 4 ++-- 7 files changed, 35 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 784c12b16..7bc424d52 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -783,7 +783,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * auto outPath = state.store->makeFixedOutputPath(ingestionMethod, h, drvName); if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign("out", DerivationOutput { - .output = DerivationOutputFixed { + .output = DerivationOutputCAFixed { .hash = FixedOutputHash { .method = ingestionMethod, .hash = std::move(h), @@ -797,7 +797,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * for (auto & i : outputs) { if (!jsonObject) drv.env[i] = hashPlaceholder(i); drv.outputs.insert_or_assign(i, DerivationOutput { - .output = DerivationOutputFloating { + .output = DerivationOutputCAFloating { .method = ingestionMethod, .hashType = std::move(ht), }, diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 7c8323c29..38897a9e2 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3722,18 +3722,18 @@ void DerivationGoal::registerOutputs() std::optional ca; if (! std::holds_alternative(i.second.output)) { - DerivationOutputFloating outputHash; + DerivationOutputCAFloating outputHash; std::visit(overloaded { [&](DerivationOutputInputAddressed doi) { assert(false); // Enclosing `if` handles this case in other branch }, - [&](DerivationOutputFixed dof) { - outputHash = DerivationOutputFloating { + [&](DerivationOutputCAFixed dof) { + outputHash = DerivationOutputCAFloating { .method = dof.hash.method, .hashType = dof.hash.hash.type, }; }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { outputHash = dof; }, }, i.second.output); @@ -3758,7 +3758,7 @@ void DerivationGoal::registerOutputs() // true if either floating CA, or incorrect fixed hash. bool needsMove = true; - if (auto p = std::get_if(& i.second.output)) { + if (auto p = std::get_if(& i.second.output)) { Hash & h = p->hash.hash; if (h != h2) { diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 03bb77488..8291c745c 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -63,7 +63,7 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) auto & output = drv.outputs.begin()->second; /* Try the hashed mirrors first. */ - if (auto hash = std::get_if(&output.output)) { + if (auto hash = std::get_if(&output.output)) { if (hash->hash.method == FileIngestionMethod::Flat) { for (auto hashedMirror : settings.hashedMirrors.get()) { try { diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 6a12e8734..03cdc1760 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -13,12 +13,12 @@ std::optional DerivationOutput::pathOpt(const Store & store, std::str [](DerivationOutputInputAddressed doi) -> std::optional { return { doi.path }; }, - [&](DerivationOutputFixed dof) -> std::optional { + [&](DerivationOutputCAFixed dof) -> std::optional { return { store.makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName) }; }, - [](DerivationOutputFloating dof) -> std::optional { + [](DerivationOutputCAFloating dof) -> std::optional { return std::nullopt; }, }, output); @@ -27,7 +27,7 @@ std::optional DerivationOutput::pathOpt(const Store & store, std::str bool derivationIsCA(DerivationType dt) { switch (dt) { - case DerivationType::Regular: return false; + case DerivationType::InputAddressed: return false; case DerivationType::CAFixed: return true; case DerivationType::CAFloating: return true; }; @@ -38,7 +38,7 @@ bool derivationIsCA(DerivationType dt) { bool derivationIsFixed(DerivationType dt) { switch (dt) { - case DerivationType::Regular: return false; + case DerivationType::InputAddressed: return false; case DerivationType::CAFixed: return true; case DerivationType::CAFloating: return false; }; @@ -47,7 +47,7 @@ bool derivationIsFixed(DerivationType dt) { bool derivationIsImpure(DerivationType dt) { switch (dt) { - case DerivationType::Regular: return false; + case DerivationType::InputAddressed: return false; case DerivationType::CAFixed: return true; case DerivationType::CAFloating: return false; }; @@ -156,7 +156,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, std::istrings return hash != "" ? DerivationOutput { - .output = DerivationOutputFixed { + .output = DerivationOutputCAFixed { .hash = FixedOutputHash { .method = std::move(method), .hash = Hash(hash, hashType), @@ -165,7 +165,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, std::istrings } : (settings.requireExperimentalFeature("ca-derivations"), DerivationOutput { - .output = DerivationOutputFloating { + .output = DerivationOutputCAFloating { .method = std::move(method), .hashType = std::move(hashType), }, @@ -322,11 +322,11 @@ string Derivation::unparse(const Store & store, bool maskOutputs, s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); }, - [&](DerivationOutputFixed dof) { + [&](DerivationOutputCAFixed dof) { s += ','; printUnquotedString(s, dof.hash.printMethodAlgo()); s += ','; printUnquotedString(s, dof.hash.hash.to_string(Base16, false)); }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { s += ','; printUnquotedString(s, makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); s += ','; printUnquotedString(s, ""); }, @@ -391,10 +391,10 @@ DerivationType BasicDerivation::type() const [&](DerivationOutputInputAddressed _) { inputAddressedOutputs.insert(i.first); }, - [&](DerivationOutputFixed _) { + [&](DerivationOutputCAFixed _) { fixedCAOutputs.insert(i.first); }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { floatingCAOutputs.insert(i.first); if (!floatingHashType) { floatingHashType = dof.hashType; @@ -409,7 +409,7 @@ DerivationType BasicDerivation::type() const if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty()) { throw Error("Must have at least one output"); } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty()) { - return DerivationType::Regular; + return DerivationType::InputAddressed; } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.empty() && floatingCAOutputs.empty()) { if (fixedCAOutputs.size() > 1) // FIXME: Experimental feature? @@ -475,7 +475,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m case DerivationType::CAFixed: { std::map outputHashes; for (const auto & i : drv.outputs) { - auto & dof = std::get(i.second.output); + auto & dof = std::get(i.second.output); auto hash = hashString(htSHA256, "fixed:out:" + dof.hash.printMethodAlgo() + ":" + dof.hash.hash.to_string(Base16, false) + ":" @@ -484,7 +484,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m } return outputHashes; } - case DerivationType::Regular: + case DerivationType::InputAddressed: break; } @@ -553,7 +553,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) auto hashType = parseHashType(hashAlgo); return hash != "" ? DerivationOutput { - .output = DerivationOutputFixed { + .output = DerivationOutputCAFixed { .hash = FixedOutputHash { .method = std::move(method), .hash = Hash(hash, hashType), @@ -562,7 +562,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) } : (settings.requireExperimentalFeature("ca-derivations"), DerivationOutput { - .output = DerivationOutputFloating { + .output = DerivationOutputCAFloating { .method = std::move(method), .hashType = std::move(hashType), }, @@ -630,11 +630,11 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr [&](DerivationOutputInputAddressed doi) { out << "" << ""; }, - [&](DerivationOutputFixed dof) { + [&](DerivationOutputCAFixed dof) { out << dof.hash.printMethodAlgo() << dof.hash.hash.to_string(Base16, false); }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { out << (makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)) << ""; }, diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index b1cda85cb..09d51649e 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -21,12 +21,12 @@ struct DerivationOutputInputAddressed StorePath path; }; -struct DerivationOutputFixed +struct DerivationOutputCAFixed { FixedOutputHash hash; /* hash used for expected hash computation */ }; -struct DerivationOutputFloating +struct DerivationOutputCAFloating { /* information used for expected hash computation */ FileIngestionMethod method; @@ -37,8 +37,8 @@ struct DerivationOutput { std::variant< DerivationOutputInputAddressed, - DerivationOutputFixed, - DerivationOutputFloating + DerivationOutputCAFixed, + DerivationOutputCAFloating > output; std::optional hashAlgoOpt(const Store & store) const; std::optional pathOpt(const Store & store, std::string_view drvName) const; @@ -59,7 +59,7 @@ typedef std::map DerivationInputs; typedef std::map StringPairs; enum struct DerivationType : uint8_t { - Regular, + InputAddressed, CAFixed, CAFloating, }; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index ccf2a9a27..edcb17607 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -573,11 +573,11 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed)); envHasRightPath(doia.path, i.first); }, - [&](DerivationOutputFixed dof) { + [&](DerivationOutputCAFixed dof) { StorePath path = makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName); envHasRightPath(path, i.first); }, - [&](DerivationOutputFloating _) { + [&](DerivationOutputCAFloating _) { throw UnimplementedError("Floating CA output derivations are not yet implemented"); }, }, i.second.output); diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index 25ea19834..1b51d114f 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -74,11 +74,11 @@ struct CmdShowDerivation : InstallablesCommand std::visit(overloaded { [&](DerivationOutputInputAddressed doi) { }, - [&](DerivationOutputFixed dof) { + [&](DerivationOutputCAFixed dof) { outputObj.attr("hashAlgo", dof.hash.printMethodAlgo()); outputObj.attr("hash", dof.hash.hash.to_string(Base16, false)); }, - [&](DerivationOutputFloating dof) { + [&](DerivationOutputCAFloating dof) { outputObj.attr("hashAlgo", makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); }, }, output.second.output); -- cgit v1.2.3 From 790b694be71c102ca9c67a85a3e9fd6f076811b1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 5 Aug 2020 16:51:06 +0200 Subject: Style fix --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index de7ddb84b..3c66a4dfd 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -578,7 +578,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat envHasRightPath(path, i.first); }, [&](DerivationOutputCAFloating _) { - throw UnimplementedError("Floating CA output derivations are not yet implemented"); + throw UnimplementedError("floating CA output derivations are not yet implemented"); }, }, i.second.output); } -- cgit v1.2.3 From b3e73547a03f068ae4dd9cca4bc865cde85c8dec Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 11:05:46 -0400 Subject: Update src/libexpr/primops.cc Co-authored-by: Eelco Dolstra --- src/libexpr/primops.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 9ecf99f17..65d36ca0e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -647,7 +647,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * /* The `args' attribute is special: it supplies the command-line arguments to the builder. */ - if (i->name == state.sArgs) { + else if (i->name == state.sArgs) { state.forceList(*i->value, pos); for (unsigned int n = 0; n < i->value->listSize(); ++n) { string s = state.coerceToString(posDrvName, *i->value->listElems()[n], context, true); -- cgit v1.2.3