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