From 2be64efb02e9eeb0fdacfec5d3314bf02ab13395 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 15 Mar 2020 02:23:17 -0400 Subject: [PATCH 01/74] 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 | 36 +++++++++++++++++----------- 4 files changed, 106 insertions(+), 40 deletions(-) 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)); + // 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(); - 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); + 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); + } } } From e5178fd22d35d58be00902b5425359b8b33019a0 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 16 Mar 2020 16:40:13 -0400 Subject: [PATCH 02/74] Fix typos Thanks @asymmetric! Co-Authored-By: asymmetric --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; From 049179ba0776e293cd478cbb86ce7a167b64cdb0 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 18 Mar 2020 19:07:05 -0400 Subject: [PATCH 03/74] 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(-) 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(); From f1cf3ab870343a6894c08e2bb893ea69badfc397 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 19 Mar 2020 00:37:57 -0400 Subject: [PATCH 04/74] 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 | 94 +++++++++++++++++++++++++------------ src/libstore/derivations.hh | 33 ++++++++++++- src/libstore/local-store.cc | 4 +- 4 files changed, 100 insertions(+), 35 deletions(-) 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. +/* 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; +} - 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. +/* See the header for interface details. These are the implementation details. - 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) + 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); } From d5b3328dd1e3de8910b237d54fb9dbf36629870f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 19 Mar 2020 23:37:52 -0400 Subject: [PATCH 05/74] 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(-) 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. From e3173242362c8421429633c0e9f64ab0211760bd Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 19 Mar 2020 23:38:51 -0400 Subject: [PATCH 06/74] Apply suggestions from code review --- src/libstore/derivations.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 From 25004030591b3d5a6ce5219f36309ac05344c92b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 3 Jun 2020 17:38:54 +0000 Subject: [PATCH 07/74] 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(-) 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 */ From 3a9e4c32624b36b70cf8d553fd76a85ee97773ab Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 3 Jun 2020 18:50:45 +0000 Subject: [PATCH 08/74] Don't anticipate CA but not fixed outputs for now --- src/libstore/build.cc | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) 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)) From 74b251b2f3d6414de051c8523011c0ee3c5ea154 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 3 Jun 2020 18:53:04 +0000 Subject: [PATCH 09/74] Don't anticipate multiple CA outputs for now --- src/libstore/local-store.cc | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) 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(); + 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)); - 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); - } + 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); } } From bf9f040112c33213910faef40077122f1932d462 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 21 Jun 2020 16:51:39 +0000 Subject: [PATCH 10/74] Tweak declaration I think this is clearer --- src/libstore/derivations.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); From 3804e3df9bb479fe1d399f29d16a1aabaf352c19 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 21 Jun 2020 21:05:37 +0000 Subject: [PATCH 11/74] 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(-) 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; } From 55d4bd6e0ea23dcf55dc33a964514230ee785bbe Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 22 Jun 2020 18:08:27 +0000 Subject: [PATCH 12/74] Improve content address parsing - Ensure hash is in form - and not SRI. - Better errors if something goes wrong - string_view for no coppying --- src/libstore/content-address.cc | 77 ++++++++++++++++++++------------- src/libutil/hash.cc | 4 +- src/libutil/hash.hh | 4 +- 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 3d753836f..216d7eb03 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -1,3 +1,4 @@ +#include "args.hh" #include "content-address.hh" namespace nix { @@ -40,38 +41,52 @@ std::string renderContentAddress(ContentAddress ca) { } ContentAddress parseContentAddress(std::string_view rawCa) { - auto prefixSeparator = rawCa.find(':'); - if (prefixSeparator != string::npos) { - auto prefix = string(rawCa, 0, prefixSeparator); - if (prefix == "text") { - auto hashTypeAndHash = rawCa.substr(prefixSeparator+1, string::npos); - Hash hash = Hash(string(hashTypeAndHash)); - if (*hash.type != htSHA256) { - throw Error("parseContentAddress: the text hash should have type SHA256"); - } - return TextHash { hash }; - } else if (prefix == "fixed") { - // This has to be an inverse of makeFixedOutputCA - auto methodAndHash = rawCa.substr(prefixSeparator+1, string::npos); - if (methodAndHash.substr(0,2) == "r:") { - std::string_view hashRaw = methodAndHash.substr(2,string::npos); - return FixedOutputHash { - .method = FileIngestionMethod::Recursive, - .hash = Hash(string(hashRaw)), - }; - } else { - std::string_view hashRaw = methodAndHash; - return FixedOutputHash { - .method = FileIngestionMethod::Flat, - .hash = Hash(string(hashRaw)), - }; - } - } else { - throw Error("parseContentAddress: format not recognized; has to be text or fixed"); + auto rest = rawCa; + + // Ensure prefix + const auto prefixSeparator = rawCa.find(':'); + if (prefixSeparator == string::npos) + throw UsageError("not a content address because it is not in the form \":\": %s", rawCa); + auto prefix = rest.substr(0, prefixSeparator); + rest = rest.substr(prefixSeparator + 1); + + auto parseHashType_ = [&](){ + // Parse hash type + auto algoSeparator = rest.find(':'); + HashType hashType; + if (algoSeparator == string::npos) + throw UsageError("content address hash must be in form \":\", but found: %s", rest); + hashType = parseHashType(rest.substr(0, algoSeparator)); + + rest = rest.substr(algoSeparator + 1); + + return std::move(hashType); + }; + + // Switch on prefix + if (prefix == "text") { + // No parsing of the method, "text" only support flat. + HashType hashType = parseHashType_(); + if (hashType != htSHA256) + throw Error("text content address hash should use %s, but instead uses %s", + printHashType(htSHA256), printHashType(hashType)); + return TextHash { + .hash = Hash { rest, std::move(hashType) }, + }; + } else if (prefix == "fixed") { + // Parse method + auto method = FileIngestionMethod::Flat; + if (rest.substr(0, 2) == "r:") { + method = FileIngestionMethod::Recursive; + rest = rest.substr(2); } - } else { - throw Error("Not a content address because it lacks an appropriate prefix"); - } + HashType hashType = parseHashType_(); + return FixedOutputHash { + .method = method, + .hash = Hash { rest, std::move(hashType) }, + }; + } else + throw UsageError("content address prefix \"%s\" is unrecognized. Recogonized prefixes are \"text\" or \"fixed\"", prefix); }; std::optional parseContentAddressOpt(std::string_view rawCaOpt) { diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index c8fcdfed0..012fa727d 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -342,7 +342,7 @@ Hash compressHash(const Hash & hash, unsigned int newSize) } -std::optional parseHashTypeOpt(const string & s) +std::optional parseHashTypeOpt(std::string_view s) { if (s == "md5") return htMD5; else if (s == "sha1") return htSHA1; @@ -351,7 +351,7 @@ std::optional parseHashTypeOpt(const string & s) else return std::optional {}; } -HashType parseHashType(const string & s) +HashType parseHashType(std::string_view s) { auto opt_h = parseHashTypeOpt(s); if (opt_h) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 0d9916508..e4abe72ce 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -121,9 +121,9 @@ HashResult hashPath(HashType ht, const Path & path, Hash compressHash(const Hash & hash, unsigned int newSize); /* Parse a string representing a hash type. */ -HashType parseHashType(const string & s); +HashType parseHashType(std::string_view s); /* Will return nothing on parse error */ -std::optional parseHashTypeOpt(const string & s); +std::optional parseHashTypeOpt(std::string_view s); /* And the reverse. */ string printHashType(HashType ht); From d92d4f85a5c8a2a2385c084500a8b6bd54b54e6c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 24 Jun 2020 21:22:13 +0000 Subject: [PATCH 13/74] Move ValidPathInfo to its own header --- src/libstore/nar-info.cc | 1 + src/libstore/nar-info.hh | 2 +- src/libstore/path-info.hh | 99 +++++++++++++++++++++++++++++++++++++++ src/libstore/store-api.hh | 78 +----------------------------- 4 files changed, 102 insertions(+), 78 deletions(-) create mode 100644 src/libstore/path-info.hh diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index 04550ed97..ef04bc859 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -1,5 +1,6 @@ #include "globals.hh" #include "nar-info.hh" +#include "store-api.hh" namespace nix { diff --git a/src/libstore/nar-info.hh b/src/libstore/nar-info.hh index 373c33427..fa38ccd78 100644 --- a/src/libstore/nar-info.hh +++ b/src/libstore/nar-info.hh @@ -2,7 +2,7 @@ #include "types.hh" #include "hash.hh" -#include "store-api.hh" +#include "path-info.hh" namespace nix { diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh new file mode 100644 index 000000000..f5dee00a6 --- /dev/null +++ b/src/libstore/path-info.hh @@ -0,0 +1,99 @@ +#pragma once + +// TODO many of thes eare not needed. +#include "path.hh" +#include "hash.hh" +#include "content-address.hh" +#include "serialise.hh" +#include "crypto.hh" +#include "lru-cache.hh" +#include "sync.hh" +#include "globals.hh" +#include "config.hh" + +#include +#include +#include + +namespace nix { + + +class Store; + +struct ValidPathInfo +{ + StorePath path; + std::optional deriver; + Hash narHash; + StorePathSet references; + time_t registrationTime = 0; + uint64_t narSize = 0; // 0 = unknown + uint64_t id; // internal use only + + /* Whether the path is ultimately trusted, that is, it's a + derivation output that was built locally. */ + bool ultimate = false; + + StringSet sigs; // note: not necessarily verified + + /* If non-empty, an assertion that the path is content-addressed, + i.e., that the store path is computed from a cryptographic hash + of the contents of the path, plus some other bits of data like + the "name" part of the path. Such a path doesn't need + signatures, since we don't have to trust anybody's claim that + the path is the output of a particular derivation. (In the + extensional store model, we have to trust that the *contents* + of an output path of a derivation were actually produced by + that derivation. In the intensional model, we have to trust + that a particular output path was produced by a derivation; the + path then implies the contents.) + + Ideally, the content-addressability assertion would just be a Boolean, + and the store path would be computed from the name component, ‘narHash’ + and ‘references’. However, we support many types of content addresses. + */ + std::optional ca; + + bool operator == (const ValidPathInfo & i) const + { + return + path == i.path + && narHash == i.narHash + && references == i.references; + } + + /* Return a fingerprint of the store path to be used in binary + cache signatures. It contains the store path, the base-32 + SHA-256 hash of the NAR serialisation of the path, the size of + the NAR, and the sorted references. The size field is strictly + speaking superfluous, but might prevent endless/excessive data + attacks. */ + std::string fingerprint(const Store & store) const; + + void sign(const Store & store, const SecretKey & secretKey); + + /* Return true iff the path is verifiably content-addressed. */ + bool isContentAddressed(const Store & store) const; + + static const size_t maxSigs = std::numeric_limits::max(); + + /* Return the number of signatures on this .narinfo that were + produced by one of the specified keys, or maxSigs if the path + is content-addressed. */ + size_t checkSignatures(const Store & store, const PublicKeys & publicKeys) const; + + /* Verify a single signature. */ + bool checkSignature(const Store & store, const PublicKeys & publicKeys, const std::string & sig) const; + + Strings shortRefs() const; + + ValidPathInfo(const ValidPathInfo & other) = default; + + ValidPathInfo(StorePath && path) : path(std::move(path)) { }; + ValidPathInfo(const StorePath & path) : path(path) { }; + + virtual ~ValidPathInfo() { } +}; + +typedef list ValidPathInfos; +} diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 25d78c297..00b9c385c 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -10,6 +10,7 @@ #include "globals.hh" #include "config.hh" #include "derivations.hh" +#include "path-info.hh" #include #include @@ -111,83 +112,6 @@ struct SubstitutablePathInfo typedef std::map SubstitutablePathInfos; -struct ValidPathInfo -{ - StorePath path; - std::optional deriver; - Hash narHash; - StorePathSet references; - time_t registrationTime = 0; - uint64_t narSize = 0; // 0 = unknown - uint64_t id; // internal use only - - /* Whether the path is ultimately trusted, that is, it's a - derivation output that was built locally. */ - bool ultimate = false; - - StringSet sigs; // note: not necessarily verified - - /* If non-empty, an assertion that the path is content-addressed, - i.e., that the store path is computed from a cryptographic hash - of the contents of the path, plus some other bits of data like - the "name" part of the path. Such a path doesn't need - signatures, since we don't have to trust anybody's claim that - the path is the output of a particular derivation. (In the - extensional store model, we have to trust that the *contents* - of an output path of a derivation were actually produced by - that derivation. In the intensional model, we have to trust - that a particular output path was produced by a derivation; the - path then implies the contents.) - - Ideally, the content-addressability assertion would just be a Boolean, - and the store path would be computed from the name component, ‘narHash’ - and ‘references’. However, we support many types of content addresses. - */ - std::optional ca; - - bool operator == (const ValidPathInfo & i) const - { - return - path == i.path - && narHash == i.narHash - && references == i.references; - } - - /* Return a fingerprint of the store path to be used in binary - cache signatures. It contains the store path, the base-32 - SHA-256 hash of the NAR serialisation of the path, the size of - the NAR, and the sorted references. The size field is strictly - speaking superfluous, but might prevent endless/excessive data - attacks. */ - std::string fingerprint(const Store & store) const; - - void sign(const Store & store, const SecretKey & secretKey); - - /* Return true iff the path is verifiably content-addressed. */ - bool isContentAddressed(const Store & store) const; - - static const size_t maxSigs = std::numeric_limits::max(); - - /* Return the number of signatures on this .narinfo that were - produced by one of the specified keys, or maxSigs if the path - is content-addressed. */ - size_t checkSignatures(const Store & store, const PublicKeys & publicKeys) const; - - /* Verify a single signature. */ - bool checkSignature(const Store & store, const PublicKeys & publicKeys, const std::string & sig) const; - - Strings shortRefs() const; - - ValidPathInfo(const ValidPathInfo & other) = default; - - ValidPathInfo(StorePath && path) : path(std::move(path)) { }; - ValidPathInfo(const StorePath & path) : path(path) { }; - - virtual ~ValidPathInfo() { } -}; - -typedef list ValidPathInfos; - enum BuildMode { bmNormal, bmRepair, bmCheck }; From 7ba0fae0ddc815d7d7b2c3a9cd3d60879baeab54 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 30 Jun 2020 11:57:09 -0400 Subject: [PATCH 14/74] Create the spitPrefix function in parser.hh --- src/libutil/parser.hh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/libutil/parser.hh diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh new file mode 100644 index 000000000..64689e283 --- /dev/null +++ b/src/libutil/parser.hh @@ -0,0 +1,25 @@ +#pragma once + +#include + +namespace nix { + +// If `separator` is found, we return the portion of the string before the +// separator, and modify the string argument to contain only the part after the +// separator. Otherwise, wer return `std::nullopt`, and we leave the argument +// string alone. +std::optional splitPrefix(std::string_view & string, char separator); + +std::optional splitPrefix(std::string_view & string, char separator) { + auto sepInstance = string.find(separator); + + if (sepInstance != std::string_view::npos) { + auto prefix = string.substr(0, sepInstance); + string.remove_prefix(sepInstance+1); + return prefix; + } + + return std::nullopt; +} + +} From 77b51f4598763f0b3a435ed0e4ce98178d9e99da Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 30 Jun 2020 11:57:46 -0400 Subject: [PATCH 15/74] Factor the prefix splitting in content-address --- src/libstore/content-address.cc | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 4599cd924..8152e5215 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -1,5 +1,6 @@ #include "args.hh" #include "content-address.hh" +#include "parser.hh" namespace nix { @@ -43,23 +44,19 @@ std::string renderContentAddress(ContentAddress ca) { ContentAddress parseContentAddress(std::string_view rawCa) { auto rest = rawCa; - // Ensure prefix - const auto prefixSeparator = rawCa.find(':'); - if (prefixSeparator == string::npos) - throw UsageError("not a content address because it is not in the form \":\": %s", rawCa); - auto prefix = rest.substr(0, prefixSeparator); - rest = rest.substr(prefixSeparator + 1); + std::string_view prefix; + { + auto optPrefix = splitPrefix(rest, ':'); + if (!optPrefix) + throw UsageError("not a content address because it is not in the form \":\": %s", rawCa); + prefix = *optPrefix; + } auto parseHashType_ = [&](){ - // Parse hash type - auto algoSeparator = rest.find(':'); - HashType hashType; - if (algoSeparator == string::npos) - throw UsageError("content address hash must be in form \":\", but found: %s", rest); - hashType = parseHashType(rest.substr(0, algoSeparator)); - - rest = rest.substr(algoSeparator + 1); - + auto hashTypeRaw = splitPrefix(rest, ':'); + if (!hashTypeRaw) + throw UsageError("content address hash must be in form \":\", but found: %s", rawCa); + HashType hashType = parseHashType(*hashTypeRaw); return std::move(hashType); }; From a1f66d1d9e70d4204a3686002b02277465a6b7ab Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 30 Jun 2020 12:49:00 -0400 Subject: [PATCH 16/74] Factor the prefix splitting in hash --- src/libutil/hash.cc | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index e060700d9..26ab5a110 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -7,6 +7,7 @@ #include "args.hh" #include "hash.hh" #include "archive.hh" +#include "parser.hh" #include "util.hh" #include "istringstream_nocopy.hh" @@ -144,17 +145,15 @@ Hash::Hash(std::string_view original, std::optional optType) // Parse the has type before the separater, if there was one. std::optional optParsedType; { - auto sep = rest.find(':'); - if (sep == std::string_view::npos) { - sep = rest.find('-'); - if (sep != std::string_view::npos) + auto hashRaw = splitPrefix(rest, ':'); + + if (!hashRaw) { + hashRaw = splitPrefix(rest, '-'); + if (hashRaw) isSRI = true; } - if (sep != std::string_view::npos) { - auto hashRaw = rest.substr(0, sep); - optParsedType = parseHashType(hashRaw); - rest = rest.substr(sep + 1); - } + if (hashRaw) + optParsedType = parseHashType(*hashRaw); } // Either the string or user must provide the type, if they both do they From b798efb829415eb47a532e9479523afdff74eca7 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 30 Jun 2020 14:10:30 -0400 Subject: [PATCH 17/74] WIP initial design --- src/libutil/hash.cc | 17 ++++++++++++----- src/libutil/hash.hh | 7 +++++-- src/libutil/parser.hh | 5 ++--- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 26ab5a110..36de293bb 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -132,10 +132,13 @@ std::string Hash::to_string(Base base, bool includeType) const return s; } -Hash::Hash(std::string_view s, HashType type) : Hash(s, std::optional { type }) { } -Hash::Hash(std::string_view s) : Hash(s, std::optional{}) { } +Hash fromSRI(std::string_view original) { -Hash::Hash(std::string_view original, std::optional optType) +} + +Hash::Hash(std::string_view s) : Hash(s, std::nullopt) { } + +static HashType newFunction(std::string_view & rest, std::optional optType) { auto rest = original; @@ -161,13 +164,17 @@ Hash::Hash(std::string_view original, std::optional optType) if (!optParsedType && !optType) { throw BadHash("hash '%s' does not include a type, nor is the type otherwise known from context.", rest); } else { - this->type = optParsedType ? *optParsedType : *optType; if (optParsedType && optType && *optParsedType != *optType) throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); + return optParsedType ? *optParsedType : *optType; } +} - init(); +// mutates the string_view +Hash::Hash(std::string_view original, std::optional optType) + : Hash(original, newFunction(original, optType)) +Hash::Hash(std::string_view original, HashType type) : Hash(type) { if (!isSRI && rest.size() == base16Len()) { auto parseHexDigit = [&](char c) { diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index a55295912..42bd585a2 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -40,13 +40,16 @@ struct Hash is not present, then the hash type must be specified in the string. */ Hash(std::string_view s, std::optional type); - // type must be provided - Hash(std::string_view s, HashType type); // hash type must be part of string Hash(std::string_view s); +private: + // type must be provided, s must not include prefix + Hash(std::string_view s, HashType type); + void init(); +public: /* Check whether a hash is set. */ operator bool () const { return (bool) type; } diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh index 64689e283..d3bfafe75 100644 --- a/src/libutil/parser.hh +++ b/src/libutil/parser.hh @@ -1,6 +1,7 @@ #pragma once #include +#include namespace nix { @@ -8,9 +9,7 @@ namespace nix { // separator, and modify the string argument to contain only the part after the // separator. Otherwise, wer return `std::nullopt`, and we leave the argument // string alone. -std::optional splitPrefix(std::string_view & string, char separator); - -std::optional splitPrefix(std::string_view & string, char separator) { +static inline std::optional splitPrefix(std::string_view & string, char separator) { auto sepInstance = string.find(separator); if (sepInstance != std::string_view::npos) { From c2e7f7a7129fc5366a4cad337fcd6ae319a58ce5 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 17:32:06 -0400 Subject: [PATCH 18/74] Fixed build, we still have test errors --- src/libutil/hash.cc | 46 +++++++++++++++++++++++++++++---------------- src/libutil/hash.hh | 4 +++- src/libutil/util.cc | 2 +- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 36de293bb..e8d290d13 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -132,17 +132,24 @@ std::string Hash::to_string(Base base, bool includeType) const return s; } -Hash fromSRI(std::string_view original) { +Hash Hash::fromSRI(std::string_view original) { + auto rest = original; + // Parse the has type before the separater, if there was one. + auto hashRaw = splitPrefix(rest, '-'); + if (!hashRaw) + throw BadHash("hash '%s' is not SRI", original); + HashType parsedType = parseHashType(*hashRaw); + + return Hash(rest, std::make_pair(parsedType, true)); } -Hash::Hash(std::string_view s) : Hash(s, std::nullopt) { } +Hash::Hash(std::string_view s) : Hash(s, std::nullopt) {} -static HashType newFunction(std::string_view & rest, std::optional optType) +static std::pair newFunction(std::string_view & original, std::optional optType) { auto rest = original; - size_t pos = 0; bool isSRI = false; // Parse the has type before the separater, if there was one. @@ -166,16 +173,23 @@ static HashType newFunction(std::string_view & rest, std::optional opt } else { if (optParsedType && optType && *optParsedType != *optType) throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); - return optParsedType ? *optParsedType : *optType; + return { + optParsedType ? *optParsedType : *optType, + isSRI, + }; } } // mutates the string_view Hash::Hash(std::string_view original, std::optional optType) - : Hash(original, newFunction(original, optType)) + : Hash(original, newFunction(original, optType)) {} -Hash::Hash(std::string_view original, HashType type) : Hash(type) { - if (!isSRI && rest.size() == base16Len()) { +Hash::Hash(std::string_view original, std::pair typeAndSRI) + : Hash(typeAndSRI.first) +{ + auto [type, isSRI] = std::move(typeAndSRI); + + if (!isSRI && original.size() == base16Len()) { auto parseHexDigit = [&](char c) { if (c >= '0' && c <= '9') return c - '0'; @@ -186,15 +200,15 @@ Hash::Hash(std::string_view original, HashType type) : Hash(type) { for (unsigned int i = 0; i < hashSize; i++) { hash[i] = - parseHexDigit(rest[pos + i * 2]) << 4 - | parseHexDigit(rest[pos + i * 2 + 1]); + parseHexDigit(original[i * 2]) << 4 + | parseHexDigit(original[i * 2 + 1]); } } - else if (!isSRI && rest.size() == base32Len()) { + else if (!isSRI && original.size() == base32Len()) { - for (unsigned int n = 0; n < rest.size(); ++n) { - char c = rest[rest.size() - n - 1]; + for (unsigned int n = 0; n < original.size(); ++n) { + char c = original[original.size() - n - 1]; unsigned char digit; for (digit = 0; digit < base32Chars.size(); ++digit) /* !!! slow */ if (base32Chars[digit] == c) break; @@ -214,8 +228,8 @@ Hash::Hash(std::string_view original, HashType type) : Hash(type) { } } - else if (isSRI || rest.size() == base64Len()) { - auto d = base64Decode(rest); + else if (isSRI || original.size() == base64Len()) { + auto d = base64Decode(original); if (d.size() != hashSize) throw BadHash("invalid %s hash '%s'", isSRI ? "SRI" : "base-64", original); assert(hashSize); @@ -223,7 +237,7 @@ Hash::Hash(std::string_view original, HashType type) : Hash(type) { } else - throw BadHash("hash '%s' has wrong length for hash type '%s'", rest, printHashType(this->type)); + throw BadHash("hash '%s' has wrong length for hash type '%s'", original, printHashType(this->type)); } Hash newHashAllowEmpty(std::string hashStr, std::optional ht) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 42bd585a2..570e50dd4 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -43,9 +43,11 @@ struct Hash // hash type must be part of string Hash(std::string_view s); + Hash fromSRI(std::string_view original); + private: // type must be provided, s must not include prefix - Hash(std::string_view s, HashType type); + Hash(std::string_view s, std::pair typeAndSRI); void init(); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 1268b146a..ed43c403f 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1433,7 +1433,7 @@ string base64Decode(std::string_view s) char digit = decode[(unsigned char) c]; if (digit == -1) - throw Error("invalid character in Base64 string"); + throw Error("invalid character in Base64 string: '%c'", c); bits += 6; d = d << 6 | digit; From 274a8136fbf3d0fffb564f33464da26aab924b60 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 17:47:15 -0400 Subject: [PATCH 19/74] Correct FIXMEs in libfetchers --- src/libfetchers/fetchers.cc | 3 +-- src/libfetchers/tarball.cc | 6 ++---- src/libutil/hash.hh | 2 +- src/libutil/util.cc | 1 + 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 9174c3de4..91d0d6a1d 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -35,8 +35,7 @@ std::unique_ptr inputFromAttrs(const Attrs & attrs) auto res = inputScheme->inputFromAttrs(attrs2); if (res) { if (auto narHash = maybeGetStrAttr(attrs, "narHash")) - // FIXME: require SRI hash. - res->narHash = newHashAllowEmpty(*narHash, {}); + res->narHash = Hash::fromSRI(*narHash); return res; } } diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index f5356f0af..732fac8c3 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -242,15 +242,13 @@ struct TarballInputScheme : InputScheme auto hash = input->url.query.find("hash"); if (hash != input->url.query.end()) { - // FIXME: require SRI hash. - input->hash = Hash(hash->second); + input->hash = Hash::fromSRI(hash->second); input->url.query.erase(hash); } auto narHash = input->url.query.find("narHash"); if (narHash != input->url.query.end()) { - // FIXME: require SRI hash. - input->narHash = Hash(narHash->second); + input->narHash = Hash::fromSRI(narHash->second); input->url.query.erase(narHash); } diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 570e50dd4..887952ce5 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -43,7 +43,7 @@ struct Hash // hash type must be part of string Hash(std::string_view s); - Hash fromSRI(std::string_view original); + static Hash fromSRI(std::string_view original); private: // type must be provided, s must not include prefix diff --git a/src/libutil/util.cc b/src/libutil/util.cc index ed43c403f..ea28ee70d 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1,5 +1,6 @@ #include "lazy.hh" #include "util.hh" +#include "hash.hh" #include "affinity.hh" #include "sync.hh" #include "finally.hh" From 6faeec3b2a5446e3fa511ce8cb4b1a12621c6186 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 17:50:34 -0400 Subject: [PATCH 20/74] Keep the previous name, for diffing --- src/libutil/hash.cc | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index e8d290d13..448eb25f9 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -184,36 +184,36 @@ static std::pair newFunction(std::string_view & original, std::o Hash::Hash(std::string_view original, std::optional optType) : Hash(original, newFunction(original, optType)) {} -Hash::Hash(std::string_view original, std::pair typeAndSRI) +Hash::Hash(std::string_view rest, std::pair typeAndSRI) : Hash(typeAndSRI.first) { auto [type, isSRI] = std::move(typeAndSRI); - if (!isSRI && original.size() == base16Len()) { + if (!isSRI && rest.size() == base16Len()) { auto parseHexDigit = [&](char c) { if (c >= '0' && c <= '9') return c - '0'; if (c >= 'A' && c <= 'F') return c - 'A' + 10; if (c >= 'a' && c <= 'f') return c - 'a' + 10; - throw BadHash("invalid base-16 hash '%s'", original); + throw BadHash("invalid base-16 hash '%s'", rest); }; for (unsigned int i = 0; i < hashSize; i++) { hash[i] = - parseHexDigit(original[i * 2]) << 4 - | parseHexDigit(original[i * 2 + 1]); + parseHexDigit(rest[i * 2]) << 4 + | parseHexDigit(rest[i * 2 + 1]); } } - else if (!isSRI && original.size() == base32Len()) { + else if (!isSRI && rest.size() == base32Len()) { - for (unsigned int n = 0; n < original.size(); ++n) { - char c = original[original.size() - n - 1]; + for (unsigned int n = 0; n < rest.size(); ++n) { + char c = rest[rest.size() - n - 1]; unsigned char digit; for (digit = 0; digit < base32Chars.size(); ++digit) /* !!! slow */ if (base32Chars[digit] == c) break; if (digit >= 32) - throw BadHash("invalid base-32 hash '%s'", original); + throw BadHash("invalid base-32 hash '%s'", rest); unsigned int b = n * 5; unsigned int i = b / 8; unsigned int j = b % 8; @@ -223,21 +223,21 @@ Hash::Hash(std::string_view original, std::pair typeAndSRI) hash[i + 1] |= digit >> (8 - j); } else { if (digit >> (8 - j)) - throw BadHash("invalid base-32 hash '%s'", original); + throw BadHash("invalid base-32 hash '%s'", rest); } } } - else if (isSRI || original.size() == base64Len()) { - auto d = base64Decode(original); + else if (isSRI || rest.size() == base64Len()) { + auto d = base64Decode(rest); if (d.size() != hashSize) - throw BadHash("invalid %s hash '%s'", isSRI ? "SRI" : "base-64", original); + throw BadHash("invalid %s hash '%s'", isSRI ? "SRI" : "base-64", rest); assert(hashSize); memcpy(hash, d.data(), hashSize); } else - throw BadHash("hash '%s' has wrong length for hash type '%s'", original, printHashType(this->type)); + throw BadHash("hash '%s' has wrong length for hash type '%s'", rest, printHashType(this->type)); } Hash newHashAllowEmpty(std::string hashStr, std::optional ht) From d63a5ded76079008b09256aa36ef0c222919e8fa Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 17:53:24 -0400 Subject: [PATCH 21/74] Remove unused import --- src/libutil/util.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index ea28ee70d..ed43c403f 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1,6 +1,5 @@ #include "lazy.hh" #include "util.hh" -#include "hash.hh" #include "affinity.hh" #include "sync.hh" #include "finally.hh" From c8c4bcf90e065b47c3ee2984b1f8ff696da889af Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 18:03:22 -0400 Subject: [PATCH 22/74] Inline Hash::init() --- src/libutil/hash.cc | 2 +- src/libutil/hash.hh | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 448eb25f9..2087e3464 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -27,7 +27,7 @@ static size_t regularHashSize(HashType type) { abort(); } -void Hash::init() +Hash::Hash(HashType type) : type(type) { hashSize = regularHashSize(type); assert(hashSize <= maxHashSize); diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 887952ce5..766009438 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -32,7 +32,7 @@ struct Hash HashType type; /* Create a zero-filled hash object. */ - Hash(HashType type) : type(type) { init(); }; + Hash(HashType type); /* Initialize the hash from a string representation, in the format "[:]" or "-" (a @@ -49,8 +49,6 @@ private: // type must be provided, s must not include prefix Hash(std::string_view s, std::pair typeAndSRI); - void init(); - public: /* Check whether a hash is set. */ operator bool () const { return (bool) type; } From 263ccdd48923b730fd7e6f687583160d7b24039b Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 18:34:18 -0400 Subject: [PATCH 23/74] Rename two hash constructors to proper functions --- src/libexpr/primops/fetchGit.cc | 2 +- src/libexpr/primops/fetchMercurial.cc | 2 +- src/libfetchers/git.cc | 8 ++++---- src/libfetchers/github.cc | 8 ++++---- src/libfetchers/mercurial.cc | 6 +++--- src/libfetchers/path.cc | 4 ++-- src/libstore/content-address.cc | 4 ++-- src/libstore/daemon.cc | 2 +- src/libstore/derivations.cc | 4 ++-- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/local-store.cc | 2 +- src/libstore/nar-info-disk-cache.cc | 4 ++-- src/libstore/nar-info.cc | 2 +- src/libstore/remote-store.cc | 2 +- src/libstore/store-api.cc | 2 +- src/libutil/hash.cc | 14 ++++++++++---- src/libutil/hash.hh | 6 ++++-- src/nix-prefetch-url/nix-prefetch-url.cc | 2 +- src/nix-store/nix-store.cc | 4 ++-- src/nix/hash.cc | 2 +- 20 files changed, 45 insertions(+), 37 deletions(-) diff --git a/src/libexpr/primops/fetchGit.cc b/src/libexpr/primops/fetchGit.cc index dd7229a3d..0421318d1 100644 --- a/src/libexpr/primops/fetchGit.cc +++ b/src/libexpr/primops/fetchGit.cc @@ -29,7 +29,7 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va else if (n == "ref") ref = state.forceStringNoCtx(*attr.value, *attr.pos); else if (n == "rev") - rev = Hash(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA1); + rev = Hash::parseAny(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA1); else if (n == "name") name = state.forceStringNoCtx(*attr.value, *attr.pos); else if (n == "submodules") diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index 9bace8f89..236219a6f 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -31,7 +31,7 @@ static void prim_fetchMercurial(EvalState & state, const Pos & pos, Value * * ar // be both a revision or a branch/tag name. auto value = state.forceStringNoCtx(*attr.value, *attr.pos); if (std::regex_match(value, revRegex)) - rev = Hash(value, htSHA1); + rev = Hash::parseAny(value, htSHA1); else ref = value; } diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 75ce5ee8b..909bac78d 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -225,14 +225,14 @@ struct GitInput : Input if (isLocal) { if (!input->rev) - input->rev = Hash(chomp(runProgram("git", true, { "-C", actualUrl, "rev-parse", *input->ref })), htSHA1); + input->rev = Hash::parseAny(chomp(runProgram("git", true, { "-C", actualUrl, "rev-parse", *input->ref })), htSHA1); repoDir = actualUrl; } else { if (auto res = getCache()->lookup(store, mutableAttrs)) { - auto rev2 = Hash(getStrAttr(res->first, "rev"), htSHA1); + auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1); if (!rev || rev == rev2) { input->rev = rev2; return makeResult(res->first, std::move(res->second)); @@ -301,7 +301,7 @@ struct GitInput : Input } if (!input->rev) - input->rev = Hash(chomp(readFile(localRefFile)), htSHA1); + input->rev = Hash::parseAny(chomp(readFile(localRefFile)), htSHA1); } bool isShallow = chomp(runProgram("git", true, { "-C", repoDir, "rev-parse", "--is-shallow-repository" })) == "true"; @@ -426,7 +426,7 @@ struct GitInputScheme : InputScheme input->ref = *ref; } if (auto rev = maybeGetStrAttr(attrs, "rev")) - input->rev = Hash(*rev, htSHA1); + input->rev = Hash::parseAny(*rev, htSHA1); input->shallow = maybeGetBoolAttr(attrs, "shallow").value_or(false); diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 0bee1d6b3..449481092 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -76,7 +76,7 @@ struct GitHubInput : Input readFile( store->toRealPath( downloadFile(store, url, "source", false).storePath))); - rev = Hash(std::string { json["sha"] }, htSHA1); + rev = Hash::parseAny(std::string { json["sha"] }, htSHA1); debug("HEAD revision for '%s' is %s", url, rev->gitRev()); } @@ -140,7 +140,7 @@ struct GitHubInputScheme : InputScheme if (path.size() == 2) { } else if (path.size() == 3) { if (std::regex_match(path[2], revRegex)) - input->rev = Hash(path[2], htSHA1); + input->rev = Hash::parseAny(path[2], htSHA1); else if (std::regex_match(path[2], refRegex)) input->ref = path[2]; else @@ -152,7 +152,7 @@ struct GitHubInputScheme : InputScheme if (name == "rev") { if (input->rev) throw BadURL("GitHub URL '%s' contains multiple commit hashes", url.url); - input->rev = Hash(value, htSHA1); + input->rev = Hash::parseAny(value, htSHA1); } else if (name == "ref") { if (!std::regex_match(value, refRegex)) @@ -185,7 +185,7 @@ struct GitHubInputScheme : InputScheme input->repo = getStrAttr(attrs, "repo"); input->ref = maybeGetStrAttr(attrs, "ref"); if (auto rev = maybeGetStrAttr(attrs, "rev")) - input->rev = Hash(*rev, htSHA1); + input->rev = Hash::parseAny(*rev, htSHA1); return input; } }; diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 2e0d4bf4d..29f2a9d5b 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -167,7 +167,7 @@ struct MercurialInput : Input }); if (auto res = getCache()->lookup(store, mutableAttrs)) { - auto rev2 = Hash(getStrAttr(res->first, "rev"), htSHA1); + auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1); if (!rev || rev == rev2) { input->rev = rev2; return makeResult(res->first, std::move(res->second)); @@ -210,7 +210,7 @@ struct MercurialInput : Input runProgram("hg", true, { "log", "-R", cacheDir, "-r", revOrRef, "--template", "{node} {rev} {branch}" })); assert(tokens.size() == 3); - input->rev = Hash(tokens[0], htSHA1); + input->rev = Hash::parseAny(tokens[0], htSHA1); auto revCount = std::stoull(tokens[1]); input->ref = tokens[2]; @@ -293,7 +293,7 @@ struct MercurialInputScheme : InputScheme input->ref = *ref; } if (auto rev = maybeGetStrAttr(attrs, "rev")) - input->rev = Hash(*rev, htSHA1); + input->rev = Hash::parseAny(*rev, htSHA1); return input; } }; diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index ba2cc192e..1caab4165 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -101,7 +101,7 @@ struct PathInputScheme : InputScheme for (auto & [name, value] : url.query) if (name == "rev") - input->rev = Hash(value, htSHA1); + input->rev = Hash::parseAny(value, htSHA1); else if (name == "revCount") { uint64_t revCount; if (!string2Int(value, revCount)) @@ -129,7 +129,7 @@ struct PathInputScheme : InputScheme for (auto & [name, value] : attrs) if (name == "rev") - input->rev = Hash(getStrAttr(attrs, "rev"), htSHA1); + input->rev = Hash::parseAny(getStrAttr(attrs, "rev"), htSHA1); else if (name == "revCount") input->revCount = getIntAttr(attrs, "revCount"); else if (name == "lastModified") diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 8152e5215..2d96fb0c0 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -68,7 +68,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { throw Error("text content address hash should use %s, but instead uses %s", printHashType(htSHA256), printHashType(hashType)); return TextHash { - .hash = Hash { rest, std::move(hashType) }, + .hash = Hash::parseAny(rest, std::move(hashType)), }; } else if (prefix == "fixed") { // Parse method @@ -80,7 +80,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { HashType hashType = parseHashType_(); return FixedOutputHash { .method = method, - .hash = Hash { rest, std::move(hashType) }, + .hash = Hash::parseAny(rest, std::move(hashType)), }; } else throw UsageError("content address prefix \"%s\" is unrecognized. Recogonized prefixes are \"text\" or \"fixed\"", prefix); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 0b48e04c0..cc4827e64 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -706,7 +706,7 @@ static void performOp(TunnelLogger * logger, ref store, auto deriver = readString(from); if (deriver != "") info.deriver = store->parseStorePath(deriver); - info.narHash = Hash(readString(from), htSHA256); + info.narHash = Hash::parseAny(readString(from), htSHA256); info.references = readStorePaths(*store, from); from >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(from); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 42551ef6b..30ce32354 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -118,7 +118,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream const HashType hashType = parseHashType(hashAlgo); fsh = FixedOutputHash { .method = std::move(method), - .hash = Hash(hash, hashType), + .hash = Hash::parseAny(hash, hashType), }; } @@ -416,7 +416,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) auto hashType = parseHashType(hashAlgo); fsh = FixedOutputHash { .method = std::move(method), - .hash = Hash(hash, hashType), + .hash = Hash::parseAny(hash, hashType), }; } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 35caf23e7..f9d95fd2b 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -113,7 +113,7 @@ struct LegacySSHStore : public Store if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) { auto s = readString(conn->from); - info->narHash = s.empty() ? std::optional{} : Hash{s}; + info->narHash = s.empty() ? std::optional{} : Hash::parseAnyPrefixed(s); info->ca = parseContentAddressOpt(readString(conn->from)); info->sigs = readStrings(conn->from); } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9259d8f61..b75e2bdfe 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -647,7 +647,7 @@ void LocalStore::queryPathInfoUncached(const StorePath & path, info->id = useQueryPathInfo.getInt(0); try { - info->narHash = Hash(useQueryPathInfo.getStr(1)); + info->narHash = Hash::parseAnyPrefixed(useQueryPathInfo.getStr(1)); } catch (BadHash & e) { throw Error("in valid-path entry for '%s': %s", printStorePath(path), e.what()); } diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 9ddb9957f..92da14e23 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -193,9 +193,9 @@ public: narInfo->url = queryNAR.getStr(2); narInfo->compression = queryNAR.getStr(3); if (!queryNAR.isNull(4)) - narInfo->fileHash = Hash(queryNAR.getStr(4)); + narInfo->fileHash = Hash::parseAnyPrefixed(queryNAR.getStr(4)); narInfo->fileSize = queryNAR.getInt(5); - narInfo->narHash = Hash(queryNAR.getStr(6)); + narInfo->narHash = Hash::parseAnyPrefixed(queryNAR.getStr(6)); narInfo->narSize = queryNAR.getInt(7); for (auto & r : tokenizeString(queryNAR.getStr(8), " ")) narInfo->references.insert(StorePath(r)); diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index c403d4bec..2a52e4098 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -12,7 +12,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & auto parseHashField = [&](const string & s) { try { - return Hash(s); + return Hash::parseAnyPrefixed(s); } catch (BadHash &) { throw corrupt(); } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 305a47340..890d96388 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -375,7 +375,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, info = std::make_shared(StorePath(path)); auto deriver = readString(conn->from); if (deriver != "") info->deriver = parseStorePath(deriver); - info->narHash = Hash(readString(conn->from), htSHA256); + info->narHash = Hash::parseAny(readString(conn->from), htSHA256); info->references = readStorePaths(*this, conn->from); conn->from >> info->registrationTime >> info->narSize; if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) { diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index e4083bbe1..080ce9823 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -703,7 +703,7 @@ std::optional decodeValidPathInfo(const Store & store, std::istre if (hashGiven) { string s; getline(str, s); - info.narHash = Hash(s, htSHA256); + info.narHash = Hash::parseAny(s, htSHA256); getline(str, s); if (!string2Int(s, info.narSize)) throw Error("number expected"); } diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 2087e3464..fcc0b9eb7 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -144,7 +144,10 @@ Hash Hash::fromSRI(std::string_view original) { return Hash(rest, std::make_pair(parsedType, true)); } -Hash::Hash(std::string_view s) : Hash(s, std::nullopt) {} +Hash Hash::parseAnyPrefixed(std::string_view s) +{ + return parseAny(s, std::nullopt); +} static std::pair newFunction(std::string_view & original, std::optional optType) { @@ -181,8 +184,11 @@ static std::pair newFunction(std::string_view & original, std::o } // mutates the string_view -Hash::Hash(std::string_view original, std::optional optType) - : Hash(original, newFunction(original, optType)) {} +Hash Hash::parseAny(std::string_view original, std::optional optType) +{ + auto typeAndSRI = newFunction(original, optType); + return Hash(original, typeAndSRI); +} Hash::Hash(std::string_view rest, std::pair typeAndSRI) : Hash(typeAndSRI.first) @@ -249,7 +255,7 @@ Hash newHashAllowEmpty(std::string hashStr, std::optional ht) warn("found empty hash, assuming '%s'", h.to_string(SRI, true)); return h; } else - return Hash(hashStr, ht); + return Hash::parseAny(hashStr, ht); } diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 766009438..3e413a52c 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -39,9 +39,11 @@ struct Hash Subresource Integrity hash expression). If the 'type' argument is not present, then the hash type must be specified in the string. */ - Hash(std::string_view s, std::optional type); + static Hash parseAny(std::string_view s, std::optional type); // hash type must be part of string - Hash(std::string_view s); + static Hash parseAnyPrefixed(std::string_view s); + // prefix parsed separately; non SRI hash + static Hash parseAnyUnprefixed(std::string_view s, HashType type); static Hash fromSRI(std::string_view original); diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index 22410c44c..f752e0448 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -156,7 +156,7 @@ static int _main(int argc, char * * argv) Hash hash(ht), expectedHash(ht); std::optional storePath; if (args.size() == 2) { - expectedHash = Hash(args[1], ht); + expectedHash = Hash::parseAny(args[1], ht); const auto recursive = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; storePath = store->makeFixedOutputPath(recursive, expectedHash, name); if (store->isValidPath(*storePath)) diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index ff04cbefc..cc3b07c31 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -208,7 +208,7 @@ static void opPrintFixedPath(Strings opFlags, Strings opArgs) string hash = *i++; string name = *i++; - cout << fmt("%s\n", store->printStorePath(store->makeFixedOutputPath(recursive, Hash(hash, hashAlgo), name))); + cout << fmt("%s\n", store->printStorePath(store->makeFixedOutputPath(recursive, Hash::parseAny(hash, hashAlgo), name))); } @@ -950,7 +950,7 @@ static void opServe(Strings opFlags, Strings opArgs) auto deriver = readString(in); if (deriver != "") info.deriver = store->parseStorePath(deriver); - info.narHash = Hash(readString(in), htSHA256); + info.narHash = Hash::parseAny(readString(in), htSHA256); info.references = readStorePaths(*store, in); in >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(in); diff --git a/src/nix/hash.cc b/src/nix/hash.cc index b97c6d21f..fb0843ce2 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -103,7 +103,7 @@ struct CmdToBase : Command void run() override { for (auto s : args) - logger->stdout(Hash(s, ht).to_string(base, base == SRI)); + logger->stdout(Hash::parseAny(s, ht).to_string(base, base == SRI)); } }; From 343d1569b193e4456e2e5365921371bfd6e37205 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 10:48:47 -0400 Subject: [PATCH 24/74] Fix test suite --- src/libutil/hash.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index fcc0b9eb7..084d24170 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -149,9 +149,9 @@ Hash Hash::parseAnyPrefixed(std::string_view s) return parseAny(s, std::nullopt); } -static std::pair newFunction(std::string_view & original, std::optional optType) +static std::pair newFunction(std::string_view & rest, std::optional optType) { - auto rest = original; + auto original = rest; bool isSRI = false; From 27c8029573ab49ad11c069ef547e3c087c73939f Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 10:58:29 -0400 Subject: [PATCH 25/74] Inline newFunction --- src/libutil/hash.cc | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 084d24170..2908a3445 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -149,11 +149,12 @@ Hash Hash::parseAnyPrefixed(std::string_view s) return parseAny(s, std::nullopt); } -static std::pair newFunction(std::string_view & rest, std::optional optType) +Hash Hash::parseAny(std::string_view original, std::optional optType) { - auto original = rest; + auto rest = original; bool isSRI = false; + HashType hashType; // Parse the has type before the separater, if there was one. std::optional optParsedType; @@ -171,23 +172,13 @@ static std::pair newFunction(std::string_view & rest, std::optio // Either the string or user must provide the type, if they both do they // must agree. - if (!optParsedType && !optType) { + if (!optParsedType && !optType) throw BadHash("hash '%s' does not include a type, nor is the type otherwise known from context.", rest); - } else { - if (optParsedType && optType && *optParsedType != *optType) - throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); - return { - optParsedType ? *optParsedType : *optType, - isSRI, - }; - } -} + else if (optParsedType && optType && *optParsedType != *optType) + throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); -// mutates the string_view -Hash Hash::parseAny(std::string_view original, std::optional optType) -{ - auto typeAndSRI = newFunction(original, optType); - return Hash(original, typeAndSRI); + hashType = optParsedType ? *optParsedType : *optType; + return Hash(rest, std::make_pair(hashType, isSRI)); } Hash::Hash(std::string_view rest, std::pair typeAndSRI) From f61bc45d192809a040a359b22f3dbd0722613af6 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:09:04 -0400 Subject: [PATCH 26/74] Get rid of the std::pair --- src/libutil/hash.cc | 10 ++++------ src/libutil/hash.hh | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 2908a3445..2f006ce1e 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -141,7 +141,7 @@ Hash Hash::fromSRI(std::string_view original) { throw BadHash("hash '%s' is not SRI", original); HashType parsedType = parseHashType(*hashRaw); - return Hash(rest, std::make_pair(parsedType, true)); + return Hash(rest, parsedType, true); } Hash Hash::parseAnyPrefixed(std::string_view s) @@ -178,14 +178,12 @@ Hash Hash::parseAny(std::string_view original, std::optional optType) throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); hashType = optParsedType ? *optParsedType : *optType; - return Hash(rest, std::make_pair(hashType, isSRI)); + return Hash(rest, hashType, isSRI); } -Hash::Hash(std::string_view rest, std::pair typeAndSRI) - : Hash(typeAndSRI.first) +Hash::Hash(std::string_view rest, HashType type, bool isSRI) + : Hash(type) { - auto [type, isSRI] = std::move(typeAndSRI); - if (!isSRI && rest.size() == base16Len()) { auto parseHexDigit = [&](char c) { diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 3e413a52c..4e3591a04 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -49,7 +49,7 @@ struct Hash private: // type must be provided, s must not include prefix - Hash(std::string_view s, std::pair typeAndSRI); + Hash(std::string_view s, HashType type, bool isSRI); public: /* Check whether a hash is set. */ From 9462d8a50b5443bc2dac616f94ded9ad37020094 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:11:18 -0400 Subject: [PATCH 27/74] Rename fromSRI to parseSRI for constistency --- src/libfetchers/fetchers.cc | 2 +- src/libfetchers/tarball.cc | 4 ++-- src/libutil/hash.cc | 2 +- src/libutil/hash.hh | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 91d0d6a1d..b1782feab 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -35,7 +35,7 @@ std::unique_ptr inputFromAttrs(const Attrs & attrs) auto res = inputScheme->inputFromAttrs(attrs2); if (res) { if (auto narHash = maybeGetStrAttr(attrs, "narHash")) - res->narHash = Hash::fromSRI(*narHash); + res->narHash = Hash::parseSRI(*narHash); return res; } } diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 732fac8c3..6ca51269b 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -242,13 +242,13 @@ struct TarballInputScheme : InputScheme auto hash = input->url.query.find("hash"); if (hash != input->url.query.end()) { - input->hash = Hash::fromSRI(hash->second); + input->hash = Hash::parseSRI(hash->second); input->url.query.erase(hash); } auto narHash = input->url.query.find("narHash"); if (narHash != input->url.query.end()) { - input->narHash = Hash::fromSRI(narHash->second); + input->narHash = Hash::parseSRI(narHash->second); input->url.query.erase(narHash); } diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 2f006ce1e..a077d40a0 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -132,7 +132,7 @@ std::string Hash::to_string(Base base, bool includeType) const return s; } -Hash Hash::fromSRI(std::string_view original) { +Hash Hash::parseSRI(std::string_view original) { auto rest = original; // Parse the has type before the separater, if there was one. diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 4e3591a04..d321cc8e1 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -45,7 +45,7 @@ struct Hash // prefix parsed separately; non SRI hash static Hash parseAnyUnprefixed(std::string_view s, HashType type); - static Hash fromSRI(std::string_view original); + static Hash parseSRI(std::string_view original); private: // type must be provided, s must not include prefix From 36cbc74689321399aeae26ff506809b8d9b24674 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:21:00 -0400 Subject: [PATCH 28/74] Inline and simplify in parseAnyPrefixed --- src/libutil/hash.cc | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index a077d40a0..75f8f319c 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -144,9 +144,32 @@ Hash Hash::parseSRI(std::string_view original) { return Hash(rest, parsedType, true); } -Hash Hash::parseAnyPrefixed(std::string_view s) +Hash Hash::parseAnyPrefixed(std::string_view original) { - return parseAny(s, std::nullopt); + auto rest = original; + + bool isSRI = false; + + // Parse the has type before the separater, if there was one. + std::optional optParsedType; + { + auto hashRaw = splitPrefix(rest, ':'); + + if (!hashRaw) { + hashRaw = splitPrefix(rest, '-'); + if (hashRaw) + isSRI = true; + } + if (hashRaw) + optParsedType = parseHashType(*hashRaw); + } + + // Either the string or user must provide the type, if they both do they + // must agree. + if (!optParsedType) + throw BadHash("hash '%s' does not include a type.", rest); + + return Hash(rest, *optParsedType, isSRI); } Hash Hash::parseAny(std::string_view original, std::optional optType) From ea48e3a5b5f1051c251184792417326c513bf00f Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:29:33 -0400 Subject: [PATCH 29/74] Abstract common parsing functionality --- src/libutil/hash.cc | 37 +++---------------------------------- src/libutil/parser.hh | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 75f8f319c..7d6b8d96e 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -147,22 +147,7 @@ Hash Hash::parseSRI(std::string_view original) { Hash Hash::parseAnyPrefixed(std::string_view original) { auto rest = original; - - bool isSRI = false; - - // Parse the has type before the separater, if there was one. - std::optional optParsedType; - { - auto hashRaw = splitPrefix(rest, ':'); - - if (!hashRaw) { - hashRaw = splitPrefix(rest, '-'); - if (hashRaw) - isSRI = true; - } - if (hashRaw) - optParsedType = parseHashType(*hashRaw); - } + auto [optParsedType, isSRI] = getParsedTypeAndSRI(rest); // Either the string or user must provide the type, if they both do they // must agree. @@ -175,23 +160,7 @@ Hash Hash::parseAnyPrefixed(std::string_view original) Hash Hash::parseAny(std::string_view original, std::optional optType) { auto rest = original; - - bool isSRI = false; - HashType hashType; - - // Parse the has type before the separater, if there was one. - std::optional optParsedType; - { - auto hashRaw = splitPrefix(rest, ':'); - - if (!hashRaw) { - hashRaw = splitPrefix(rest, '-'); - if (hashRaw) - isSRI = true; - } - if (hashRaw) - optParsedType = parseHashType(*hashRaw); - } + auto [optParsedType, isSRI] = getParsedTypeAndSRI(rest); // Either the string or user must provide the type, if they both do they // must agree. @@ -200,7 +169,7 @@ Hash Hash::parseAny(std::string_view original, std::optional optType) else if (optParsedType && optType && *optParsedType != *optType) throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); - hashType = optParsedType ? *optParsedType : *optType; + HashType hashType = optParsedType ? *optParsedType : *optType; return Hash(rest, hashType, isSRI); } diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh index d3bfafe75..d20e4dfc6 100644 --- a/src/libutil/parser.hh +++ b/src/libutil/parser.hh @@ -21,4 +21,26 @@ static inline std::optional splitPrefix(std::string_view & str return std::nullopt; } +// Mutates the string to eliminate the prefixes when found +std::pair, bool> getParsedTypeAndSRI(std::string_view & rest) { + bool isSRI = false; + + // Parse the has type before the separater, if there was one. + std::optional optParsedType; + { + auto hashRaw = splitPrefix(rest, ':'); + + if (!hashRaw) { + hashRaw = splitPrefix(rest, '-'); + if (hashRaw) + isSRI = true; + } + if (hashRaw) + optParsedType = parseHashType(*hashRaw); + } + + return std::make_pair(optParsedType, isSRI); +} + + } From b6b10b1d4cb1cd487bbb5d2cc063ca743ae79004 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:34:40 -0400 Subject: [PATCH 30/74] Write the implementation for parseNonSRIUnprefixed --- src/libutil/hash.cc | 5 +++++ src/libutil/hash.hh | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 7d6b8d96e..1150e74ed 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -173,6 +173,11 @@ Hash Hash::parseAny(std::string_view original, std::optional optType) return Hash(rest, hashType, isSRI); } +Hash Hash::parseNonSRIUnprefixed(std::string_view s, HashType type) +{ + return Hash(s, type, false); +} + Hash::Hash(std::string_view rest, HashType type, bool isSRI) : Hash(type) { diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index d321cc8e1..af11a028d 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -43,7 +43,7 @@ struct Hash // hash type must be part of string static Hash parseAnyPrefixed(std::string_view s); // prefix parsed separately; non SRI hash - static Hash parseAnyUnprefixed(std::string_view s, HashType type); + static Hash parseNonSRIUnprefixed(std::string_view s, HashType type); static Hash parseSRI(std::string_view original); From 1fc835aa223520f37e4945fa8626a096f170b188 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:57:21 -0400 Subject: [PATCH 31/74] Tighten parsing for drv files and pathinfo --- src/libstore/content-address.cc | 4 ++-- src/libstore/derivations.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 2d96fb0c0..02ab6710f 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -68,7 +68,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { throw Error("text content address hash should use %s, but instead uses %s", printHashType(htSHA256), printHashType(hashType)); return TextHash { - .hash = Hash::parseAny(rest, std::move(hashType)), + .hash = Hash::parseNonSRIUnprefixed(rest, std::move(hashType)), }; } else if (prefix == "fixed") { // Parse method @@ -80,7 +80,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { HashType hashType = parseHashType_(); return FixedOutputHash { .method = method, - .hash = Hash::parseAny(rest, std::move(hashType)), + .hash = Hash::parseNonSRIUnprefixed(rest, std::move(hashType)), }; } else throw UsageError("content address prefix \"%s\" is unrecognized. Recogonized prefixes are \"text\" or \"fixed\"", prefix); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 30ce32354..149994e3e 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -118,7 +118,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream const HashType hashType = parseHashType(hashAlgo); fsh = FixedOutputHash { .method = std::move(method), - .hash = Hash::parseAny(hash, hashType), + .hash = Hash::parseNonSRIUnprefixed(hash, hashType), }; } @@ -416,7 +416,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) auto hashType = parseHashType(hashAlgo); fsh = FixedOutputHash { .method = std::move(method), - .hash = Hash::parseAny(hash, hashType), + .hash = Hash::parseNonSRIUnprefixed(hash, hashType), }; } From a7cd7425d9341cf8a2c3af80b84cc55e874515c6 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 2 Jul 2020 23:10:11 +0000 Subject: [PATCH 32/74] Move `getParsedTypeAndSRI` to a more suitable location Also mark it static --- src/libutil/hash.cc | 21 +++++++++++++++++++++ src/libutil/parser.hh | 21 --------------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 1150e74ed..76fa67086 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -144,6 +144,27 @@ Hash Hash::parseSRI(std::string_view original) { return Hash(rest, parsedType, true); } +// Mutates the string to eliminate the prefixes when found +static std::pair, bool> getParsedTypeAndSRI(std::string_view & rest) { + bool isSRI = false; + + // Parse the has type before the separater, if there was one. + std::optional optParsedType; + { + auto hashRaw = splitPrefix(rest, ':'); + + if (!hashRaw) { + hashRaw = splitPrefix(rest, '-'); + if (hashRaw) + isSRI = true; + } + if (hashRaw) + optParsedType = parseHashType(*hashRaw); + } + + return {optParsedType, isSRI}; +} + Hash Hash::parseAnyPrefixed(std::string_view original) { auto rest = original; diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh index d20e4dfc6..a6a83ce89 100644 --- a/src/libutil/parser.hh +++ b/src/libutil/parser.hh @@ -21,26 +21,5 @@ static inline std::optional splitPrefix(std::string_view & str return std::nullopt; } -// Mutates the string to eliminate the prefixes when found -std::pair, bool> getParsedTypeAndSRI(std::string_view & rest) { - bool isSRI = false; - - // Parse the has type before the separater, if there was one. - std::optional optParsedType; - { - auto hashRaw = splitPrefix(rest, ':'); - - if (!hashRaw) { - hashRaw = splitPrefix(rest, '-'); - if (hashRaw) - isSRI = true; - } - if (hashRaw) - optParsedType = parseHashType(*hashRaw); - } - - return std::make_pair(optParsedType, isSRI); -} - } From 13796be78dfa9d3a189ea6b482659c56b1301634 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 2 Jul 2020 23:16:57 +0000 Subject: [PATCH 33/74] Have `splitPrefix` and `splitPrefixTo` parser helpers --- src/libstore/content-address.cc | 8 +++----- src/libutil/hash.cc | 6 +++--- src/libutil/parser.hh | 10 +++++++++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 02ab6710f..470cc62c9 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -46,14 +46,14 @@ ContentAddress parseContentAddress(std::string_view rawCa) { std::string_view prefix; { - auto optPrefix = splitPrefix(rest, ':'); + auto optPrefix = splitPrefixTo(rest, ':'); if (!optPrefix) throw UsageError("not a content address because it is not in the form \":\": %s", rawCa); prefix = *optPrefix; } auto parseHashType_ = [&](){ - auto hashTypeRaw = splitPrefix(rest, ':'); + auto hashTypeRaw = splitPrefixTo(rest, ':'); if (!hashTypeRaw) throw UsageError("content address hash must be in form \":\", but found: %s", rawCa); HashType hashType = parseHashType(*hashTypeRaw); @@ -73,10 +73,8 @@ ContentAddress parseContentAddress(std::string_view rawCa) { } else if (prefix == "fixed") { // Parse method auto method = FileIngestionMethod::Flat; - if (rest.substr(0, 2) == "r:") { + if (splitPrefix(rest, "r:")) method = FileIngestionMethod::Recursive; - rest = rest.substr(2); - } HashType hashType = parseHashType_(); return FixedOutputHash { .method = method, diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 76fa67086..35054462c 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -136,7 +136,7 @@ Hash Hash::parseSRI(std::string_view original) { auto rest = original; // Parse the has type before the separater, if there was one. - auto hashRaw = splitPrefix(rest, '-'); + auto hashRaw = splitPrefixTo(rest, '-'); if (!hashRaw) throw BadHash("hash '%s' is not SRI", original); HashType parsedType = parseHashType(*hashRaw); @@ -151,10 +151,10 @@ static std::pair, bool> getParsedTypeAndSRI(std::string_ // Parse the has type before the separater, if there was one. std::optional optParsedType; { - auto hashRaw = splitPrefix(rest, ':'); + auto hashRaw = splitPrefixTo(rest, ':'); if (!hashRaw) { - hashRaw = splitPrefix(rest, '-'); + hashRaw = splitPrefixTo(rest, '-'); if (hashRaw) isSRI = true; } diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh index a6a83ce89..d19d7d8ed 100644 --- a/src/libutil/parser.hh +++ b/src/libutil/parser.hh @@ -3,13 +3,15 @@ #include #include +#include "util.hh" + namespace nix { // If `separator` is found, we return the portion of the string before the // separator, and modify the string argument to contain only the part after the // separator. Otherwise, wer return `std::nullopt`, and we leave the argument // string alone. -static inline std::optional splitPrefix(std::string_view & string, char separator) { +static inline std::optional splitPrefixTo(std::string_view & string, char separator) { auto sepInstance = string.find(separator); if (sepInstance != std::string_view::npos) { @@ -21,5 +23,11 @@ static inline std::optional splitPrefix(std::string_view & str return std::nullopt; } +static inline bool splitPrefix(std::string_view & string, std::string_view prefix) { + bool res = hasPrefix(string, prefix); + if (res) + string.remove_prefix(prefix.length()); + return res; +} } From d291be444bf6e50ba13235be975db4443b162903 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 3 Jul 2020 14:49:22 +0000 Subject: [PATCH 34/74] Fix Perl --- perl/lib/Nix/Store.xs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 2a2a0d429..595153e36 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -285,7 +285,7 @@ SV * addToStore(char * srcPath, int recursive, char * algo) SV * makeFixedOutputPath(int recursive, char * algo, char * hash, char * name) PPCODE: try { - Hash h(hash, parseHashType(algo)); + auto h = Hash::parseAny(hash, parseHashType(algo)); auto method = recursive ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; auto path = store()->makeFixedOutputPath(method, h, name); XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(path).c_str(), 0))); From d4250fef239b28b4a098e70e1deac889616dbb9a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 3 Jul 2020 15:17:20 +0000 Subject: [PATCH 35/74] Fix Perl, again... --- perl/lib/Nix/Store.xs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 595153e36..97cc4d94e 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -224,7 +224,7 @@ SV * hashString(char * algo, int base32, char * s) SV * convertHash(char * algo, char * s, int toBase32) PPCODE: try { - Hash h(s, parseHashType(algo)); + auto h = Hash::parseAny(s, parseHashType(algo)); string s = h.to_string(toBase32 ? Base32 : Base16, false); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); } catch (Error & e) { From ffc18583b1a086849ac0efd17da40ff510299b52 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 22:15:14 +0000 Subject: [PATCH 36/74] 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(-) 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; + + } From fedfc913ad75984b476e7838d6254c547f9134d5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 16:12:21 +0000 Subject: [PATCH 37/74] 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(-) 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); } } From 230c9b4329b3d285e57f4cce058c121256187da1 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 16:12:21 +0000 Subject: [PATCH 38/74] 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(-) 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); } } From 1feb8981df6adf8519a1f2d31883eb12db11fcb5 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 17 Jul 2020 11:33:27 -0400 Subject: [PATCH 39/74] 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(-) 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"); } } From 205dcd140d46db94481329578b4fee8275e6c534 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 17 Jul 2020 12:43:46 -0400 Subject: [PATCH 40/74] 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(-) 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); } } From bbc633c98ca2c2f11303efafe4d58edd6d9b1018 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 17 Jul 2020 13:10:32 -0400 Subject: [PATCH 41/74] 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(-) 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)) From 6756cecfcf266801219b1e2da7e79f14695ccecf Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 17 Jul 2020 19:55:41 +0000 Subject: [PATCH 42/74] Add `DerivationType::CAFloating` --- src/libstore/derivations.cc | 28 +++++++++++++++++++++------- src/libstore/derivations.hh | 1 + 2 files changed, 22 insertions(+), 7 deletions(-) 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, From 5ae747b43498076efe9b60d1a871f9956dd3fc84 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 17 Jul 2020 21:50:09 +0000 Subject: [PATCH 43/74] Remove stray added file --- local-store-temp.cc | 1787 ------------------------------------------- 1 file changed, 1787 deletions(-) delete mode 100644 local-store-temp.cc diff --git a/local-store-temp.cc b/local-store-temp.cc deleted file mode 100644 index be6e081cb..000000000 --- a/local-store-temp.cc +++ /dev/null @@ -1,1787 +0,0 @@ -#include "local-store.hh" -#include "globals.hh" -#include "git.hh" -#include "archive.hh" -#include "pathlocks.hh" -#include "worker-protocol.hh" -#include "derivations.hh" -#include "nar-info.hh" -#include "references.hh" - -#include -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#if __linux__ -#include -#include -#include -#include -#include -#endif - -#ifdef __CYGWIN__ -#include -#endif - -#include - - -namespace nix { - - -LocalStore::LocalStore(const Params & params) - : Store(params) - , LocalFSStore(params) - , realStoreDir_{this, false, rootDir != "" ? rootDir + "/nix/store" : storeDir, "real", - "physical path to the Nix store"} - , realStoreDir(realStoreDir_) - , dbDir(stateDir + "/db") - , linksDir(realStoreDir + "/.links") - , reservedPath(dbDir + "/reserved") - , schemaPath(dbDir + "/schema") - , trashDir(realStoreDir + "/trash") - , tempRootsDir(stateDir + "/temproots") - , fnTempRoots(fmt("%s/%d", tempRootsDir, getpid())) - , locksHeld(tokenizeString(getEnv("NIX_HELD_LOCKS").value_or(""))) -{ - auto state(_state.lock()); - - /* Create missing state directories if they don't already exist. */ - createDirs(realStoreDir); - makeStoreWritable(); - createDirs(linksDir); - Path profilesDir = stateDir + "/profiles"; - createDirs(profilesDir); - createDirs(tempRootsDir); - createDirs(dbDir); - Path gcRootsDir = stateDir + "/gcroots"; - if (!pathExists(gcRootsDir)) { - createDirs(gcRootsDir); - createSymlink(profilesDir, gcRootsDir + "/profiles"); - } - - for (auto & perUserDir : {profilesDir + "/per-user", gcRootsDir + "/per-user"}) { - createDirs(perUserDir); - if (chmod(perUserDir.c_str(), 0755) == -1) - throw SysError("could not set permissions on '%s' to 755", perUserDir); - } - - createUser(getUserName(), getuid()); - - /* Optionally, create directories and set permissions for a - multi-user install. */ - if (getuid() == 0 && settings.buildUsersGroup != "") { - mode_t perm = 01775; - - struct group * gr = getgrnam(settings.buildUsersGroup.get().c_str()); - if (!gr) - logError({ - .name = "'build-users-group' not found", - .hint = hintfmt( - "warning: the group '%1%' specified in 'build-users-group' does not exist", - settings.buildUsersGroup) - }); - else { - struct stat st; - if (stat(realStoreDir.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", realStoreDir); - - if (st.st_uid != 0 || st.st_gid != gr->gr_gid || (st.st_mode & ~S_IFMT) != perm) { - if (chown(realStoreDir.c_str(), 0, gr->gr_gid) == -1) - throw SysError("changing ownership of path '%1%'", realStoreDir); - if (chmod(realStoreDir.c_str(), perm) == -1) - throw SysError("changing permissions on path '%1%'", realStoreDir); - } - } - } - - /* Ensure that the store and its parents are not symlinks. */ - if (getEnv("NIX_IGNORE_SYMLINK_STORE") != "1") { - Path path = realStoreDir; - struct stat st; - while (path != "/") { - if (lstat(path.c_str(), &st)) - throw SysError("getting status of '%1%'", path); - if (S_ISLNK(st.st_mode)) - throw Error( - "the path '%1%' is a symlink; " - "this is not allowed for the Nix store and its parent directories", - path); - path = dirOf(path); - } - } - - /* We can't open a SQLite database if the disk is full. Since - this prevents the garbage collector from running when it's most - needed, we reserve some dummy space that we can free just - before doing a garbage collection. */ - try { - struct stat st; - if (stat(reservedPath.c_str(), &st) == -1 || - st.st_size != settings.reservedSize) - { - AutoCloseFD fd = open(reservedPath.c_str(), O_WRONLY | O_CREAT | O_CLOEXEC, 0600); - int res = -1; -#if HAVE_POSIX_FALLOCATE - res = posix_fallocate(fd.get(), 0, settings.reservedSize); -#endif - if (res == -1) { - writeFull(fd.get(), string(settings.reservedSize, 'X')); - [[gnu::unused]] auto res2 = ftruncate(fd.get(), settings.reservedSize); - } - } - } catch (SysError & e) { /* don't care about errors */ - } - - /* Acquire the big fat lock in shared mode to make sure that no - schema upgrade is in progress. */ - Path globalLockPath = dbDir + "/big-lock"; - globalLock = openLockFile(globalLockPath.c_str(), true); - - if (!lockFile(globalLock.get(), ltRead, false)) { - printInfo("waiting for the big Nix store lock..."); - lockFile(globalLock.get(), ltRead, true); - } - - /* Check the current database schema and if necessary do an - upgrade. */ - int curSchema = getSchema(); - if (curSchema > nixSchemaVersion) - throw Error("current Nix store schema is version %1%, but I only support %2%", - curSchema, nixSchemaVersion); - - else if (curSchema == 0) { /* new store */ - curSchema = nixSchemaVersion; - openDB(*state, true); - writeFile(schemaPath, (format("%1%") % nixSchemaVersion).str()); - } - - else if (curSchema < nixSchemaVersion) { - if (curSchema < 5) - throw Error( - "Your Nix store has a database in Berkeley DB format,\n" - "which is no longer supported. To convert to the new format,\n" - "please upgrade Nix to version 0.12 first."); - - if (curSchema < 6) - throw Error( - "Your Nix store has a database in flat file format,\n" - "which is no longer supported. To convert to the new format,\n" - "please upgrade Nix to version 1.11 first."); - - if (!lockFile(globalLock.get(), ltWrite, false)) { - printInfo("waiting for exclusive access to the Nix store..."); - lockFile(globalLock.get(), ltWrite, true); - } - - /* Get the schema version again, because another process may - have performed the upgrade already. */ - curSchema = getSchema(); - - if (curSchema < 7) { upgradeStore7(); } - - openDB(*state, false); - - if (curSchema < 8) { - SQLiteTxn txn(state->db); - state->db.exec("alter table ValidPaths add column ultimate integer"); - state->db.exec("alter table ValidPaths add column sigs text"); - txn.commit(); - } - - if (curSchema < 9) { - SQLiteTxn txn(state->db); - state->db.exec("drop table FailedPaths"); - txn.commit(); - } - - if (curSchema < 10) { - SQLiteTxn txn(state->db); - state->db.exec("alter table ValidPaths add column ca text"); - txn.commit(); - } - - writeFile(schemaPath, (format("%1%") % nixSchemaVersion).str()); - - lockFile(globalLock.get(), ltRead, true); - } - - else openDB(*state, false); - - /* Prepare SQL statements. */ - state->stmtRegisterValidPath.create(state->db, - "insert into ValidPaths (path, hash, registrationTime, deriver, narSize, ultimate, sigs, ca) values (?, ?, ?, ?, ?, ?, ?, ?);"); - state->stmtUpdatePathInfo.create(state->db, - "update ValidPaths set narSize = ?, hash = ?, ultimate = ?, sigs = ?, ca = ? where path = ?;"); - state->stmtAddReference.create(state->db, - "insert or replace into Refs (referrer, reference) values (?, ?);"); - state->stmtQueryPathInfo.create(state->db, - "select id, hash, registrationTime, deriver, narSize, ultimate, sigs, ca from ValidPaths where path = ?;"); - state->stmtQueryReferences.create(state->db, - "select path from Refs join ValidPaths on reference = id where referrer = ?;"); - state->stmtQueryReferrers.create(state->db, - "select path from Refs join ValidPaths on referrer = id where reference = (select id from ValidPaths where path = ?);"); - state->stmtInvalidatePath.create(state->db, - "delete from ValidPaths where path = ?;"); - state->stmtAddDerivationOutput.create(state->db, - "insert or replace into DerivationOutputs (drv, id, path) values (?, ?, ?);"); - state->stmtQueryValidDerivers.create(state->db, - "select v.id, v.path from DerivationOutputs d join ValidPaths v on d.drv = v.id where d.path = ?;"); - state->stmtQueryDerivationOutputs.create(state->db, - "select id, path from DerivationOutputs where drv = ?;"); - // Use "path >= ?" with limit 1 rather than "path like '?%'" to - // ensure efficient lookup. - state->stmtQueryPathFromHashPart.create(state->db, - "select path from ValidPaths where path >= ? limit 1;"); - state->stmtQueryValidPaths.create(state->db, "select path from ValidPaths"); -} - - -LocalStore::~LocalStore() -{ - std::shared_future future; - - { - auto state(_state.lock()); - if (state->gcRunning) - future = state->gcFuture; - } - - if (future.valid()) { - printInfo("waiting for auto-GC to finish on exit..."); - future.get(); - } - - try { - auto state(_state.lock()); - if (state->fdTempRoots) { - state->fdTempRoots = -1; - unlink(fnTempRoots.c_str()); - } - } catch (...) { - ignoreException(); - } -} - - -std::string LocalStore::getUri() -{ - return "local"; -} - - -int LocalStore::getSchema() -{ - int curSchema = 0; - if (pathExists(schemaPath)) { - string s = readFile(schemaPath); - if (!string2Int(s, curSchema)) - throw Error("'%1%' is corrupt", schemaPath); - } - return curSchema; -} - - -void LocalStore::openDB(State & state, bool create) -{ - if (access(dbDir.c_str(), R_OK | W_OK)) - throw SysError("Nix database directory '%1%' is not writable", dbDir); - - /* Open the Nix database. */ - string dbPath = dbDir + "/db.sqlite"; - auto & db(state.db); - state.db = SQLite(dbPath, create); - -#ifdef __CYGWIN__ - /* The cygwin version of sqlite3 has a patch which calls - SetDllDirectory("/usr/bin") on init. It was intended to fix extension - loading, which we don't use, and the effect of SetDllDirectory is - inherited by child processes, and causes libraries to be loaded from - /usr/bin instead of $PATH. This breaks quite a few things (e.g. - checkPhase on openssh), so we set it back to default behaviour. */ - SetDllDirectoryW(L""); -#endif - - /* !!! check whether sqlite has been built with foreign key - support */ - - /* Whether SQLite should fsync(). "Normal" synchronous mode - should be safe enough. If the user asks for it, don't sync at - all. This can cause database corruption if the system - crashes. */ - string syncMode = settings.fsyncMetadata ? "normal" : "off"; - db.exec("pragma synchronous = " + syncMode); - - /* Set the SQLite journal mode. WAL mode is fastest, so it's the - default. */ - string mode = settings.useSQLiteWAL ? "wal" : "truncate"; - string prevMode; - { - SQLiteStmt stmt; - stmt.create(db, "pragma main.journal_mode;"); - if (sqlite3_step(stmt) != SQLITE_ROW) - throwSQLiteError(db, "querying journal mode"); - prevMode = string((const char *) sqlite3_column_text(stmt, 0)); - } - if (prevMode != mode && - sqlite3_exec(db, ("pragma main.journal_mode = " + mode + ";").c_str(), 0, 0, 0) != SQLITE_OK) - throwSQLiteError(db, "setting journal mode"); - - /* Increase the auto-checkpoint interval to 40000 pages. This - seems enough to ensure that instantiating the NixOS system - derivation is done in a single fsync(). */ - if (mode == "wal" && sqlite3_exec(db, "pragma wal_autocheckpoint = 40000;", 0, 0, 0) != SQLITE_OK) - throwSQLiteError(db, "setting autocheckpoint interval"); - - /* Initialise the database schema, if necessary. */ - if (create) { - static const char schema[] = -#include "schema.sql.gen.hh" - ; - db.exec(schema); - } -} - - -/* To improve purity, users may want to make the Nix store a read-only - bind mount. So make the Nix store writable for this process. */ -void LocalStore::makeStoreWritable() -{ -#if __linux__ - if (getuid() != 0) return; - /* Check if /nix/store is on a read-only mount. */ - struct statvfs stat; - if (statvfs(realStoreDir.c_str(), &stat) != 0) - throw SysError("getting info about the Nix store mount point"); - - if (stat.f_flag & ST_RDONLY) { - if (unshare(CLONE_NEWNS) == -1) - throw SysError("setting up a private mount namespace"); - - if (mount(0, realStoreDir.c_str(), "none", MS_REMOUNT | MS_BIND, 0) == -1) - throw SysError("remounting %1% writable", realStoreDir); - } -#endif -} - - -const time_t mtimeStore = 1; /* 1 second into the epoch */ - - -static void canonicaliseTimestampAndPermissions(const Path & path, const struct stat & st) -{ - if (!S_ISLNK(st.st_mode)) { - - /* Mask out all type related bits. */ - mode_t mode = st.st_mode & ~S_IFMT; - - if (mode != 0444 && mode != 0555) { - mode = (st.st_mode & S_IFMT) - | 0444 - | (st.st_mode & S_IXUSR ? 0111 : 0); - if (chmod(path.c_str(), mode) == -1) - throw SysError("changing mode of '%1%' to %2$o", path, mode); - } - - } - - if (st.st_mtime != mtimeStore) { - struct timeval times[2]; - times[0].tv_sec = st.st_atime; - times[0].tv_usec = 0; - times[1].tv_sec = mtimeStore; - times[1].tv_usec = 0; -#if HAVE_LUTIMES - if (lutimes(path.c_str(), times) == -1) - if (errno != ENOSYS || - (!S_ISLNK(st.st_mode) && utimes(path.c_str(), times) == -1)) -#else - if (!S_ISLNK(st.st_mode) && utimes(path.c_str(), times) == -1) -#endif - throw SysError("changing modification time of '%1%'", path); - } -} - - -void canonicaliseTimestampAndPermissions(const Path & path) -{ - struct stat st; - if (lstat(path.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", path); - canonicaliseTimestampAndPermissions(path, st); -} - - -static void canonicalisePathMetaData_(const Path & path, uid_t fromUid, InodesSeen & inodesSeen) -{ - checkInterrupt(); - -#if __APPLE__ - /* Remove flags, in particular UF_IMMUTABLE which would prevent - the file from being garbage-collected. FIXME: Use - setattrlist() to remove other attributes as well. */ - if (lchflags(path.c_str(), 0)) { - if (errno != ENOTSUP) - throw SysError("clearing flags of path '%1%'", path); - } -#endif - - struct stat st; - if (lstat(path.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", path); - - /* Really make sure that the path is of a supported type. */ - if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode))) - throw Error("file '%1%' has an unsupported type", path); - -#if __linux__ - /* Remove extended attributes / ACLs. */ - ssize_t eaSize = llistxattr(path.c_str(), nullptr, 0); - - if (eaSize < 0) { - if (errno != ENOTSUP && errno != ENODATA) - throw SysError("querying extended attributes of '%s'", path); - } else if (eaSize > 0) { - std::vector eaBuf(eaSize); - - if ((eaSize = llistxattr(path.c_str(), eaBuf.data(), eaBuf.size())) < 0) - throw SysError("querying extended attributes of '%s'", path); - - for (auto & eaName: tokenizeString(std::string(eaBuf.data(), eaSize), std::string("\000", 1))) { - /* Ignore SELinux security labels since these cannot be - removed even by root. */ - if (eaName == "security.selinux") continue; - if (lremovexattr(path.c_str(), eaName.c_str()) == -1) - throw SysError("removing extended attribute '%s' from '%s'", eaName, path); - } - } -#endif - - /* Fail if the file is not owned by the build user. This prevents - us from messing up the ownership/permissions of files - hard-linked into the output (e.g. "ln /etc/shadow $out/foo"). - However, ignore files that we chown'ed ourselves previously to - ensure that we don't fail on hard links within the same build - (i.e. "touch $out/foo; ln $out/foo $out/bar"). */ - if (fromUid != (uid_t) -1 && st.st_uid != fromUid) { - assert(!S_ISDIR(st.st_mode)); - if (inodesSeen.find(Inode(st.st_dev, st.st_ino)) == inodesSeen.end()) - throw BuildError("invalid ownership on file '%1%'", path); - mode_t mode = st.st_mode & ~S_IFMT; - assert(S_ISLNK(st.st_mode) || (st.st_uid == geteuid() && (mode == 0444 || mode == 0555) && st.st_mtime == mtimeStore)); - return; - } - - inodesSeen.insert(Inode(st.st_dev, st.st_ino)); - - canonicaliseTimestampAndPermissions(path, st); - - /* Change ownership to the current uid. If it's a symlink, use - lchown if available, otherwise don't bother. Wrong ownership - of a symlink doesn't matter, since the owning user can't change - the symlink and can't delete it because the directory is not - writable. The only exception is top-level paths in the Nix - store (since that directory is group-writable for the Nix build - users group); we check for this case below. */ - if (st.st_uid != geteuid()) { -#if HAVE_LCHOWN - if (lchown(path.c_str(), geteuid(), getegid()) == -1) -#else - if (!S_ISLNK(st.st_mode) && - chown(path.c_str(), geteuid(), getegid()) == -1) -#endif - throw SysError("changing owner of '%1%' to %2%", - path, geteuid()); - } - - if (S_ISDIR(st.st_mode)) { - DirEntries entries = readDirectory(path); - for (auto & i : entries) - canonicalisePathMetaData_(path + "/" + i.name, fromUid, inodesSeen); - } -} - - -void canonicalisePathMetaData(const Path & path, uid_t fromUid, InodesSeen & inodesSeen) -{ - canonicalisePathMetaData_(path, fromUid, inodesSeen); - - /* On platforms that don't have lchown(), the top-level path can't - be a symlink, since we can't change its ownership. */ - struct stat st; - if (lstat(path.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", path); - - if (st.st_uid != geteuid()) { - assert(S_ISLNK(st.st_mode)); - throw Error("wrong ownership of top-level store path '%1%'", path); - } -} - - -void canonicalisePathMetaData(const Path & path, uid_t fromUid) -{ - InodesSeen inodesSeen; - canonicalisePathMetaData(path, fromUid, inodesSeen); -} - - -void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivation & drv) -{ - assert(drvPath.isDerivation()); - 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) - { - 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'", - printStorePath(drvPath), varName, printStorePath(actual)); - }; - - - 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)); - - check( - makeFixedOutputPath( - out->second.hash->method, - out->second.hash->hash, - 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); - } -} - - -uint64_t LocalStore::addValidPath(State & state, - const ValidPathInfo & info, bool checkOutputs) -{ - if (info.ca.has_value() && !info.isContentAddressed(*this)) - throw Error("cannot add path '%s' to the Nix store because it claims to be content-addressed but isn't", - printStorePath(info.path)); - - state.stmtRegisterValidPath.use() - (printStorePath(info.path)) - (info.narHash.to_string(Base16, true)) - (info.registrationTime == 0 ? time(0) : info.registrationTime) - (info.deriver ? printStorePath(*info.deriver) : "", (bool) info.deriver) - (info.narSize, info.narSize != 0) - (info.ultimate ? 1 : 0, info.ultimate) - (concatStringsSep(" ", info.sigs), !info.sigs.empty()) - (renderContentAddress(info.ca), (bool) info.ca) - .exec(); - uint64_t id = sqlite3_last_insert_rowid(state.db); - - /* If this is a derivation, then store the derivation outputs in - the database. This is useful for the garbage collector: it can - efficiently query whether a path is an output of some - derivation. */ - if (info.path.isDerivation()) { - auto drv = readDerivation(info.path); - - /* Verify that the output paths in the derivation are correct - (i.e., follow the scheme for computing output paths from - derivations). Note that if this throws an error, then the - DB transaction is rolled back, so the path validity - registration above is undone. */ - if (checkOutputs) checkDerivationOutputs(info.path, drv); - - for (auto & i : drv.outputs) { - state.stmtAddDerivationOutput.use() - (id) - (i.first) - (printStorePath(i.second.path)) - .exec(); - } - } - - { - auto state_(Store::state.lock()); - state_->pathInfoCache.upsert(std::string(info.path.hashPart()), - PathInfoCacheValue{ .value = std::make_shared(info) }); - } - - return id; -} - - -void LocalStore::queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept -{ - try { - auto info = std::make_shared(path); - - callback(retrySQLite>([&]() { - auto state(_state.lock()); - - /* Get the path info. */ - auto useQueryPathInfo(state->stmtQueryPathInfo.use()(printStorePath(info->path))); - - if (!useQueryPathInfo.next()) - return std::shared_ptr(); - - info->id = useQueryPathInfo.getInt(0); - - try { - info->narHash = Hash(useQueryPathInfo.getStr(1)); - } catch (BadHash & e) { - throw Error("in valid-path entry for '%s': %s", printStorePath(path), e.what()); - } - - info->registrationTime = useQueryPathInfo.getInt(2); - - auto s = (const char *) sqlite3_column_text(state->stmtQueryPathInfo, 3); - if (s) info->deriver = parseStorePath(s); - - /* Note that narSize = NULL yields 0. */ - info->narSize = useQueryPathInfo.getInt(4); - - info->ultimate = useQueryPathInfo.getInt(5) == 1; - - s = (const char *) sqlite3_column_text(state->stmtQueryPathInfo, 6); - if (s) info->sigs = tokenizeString(s, " "); - - s = (const char *) sqlite3_column_text(state->stmtQueryPathInfo, 7); - if (s) info->ca = parseContentAddressOpt(s); - - /* Get the references. */ - auto useQueryReferences(state->stmtQueryReferences.use()(info->id)); - - while (useQueryReferences.next()) - info->references.insert(parseStorePath(useQueryReferences.getStr(0))); - - return info; - })); - - } catch (...) { callback.rethrow(); } -} - - -/* Update path info in the database. */ -void LocalStore::updatePathInfo(State & state, const ValidPathInfo & info) -{ - state.stmtUpdatePathInfo.use() - (info.narSize, info.narSize != 0) - (info.narHash.to_string(Base16, true)) - (info.ultimate ? 1 : 0, info.ultimate) - (concatStringsSep(" ", info.sigs), !info.sigs.empty()) - (renderContentAddress(info.ca), (bool) info.ca) - (printStorePath(info.path)) - .exec(); -} - - -uint64_t LocalStore::queryValidPathId(State & state, const StorePath & path) -{ - auto use(state.stmtQueryPathInfo.use()(printStorePath(path))); - if (!use.next()) - throw Error("path '%s' is not valid", printStorePath(path)); - return use.getInt(0); -} - - -bool LocalStore::isValidPath_(State & state, const StorePath & path) -{ - return state.stmtQueryPathInfo.use()(printStorePath(path)).next(); -} - - -bool LocalStore::isValidPathUncached(const StorePath & path) -{ - return retrySQLite([&]() { - auto state(_state.lock()); - return isValidPath_(*state, path); - }); -} - - -StorePathSet LocalStore::queryValidPaths(const StorePathSet & paths, SubstituteFlag maybeSubstitute) -{ - StorePathSet res; - for (auto & i : paths) - if (isValidPath(i)) res.insert(i); - return res; -} - - -StorePathSet LocalStore::queryAllValidPaths() -{ - return retrySQLite([&]() { - auto state(_state.lock()); - auto use(state->stmtQueryValidPaths.use()); - StorePathSet res; - while (use.next()) res.insert(parseStorePath(use.getStr(0))); - return res; - }); -} - - -void LocalStore::queryReferrers(State & state, const StorePath & path, StorePathSet & referrers) -{ - auto useQueryReferrers(state.stmtQueryReferrers.use()(printStorePath(path))); - - while (useQueryReferrers.next()) - referrers.insert(parseStorePath(useQueryReferrers.getStr(0))); -} - - -void LocalStore::queryReferrers(const StorePath & path, StorePathSet & referrers) -{ - return retrySQLite([&]() { - auto state(_state.lock()); - queryReferrers(*state, path, referrers); - }); -} - - -StorePathSet LocalStore::queryValidDerivers(const StorePath & path) -{ - return retrySQLite([&]() { - auto state(_state.lock()); - - auto useQueryValidDerivers(state->stmtQueryValidDerivers.use()(printStorePath(path))); - - StorePathSet derivers; - while (useQueryValidDerivers.next()) - derivers.insert(parseStorePath(useQueryValidDerivers.getStr(1))); - - return derivers; - }); -} - - -OutputPathMap LocalStore::queryDerivationOutputMap(const StorePath & path) -{ - return retrySQLite([&]() { - auto state(_state.lock()); - - auto useQueryDerivationOutputs(state->stmtQueryDerivationOutputs.use() - (queryValidPathId(*state, path))); - - OutputPathMap outputs; - while (useQueryDerivationOutputs.next()) - outputs.emplace( - useQueryDerivationOutputs.getStr(0), - parseStorePath(useQueryDerivationOutputs.getStr(1)) - ); - - return outputs; - }); -} - - -std::optional LocalStore::queryPathFromHashPart(const std::string & hashPart) -{ - if (hashPart.size() != StorePath::HashLen) throw Error("invalid hash part"); - - Path prefix = storeDir + "/" + hashPart; - - return retrySQLite>([&]() -> std::optional { - auto state(_state.lock()); - - auto useQueryPathFromHashPart(state->stmtQueryPathFromHashPart.use()(prefix)); - - if (!useQueryPathFromHashPart.next()) return {}; - - const char * s = (const char *) sqlite3_column_text(state->stmtQueryPathFromHashPart, 0); - if (s && prefix.compare(0, prefix.size(), s, prefix.size()) == 0) - return parseStorePath(s); - return {}; - }); -} - - -StorePathSet LocalStore::querySubstitutablePaths(const StorePathSet & paths) -{ - if (!settings.useSubstitutes) return StorePathSet(); - - StorePathSet remaining; - for (auto & i : paths) - remaining.insert(i); - - StorePathSet res; - - for (auto & sub : getDefaultSubstituters()) { - if (remaining.empty()) break; - if (sub->storeDir != storeDir) continue; - if (!sub->wantMassQuery) continue; - - auto valid = sub->queryValidPaths(remaining); - - StorePathSet remaining2; - for (auto & path : remaining) - if (valid.count(path)) - res.insert(path); - else - remaining2.insert(path); - - std::swap(remaining, remaining2); - } - - return res; -} - - -void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos) -{ - if (!settings.useSubstitutes) return; - for (auto & sub : getDefaultSubstituters()) { - if (sub->storeDir != storeDir) continue; - for (auto & path : paths) { - if (infos.count(path)) continue; - debug("checking substituter '%s' for path '%s'", sub->getUri(), printStorePath(path)); - try { - auto info = sub->queryPathInfo(path); - auto narInfo = std::dynamic_pointer_cast( - std::shared_ptr(info)); - infos.insert_or_assign(path, SubstitutablePathInfo{ - info->deriver, - info->references, - narInfo ? narInfo->fileSize : 0, - info->narSize}); - } catch (InvalidPath &) { - } catch (SubstituterDisabled &) { - } catch (Error & e) { - if (settings.tryFallback) - logError(e.info()); - else - throw; - } - } - } -} - - -void LocalStore::registerValidPath(const ValidPathInfo & info) -{ - ValidPathInfos infos; - infos.push_back(info); - registerValidPaths(infos); -} - - -void LocalStore::registerValidPaths(const ValidPathInfos & infos) -{ - /* SQLite will fsync by default, but the new valid paths may not - be fsync-ed. So some may want to fsync them before registering - the validity, at the expense of some speed of the path - registering operation. */ - if (settings.syncBeforeRegistering) sync(); - - return retrySQLite([&]() { - auto state(_state.lock()); - - SQLiteTxn txn(state->db); - StorePathSet paths; - - for (auto & i : infos) { - assert(i.narHash.type == htSHA256); - if (isValidPath_(*state, i.path)) - updatePathInfo(*state, i); - else - addValidPath(*state, i, false); - paths.insert(i.path); - } - - for (auto & i : infos) { - auto referrer = queryValidPathId(*state, i.path); - for (auto & j : i.references) - state->stmtAddReference.use()(referrer)(queryValidPathId(*state, j)).exec(); - } - - /* Check that the derivation outputs are correct. We can't do - this in addValidPath() above, because the references might - not be valid yet. */ - for (auto & i : infos) - if (i.path.isDerivation()) { - // FIXME: inefficient; we already loaded the derivation in addValidPath(). - checkDerivationOutputs(i.path, readDerivation(i.path)); - } - - /* Do a topological sort of the paths. This will throw an - error if a cycle is detected and roll back the - transaction. Cycles can only occur when a derivation - has multiple outputs. */ - topoSortPaths(paths); - - txn.commit(); - }); -} - - -/* Invalidate a path. The caller is responsible for checking that - there are no referrers. */ -void LocalStore::invalidatePath(State & state, const StorePath & path) -{ - debug("invalidating path '%s'", printStorePath(path)); - - state.stmtInvalidatePath.use()(printStorePath(path)).exec(); - - /* Note that the foreign key constraints on the Refs table take - care of deleting the references entries for `path'. */ - - { - auto state_(Store::state.lock()); - state_->pathInfoCache.erase(std::string(path.hashPart())); - } -} - - -const PublicKeys & LocalStore::getPublicKeys() -{ - auto state(_state.lock()); - if (!state->publicKeys) - state->publicKeys = std::make_unique(getDefaultPublicKeys()); - return *state->publicKeys; -} - - -void LocalStore::addToStore(const ValidPathInfo & info, Source & source, - RepairFlag repair, CheckSigsFlag checkSigs) -{ - if (!info.narHash) - throw Error("cannot add path '%s' because it lacks a hash", printStorePath(info.path)); - - if (requireSigs && checkSigs && !info.checkSignatures(*this, getPublicKeys())) - throw Error("cannot add path '%s' because it lacks a valid signature", printStorePath(info.path)); - - addTempRoot(info.path); - - if (repair || !isValidPath(info.path)) { - - PathLocks outputLock; - - auto realPath = Store::toRealPath(info.path); - - /* Lock the output path. But don't lock if we're being called - from a build hook (whose parent process already acquired a - lock on this path). */ - if (!locksHeld.count(printStorePath(info.path))) - outputLock.lockPaths({realPath}); - - if (repair || !isValidPath(info.path)) { - - deletePath(realPath); - - // text hashing has long been allowed to have non-self-references because it is used for drv files. - bool refersToSelf = info.references.count(info.path) > 0; - if (info.ca.has_value() && !info.references.empty() && !(std::holds_alternative(*info.ca) && !refersToSelf)) - settings.requireExperimentalFeature("ca-references"); - - /* While restoring the path from the NAR, compute the hash - of the NAR. */ - std::unique_ptr hashSink; - if (!info.ca.has_value() || !info.references.count(info.path)) - hashSink = std::make_unique(htSHA256); - else - hashSink = std::make_unique(htSHA256, std::string(info.path.hashPart())); - - LambdaSource wrapperSource([&](unsigned char * data, size_t len) -> size_t { - size_t n = source.read(data, len); - (*hashSink)(data, n); - return n; - }); - - restorePath(realPath, wrapperSource); - - auto hashResult = hashSink->finish(); - - if (hashResult.first != info.narHash) - throw Error("hash mismatch importing path '%s';\n wanted: %s\n got: %s", - printStorePath(info.path), info.narHash.to_string(Base32, true), hashResult.first.to_string(Base32, true)); - - if (hashResult.second != info.narSize) - throw Error("size mismatch importing path '%s';\n wanted: %s\n got: %s", - printStorePath(info.path), info.narSize, hashResult.second); - - autoGC(); - - canonicalisePathMetaData(realPath, -1); - - optimisePath(realPath); // FIXME: combine with hashPath() - - registerValidPath(info); - } - - outputLock.setDeletion(true); - } -} - - -<<<<<<< HEAD -StorePath LocalStore::addToStoreFromDump(const string & dump, const string & name, - FileIngestionMethod method, HashType hashAlgo, RepairFlag repair) -{ - if (method == FileIngestionMethod::Git && hashAlgo != htSHA1) - throw Error("git ingestion must use sha1 hash"); - - Hash h = hashString(hashAlgo, dump); - - auto dstPath = makeFixedOutputPath(method, h, name); - - addTempRoot(dstPath); - - if (repair || !isValidPath(dstPath)) { - - /* The first check above is an optimisation to prevent - unnecessary lock acquisition. */ - - auto realPath = Store::toRealPath(dstPath); - - PathLocks outputLock({realPath}); - - if (repair || !isValidPath(dstPath)) { - - deletePath(realPath); - - autoGC(); - - switch (method) { - case FileIngestionMethod::Flat: - writeFile(realPath, dump); - break; - case FileIngestionMethod::Recursive: { - StringSource source(dump); - restorePath(realPath, source); - break; - } - case FileIngestionMethod::Git: { - StringSource source(dump); - restoreGit(realPath, source, realStoreDir, storeDir); - break; - } - } - - canonicalisePathMetaData(realPath, -1); - - /* Register the SHA-256 hash of the NAR serialisation of - the path in the database. We may just have computed it - above (if called with recursive == true and hashAlgo == - sha256); otherwise, compute it here. */ - HashResult hash; - if (method == FileIngestionMethod::Recursive) { - hash.first = hashAlgo == htSHA256 ? h : hashString(htSHA256, dump); - hash.second = dump.size(); - } else - hash = hashPath(htSHA256, realPath); - - optimisePath(realPath); // FIXME: combine with hashPath() - - ValidPathInfo info(dstPath); - info.narHash = hash.first; - info.narSize = hash.second; - info.ca = FixedOutputHash { .method = method, .hash = h }; - registerValidPath(info); - } - - outputLock.setDeletion(true); - } - - return dstPath; -} - - -||||||| merged common ancestors -StorePath LocalStore::addToStoreFromDump(const string & dump, const string & name, - FileIngestionMethod method, HashType hashAlgo, RepairFlag repair) -{ - Hash h = hashString(hashAlgo, dump); - - auto dstPath = makeFixedOutputPath(method, h, name); - - addTempRoot(dstPath); - - if (repair || !isValidPath(dstPath)) { - - /* The first check above is an optimisation to prevent - unnecessary lock acquisition. */ - - auto realPath = Store::toRealPath(dstPath); - - PathLocks outputLock({realPath}); - - if (repair || !isValidPath(dstPath)) { - - deletePath(realPath); - - autoGC(); - - if (method == FileIngestionMethod::Recursive) { - StringSource source(dump); - restorePath(realPath, source); - } else - writeFile(realPath, dump); - - canonicalisePathMetaData(realPath, -1); - - /* Register the SHA-256 hash of the NAR serialisation of - the path in the database. We may just have computed it - above (if called with recursive == true and hashAlgo == - sha256); otherwise, compute it here. */ - HashResult hash; - if (method == FileIngestionMethod::Recursive) { - hash.first = hashAlgo == htSHA256 ? h : hashString(htSHA256, dump); - hash.second = dump.size(); - } else - hash = hashPath(htSHA256, realPath); - - optimisePath(realPath); // FIXME: combine with hashPath() - - ValidPathInfo info(dstPath); - info.narHash = hash.first; - info.narSize = hash.second; - info.ca = FixedOutputHash { .method = method, .hash = h }; - registerValidPath(info); - } - - outputLock.setDeletion(true); - } - - return dstPath; -} - - -======= ->>>>>>> 3dcca18c30cbc09652f5ac644a9f8750f9ced0c9 -StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, - FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) -{ - Path srcPath(absPath(_srcPath)); - auto source = sinkToSource([&](Sink & sink) { - if (method == FileIngestionMethod::Recursive) - dumpPath(srcPath, sink, filter); - else - readFile(srcPath, sink); - }); - return addToStoreFromDump(*source, name, method, hashAlgo, repair); -} - - -StorePath LocalStore::addToStoreFromDump(Source & source0, const string & name, - FileIngestionMethod method, HashType hashAlgo, RepairFlag repair) -{ - /* For computing the store path. */ - auto hashSink = std::make_unique(hashAlgo); - TeeSource source { source0, *hashSink }; - - /* Read the source path into memory, but only if it's up to - narBufferSize bytes. If it's larger, write it to a temporary - location in the Nix store. If the subsequently computed - destination store path is already valid, we just delete the - temporary path. Otherwise, we move it to the destination store - path. */ -<<<<<<< HEAD - bool inMemory = true; - std::string nar; // TODO rename from "nar" to "dump" - - auto source = sinkToSource([&](Sink & sink) { - - LambdaSink sink2([&](const unsigned char * buf, size_t len) { - (*hashSink)(buf, len); - - if (inMemory) { - if (nar.size() + len > settings.narBufferSize) { - inMemory = false; - sink << 1; - sink((const unsigned char *) nar.data(), nar.size()); - nar.clear(); - } else { - nar.append((const char *) buf, len); - } - } - - if (!inMemory) sink(buf, len); - }); - - switch (method) { - case FileIngestionMethod::Recursive: { - dumpPath(srcPath, sink2, filter); - break; - } - case FileIngestionMethod::Git: { - // recursively add to store if path is a directory - struct stat st; - if (lstat(srcPath.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", srcPath); - if (S_ISDIR(st.st_mode)) - for (auto & i : readDirectory(srcPath)) - addToStore("git", srcPath + "/" + i.name, method, hashAlgo, filter, repair); - - dumpGit(hashAlgo, srcPath, sink2, filter); - break; - } - case FileIngestionMethod::Flat: { - readFile(srcPath, sink2); - break; - } - } - }); -||||||| merged common ancestors - bool inMemory = true; - std::string nar; // TODO rename from "nar" to "dump" - - auto source = sinkToSource([&](Sink & sink) { - - LambdaSink sink2([&](const unsigned char * buf, size_t len) { - (*hashSink)(buf, len); - - if (inMemory) { - if (nar.size() + len > settings.narBufferSize) { - inMemory = false; - sink << 1; - sink((const unsigned char *) nar.data(), nar.size()); - nar.clear(); - } else { - nar.append((const char *) buf, len); - } - } - - if (!inMemory) sink(buf, len); - }); - - if (method == FileIngestionMethod::Recursive) - dumpPath(srcPath, sink2, filter); - else - readFile(srcPath, sink2); - }); -======= - bool inMemory = false; - - std::string dump; - - /* Fill out buffer, and decide whether we are working strictly in - memory based on whether we break out because the buffer is full - or the original source is empty */ - while (dump.size() < settings.narBufferSize) { - auto oldSize = dump.size(); - constexpr size_t chunkSize = 1024; - auto want = std::min(chunkSize, settings.narBufferSize - oldSize); - dump.resize(oldSize + want); - auto got = 0; - try { - got = source.read((uint8_t *) dump.data() + oldSize, want); - } catch (EndOfFile &) { - inMemory = true; - break; - } - dump.resize(oldSize + got); - } ->>>>>>> 3dcca18c30cbc09652f5ac644a9f8750f9ced0c9 - - std::unique_ptr delTempDir; - Path tempPath; - - if (!inMemory) { - /* Drain what we pulled so far, and then keep on pulling */ - StringSource dumpSource { dump }; - ChainSource bothSource { dumpSource, source }; - - auto tempDir = createTempDir(realStoreDir, "add"); - delTempDir = std::make_unique(tempDir); - tempPath = tempDir + "/x"; - -<<<<<<< HEAD - switch (method) { - case FileIngestionMethod::Flat: - writeFile(tempPath, *source); - break; - case FileIngestionMethod::Recursive: - restorePath(tempPath, *source); - break; - case FileIngestionMethod::Git: - restoreGit(tempPath, *source, realStoreDir, storeDir); - break; - } -||||||| merged common ancestors - if (method == FileIngestionMethod::Recursive) - restorePath(tempPath, *source); - else - writeFile(tempPath, *source); -======= - if (method == FileIngestionMethod::Recursive) - restorePath(tempPath, bothSource); - else - writeFile(tempPath, bothSource); ->>>>>>> 3dcca18c30cbc09652f5ac644a9f8750f9ced0c9 - - dump.clear(); - } - - auto [hash, size] = hashSink->finish(); - - auto dstPath = makeFixedOutputPath(method, hash, name); - - addTempRoot(dstPath); - - if (repair || !isValidPath(dstPath)) { - - /* The first check above is an optimisation to prevent - unnecessary lock acquisition. */ - - auto realPath = Store::toRealPath(dstPath); - - PathLocks outputLock({realPath}); - - if (repair || !isValidPath(dstPath)) { - - deletePath(realPath); - - autoGC(); - - if (inMemory) { - StringSource dumpSource { dump }; - /* Restore from the NAR in memory. */ -<<<<<<< HEAD - StringSource source(nar); - switch (method) { - case FileIngestionMethod::Flat: - writeFile(realPath, source); - break; - case FileIngestionMethod::Recursive: { - restorePath(realPath, source); - break; - } - case FileIngestionMethod::Git: { - restoreGit(realPath, source, realStoreDir, storeDir); - break; - } - } -||||||| merged common ancestors - StringSource source(nar); - if (method == FileIngestionMethod::Recursive) - restorePath(realPath, source); - else - writeFile(realPath, source); -======= - if (method == FileIngestionMethod::Recursive) - restorePath(realPath, dumpSource); - else - writeFile(realPath, dumpSource); ->>>>>>> 3dcca18c30cbc09652f5ac644a9f8750f9ced0c9 - } else { - /* Move the temporary path we restored above. */ - if (rename(tempPath.c_str(), realPath.c_str())) - throw Error("renaming '%s' to '%s'", tempPath, realPath); - } - - /* For computing the nar hash. In recursive SHA-256 mode, this - is the same as the store hash, so no need to do it again. */ - auto narHash = std::pair { hash, size }; - if (method != FileIngestionMethod::Recursive || hashAlgo != htSHA256) { - HashSink narSink { htSHA256 }; - dumpPath(realPath, narSink); - narHash = narSink.finish(); - } - - canonicalisePathMetaData(realPath, -1); // FIXME: merge into restorePath - - optimisePath(realPath); - - ValidPathInfo info(dstPath); - info.narHash = narHash.first; - info.narSize = narHash.second; - info.ca = FixedOutputHash { .method = method, .hash = hash }; - registerValidPath(info); - } - - outputLock.setDeletion(true); - } - - return dstPath; -} - - -StorePath LocalStore::addTextToStore(const string & name, const string & s, - const StorePathSet & references, RepairFlag repair) -{ - auto hash = hashString(htSHA256, s); - auto dstPath = makeTextPath(name, hash, references); - - addTempRoot(dstPath); - - if (repair || !isValidPath(dstPath)) { - - auto realPath = Store::toRealPath(dstPath); - - PathLocks outputLock({realPath}); - - if (repair || !isValidPath(dstPath)) { - - deletePath(realPath); - - autoGC(); - - writeFile(realPath, s); - - canonicalisePathMetaData(realPath, -1); - - StringSink sink; - dumpString(s, sink); - auto narHash = hashString(htSHA256, *sink.s); - - optimisePath(realPath); - - ValidPathInfo info(dstPath); - info.narHash = narHash; - info.narSize = sink.s->size(); - info.references = references; - info.ca = TextHash { .hash = hash }; - registerValidPath(info); - } - - outputLock.setDeletion(true); - } - - return dstPath; -} - - -/* Create a temporary directory in the store that won't be - garbage-collected. */ -Path LocalStore::createTempDirInStore() -{ - Path tmpDir; - do { - /* There is a slight possibility that `tmpDir' gets deleted by - the GC between createTempDir() and addTempRoot(), so repeat - until `tmpDir' exists. */ - tmpDir = createTempDir(realStoreDir); - addTempRoot(parseStorePath(tmpDir)); - } while (!pathExists(tmpDir)); - return tmpDir; -} - - -void LocalStore::invalidatePathChecked(const StorePath & path) -{ - retrySQLite([&]() { - auto state(_state.lock()); - - SQLiteTxn txn(state->db); - - if (isValidPath_(*state, path)) { - StorePathSet referrers; queryReferrers(*state, path, referrers); - referrers.erase(path); /* ignore self-references */ - if (!referrers.empty()) - throw PathInUse("cannot delete path '%s' because it is in use by %s", - printStorePath(path), showPaths(referrers)); - invalidatePath(*state, path); - } - - txn.commit(); - }); -} - - -bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) -{ - printInfo(format("reading the Nix store...")); - - bool errors = false; - - /* Acquire the global GC lock to get a consistent snapshot of - existing and valid paths. */ - AutoCloseFD fdGCLock = openGCLock(ltWrite); - - StringSet store; - for (auto & i : readDirectory(realStoreDir)) store.insert(i.name); - - /* Check whether all valid paths actually exist. */ - printInfo("checking path existence..."); - - StorePathSet validPaths; - PathSet done; - - fdGCLock = -1; - - for (auto & i : queryAllValidPaths()) - verifyPath(printStorePath(i), store, done, validPaths, repair, errors); - - /* Optionally, check the content hashes (slow). */ - if (checkContents) { - - printInfo("checking link hashes..."); - - for (auto & link : readDirectory(linksDir)) { - printMsg(lvlTalkative, "checking contents of '%s'", link.name); - Path linkPath = linksDir + "/" + link.name; - string hash = hashPath(htSHA256, linkPath).first.to_string(Base32, false); - if (hash != link.name) { - logError({ - .name = "Invalid hash", - .hint = hintfmt( - "link '%s' was modified! expected hash '%s', got '%s'", - linkPath, link.name, hash) - }); - if (repair) { - if (unlink(linkPath.c_str()) == 0) - printInfo("removed link '%s'", linkPath); - else - throw SysError("removing corrupt link '%s'", linkPath); - } else { - errors = true; - } - } - } - - printInfo("checking store hashes..."); - - Hash nullHash(htSHA256); - - for (auto & i : validPaths) { - try { - auto info = std::const_pointer_cast(std::shared_ptr(queryPathInfo(i))); - - /* Check the content hash (optionally - slow). */ - printMsg(lvlTalkative, "checking contents of '%s'", printStorePath(i)); - - std::unique_ptr hashSink; - if (!info->ca || !info->references.count(info->path)) - hashSink = std::make_unique(*info->narHash.type); - else - hashSink = std::make_unique(*info->narHash.type, std::string(info->path.hashPart())); - - dumpPath(Store::toRealPath(i), *hashSink); - auto current = hashSink->finish(); - - if (info->narHash != nullHash && info->narHash != current.first) { - logError({ - .name = "Invalid hash - path modified", - .hint = hintfmt("path '%s' was modified! expected hash '%s', got '%s'", - printStorePath(i), info->narHash.to_string(Base32, true), current.first.to_string(Base32, true)) - }); - if (repair) repairPath(i); else errors = true; - } else { - - bool update = false; - - /* Fill in missing hashes. */ - if (info->narHash == nullHash) { - printInfo("fixing missing hash on '%s'", printStorePath(i)); - info->narHash = current.first; - update = true; - } - - /* Fill in missing narSize fields (from old stores). */ - if (info->narSize == 0) { - printInfo("updating size field on '%s' to %s", printStorePath(i), current.second); - info->narSize = current.second; - update = true; - } - - if (update) { - auto state(_state.lock()); - updatePathInfo(*state, *info); - } - - } - - } catch (Error & e) { - /* It's possible that the path got GC'ed, so ignore - errors on invalid paths. */ - if (isValidPath(i)) - logError(e.info()); - else - warn(e.msg()); - errors = true; - } - } - } - - return errors; -} - - -void LocalStore::verifyPath(const Path & pathS, const StringSet & store, - PathSet & done, StorePathSet & validPaths, RepairFlag repair, bool & errors) -{ - checkInterrupt(); - - if (!done.insert(pathS).second) return; - - if (!isStorePath(pathS)) { - logError({ - .name = "Nix path not found", - .hint = hintfmt("path '%s' is not in the Nix store", pathS) - }); - return; - } - - auto path = parseStorePath(pathS); - - if (!store.count(std::string(path.to_string()))) { - /* Check any referrers first. If we can invalidate them - first, then we can invalidate this path as well. */ - bool canInvalidate = true; - StorePathSet referrers; queryReferrers(path, referrers); - for (auto & i : referrers) - if (i != path) { - verifyPath(printStorePath(i), store, done, validPaths, repair, errors); - if (validPaths.count(i)) - canInvalidate = false; - } - - if (canInvalidate) { - printInfo("path '%s' disappeared, removing from database...", pathS); - auto state(_state.lock()); - invalidatePath(*state, path); - } else { - logError({ - .name = "Missing path with referrers", - .hint = hintfmt("path '%s' disappeared, but it still has valid referrers!", pathS) - }); - if (repair) - try { - repairPath(path); - } catch (Error & e) { - logWarning(e.info()); - errors = true; - } - else errors = true; - } - - return; - } - - validPaths.insert(std::move(path)); -} - - -unsigned int LocalStore::getProtocol() -{ - return PROTOCOL_VERSION; -} - - -#if defined(FS_IOC_SETFLAGS) && defined(FS_IOC_GETFLAGS) && defined(FS_IMMUTABLE_FL) - -static void makeMutable(const Path & path) -{ - checkInterrupt(); - - struct stat st = lstat(path); - - if (!S_ISDIR(st.st_mode) && !S_ISREG(st.st_mode)) return; - - if (S_ISDIR(st.st_mode)) { - for (auto & i : readDirectory(path)) - makeMutable(path + "/" + i.name); - } - - /* The O_NOFOLLOW is important to prevent us from changing the - mutable bit on the target of a symlink (which would be a - security hole). */ - AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC); - if (fd == -1) { - if (errno == ELOOP) return; // it's a symlink - throw SysError("opening file '%1%'", path); - } - - unsigned int flags = 0, old; - - /* Silently ignore errors getting/setting the immutable flag so - that we work correctly on filesystems that don't support it. */ - if (ioctl(fd, FS_IOC_GETFLAGS, &flags)) return; - old = flags; - flags &= ~FS_IMMUTABLE_FL; - if (old == flags) return; - if (ioctl(fd, FS_IOC_SETFLAGS, &flags)) return; -} - -/* Upgrade from schema 6 (Nix 0.15) to schema 7 (Nix >= 1.3). */ -void LocalStore::upgradeStore7() -{ - if (getuid() != 0) return; - printInfo("removing immutable bits from the Nix store (this may take a while)..."); - makeMutable(realStoreDir); -} - -#else - -void LocalStore::upgradeStore7() -{ -} - -#endif - - -void LocalStore::vacuumDB() -{ - auto state(_state.lock()); - state->db.exec("vacuum"); -} - - -void LocalStore::addSignatures(const StorePath & storePath, const StringSet & sigs) -{ - retrySQLite([&]() { - auto state(_state.lock()); - - SQLiteTxn txn(state->db); - - auto info = std::const_pointer_cast(std::shared_ptr(queryPathInfo(storePath))); - - info->sigs.insert(sigs.begin(), sigs.end()); - - updatePathInfo(*state, *info); - - txn.commit(); - }); -} - - -void LocalStore::signPathInfo(ValidPathInfo & info) -{ - // FIXME: keep secret keys in memory. - - auto secretKeyFiles = settings.secretKeyFiles; - - for (auto & secretKeyFile : secretKeyFiles.get()) { - SecretKey secretKey(readFile(secretKeyFile)); - info.sign(*this, secretKey); - } -} - - -void LocalStore::createUser(const std::string & userName, uid_t userId) -{ - for (auto & dir : { - fmt("%s/profiles/per-user/%s", stateDir, userName), - fmt("%s/gcroots/per-user/%s", stateDir, userName) - }) { - createDirs(dir); - if (chmod(dir.c_str(), 0755) == -1) - throw SysError("changing permissions of directory '%s'", dir); - if (chown(dir.c_str(), userId, getgid()) == -1) - throw SysError("changing owner of directory '%s'", dir); - } -} - - -} From bf61871271971aa45237fb9ba7fa4c63ae083ff2 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 20 Jul 2020 17:42:34 +0000 Subject: [PATCH 44/74] parser.hh -> split.hh --- src/libstore/content-address.cc | 2 +- src/libutil/hash.cc | 2 +- src/libutil/{parser.hh => split.hh} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/libutil/{parser.hh => split.hh} (100%) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index a562f2d23..749551d1a 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -1,6 +1,6 @@ #include "args.hh" #include "content-address.hh" -#include "parser.hh" +#include "split.hh" namespace nix { diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index c7ccb809d..65ba1dc81 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -7,7 +7,7 @@ #include "args.hh" #include "hash.hh" #include "archive.hh" -#include "parser.hh" +#include "split.hh" #include "util.hh" #include diff --git a/src/libutil/parser.hh b/src/libutil/split.hh similarity index 100% rename from src/libutil/parser.hh rename to src/libutil/split.hh From c58c6165c554d671f87b463c9ab1d47a5d75bbbb Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 20 Jul 2020 17:43:19 +0000 Subject: [PATCH 45/74] Remove period at the end of the exception message --- src/libutil/hash.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 65ba1dc81..dfb3668f1 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -177,7 +177,7 @@ Hash Hash::parseAnyPrefixed(std::string_view original) // Either the string or user must provide the type, if they both do they // must agree. if (!optParsedType) - throw BadHash("hash '%s' does not include a type.", rest); + throw BadHash("hash '%s' does not include a type", rest); return Hash(rest, *optParsedType, isSRI); } From 362ae93851830ecce6ade70462fe991cc522d27b Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Mon, 20 Jul 2020 14:13:37 -0400 Subject: [PATCH 46/74] 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(-) 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 { From 6357b1b0fb33bfa26f1d44326fc01d0c86d86f2c Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Mon, 20 Jul 2020 14:17:25 -0400 Subject: [PATCH 47/74] Add another Unimplemented case --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); } From 5ce95b9529ad8c53b4395d98635d035d92913091 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 21 Jul 2020 09:47:40 -0400 Subject: [PATCH 48/74] Update src/libstore/build.cc --- src/libstore/build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 { From 9423f64ee2b9fe84618e06654fb6b55766b0cf44 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 22 Jul 2020 23:59:25 +0000 Subject: [PATCH 49/74] 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(-) 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; }; } From e1de1fe0d82d8ba702947dcad3b678cbb9ce9333 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 23 Jul 2020 19:02:57 +0000 Subject: [PATCH 50/74] Make `Buildable` a `std::variant` I think this better captures the intent of what's going on: we either have an opaque store path, or a drv path with some outputs. Having this structure will also help us support CA derivations: we'll have to allow the outpath paths to be optional, so the structure we gain now makes up for the structure we loose then. --- src/libstore/content-address.cc | 4 -- src/libstore/store-api.cc | 4 -- src/libutil/util.hh | 5 ++ src/nix/build.cc | 29 +++++++---- src/nix/command.cc | 17 ++++-- src/nix/installables.cc | 91 ++++++++++++++++----------------- src/nix/installables.hh | 14 +++-- src/nix/log.cc | 13 +++-- 8 files changed, 98 insertions(+), 79 deletions(-) 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/store-api.cc b/src/libstore/store-api.cc index ab21b0bd5..0f6f3b98f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -865,10 +865,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 42130f6dc..630303a5d 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -601,4 +601,9 @@ constexpr auto enumerate(T && iterable) } +// C++17 std::visit boilerplate +template struct overloaded : Ts... { using Ts::operator()...; }; +template overloaded(Ts...) -> overloaded; + + } diff --git a/src/nix/build.cc b/src/nix/build.cc index 0f7e0e123..927301314 100644 --- a/src/nix/build.cc +++ b/src/nix/build.cc @@ -57,17 +57,24 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixProfile if (dryRun) return; - if (outLink != "") { - for (size_t i = 0; i < buildables.size(); ++i) { - for (auto & output : buildables[i].outputs) - if (auto store2 = store.dynamic_pointer_cast()) { - std::string symlink = outLink; - if (i) symlink += fmt("-%d", i); - if (output.first != "out") symlink += fmt("-%s", output.first); - store2->addPermRoot(output.second, absPath(symlink), true); - } - } - } + if (outLink != "") + if (auto store2 = store.dynamic_pointer_cast()) + for (size_t i = 0; i < buildables.size(); ++i) + std::visit(overloaded { + [&](BuildableOpaque bo) { + std::string symlink = outLink; + if (i) symlink += fmt("-%d", i); + store2->addPermRoot(bo.path, absPath(symlink), true); + }, + [&](BuildableFromDrv bfd) { + for (auto & output : bfd.outputs) { + std::string symlink = outLink; + if (i) symlink += fmt("-%d", i); + if (output.first != "out") symlink += fmt("-%s", output.first); + store2->addPermRoot(output.second, absPath(symlink), true); + } + }, + }, buildables[i]); updateProfile(buildables); } diff --git a/src/nix/command.cc b/src/nix/command.cc index af36dda89..04715b43a 100644 --- a/src/nix/command.cc +++ b/src/nix/command.cc @@ -131,11 +131,18 @@ void MixProfile::updateProfile(const Buildables & buildables) std::optional result; for (auto & buildable : buildables) { - for (auto & output : buildable.outputs) { - if (result) - throw Error("'--profile' requires that the arguments produce a single store path, but there are multiple"); - result = output.second; - } + std::visit(overloaded { + [&](BuildableOpaque bo) { + result = bo.path; + }, + [&](BuildableFromDrv bfd) { + for (auto & output : bfd.outputs) { + if (result) + throw Error("'--profile' requires that the arguments produce a single store path, but there are multiple"); + result = output.second; + } + }, + }, buildable); } if (!result) diff --git a/src/nix/installables.cc b/src/nix/installables.cc index a13e5a3df..a051950cd 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -307,16 +307,15 @@ struct InstallableStorePath : Installable for (auto & [name, output] : store->readDerivation(storePath).outputs) outputs.emplace(name, output.path); return { - Buildable { + BuildableFromDrv { .drvPath = storePath, .outputs = std::move(outputs) } }; } else { return { - Buildable { - .drvPath = {}, - .outputs = {{"out", storePath}} + BuildableOpaque { + .path = storePath, } }; } @@ -332,33 +331,20 @@ Buildables InstallableValue::toBuildables() { Buildables res; - StorePathSet drvPaths; + std::map drvsToOutputs; + // Group by derivation, helps with .all in particular for (auto & drv : toDerivations()) { - Buildable b{.drvPath = drv.drvPath}; - drvPaths.insert(drv.drvPath); - auto outputName = drv.outputName; if (outputName == "") - throw Error("derivation '%s' lacks an 'outputName' attribute", state->store->printStorePath(*b.drvPath)); - - b.outputs.emplace(outputName, drv.outPath); - - res.push_back(std::move(b)); + throw Error("derivation '%s' lacks an 'outputName' attribute", state->store->printStorePath(drv.drvPath)); + drvsToOutputs[drv.drvPath].insert_or_assign(outputName, drv.outPath); } - // Hack to recognize .all: if all drvs have the same drvPath, - // merge the buildables. - if (drvPaths.size() == 1) { - Buildable b{.drvPath = *drvPaths.begin()}; - for (auto & b2 : res) - for (auto & output : b2.outputs) - b.outputs.insert_or_assign(output.first, output.second); - Buildables bs; - bs.push_back(std::move(b)); - return bs; - } else - return res; + for (auto & i : drvsToOutputs) + res.push_back(BuildableFromDrv { i.first, i.second }); + + return res; } struct InstallableAttrPath : InstallableValue @@ -653,14 +639,17 @@ Buildables build(ref store, Realise mode, for (auto & i : installables) { for (auto & b : i->toBuildables()) { - if (b.drvPath) { - StringSet outputNames; - for (auto & output : b.outputs) - outputNames.insert(output.first); - pathsToBuild.push_back({*b.drvPath, outputNames}); - } else - for (auto & output : b.outputs) - pathsToBuild.push_back({output.second}); + std::visit(overloaded { + [&](BuildableOpaque bo) { + pathsToBuild.push_back({bo.path}); + }, + [&](BuildableFromDrv bfd) { + StringSet outputNames; + for (auto & output : bfd.outputs) + outputNames.insert(output.first); + pathsToBuild.push_back({bfd.drvPath, outputNames}); + }, + }, b); buildables.push_back(std::move(b)); } } @@ -681,16 +670,23 @@ StorePathSet toStorePaths(ref store, if (operateOn == OperateOn::Output) { for (auto & b : build(store, mode, installables)) - for (auto & output : b.outputs) - outPaths.insert(output.second); + std::visit(overloaded { + [&](BuildableOpaque bo) { + outPaths.insert(bo.path); + }, + [&](BuildableFromDrv bfd) { + for (auto & output : bfd.outputs) + outPaths.insert(output.second); + }, + }, b); } else { if (mode == Realise::Nothing) settings.readOnlyMode = true; for (auto & i : installables) for (auto & b : i->toBuildables()) - if (b.drvPath) - outPaths.insert(*b.drvPath); + if (auto bfd = std::get_if(&b)) + outPaths.insert(bfd->drvPath); } return outPaths; @@ -714,20 +710,21 @@ StorePathSet toDerivations(ref store, StorePathSet drvPaths; for (auto & i : installables) - for (auto & b : i->toBuildables()) { - if (!b.drvPath) { - if (!useDeriver) - throw Error("argument '%s' did not evaluate to a derivation", i->what()); - for (auto & output : b.outputs) { - auto derivers = store->queryValidDerivers(output.second); + for (auto & b : i->toBuildables()) + std::visit(overloaded { + [&](BuildableOpaque bo) { + if (!useDeriver) + throw Error("argument '%s' did not evaluate to a derivation", i->what()); + auto derivers = store->queryValidDerivers(bo.path); if (derivers.empty()) throw Error("'%s' does not have a known deriver", i->what()); // FIXME: use all derivers? drvPaths.insert(*derivers.begin()); - } - } else - drvPaths.insert(*b.drvPath); - } + }, + [&](BuildableFromDrv bfd) { + drvPaths.insert(bfd.drvPath); + }, + }, b); return drvPaths; } diff --git a/src/nix/installables.hh b/src/nix/installables.hh index eb34365d4..9edff3331 100644 --- a/src/nix/installables.hh +++ b/src/nix/installables.hh @@ -14,12 +14,20 @@ struct SourceExprCommand; namespace eval_cache { class EvalCache; class AttrCursor; } -struct Buildable -{ - std::optional drvPath; +struct BuildableOpaque { + StorePath path; +}; + +struct BuildableFromDrv { + StorePath drvPath; std::map outputs; }; +typedef std::variant< + BuildableOpaque, + BuildableFromDrv +> Buildable; + typedef std::vector Buildables; struct App diff --git a/src/nix/log.cc b/src/nix/log.cc index 7e10d373a..33380dcf5 100644 --- a/src/nix/log.cc +++ b/src/nix/log.cc @@ -45,11 +45,14 @@ struct CmdLog : InstallableCommand RunPager pager; for (auto & sub : subs) { - auto log = b.drvPath ? sub->getBuildLog(*b.drvPath) : nullptr; - for (auto & output : b.outputs) { - if (log) break; - log = sub->getBuildLog(output.second); - } + auto log = std::visit(overloaded { + [&](BuildableOpaque bo) { + return sub->getBuildLog(bo.path); + }, + [&](BuildableFromDrv bfd) { + return sub->getBuildLog(bfd.drvPath); + }, + }, b); if (!log) continue; stopProgressBar(); printInfo("got build log for '%s' from '%s'", installable->what(), sub->getUri()); From 951415b5685fe52d31770eadabd66d95ea75cfae Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 27 Jul 2020 17:56:36 +0000 Subject: [PATCH 51/74] 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(-) 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 { From 8065c6d1606402e936b048aa75ad98ffdd7c8d60 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 27 Jul 2020 20:45:34 +0000 Subject: [PATCH 52/74] 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 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; +} + +} From 2980b244b7e5f1660c4a07d7589f4a2dd47f9acd Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 28 Jul 2020 15:39:45 -0400 Subject: [PATCH 53/74] Use assert(false) instead of abort() --- src/libstore/derivations.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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? From e3a2154f5ac91a5cbab5d0715984972e1dd7d40d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 31 Jul 2020 01:07:59 +0000 Subject: [PATCH 54/74] Fix indentation --- src/libfetchers/mercurial.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index aee42e136..3e76ffc4d 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -209,7 +209,7 @@ struct MercurialInputScheme : InputScheme }); if (auto res = getCache()->lookup(store, mutableAttrs)) { - auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1); + auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1); if (!input.getRev() || input.getRev() == rev2) { input.attrs.insert_or_assign("rev", rev2.gitRev()); return makeResult(res->first, std::move(res->second)); From 3cbee1e840ea1beff566555f1221b2791091e20c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Aug 2020 15:26:57 +0000 Subject: [PATCH 55/74] Convert to C-style comments --- src/libutil/hash.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index d4badbab3..ffc397ce0 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -42,9 +42,9 @@ struct Hash is not present, then the hash type must be specified in the string. */ static Hash parseAny(std::string_view s, std::optional type); - // hash type must be part of string + /* hash type must be part of string */ static Hash parseAnyPrefixed(std::string_view s); - // prefix parsed separately; non SRI hash + /* prefix parsed separately; non SRI hash */ static Hash parseNonSRIUnprefixed(std::string_view s, HashType type); static Hash parseSRI(std::string_view original); From bc165e28aee689a45535afda8012c9b63f87b24c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Aug 2020 15:32:20 +0000 Subject: [PATCH 56/74] Embelish documentation of new Hash functions --- src/libutil/hash.hh | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index ffc397ce0..00ce7bb6f 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -36,21 +36,26 @@ struct Hash /* Create a zero-filled hash object. */ Hash(HashType type); - /* Initialize the hash from a string representation, in the format + /* Parse the hash from a string representation in the format "[:]" or "-" (a Subresource Integrity hash expression). If the 'type' argument is not present, then the hash type must be specified in the string. */ static Hash parseAny(std::string_view s, std::optional type); - /* hash type must be part of string */ + + /* Parse a hash from a string representation like the above, except the + type prefix is mandatory is there is no separate arguement. */ static Hash parseAnyPrefixed(std::string_view s); - /* prefix parsed separately; non SRI hash */ + + /* Parse a plain hash that musst not have any prefix indicating the type. + The type is passed in to disambiguate. */ static Hash parseNonSRIUnprefixed(std::string_view s, HashType type); static Hash parseSRI(std::string_view original); private: - // type must be provided, s must not include prefix + /* The type must be provided, the string view must not include + prefix. `isSRI` helps disambigate the various base-* encodings. */ Hash(std::string_view s, HashType type, bool isSRI); public: From c4ada76e860a595e3f034b89f27374ce79513d9f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Aug 2020 16:22:50 +0000 Subject: [PATCH 57/74] Fix error message and avoid recalculation --- src/libfetchers/fetchers.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 08b83b0db..9c69fc564 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -203,8 +203,8 @@ std::optional Input::getNarHash() const if (auto s = maybeGetStrAttr(attrs, "narHash")) { auto hash = s->empty() ? Hash(htSHA256) : Hash::parseSRI(*s); if (hash.type != htSHA256) - throw UsageError("narHash must be specified with SRI notation"); - return newHashAllowEmpty(*s, htSHA256); + throw UsageError("narHash must use SHA-256"); + return hash; } return {}; } From fe7e57a80d1f698565aaa13596de3fa13dbafe1f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 4 Aug 2020 03:46:28 +0000 Subject: [PATCH 58/74] tab -> space --- src/nix/installables.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/installables.cc b/src/nix/installables.cc index babb0e5fe..59b52ce95 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -437,7 +437,7 @@ ref openEvalCache( std::shared_ptr lockedFlake, bool useEvalCache) { - auto fingerprint = lockedFlake->getFingerprint(); + auto fingerprint = lockedFlake->getFingerprint(); return make_ref( useEvalCache && evalSettings.pureEval ? std::optional { std::cref(fingerprint) } From 327b1bf378ebdfe7acc6357b651470b45cb44472 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Aug 2020 14:50:43 +0200 Subject: [PATCH 59/74] BinaryCacheStore: Explicitly flush file sink The file sink is also flushed in its destructor, but we ignore any exceptions in the destructor. Issue #3886. --- src/libstore/binary-cache-store.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 3d3195e03..7d103e0cc 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -169,6 +169,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource TeeSource teeSource(narSource, *compressionSink); narAccessor = makeNarAccessor(teeSource); compressionSink->finish(); + fileSink.flush(); } auto now2 = std::chrono::steady_clock::now(); From dfe66420e7e5395acfa8068fafcadc3a38a56184 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Aug 2020 15:56:10 +0200 Subject: [PATCH 60/74] Revert "Remove putBytes" This reverts commit b8eea7e81af53905be7845dffc6d0a83ea8edc97. --- src/libstore/s3-binary-cache-store.cc | 7 +++++-- src/libstore/s3-binary-cache-store.hh | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 67935f3ba..1b7dff085 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -343,10 +343,13 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore std::chrono::duration_cast(now2 - now1) .count(); - printInfo("uploaded 's3://%s/%s' in %d ms", - bucketName, path, duration); + auto size = istream->tellg(); + + printInfo("uploaded 's3://%s/%s' (%d bytes) in %d ms", + bucketName, path, size, duration); stats.putTimeMs += duration; + stats.putBytes += size; stats.put++; } diff --git a/src/libstore/s3-binary-cache-store.hh b/src/libstore/s3-binary-cache-store.hh index b2b75d498..4d43fe4d2 100644 --- a/src/libstore/s3-binary-cache-store.hh +++ b/src/libstore/s3-binary-cache-store.hh @@ -19,6 +19,7 @@ public: struct Stats { std::atomic put{0}; + std::atomic putBytes{0}; std::atomic putTimeMs{0}; std::atomic get{0}; std::atomic getBytes{0}; From 4e7f1c7f11fb22d7954b2dacbe87c61548fab82d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Aug 2020 16:00:59 +0200 Subject: [PATCH 61/74] S3BinaryCacheStore: Fix size determination --- src/libstore/binary-cache-store.cc | 2 +- src/libstore/s3-binary-cache-store.cc | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 7d103e0cc..9682db730 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -283,7 +283,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource if (repair || !fileExists(narInfo->url)) { stats.narWrite++; upsertFile(narInfo->url, - std::make_shared(fnTemp, std::ios_base::in), + std::make_shared(fnTemp, std::ios_base::in | std::ios_base::binary), "application/x-nix-nar"); } else stats.narWriteAverted++; diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 1b7dff085..a0a446bd3 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -266,6 +266,10 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore const std::string & mimeType, const std::string & contentEncoding) { + istream->seekg(0, istream->end); + auto size = istream->tellg(); + istream->seekg(0, istream->beg); + auto maxThreads = std::thread::hardware_concurrency(); static std::shared_ptr @@ -343,13 +347,11 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore std::chrono::duration_cast(now2 - now1) .count(); - auto size = istream->tellg(); - printInfo("uploaded 's3://%s/%s' (%d bytes) in %d ms", bucketName, path, size, duration); stats.putTimeMs += duration; - stats.putBytes += size; + stats.putBytes += std::max(size, (decltype(size)) 0); stats.put++; } From 6d9ccde20d783a660aca3cab0e6e5c20e8444641 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 4 Aug 2020 01:09:52 +0000 Subject: [PATCH 62/74] Make JSON equality tests agnostic to ordering It is in fact more sorted than before, but I don't think we want to guarantee anything about the ordering. --- tests/binary-cache.sh | 8 ++++++-- tests/nar-access.sh | 24 ++++++++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh index 40f1a4f76..b05a7d167 100644 --- a/tests/binary-cache.sh +++ b/tests/binary-cache.sh @@ -218,7 +218,9 @@ outPath=$(nix-build --no-out-link -E ' nix copy --to file://$cacheDir?write-nar-listing=1 $outPath -[[ $(cat $cacheDir/$(basename $outPath).ls) = '{"version":1,"root":{"type":"directory","entries":{"bar":{"type":"regular","size":4,"narOffset":232},"link":{"type":"symlink","target":"xyzzy"}}}}' ]] +diff -u \ + <(jq -S < $cacheDir/$(basename $outPath).ls) \ + <(echo '{"version":1,"root":{"type":"directory","entries":{"bar":{"type":"regular","size":4,"narOffset":232},"link":{"type":"symlink","target":"xyzzy"}}}}' | jq -S) # Test debug info index generation. @@ -234,4 +236,6 @@ outPath=$(nix-build --no-out-link -E ' nix copy --to "file://$cacheDir?index-debug-info=1&compression=none" $outPath -[[ $(cat $cacheDir/debuginfo/02623eda209c26a59b1a8638ff7752f6b945c26b.debug) = '{"archive":"../nar/100vxs724qr46phz8m24iswmg9p3785hsyagz0kchf6q6gf06sw6.nar","member":"lib/debug/.build-id/02/623eda209c26a59b1a8638ff7752f6b945c26b.debug"}' ]] +diff -u \ + <(cat $cacheDir/debuginfo/02623eda209c26a59b1a8638ff7752f6b945c26b.debug | jq -S) \ + <(echo '{"archive":"../nar/100vxs724qr46phz8m24iswmg9p3785hsyagz0kchf6q6gf06sw6.nar","member":"lib/debug/.build-id/02/623eda209c26a59b1a8638ff7752f6b945c26b.debug"}' | jq -S) diff --git a/tests/nar-access.sh b/tests/nar-access.sh index 553d6ca89..88b997ca6 100644 --- a/tests/nar-access.sh +++ b/tests/nar-access.sh @@ -26,12 +26,24 @@ nix cat-store $storePath/foo/baz > baz.cat-nar diff -u baz.cat-nar $storePath/foo/baz # Test --json. -[[ $(nix ls-nar --json $narFile /) = '{"type":"directory","entries":{"foo":{},"foo-x":{},"qux":{},"zyx":{}}}' ]] -[[ $(nix ls-nar --json -R $narFile /foo) = '{"type":"directory","entries":{"bar":{"type":"regular","size":0,"narOffset":368},"baz":{"type":"regular","size":0,"narOffset":552},"data":{"type":"regular","size":58,"narOffset":736}}}' ]] -[[ $(nix ls-nar --json -R $narFile /foo/bar) = '{"type":"regular","size":0,"narOffset":368}' ]] -[[ $(nix ls-store --json $storePath) = '{"type":"directory","entries":{"foo":{},"foo-x":{},"qux":{},"zyx":{}}}' ]] -[[ $(nix ls-store --json -R $storePath/foo) = '{"type":"directory","entries":{"bar":{"type":"regular","size":0},"baz":{"type":"regular","size":0},"data":{"type":"regular","size":58}}}' ]] -[[ $(nix ls-store --json -R $storePath/foo/bar) = '{"type":"regular","size":0}' ]] +diff -u \ + <(nix ls-nar --json $narFile / | jq -S) \ + <(echo '{"type":"directory","entries":{"foo":{},"foo-x":{},"qux":{},"zyx":{}}}' | jq -S) +diff -u \ + <(nix ls-nar --json -R $narFile /foo | jq -S) \ + <(echo '{"type":"directory","entries":{"bar":{"type":"regular","size":0,"narOffset":368},"baz":{"type":"regular","size":0,"narOffset":552},"data":{"type":"regular","size":58,"narOffset":736}}}' | jq -S) +diff -u \ + <(nix ls-nar --json -R $narFile /foo/bar | jq -S) \ + <(echo '{"type":"regular","size":0,"narOffset":368}' | jq -S) +diff -u \ + <(nix ls-store --json $storePath | jq -S) \ + <(echo '{"type":"directory","entries":{"foo":{},"foo-x":{},"qux":{},"zyx":{}}}' | jq -S) +diff -u \ + <(nix ls-store --json -R $storePath/foo | jq -S) \ + <(echo '{"type":"directory","entries":{"bar":{"type":"regular","size":0},"baz":{"type":"regular","size":0},"data":{"type":"regular","size":58}}}' | jq -S) +diff -u \ + <(nix ls-store --json -R $storePath/foo/bar| jq -S) \ + <(echo '{"type":"regular","size":0}' | jq -S) # Test missing files. nix ls-store --json -R $storePath/xyzzy 2>&1 | grep 'does not exist in NAR' From 088dcea0e80bf2861fd9d6b808e76a1669b7122a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 5 Aug 2020 15:41:51 +0200 Subject: [PATCH 63/74] Typo --- src/libexpr/eval.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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; } From e7b0847f2d9674bc18532c86b2daf421347513e4 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 14:44:39 +0000 Subject: [PATCH 64/74] 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(-) 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); From e561a13a5863f25c81e8abc9d235a12925fd454e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 14:45:56 +0000 Subject: [PATCH 65/74] Reanme `DerivationType::Regular` defintion too This is the one non-prefixed occurence --- src/libstore/derivations.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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, }; From 25f79121564b21ec7c84f33c9348b169f20d2bdc Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 5 Aug 2020 16:47:48 +0200 Subject: [PATCH 66/74] Style fix --- src/libstore/content-address.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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) { From b9ebe373bbab6f19ee650ef9769ad76c32b7244d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 14:49:25 +0000 Subject: [PATCH 67/74] 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(-) 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); From 790b694be71c102ca9c67a85a3e9fd6f076811b1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 5 Aug 2020 16:51:06 +0200 Subject: [PATCH 68/74] Style fix --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); } From b3e73547a03f068ae4dd9cca4bc865cde85c8dec Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 11:05:46 -0400 Subject: [PATCH 69/74] Update src/libexpr/primops.cc Co-authored-by: Eelco Dolstra --- src/libexpr/primops.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); From a9bbfaa8515de7107457fda2a7aef87aa198788e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 16:27:15 +0000 Subject: [PATCH 70/74] Fix --profile with multiple opaque paths --- src/nix/command.cc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/nix/command.cc b/src/nix/command.cc index 04715b43a..da32819da 100644 --- a/src/nix/command.cc +++ b/src/nix/command.cc @@ -128,27 +128,25 @@ void MixProfile::updateProfile(const Buildables & buildables) { if (!profile) return; - std::optional result; + std::vector result; for (auto & buildable : buildables) { std::visit(overloaded { [&](BuildableOpaque bo) { - result = bo.path; + result.push_back(bo.path); }, [&](BuildableFromDrv bfd) { for (auto & output : bfd.outputs) { - if (result) - throw Error("'--profile' requires that the arguments produce a single store path, but there are multiple"); - result = output.second; + result.push_back(output.second); } }, }, buildable); } - if (!result) - throw Error("'--profile' requires that the arguments produce a single store path, but there are none"); + if (result.size() != 1) + throw Error("'--profile' requires that the arguments produce a single store path, but there are %d", result.size()); - updateProfile(*result); + updateProfile(result[0]); } MixDefaultProfile::MixDefaultProfile() From f1a47a96b6ab55d0b0017df5b94b3da69c65bf21 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 5 Aug 2020 10:58:00 -0600 Subject: [PATCH 71/74] error messages for issue 2238 --- src/build-remote/build-remote.cc | 33 +++++++++++++++++++++++++++++++- src/libstore/build.cc | 13 +++++++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 3579d8fff..2414a47c1 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -103,7 +103,7 @@ static int _main(int argc, char * * argv) drvPath = store->parseStorePath(readString(source)); auto requiredFeatures = readStrings>(source); - auto canBuildLocally = amWilling + auto canBuildLocally = amWilling && ( neededSystem == settings.thisSystem || settings.extraPlatforms.get().count(neededSystem) > 0) && allSupportedLocally(requiredFeatures); @@ -170,7 +170,38 @@ static int _main(int argc, char * * argv) if (rightType && !canBuildLocally) std::cerr << "# postpone\n"; else + { + // build the hint template. + string hintstring = "required (system, features): (%s, %s)"; + hintstring += "\n%s available machines:"; + hintstring += "\n(systems, maxjobs, supportedFeatures, mandatoryFeatures)"; + + for (unsigned int i = 0; i < machines.size(); ++i) { + hintstring += "\n(%s, %s, %s, %s)"; + } + + // add the template values. + auto hint = hintformat(hintstring); + hint + % neededSystem + % concatStringsSep(", ", requiredFeatures) + % machines.size(); + + for (auto & m : machines) { + hint % concatStringsSep>(", ", m.systemTypes) + % m.maxJobs + % concatStringsSep(", ", m.supportedFeatures) + % concatStringsSep(", ", m.mandatoryFeatures); + } + + logError({ + .name = "Remote build", + .description = "Failed to find a machine for remote build!", + .hint = hint + }); + std::cerr << "# decline\n"; + } break; } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 94c398b2f..76baa1a6e 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4876,8 +4876,17 @@ void Worker::run(const Goals & _topGoals) waitForInput(); else { if (awake.empty() && 0 == settings.maxBuildJobs) - throw Error("unable to start any build; either increase '--max-jobs' " - "or enable remote builds"); + { + if (getMachines().empty()) + throw Error("unable to start any build; either increase '--max-jobs' " + "or enable remote builds." + "\nhttps://nixos.org/nix/manual/#chap-distributed-builds"); + else + throw Error("unable to start any build; remote machines may not have " + "all required system features." + "\nhttps://nixos.org/nix/manual/#chap-distributed-builds"); + + } assert(!awake.empty()); } } From e4eae078a52fa4209bb5a46d21dbed09dd9c54ec Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 5 Aug 2020 11:21:36 -0600 Subject: [PATCH 72/74] add derivation path to hint --- src/build-remote/build-remote.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 2414a47c1..723e1463e 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -172,7 +172,7 @@ static int _main(int argc, char * * argv) else { // build the hint template. - string hintstring = "required (system, features): (%s, %s)"; + string hintstring = "derivation: %s\nrequired (system, features): (%s, %s)"; hintstring += "\n%s available machines:"; hintstring += "\n(systems, maxjobs, supportedFeatures, mandatoryFeatures)"; @@ -183,6 +183,7 @@ static int _main(int argc, char * * argv) // add the template values. auto hint = hintformat(hintstring); hint + % drvPath->to_string() % neededSystem % concatStringsSep(", ", requiredFeatures) % machines.size(); From 31f1af0cabb65e3c5f8f4539500c6b236c387780 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 5 Aug 2020 11:26:06 -0600 Subject: [PATCH 73/74] don't crash if there's no drvPath --- src/build-remote/build-remote.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 723e1463e..5247cefa6 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -181,9 +181,15 @@ static int _main(int argc, char * * argv) } // add the template values. + string drvstr; + if (drvPath.has_value()) + drvstr = drvPath->to_string(); + else + drvstr = ""; + auto hint = hintformat(hintstring); hint - % drvPath->to_string() + % drvstr % neededSystem % concatStringsSep(", ", requiredFeatures) % machines.size(); From 59067f0f587f75908c4f990bcbab29340c7f7652 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 6 Aug 2020 11:40:41 +0200 Subject: [PATCH 74/74] repl.cc: Check for HAVE_BOEHMGC Fixes #3906. --- src/nix/repl.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/nix/repl.cc b/src/nix/repl.cc index fb9050d0d..c3c9e54a8 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -33,12 +33,17 @@ extern "C" { #include "command.hh" #include "finally.hh" +#if HAVE_BOEHMGC #define GC_INCLUDE_NEW #include +#endif namespace nix { -struct NixRepl : gc +struct NixRepl + #if HAVE_BOEHMGC + : gc + #endif { string curDir; std::unique_ptr state;