From d564ac1c5086bda75388d21a25583b2c9809f086 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Mon, 27 Jul 2020 16:42:02 -0400 Subject: [PATCH 01/14] Offer a safer interface for pathOpt The new interface we offer provides a way of getting all the DerivationOutputs with the storePaths directly, based on the observation that it's the most common usecase. --- src/libstore/derivations.cc | 21 +++++++++++++++++++++ src/libstore/derivations.hh | 19 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index c88bb3c6d..944d89fe5 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -581,6 +581,27 @@ StringSet BasicDerivation::outputNames() const return names; } +DerivationOutputsAndPaths BasicDerivation::outputsAndPaths(const Store & store) const { + DerivationOutputsAndPaths outsAndPaths; + for (auto output : outputs) + outsAndPaths.insert(std::make_pair( + output.first, + std::make_pair(output.second, output.second.path(store, name)) + ) + ); + return outsAndPaths; +} + +DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & store) const { + DerivationOutputsAndOptPaths outsAndOptPaths; + for (auto output : outputs) + outsAndOptPaths.insert(std::make_pair( + output.first, + std::make_pair(output.second, output.second.pathOpt(store, output.first)) + ) + ); + return outsAndOptPaths; +} std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath) { auto nameWithSuffix = drvPath.name(); diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index b1cda85cb..b0cb870a2 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -41,6 +41,9 @@ struct DerivationOutput DerivationOutputFloating > output; std::optional hashAlgoOpt(const Store & store) const; + /* Note, when you use this function you should make sure that you're passing + the right derivation name. When in doubt, you should use the safer + interface provided by BasicDerivation::outputsAndPaths */ 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 { @@ -52,6 +55,15 @@ struct DerivationOutput typedef std::map DerivationOutputs; +/* These are analogues to the previous DerivationOutputs data type, but they + also contains, for each output, the (optional) store path in which it would + be written. To calculate values of these types, see the corresponding + functions in BasicDerivation */ +typedef std::map> + DerivationOutputsAndPaths; +typedef std::map>> + DerivationOutputsAndOptPaths; + /* For inputs that are sub-derivations, we specify exactly which output IDs we are interested in. */ typedef std::map DerivationInputs; @@ -101,6 +113,13 @@ struct BasicDerivation /* Return the output names of a derivation. */ StringSet outputNames() const; + /* Calculates the maps that contains all the DerivationOutputs, but + augmented with knowledge of the Store paths they would be written into. + The first one of these functions will be removed when the CA work is + completed */ + DerivationOutputsAndPaths outputsAndPaths(const Store & store) const; + DerivationOutputsAndOptPaths outputsAndOptPaths(const Store & store) const; + static std::string_view nameFromPath(const StorePath & storePath); }; From 7ef1e3cd1429f336c568ccf4d59cd89cde68efbd Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 28 Jul 2020 13:59:24 -0400 Subject: [PATCH 02/14] Use the new interface --- perl/lib/Nix/Store.xs | 4 +- src/libexpr/primops.cc | 8 ++-- src/libstore/build.cc | 84 ++++++++++++++++++------------------- src/libstore/derivations.cc | 16 +++---- src/libstore/local-store.cc | 4 +- src/libstore/misc.cc | 6 +-- src/nix-store/nix-store.cc | 8 ++-- src/nix/installables.cc | 4 +- src/nix/repl.cc | 4 +- src/nix/show-derivation.cc | 6 +-- 10 files changed, 72 insertions(+), 72 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index ec85b844a..7c3416f0b 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -303,10 +303,10 @@ SV * derivationFromPath(char * drvPath) hash = newHV(); HV * outputs = newHV(); - for (auto & i : drv.outputs) + for (auto & i : drv.outputsAndPaths(*store())) hv_store( outputs, i.first.c_str(), i.first.size(), - newSVpv(store()->printStorePath(i.second.path(*store(), drv.name)).c_str(), 0), + newSVpv(store()->printStorePath(i.second.second).c_str(), 0), 0); hv_stores(hash, "outputs", newRV((SV *) outputs)); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index d12d571ad..ee0944fba 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -113,9 +113,9 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args state.mkList(*outputsVal, drv.outputs.size()); unsigned int outputs_index = 0; - for (const auto & o : drv.outputs) { + for (const auto & o : drv.outputsAndPaths(*state.store)) { v2 = state.allocAttr(w, state.symbols.create(o.first)); - mkString(*v2, state.store->printStorePath(o.second.path(*state.store, drv.name)), {"!" + o.first + "!" + path}); + mkString(*v2, state.store->printStorePath(o.second.second), {"!" + o.first + "!" + path}); outputsVal->listElems()[outputs_index] = state.allocValue(); mkString(*(outputsVal->listElems()[outputs_index++]), o.first); } @@ -830,9 +830,9 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * state.mkAttrs(v, 1 + drv.outputs.size()); mkString(*state.allocAttr(v, state.sDrvPath), drvPathS, {"=" + drvPathS}); - for (auto & i : drv.outputs) { + for (auto & i : drv.outputsAndPaths(*state.store)) { mkString(*state.allocAttr(v, state.symbols.create(i.first)), - state.store->printStorePath(i.second.path(*state.store, drv.name)), {"!" + i.first + "!" + drvPathS}); + state.store->printStorePath(i.second.second), {"!" + i.first + "!" + drvPathS}); } v.attrs->sort(); } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 413fc9c4a..7be60396c 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1181,8 +1181,8 @@ void DerivationGoal::haveDerivation() retrySubstitution = false; - for (auto & i : drv->outputs) - worker.store.addTempRoot(i.second.path(worker.store, drv->name)); + for (auto & i : drv->outputsAndPaths(worker.store)) + worker.store.addTempRoot(i.second.second); /* Check what outputs paths are not already valid. */ auto invalidOutputs = checkPathValidity(false, buildMode == bmRepair); @@ -1288,14 +1288,14 @@ void DerivationGoal::repairClosure() /* Get the output closure. */ StorePathSet outputClosure; - for (auto & i : drv->outputs) { + for (auto & i : drv->outputsAndPaths(worker.store)) { if (!wantOutput(i.first, wantedOutputs)) continue; - worker.store.computeFSClosure(i.second.path(worker.store, drv->name), outputClosure); + worker.store.computeFSClosure(i.second.second, outputClosure); } /* Filter out our own outputs (which we have already checked). */ - for (auto & i : drv->outputs) - outputClosure.erase(i.second.path(worker.store, drv->name)); + for (auto & i : drv->outputsAndPaths(worker.store)) + outputClosure.erase(i.second.second); /* Get all dependencies of this derivation so that we know which derivation is responsible for which path in the output @@ -1306,8 +1306,8 @@ void DerivationGoal::repairClosure() for (auto & i : inputClosure) if (i.isDerivation()) { Derivation drv = worker.store.derivationFromPath(i); - for (auto & j : drv.outputs) - outputsToDrv.insert_or_assign(j.second.path(worker.store, drv.name), i); + for (auto & j : drv.outputsAndPaths(worker.store)) + outputsToDrv.insert_or_assign(j.second.second, i); } /* Check each path (slow!). */ @@ -1466,10 +1466,10 @@ void DerivationGoal::tryToBuild() /* If any of the outputs already exist but are not valid, delete them. */ - for (auto & i : drv->outputs) { - if (worker.store.isValidPath(i.second.path(worker.store, drv->name))) continue; - debug("removing invalid path '%s'", worker.store.printStorePath(i.second.path(worker.store, drv->name))); - deletePath(worker.store.Store::toRealPath(i.second.path(worker.store, drv->name))); + for (auto & i : drv->outputsAndPaths(worker.store)) { + if (worker.store.isValidPath(i.second.second)) continue; + debug("removing invalid path '%s'", worker.store.printStorePath(i.second.second)); + deletePath(worker.store.Store::toRealPath(i.second.second)); } /* Don't do a remote build if the derivation has the attribute @@ -1919,8 +1919,8 @@ StorePathSet DerivationGoal::exportReferences(const StorePathSet & storePaths) for (auto & j : paths2) { if (j.isDerivation()) { Derivation drv = worker.store.derivationFromPath(j); - for (auto & k : drv.outputs) - worker.store.computeFSClosure(k.second.path(worker.store, drv.name), paths); + for (auto & k : drv.outputsAndPaths(worker.store)) + worker.store.computeFSClosure(k.second.second, paths); } } @@ -2014,8 +2014,8 @@ void DerivationGoal::startBuilder() chownToBuilder(tmpDir); /* Substitute output placeholders with the actual output paths. */ - for (auto & output : drv->outputs) - inputRewrites[hashPlaceholder(output.first)] = worker.store.printStorePath(output.second.path(worker.store, drv->name)); + for (auto & output : drv->outputsAndPaths(worker.store)) + inputRewrites[hashPlaceholder(output.first)] = worker.store.printStorePath(output.second.second); /* Construct the environment passed to the builder. */ initEnv(); @@ -2199,8 +2199,8 @@ void DerivationGoal::startBuilder() rebuilding a path that is in settings.dirsInChroot (typically the dependencies of /bin/sh). Throw them out. */ - for (auto & i : drv->outputs) - dirsInChroot.erase(worker.store.printStorePath(i.second.path(worker.store, drv->name))); + for (auto & i : drv->outputsAndPaths(worker.store)) + dirsInChroot.erase(worker.store.printStorePath(i.second.second)); #elif __APPLE__ /* We don't really have any parent prep work to do (yet?) @@ -2612,8 +2612,8 @@ void DerivationGoal::writeStructuredAttrs() /* Add an "outputs" object containing the output paths. */ nlohmann::json outputs; - for (auto & i : drv->outputs) - outputs[i.first] = rewriteStrings(worker.store.printStorePath(i.second.path(worker.store, drv->name)), inputRewrites); + for (auto & i : drv->outputsAndPaths(worker.store)) + outputs[i.first] = rewriteStrings(worker.store.printStorePath(i.second.second), inputRewrites); json["outputs"] = outputs; /* Handle exportReferencesGraph. */ @@ -2815,9 +2815,9 @@ struct RestrictedStore : public LocalFSStore if (!goal.isAllowed(path.path)) throw InvalidPath("cannot build unknown path '%s' in recursive Nix", printStorePath(path.path)); auto drv = derivationFromPath(path.path); - for (auto & output : drv.outputs) + for (auto & output : drv.outputsAndPaths(*this)) if (wantOutput(output.first, path.outputs)) - newPaths.insert(output.second.path(*this, drv.name)); + newPaths.insert(output.second.second); } else if (!goal.isAllowed(path.path)) throw InvalidPath("cannot build unknown path '%s' in recursive Nix", printStorePath(path.path)); } @@ -3616,8 +3616,8 @@ void DerivationGoal::registerOutputs() to do anything here. */ if (hook) { bool allValid = true; - for (auto & i : drv->outputs) - if (!worker.store.isValidPath(i.second.path(worker.store, drv->name))) allValid = false; + for (auto & i : drv->outputsAndPaths(worker.store)) + if (!worker.store.isValidPath(i.second.second)) allValid = false; if (allValid) return; } @@ -3638,23 +3638,23 @@ void DerivationGoal::registerOutputs() Nix calls. */ StorePathSet referenceablePaths; for (auto & p : inputPaths) referenceablePaths.insert(p); - for (auto & i : drv->outputs) referenceablePaths.insert(i.second.path(worker.store, drv->name)); + for (auto & i : drv->outputsAndPaths(worker.store)) referenceablePaths.insert(i.second.second); for (auto & p : addedPaths) referenceablePaths.insert(p); /* Check whether the output paths were created, and grep each output path to determine what other paths it references. Also make all output paths read-only. */ - for (auto & i : drv->outputs) { - auto path = worker.store.printStorePath(i.second.path(worker.store, drv->name)); - if (!missingPaths.count(i.second.path(worker.store, drv->name))) continue; + for (auto & i : drv->outputsAndPaths(worker.store)) { + auto path = worker.store.printStorePath(i.second.second); + if (!missingPaths.count(i.second.second)) continue; Path actualPath = path; if (needsHashRewrite()) { - auto r = redirectedOutputs.find(i.second.path(worker.store, drv->name)); + auto r = redirectedOutputs.find(i.second.second); if (r != redirectedOutputs.end()) { auto redirected = worker.store.Store::toRealPath(r->second); if (buildMode == bmRepair - && redirectedBadOutputs.count(i.second.path(worker.store, drv->name)) + && redirectedBadOutputs.count(i.second.second) && pathExists(redirected)) replaceValidPath(path, redirected); if (buildMode == bmCheck) @@ -3721,7 +3721,7 @@ void DerivationGoal::registerOutputs() hash). */ std::optional ca; - if (! std::holds_alternative(i.second.output)) { + if (! std::holds_alternative(i.second.first.output)) { DerivationOutputFloating outputHash; std::visit(overloaded { [&](DerivationOutputInputAddressed doi) { @@ -3736,7 +3736,7 @@ void DerivationGoal::registerOutputs() [&](DerivationOutputFloating dof) { outputHash = dof; }, - }, i.second.output); + }, i.second.first.output); if (outputHash.method == FileIngestionMethod::Flat) { /* The output path should be a regular file without execute permission. */ @@ -3753,12 +3753,12 @@ void DerivationGoal::registerOutputs() ? 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()); + auto dest = worker.store.makeFixedOutputPath(outputHash.method, h2, i.second.second.name()); // 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.first.output)) { Hash & h = p->hash.hash; if (h != h2) { @@ -3919,8 +3919,8 @@ void DerivationGoal::registerOutputs() /* If this is the first round of several, then move the output out of the way. */ if (nrRounds > 1 && curRound == 1 && curRound < nrRounds && keepPreviousRound) { - for (auto & i : drv->outputs) { - auto path = worker.store.printStorePath(i.second.path(worker.store, drv->name)); + for (auto & i : drv->outputsAndPaths(worker.store)) { + auto path = worker.store.printStorePath(i.second.second); Path prev = path + checkSuffix; deletePath(prev); Path dst = path + checkSuffix; @@ -3937,8 +3937,8 @@ void DerivationGoal::registerOutputs() /* Remove the .check directories if we're done. FIXME: keep them if the result was not determistic? */ if (curRound == nrRounds) { - for (auto & i : drv->outputs) { - Path prev = worker.store.printStorePath(i.second.path(worker.store, drv->name)) + checkSuffix; + for (auto & i : drv->outputsAndPaths(worker.store)) { + Path prev = worker.store.printStorePath(i.second.second) + checkSuffix; deletePath(prev); } } @@ -4236,12 +4236,12 @@ void DerivationGoal::flushLine() StorePathSet DerivationGoal::checkPathValidity(bool returnValid, bool checkHash) { StorePathSet result; - for (auto & i : drv->outputs) { + for (auto & i : drv->outputsAndPaths(worker.store)) { if (!wantOutput(i.first, wantedOutputs)) continue; bool good = - worker.store.isValidPath(i.second.path(worker.store, drv->name)) && - (!checkHash || worker.pathContentsGood(i.second.path(worker.store, drv->name))); - if (good == returnValid) result.insert(i.second.path(worker.store, drv->name)); + worker.store.isValidPath(i.second.second) && + (!checkHash || worker.pathContentsGood(i.second.second)); + if (good == returnValid) result.insert(i.second.second); } return result; } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 944d89fe5..b9c913788 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -473,12 +473,12 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m 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) { - auto & dof = std::get(i.second.output); + for (const auto & i : drv.outputsAndPaths(store)) { + auto & dof = std::get(i.second.first.output); auto hash = hashString(htSHA256, "fixed:out:" + dof.hash.printMethodAlgo() + ":" + dof.hash.hash.to_string(Base16, false) + ":" - + store.printStorePath(i.second.path(store, drv.name))); + + store.printStorePath(i.second.second)); outputHashes.insert_or_assign(i.first, std::move(hash)); } return outputHashes; @@ -532,8 +532,8 @@ bool wantOutput(const string & output, const std::set & wanted) StorePathSet BasicDerivation::outputPaths(const Store & store) const { StorePathSet paths; - for (auto & i : outputs) - paths.insert(i.second.path(store, name)); + for (auto & i : outputsAndPaths(store)) + paths.insert(i.second.second); return paths; } @@ -642,9 +642,9 @@ Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, void writeDerivation(Sink & out, const Store & store, const BasicDerivation & drv) { out << drv.outputs.size(); - for (auto & i : drv.outputs) { + for (auto & i : drv.outputsAndPaths(store)) { out << i.first - << store.printStorePath(i.second.path(store, drv.name)); + << store.printStorePath(i.second.second); std::visit(overloaded { [&](DerivationOutputInputAddressed doi) { out << "" << ""; @@ -657,7 +657,7 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr out << (makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)) << ""; }, - }, i.second.output); + }, i.second.first.output); } writeStorePaths(store, out, drv.inputSrcs); out << drv.platform << drv.builder << drv.args; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 0ca21bc40..a87dde1c5 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -618,11 +618,11 @@ uint64_t LocalStore::addValidPath(State & state, registration above is undone. */ if (checkOutputs) checkDerivationOutputs(info.path, drv); - for (auto & i : drv.outputs) { + for (auto & i : drv.outputsAndPaths(*this)) { state.stmtAddDerivationOutput.use() (id) (i.first) - (printStorePath(i.second.path(*this, drv.name))) + (printStorePath(i.second.second)) .exec(); } } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index c4d22a634..46553fecb 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -196,10 +196,10 @@ void Store::queryMissing(const std::vector & targets, ParsedDerivation parsedDrv(StorePath(path.path), *drv); PathSet invalid; - for (auto & j : drv->outputs) + for (auto & j : drv->outputsAndPaths(*this)) if (wantOutput(j.first, path.outputs) - && !isValidPath(j.second.path(*this, drv->name))) - invalid.insert(printStorePath(j.second.path(*this, drv->name))); + && !isValidPath(j.second.second)) + invalid.insert(printStorePath(j.second.second)); if (invalid.empty()) return; if (settings.useSubstitutes && parsedDrv.substitutesAllowed()) { diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 9c8874fd4..87888a1d0 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -218,8 +218,8 @@ static StorePathSet maybeUseOutputs(const StorePath & storePath, bool useOutput, if (useOutput && storePath.isDerivation()) { auto drv = store->derivationFromPath(storePath); StorePathSet outputs; - for (auto & i : drv.outputs) - outputs.insert(i.second.path(*store, drv.name)); + for (auto & i : drv.outputsAndPaths(*store)) + outputs.insert(i.second.second); return outputs; } else return {storePath}; @@ -312,8 +312,8 @@ static void opQuery(Strings opFlags, Strings opArgs) auto i2 = store->followLinksToStorePath(i); if (forceRealise) realisePath({i2}); Derivation drv = store->derivationFromPath(i2); - for (auto & j : drv.outputs) - cout << fmt("%1%\n", store->printStorePath(j.second.path(*store, drv.name))); + for (auto & j : drv.outputsAndPaths(*store)) + cout << fmt("%1%\n", store->printStorePath(j.second.second)); } break; } diff --git a/src/nix/installables.cc b/src/nix/installables.cc index b245e417e..41c077c47 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -305,8 +305,8 @@ struct InstallableStorePath : Installable if (storePath.isDerivation()) { std::map outputs; auto drv = store->readDerivation(storePath); - for (auto & [name, output] : drv.outputs) - outputs.emplace(name, output.path(*store, drv.name)); + for (auto & i : drv.outputsAndPaths(*store)) + outputs.emplace(i.first, i.second.second); return { Buildable { .drvPath = storePath, diff --git a/src/nix/repl.cc b/src/nix/repl.cc index fb9050d0d..803d89018 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -485,8 +485,8 @@ bool NixRepl::processLine(string line) if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPath}) == 0) { auto drv = readDerivation(*state->store, drvPath, Derivation::nameFromPath(state->store->parseStorePath(drvPath))); std::cout << std::endl << "this derivation produced the following outputs:" << std::endl; - for (auto & i : drv.outputs) - std::cout << fmt(" %s -> %s\n", i.first, state->store->printStorePath(i.second.path(*state->store, drv.name))); + for (auto & i : drv.outputsAndPaths(*state->store)) + std::cout << fmt(" %s -> %s\n", i.first, state->store->printStorePath(i.second.second)); } } else if (command == ":i") { runProgram(settings.nixBinDir + "/nix-env", Strings{"-i", drvPath}); diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index 25ea19834..87122be87 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -67,9 +67,9 @@ struct CmdShowDerivation : InstallablesCommand { auto outputsObj(drvObj.object("outputs")); - for (auto & output : drv.outputs) { + for (auto & output : drv.outputsAndPaths(*store)) { auto outputObj(outputsObj.object(output.first)); - outputObj.attr("path", store->printStorePath(output.second.path(*store, drv.name))); + outputObj.attr("path", store->printStorePath(output.second.second)); std::visit(overloaded { [&](DerivationOutputInputAddressed doi) { @@ -81,7 +81,7 @@ struct CmdShowDerivation : InstallablesCommand [&](DerivationOutputFloating dof) { outputObj.attr("hashAlgo", makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); }, - }, output.second.output); + }, output.second.first.output); } } From 1d71028f4d43f8e3ade559f7187f697c7bb9d709 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 5 Aug 2020 14:42:48 -0400 Subject: [PATCH 03/14] Remove optionality in ValidPathInfo::narInfo --- src/libexpr/primops/fetchTree.cc | 2 +- src/libfetchers/fetchers.cc | 4 ++-- src/libstore/build.cc | 2 +- src/libstore/daemon.cc | 4 ++-- src/libstore/export-import.cc | 4 ++-- src/libstore/legacy-ssh-store.cc | 4 ++-- src/libstore/local-store.cc | 14 +++++++------- src/libstore/nar-info-disk-cache.cc | 2 +- src/libstore/nar-info.cc | 4 ++-- src/libstore/path-info.hh | 6 +++--- src/libstore/remote-store.cc | 2 +- src/libstore/store-api.cc | 6 +++--- src/nix-store/nix-store.cc | 10 +++++----- src/nix/make-content-addressable.cc | 2 +- src/nix/profile.cc | 2 +- src/nix/verify.cc | 8 ++++---- 16 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index cddcf0e59..0dbf4ae1d 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -212,7 +212,7 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v, : hashFile(htSHA256, path); if (hash != *expectedHash) throw Error((unsigned int) 102, "hash mismatch in file downloaded from '%s':\n wanted: %s\n got: %s", - *url, expectedHash->to_string(Base32, true), hash->to_string(Base32, true)); + *url, expectedHash->to_string(Base32, true), hash.to_string(Base32, true)); } if (state.allowedPaths) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 9c69fc564..eaa635595 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -130,12 +130,12 @@ std::pair Input::fetch(ref store) const tree.actualPath = store->toRealPath(tree.storePath); auto narHash = store->queryPathInfo(tree.storePath)->narHash; - input.attrs.insert_or_assign("narHash", narHash->to_string(SRI, true)); + input.attrs.insert_or_assign("narHash", narHash.to_string(SRI, true)); if (auto prevNarHash = getNarHash()) { if (narHash != *prevNarHash) throw Error((unsigned int) 102, "NAR hash mismatch in input '%s' (%s), expected '%s', got '%s'", - to_string(), tree.actualPath, prevNarHash->to_string(SRI, true), narHash->to_string(SRI, true)); + to_string(), tree.actualPath, prevNarHash->to_string(SRI, true), narHash.to_string(SRI, true)); } if (auto prevLastModified = getLastModified()) { diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 4156cd678..51dd0c094 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -5037,7 +5037,7 @@ bool Worker::pathContentsGood(const StorePath & path) if (!pathExists(store.printStorePath(path))) res = false; else { - HashResult current = hashPath(info->narHash->type, store.printStorePath(path)); + HashResult current = hashPath(info->narHash.type, store.printStorePath(path)); Hash nullHash(htSHA256); res = info->narHash == nullHash || info->narHash == current.first; } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 5e568fc94..fa3bf6e2d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -289,7 +289,7 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); auto hash = store->queryPathInfo(path)->narHash; logger->stopWork(); - to << hash->to_string(Base16, false); + to << hash.to_string(Base16, false); break; } @@ -638,7 +638,7 @@ static void performOp(TunnelLogger * logger, ref store, if (GET_PROTOCOL_MINOR(clientVersion) >= 17) to << 1; to << (info->deriver ? store->printStorePath(*info->deriver) : "") - << info->narHash->to_string(Base16, false); + << info->narHash.to_string(Base16, false); writeStorePaths(*store, to, info->references); to << info->registrationTime << info->narSize; if (GET_PROTOCOL_MINOR(clientVersion) >= 16) { diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc index a0fc22264..3f4b7c36f 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -38,9 +38,9 @@ void Store::exportPath(const StorePath & path, Sink & sink) filesystem corruption from spreading to other machines. Don't complain if the stored hash is zero (unknown). */ Hash hash = hashSink.currentHash().first; - if (hash != info->narHash && info->narHash != Hash(info->narHash->type)) + if (hash != info->narHash && info->narHash != Hash(info->narHash.type)) throw Error("hash of path '%s' has changed from '%s' to '%s'!", - printStorePath(path), info->narHash->to_string(Base32, true), hash.to_string(Base32, true)); + printStorePath(path), info->narHash.to_string(Base32, true), hash.to_string(Base32, true)); teeSink << exportMagic diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index c6eeab548..ed57a188b 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::parseAnyPrefixed(s); + info->narHash = s.empty() ? Hash(htSHA256) : Hash::parseAnyPrefixed(s); info->ca = parseContentAddressOpt(readString(conn->from)); info->sigs = readStrings(conn->from); } @@ -138,7 +138,7 @@ struct LegacySSHStore : public Store << cmdAddToStoreNar << printStorePath(info.path) << (info.deriver ? printStorePath(*info.deriver) : "") - << info.narHash->to_string(Base16, false); + << info.narHash.to_string(Base16, false); writeStorePaths(*this, conn->to, info.references); conn->to << info.registrationTime diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 6b0548538..93a86f828 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -579,7 +579,7 @@ uint64_t LocalStore::addValidPath(State & state, state.stmtRegisterValidPath.use() (printStorePath(info.path)) - (info.narHash->to_string(Base16, true)) + (info.narHash.to_string(Base16, true)) (info.registrationTime == 0 ? time(0) : info.registrationTime) (info.deriver ? printStorePath(*info.deriver) : "", (bool) info.deriver) (info.narSize, info.narSize != 0) @@ -679,7 +679,7 @@ void LocalStore::updatePathInfo(State & state, const ValidPathInfo & info) { state.stmtUpdatePathInfo.use() (info.narSize, info.narSize != 0) - (info.narHash->to_string(Base16, true)) + (info.narHash.to_string(Base16, true)) (info.ultimate ? 1 : 0, info.ultimate) (concatStringsSep(" ", info.sigs), !info.sigs.empty()) (renderContentAddress(info.ca), (bool) info.ca) @@ -905,7 +905,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos) StorePathSet paths; for (auto & i : infos) { - assert(i.narHash && i.narHash->type == htSHA256); + assert(i.narHash && i.narHash.type == htSHA256); if (isValidPath_(*state, i.path)) updatePathInfo(*state, i); else @@ -1018,7 +1018,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, if (hashResult.first != info.narHash) throw Error("hash mismatch importing path '%s';\n wanted: %s\n got: %s", - printStorePath(info.path), info.narHash->to_string(Base32, true), hashResult.first.to_string(Base32, true)); + printStorePath(info.path), info.narHash.to_string(Base32, true), hashResult.first.to_string(Base32, true)); if (hashResult.second != info.narSize) throw Error("size mismatch importing path '%s';\n wanted: %s\n got: %s", @@ -1300,9 +1300,9 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) std::unique_ptr hashSink; if (!info->ca || !info->references.count(info->path)) - hashSink = std::make_unique(info->narHash->type); + hashSink = std::make_unique(info->narHash.type); else - hashSink = std::make_unique(info->narHash->type, std::string(info->path.hashPart())); + hashSink = std::make_unique(info->narHash.type, std::string(info->path.hashPart())); dumpPath(Store::toRealPath(i), *hashSink); auto current = hashSink->finish(); @@ -1311,7 +1311,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) logError({ .name = "Invalid hash - path modified", .hint = hintfmt("path '%s' was modified! expected hash '%s', got '%s'", - printStorePath(i), info->narHash->to_string(Base32, true), current.first.to_string(Base32, true)) + printStorePath(i), info->narHash.to_string(Base32, true), current.first.to_string(Base32, true)) }); if (repair) repairPath(i); else errors = true; } else { diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 92da14e23..dd40006d7 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -232,7 +232,7 @@ public: (narInfo ? narInfo->compression : "", narInfo != 0) (narInfo && narInfo->fileHash ? narInfo->fileHash->to_string(Base32, true) : "", narInfo && narInfo->fileHash) (narInfo ? narInfo->fileSize : 0, narInfo != 0 && narInfo->fileSize) - (info->narHash->to_string(Base32, true)) + (info->narHash.to_string(Base32, true)) (info->narSize) (concatStringsSep(" ", info->shortRefs())) (info->deriver ? std::string(info->deriver->to_string()) : "", (bool) info->deriver) diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index 5812aa4ac..cadaf6267 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -89,8 +89,8 @@ std::string NarInfo::to_string(const Store & store) const assert(fileHash && fileHash->type == htSHA256); res += "FileHash: " + fileHash->to_string(Base32, true) + "\n"; res += "FileSize: " + std::to_string(fileSize) + "\n"; - assert(narHash && narHash->type == htSHA256); - res += "NarHash: " + narHash->to_string(Base32, true) + "\n"; + assert(narHash && narHash.type == htSHA256); + res += "NarHash: " + narHash.to_string(Base32, true) + "\n"; res += "NarSize: " + std::to_string(narSize) + "\n"; res += "References: " + concatStringsSep(" ", shortRefs()) + "\n"; diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh index 2a015ea3c..ba154dab7 100644 --- a/src/libstore/path-info.hh +++ b/src/libstore/path-info.hh @@ -29,7 +29,7 @@ struct ValidPathInfo StorePath path; std::optional deriver; // TODO document this - std::optional narHash; + Hash narHash; StorePathSet references; time_t registrationTime = 0; uint64_t narSize = 0; // 0 = unknown @@ -100,8 +100,8 @@ struct ValidPathInfo ValidPathInfo(const ValidPathInfo & other) = default; - ValidPathInfo(StorePath && path) : path(std::move(path)) { }; - ValidPathInfo(const StorePath & path) : path(path) { }; + ValidPathInfo(StorePath && path) : path(std::move(path)), narHash(Hash(htSHA256)) { }; + ValidPathInfo(const StorePath & path) : path(path), narHash(Hash(htSHA256)) { }; virtual ~ValidPathInfo() { } }; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 33d1e431b..b5e4d9fa6 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -521,7 +521,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, conn->to << wopAddToStoreNar << printStorePath(info.path) << (info.deriver ? printStorePath(*info.deriver) : "") - << info.narHash->to_string(Base16, false); + << info.narHash.to_string(Base16, false); writeStorePaths(*this, conn->to, info.references); conn->to << info.registrationTime << info.narSize << info.ultimate << info.sigs << renderContentAddress(info.ca) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index ea666f098..81180c484 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -569,7 +569,7 @@ string Store::makeValidityRegistration(const StorePathSet & paths, auto info = queryPathInfo(i); if (showHash) { - s += info->narHash->to_string(Base16, false) + "\n"; + s += info->narHash.to_string(Base16, false) + "\n"; s += (format("%1%\n") % info->narSize).str(); } @@ -601,7 +601,7 @@ void Store::pathInfoToJSON(JSONPlaceholder & jsonOut, const StorePathSet & store auto info = queryPathInfo(storePath); jsonPath - .attr("narHash", info->narHash->to_string(hashBase, true)) + .attr("narHash", info->narHash.to_string(hashBase, true)) .attr("narSize", info->narSize); { @@ -919,7 +919,7 @@ std::string ValidPathInfo::fingerprint(const Store & store) const store.printStorePath(path)); return "1;" + store.printStorePath(path) + ";" - + narHash->to_string(Base32, true) + ";" + + narHash.to_string(Base32, true) + ";" + std::to_string(narSize) + ";" + concatStringsSep(",", store.printStorePathSet(references)); } diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 2996e36c4..a901261f4 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -372,8 +372,8 @@ static void opQuery(Strings opFlags, Strings opArgs) for (auto & j : maybeUseOutputs(store->followLinksToStorePath(i), useOutput, forceRealise)) { auto info = store->queryPathInfo(j); if (query == qHash) { - assert(info->narHash && info->narHash->type == htSHA256); - cout << fmt("%s\n", info->narHash->to_string(Base32, true)); + assert(info->narHash && info->narHash.type == htSHA256); + cout << fmt("%s\n", info->narHash.to_string(Base32, true)); } else if (query == qSize) cout << fmt("%d\n", info->narSize); } @@ -723,7 +723,7 @@ static void opVerifyPath(Strings opFlags, Strings opArgs) auto path = store->followLinksToStorePath(i); printMsg(lvlTalkative, "checking path '%s'...", store->printStorePath(path)); auto info = store->queryPathInfo(path); - HashSink sink(info->narHash->type); + HashSink sink(info->narHash.type); store->narFromPath(path, sink); auto current = sink.finish(); if (current.first != info->narHash) { @@ -732,7 +732,7 @@ static void opVerifyPath(Strings opFlags, Strings opArgs) .hint = hintfmt( "path '%s' was modified! expected hash '%s', got '%s'", store->printStorePath(path), - info->narHash->to_string(Base32, true), + info->narHash.to_string(Base32, true), current.first.to_string(Base32, true)) }); status = 1; @@ -862,7 +862,7 @@ static void opServe(Strings opFlags, Strings opArgs) out << info->narSize // downloadSize << info->narSize; if (GET_PROTOCOL_MINOR(clientVersion) >= 4) - out << (info->narHash ? info->narHash->to_string(Base32, true) : "") + out << (info->narHash ? info->narHash.to_string(Base32, true) : "") << renderContentAddress(info->ca) << info->sigs; } catch (InvalidPath &) { diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index 2fe2e2fb2..712043978 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -84,7 +84,7 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON info.narSize = sink.s->size(); info.ca = FixedOutputHash { .method = FileIngestionMethod::Recursive, - .hash = *info.narHash, + .hash = info.narHash, }; if (!json) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 7dcc0b6d4..c6cd88c49 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -133,7 +133,7 @@ struct ProfileManifest info.references = std::move(references); info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = FixedOutputHash { .method = FileIngestionMethod::Recursive, .hash = *info.narHash }; + info.ca = FixedOutputHash { .method = FileIngestionMethod::Recursive, .hash = info.narHash }; auto source = StringSource { *sink.s }; store->addToStore(info, source); diff --git a/src/nix/verify.cc b/src/nix/verify.cc index fc7a9765c..26f755fd9 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -91,15 +91,15 @@ struct CmdVerify : StorePathsCommand std::unique_ptr hashSink; if (!info->ca) - hashSink = std::make_unique(info->narHash->type); + hashSink = std::make_unique(info->narHash.type); else - hashSink = std::make_unique(info->narHash->type, std::string(info->path.hashPart())); + hashSink = std::make_unique(info->narHash.type, std::string(info->path.hashPart())); store->narFromPath(info->path, *hashSink); auto hash = hashSink->finish(); - if (hash.first != *info->narHash) { + if (hash.first != info->narHash) { corrupted++; act2.result(resCorruptedPath, store->printStorePath(info->path)); logError({ @@ -107,7 +107,7 @@ struct CmdVerify : StorePathsCommand .hint = hintfmt( "path '%s' was modified! expected hash '%s', got '%s'", store->printStorePath(info->path), - info->narHash->to_string(Base32, true), + info->narHash.to_string(Base32, true), hash.first.to_string(Base32, true)) }); } From 1ad6394b33b5e627c27bc26247f8a5e1d7d81ce1 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 5 Aug 2020 15:11:49 -0400 Subject: [PATCH 04/14] Add Hash::dummy to signal default value We did this in the same spirit of the dummy value that's present in libstore/path.hh --- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/path-info.hh | 4 ++-- src/libutil/hash.cc | 2 ++ src/libutil/hash.hh | 2 ++ 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index ed57a188b..f0f5ec626 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() ? Hash(htSHA256) : Hash::parseAnyPrefixed(s); + info->narHash = s.empty() ? Hash::dummy : Hash::parseAnyPrefixed(s); info->ca = parseContentAddressOpt(readString(conn->from)); info->sigs = readStrings(conn->from); } diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh index ba154dab7..15484717e 100644 --- a/src/libstore/path-info.hh +++ b/src/libstore/path-info.hh @@ -100,8 +100,8 @@ struct ValidPathInfo ValidPathInfo(const ValidPathInfo & other) = default; - ValidPathInfo(StorePath && path) : path(std::move(path)), narHash(Hash(htSHA256)) { }; - ValidPathInfo(const StorePath & path) : path(path), narHash(Hash(htSHA256)) { }; + ValidPathInfo(StorePath && path) : path(std::move(path)), narHash(Hash::dummy) { }; + ValidPathInfo(const StorePath & path) : path(path), narHash(Hash::dummy) { }; virtual ~ValidPathInfo() { } }; diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index dfb3668f1..4a94f0dfd 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -136,6 +136,8 @@ std::string Hash::to_string(Base base, bool includeType) const return s; } +Hash Hash::dummy(htSHA256); + Hash Hash::parseSRI(std::string_view original) { auto rest = original; diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 00ce7bb6f..0520c6022 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -105,6 +105,8 @@ public: assert(type == htSHA1); return std::string(to_string(Base16, false), 0, 7); } + + static Hash dummy; }; /* Helper that defaults empty hashes to the 0 hash. */ From 8241e660bac5583d2924a490e5b32027e9e8e2fc Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 5 Aug 2020 15:30:38 -0400 Subject: [PATCH 05/14] Remove Hash::operator bool () Since the hash is not optional anymore --- src/libstore/binary-cache-store.cc | 2 +- src/libstore/local-store.cc | 5 +---- src/libstore/nar-info.cc | 4 ++-- src/libstore/store-api.cc | 18 ++---------------- src/libutil/hash.hh | 3 --- src/nix-store/nix-store.cc | 4 ++-- 6 files changed, 8 insertions(+), 28 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 9682db730..4bcb2c93b 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -143,7 +143,7 @@ struct FileSource : FdSource void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource, RepairFlag repair, CheckSigsFlag checkSigs) { - assert(info.narHash && info.narSize); + assert(info.narSize); if (!repair && isValidPath(info.path)) { // FIXME: copyNAR -> null sink diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 3542904bf..fc0630cae 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -920,7 +920,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos) StorePathSet paths; for (auto & i : infos) { - assert(i.narHash && i.narHash.type == htSHA256); + assert(i.narHash.type == htSHA256); if (isValidPath_(*state, i.path)) updatePathInfo(*state, i); else @@ -984,9 +984,6 @@ const PublicKeys & LocalStore::getPublicKeys() void LocalStore::addToStore(const ValidPathInfo & info, Source & source, RepairFlag repair, CheckSigsFlag checkSigs) { - if (!info.narHash) - throw Error("cannot add path '%s' because it lacks a hash", printStorePath(info.path)); - if (requireSigs && checkSigs && !info.checkSignatures(*this, getPublicKeys())) throw Error("cannot add path '%s' because it lacks a valid signature", printStorePath(info.path)); diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index cadaf6267..3a9121679 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -76,7 +76,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & if (compression == "") compression = "bzip2"; - if (!havePath || url.empty() || narSize == 0 || !narHash) throw corrupt(); + if (!havePath || url.empty() || narSize == 0) throw corrupt(); } std::string NarInfo::to_string(const Store & store) const @@ -89,7 +89,7 @@ std::string NarInfo::to_string(const Store & store) const assert(fileHash && fileHash->type == htSHA256); res += "FileHash: " + fileHash->to_string(Base32, true) + "\n"; res += "FileSize: " + std::to_string(fileSize) + "\n"; - assert(narHash && narHash.type == htSHA256); + assert(narHash.type == htSHA256); res += "NarHash: " + narHash.to_string(Base32, true) + "\n"; res += "NarSize: " + std::to_string(narSize) + "\n"; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index aead3468a..997af1195 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -725,20 +725,6 @@ void copyStorePath(ref srcStore, ref dstStore, info = info2; } - if (!info->narHash) { - StringSink sink; - srcStore->narFromPath({storePath}, sink); - auto info2 = make_ref(*info); - info2->narHash = hashString(htSHA256, *sink.s); - if (!info->narSize) info2->narSize = sink.s->size(); - if (info->ultimate) info2->ultimate = false; - info = info2; - - StringSource source(*sink.s); - dstStore->addToStore(*info, source, repair, checkSigs); - return; - } - if (info->ultimate) { auto info2 = make_ref(*info); info2->ultimate = false; @@ -910,8 +896,8 @@ string showPaths(const PathSet & paths) std::string ValidPathInfo::fingerprint(const Store & store) const { - if (narSize == 0 || !narHash) - throw Error("cannot calculate fingerprint of path '%s' because its size/hash is not known", + if (narSize == 0) + throw Error("cannot calculate fingerprint of path '%s' because its size is not known", store.printStorePath(path)); return "1;" + store.printStorePath(path) + ";" diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 0520c6022..6d6eb70ca 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -59,9 +59,6 @@ private: Hash(std::string_view s, HashType type, bool isSRI); public: - /* Check whether a hash is set. */ - operator bool () const { return (bool) type; } - /* Check whether two hash are equal. */ bool operator == (const Hash & h2) const; diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index a901261f4..cc104bf35 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -372,7 +372,7 @@ static void opQuery(Strings opFlags, Strings opArgs) for (auto & j : maybeUseOutputs(store->followLinksToStorePath(i), useOutput, forceRealise)) { auto info = store->queryPathInfo(j); if (query == qHash) { - assert(info->narHash && info->narHash.type == htSHA256); + assert(info->narHash.type == htSHA256); cout << fmt("%s\n", info->narHash.to_string(Base32, true)); } else if (query == qSize) cout << fmt("%d\n", info->narSize); @@ -862,7 +862,7 @@ static void opServe(Strings opFlags, Strings opArgs) out << info->narSize // downloadSize << info->narSize; if (GET_PROTOCOL_MINOR(clientVersion) >= 4) - out << (info->narHash ? info->narHash.to_string(Base32, true) : "") + out << info->narHash.to_string(Base32, true) << renderContentAddress(info->ca) << info->sigs; } catch (InvalidPath &) { From d5d9907a9ebd89e84d5031cd767dd4d97145803a Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 5 Aug 2020 15:57:42 -0400 Subject: [PATCH 06/14] Fix perl integration --- perl/lib/Nix/Store.xs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index de60fb939..41846e6a7 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -80,7 +80,7 @@ SV * queryReferences(char * path) SV * queryPathHash(char * path) PPCODE: try { - auto s = store()->queryPathInfo(store()->parseStorePath(path))->narHash->to_string(Base32, true); + auto s = store()->queryPathInfo(store()->parseStorePath(path))->narHash.to_string(Base32, true); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); } catch (Error & e) { croak("%s", e.what()); @@ -106,7 +106,7 @@ SV * queryPathInfo(char * path, int base32) XPUSHs(&PL_sv_undef); else XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(*info->deriver).c_str(), 0))); - auto s = info->narHash->to_string(base32 ? Base32 : Base16, true); + auto s = info->narHash.to_string(base32 ? Base32 : Base16, true); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); mXPUSHi(info->registrationTime); mXPUSHi(info->narSize); From e89b5bd0bfeb4dfdd8fe7e6929544cb9ceb8a505 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 6 Aug 2020 18:31:48 +0000 Subject: [PATCH 07/14] Minimize the usage of `Hash::dummy` --- src/libfetchers/tarball.cc | 6 ++++-- src/libstore/binary-cache-store.cc | 10 ++++++++-- src/libstore/build.cc | 6 ++++-- src/libstore/daemon.cc | 5 +++-- src/libstore/export-import.cc | 11 ++++++----- src/libstore/legacy-ssh-store.cc | 19 +++++++++++++------ src/libstore/local-store.cc | 21 +++++++++++---------- src/libstore/nar-info-disk-cache.cc | 5 +++-- src/libstore/nar-info.cc | 10 +++++++--- src/libstore/nar-info.hh | 6 ++++-- src/libstore/path-info.hh | 5 +++-- src/libstore/remote-store.cc | 4 ++-- src/libstore/store-api.cc | 19 ++++++++++++------- src/libstore/store-api.hh | 3 +-- src/nix-store/nix-store.cc | 11 ++++++++--- src/nix/add-to-store.cc | 6 ++++-- src/nix/make-content-addressable.cc | 6 ++++-- src/nix/profile.cc | 6 ++++-- 18 files changed, 101 insertions(+), 58 deletions(-) diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 55158cece..a2d16365e 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -67,8 +67,10 @@ DownloadFileResult downloadFile( StringSink sink; dumpString(*res.data, sink); auto hash = hashString(htSHA256, *res.data); - ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name)); - info.narHash = hashString(htSHA256, *sink.s); + ValidPathInfo info { + store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name), + hashString(htSHA256, *sink.s), + }; info.narSize = sink.s->size(); info.ca = FixedOutputHash { .method = FileIngestionMethod::Flat, diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 4bcb2c93b..9608467e2 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -385,7 +385,10 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath h = hashString(hashAlgo, s); } - ValidPathInfo info(makeFixedOutputPath(method, *h, name)); + ValidPathInfo info { + makeFixedOutputPath(method, *h, name), + Hash::dummy, // Will be fixed in addToStore, which recomputes nar hash + }; auto source = StringSource { *sink.s }; addToStore(info, source, repair, CheckSigs); @@ -396,7 +399,10 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) { - ValidPathInfo info(computeStorePathForText(name, s, references)); + ValidPathInfo info { + computeStorePathForText(name, s, references), + Hash::dummy, // Will be fixed in addToStore, which recomputes nar hash + }; info.references = references; if (repair || !isValidPath(info.path)) { diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 3370af63d..9effd7ec0 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3864,8 +3864,10 @@ void DerivationGoal::registerOutputs() worker.markContentsGood(worker.store.parseStorePath(path)); } - ValidPathInfo info(worker.store.parseStorePath(path)); - info.narHash = hash.first; + ValidPathInfo info { + worker.store.parseStorePath(path), + hash.first, + }; info.narSize = hash.second; info.references = std::move(references); info.deriver = drvPath; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index fa3bf6e2d..7f7f25ebf 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -694,11 +694,12 @@ static void performOp(TunnelLogger * logger, ref store, case wopAddToStoreNar: { bool repair, dontCheckSigs; - ValidPathInfo info(store->parseStorePath(readString(from))); + auto path = store->parseStorePath(readString(from)); auto deriver = readString(from); + auto narHash = Hash::parseAny(readString(from), htSHA256); + ValidPathInfo info { path, narHash }; if (deriver != "") info.deriver = store->parseStorePath(deriver); - 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/export-import.cc b/src/libstore/export-import.cc index 3f4b7c36f..ccd466d09 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -69,17 +69,18 @@ StorePaths Store::importPaths(Source & source, CheckSigsFlag checkSigs) if (magic != exportMagic) throw Error("Nix archive cannot be imported; wrong format"); - ValidPathInfo info(parseStorePath(readString(source))); + auto path = parseStorePath(readString(source)); //Activity act(*logger, lvlInfo, format("importing path '%s'") % info.path); - info.references = readStorePaths(*this, source); - + auto references = readStorePaths(*this, source); auto deriver = readString(source); + auto narHash = hashString(htSHA256, *saved.s); + + ValidPathInfo info { path, narHash }; if (deriver != "") info.deriver = parseStorePath(deriver); - - info.narHash = hashString(htSHA256, *saved.s); + info.references = references; info.narSize = saved.s->size(); // Ignore optional legacy signature. diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index f0f5ec626..9f95a9726 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -93,6 +93,9 @@ struct LegacySSHStore : public Store try { auto conn(connections->get()); + /* No longer support missing NAR hash */ + assert(GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4); + debug("querying remote host '%s' for info on '%s'", host, printStorePath(path)); conn->to << cmdQueryPathInfos << PathSet{printStorePath(path)}; @@ -100,8 +103,10 @@ struct LegacySSHStore : public Store auto p = readString(conn->from); if (p.empty()) return callback(nullptr); - auto info = std::make_shared(parseStorePath(p)); - assert(path == info->path); + auto path2 = parseStorePath(p); + assert(path == path2); + /* Hash will be set below. FIXME construct ValidPathInfo at end. */ + auto info = std::make_shared(path, Hash::dummy); PathSet references; auto deriver = readString(conn->from); @@ -111,12 +116,14 @@ struct LegacySSHStore : public Store readLongLong(conn->from); // download size info->narSize = readLongLong(conn->from); - if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) { + { auto s = readString(conn->from); - info->narHash = s.empty() ? Hash::dummy : Hash::parseAnyPrefixed(s); - info->ca = parseContentAddressOpt(readString(conn->from)); - info->sigs = readStrings(conn->from); + if (s == "") + throw Error("NAR hash is now mandatory"); + info->narHash = Hash::parseAnyPrefixed(s); } + info->ca = parseContentAddressOpt(readString(conn->from)); + info->sigs = readStrings(conn->from); auto s = readString(conn->from); assert(s == ""); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index fc0630cae..c96db68ed 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -641,25 +641,28 @@ void LocalStore::queryPathInfoUncached(const StorePath & path, Callback> callback) noexcept { try { - auto info = std::make_shared(path); - callback(retrySQLite>([&]() { auto state(_state.lock()); /* Get the path info. */ - auto useQueryPathInfo(state->stmtQueryPathInfo.use()(printStorePath(info->path))); + auto useQueryPathInfo(state->stmtQueryPathInfo.use()(printStorePath(path))); if (!useQueryPathInfo.next()) return std::shared_ptr(); - info->id = useQueryPathInfo.getInt(0); + auto id = useQueryPathInfo.getInt(0); + auto narHash = Hash::dummy; try { - info->narHash = Hash::parseAnyPrefixed(useQueryPathInfo.getStr(1)); + narHash = Hash::parseAnyPrefixed(useQueryPathInfo.getStr(1)); } catch (BadHash & e) { - throw Error("in valid-path entry for '%s': %s", printStorePath(path), e.what()); + throw Error("invalid-path entry for '%s': %s", printStorePath(path), e.what()); } + auto info = std::make_shared(path, narHash); + + info->id = id; + info->registrationTime = useQueryPathInfo.getInt(2); auto s = (const char *) sqlite3_column_text(state->stmtQueryPathInfo, 3); @@ -1152,8 +1155,7 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, const string & name, optimisePath(realPath); - ValidPathInfo info(dstPath); - info.narHash = narHash.first; + ValidPathInfo info { dstPath, narHash.first }; info.narSize = narHash.second; info.ca = FixedOutputHash { .method = method, .hash = hash }; registerValidPath(info); @@ -1196,8 +1198,7 @@ StorePath LocalStore::addTextToStore(const string & name, const string & s, optimisePath(realPath); - ValidPathInfo info(dstPath); - info.narHash = narHash; + ValidPathInfo info { dstPath, narHash }; info.narSize = sink.s->size(); info.references = references; info.ca = TextHash { .hash = hash }; diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index dd40006d7..8541cc51f 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -189,13 +189,14 @@ public: return {oInvalid, 0}; auto namePart = queryNAR.getStr(1); - auto narInfo = make_ref(StorePath(hashPart + "-" + namePart)); + auto narInfo = make_ref( + StorePath(hashPart + "-" + namePart), + Hash::parseAnyPrefixed(queryNAR.getStr(6))); narInfo->url = queryNAR.getStr(2); narInfo->compression = queryNAR.getStr(3); if (!queryNAR.isNull(4)) narInfo->fileHash = Hash::parseAnyPrefixed(queryNAR.getStr(4)); narInfo->fileSize = queryNAR.getInt(5); - 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 3a9121679..3454f34bb 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -1,10 +1,11 @@ #include "globals.hh" #include "nar-info.hh" +#include "store-api.hh" namespace nix { NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & whence) - : ValidPathInfo(StorePath(StorePath::dummy)) // FIXME: hack + : ValidPathInfo(StorePath(StorePath::dummy), Hash(Hash::dummy)) // FIXME: hack { auto corrupt = [&]() { return Error("NAR info file '%1%' is corrupt", whence); @@ -19,6 +20,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & }; bool havePath = false; + bool haveNarHash = false; size_t pos = 0; while (pos < s.size()) { @@ -46,8 +48,10 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & else if (name == "FileSize") { if (!string2Int(value, fileSize)) throw corrupt(); } - else if (name == "NarHash") + else if (name == "NarHash") { narHash = parseHashField(value); + haveNarHash = true; + } else if (name == "NarSize") { if (!string2Int(value, narSize)) throw corrupt(); } @@ -76,7 +80,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & if (compression == "") compression = "bzip2"; - if (!havePath || url.empty() || narSize == 0) throw corrupt(); + if (!havePath || !haveNarHash || url.empty() || narSize == 0) throw corrupt(); } std::string NarInfo::to_string(const Store & store) const diff --git a/src/libstore/nar-info.hh b/src/libstore/nar-info.hh index eff19f0ef..39ced76e5 100644 --- a/src/libstore/nar-info.hh +++ b/src/libstore/nar-info.hh @@ -2,10 +2,12 @@ #include "types.hh" #include "hash.hh" -#include "store-api.hh" +#include "path-info.hh" namespace nix { +class Store; + struct NarInfo : ValidPathInfo { std::string url; @@ -15,7 +17,7 @@ struct NarInfo : ValidPathInfo std::string system; NarInfo() = delete; - NarInfo(StorePath && path) : ValidPathInfo(std::move(path)) { } + NarInfo(StorePath && path, Hash narHash) : ValidPathInfo(std::move(path), narHash) { } NarInfo(const ValidPathInfo & info) : ValidPathInfo(info) { } NarInfo(const Store & store, const std::string & s, const std::string & whence); diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh index 15484717e..8ff5c466e 100644 --- a/src/libstore/path-info.hh +++ b/src/libstore/path-info.hh @@ -1,5 +1,6 @@ #pragma once +#include "crypto.hh" #include "path.hh" #include "hash.hh" #include "content-address.hh" @@ -100,8 +101,8 @@ struct ValidPathInfo ValidPathInfo(const ValidPathInfo & other) = default; - ValidPathInfo(StorePath && path) : path(std::move(path)), narHash(Hash::dummy) { }; - ValidPathInfo(const StorePath & path) : path(path), narHash(Hash::dummy) { }; + ValidPathInfo(StorePath && path, Hash narHash) : path(std::move(path)), narHash(narHash) { }; + ValidPathInfo(const StorePath & path, Hash narHash) : path(path), narHash(narHash) { }; virtual ~ValidPathInfo() { } }; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index b5e4d9fa6..8dcc1d710 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -419,10 +419,10 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, bool valid; conn->from >> valid; if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path)); } - info = std::make_shared(StorePath(path)); auto deriver = readString(conn->from); + auto narHash = Hash::parseAny(readString(conn->from), htSHA256); + info = std::make_shared(path, narHash); if (deriver != "") info->deriver = parseStorePath(deriver); - 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 997af1195..5a061b2c9 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -320,8 +320,10 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath, if (expectedCAHash && expectedCAHash != hash) throw Error("hash mismatch for '%s'", srcPath); - ValidPathInfo info(makeFixedOutputPath(method, hash, name)); - info.narHash = narHash; + ValidPathInfo info { + makeFixedOutputPath(method, hash, name), + narHash, + }; info.narSize = narSize; info.ca = FixedOutputHash { .method = method, .hash = hash }; @@ -849,19 +851,22 @@ void copyClosure(ref srcStore, ref dstStore, } -std::optional decodeValidPathInfo(const Store & store, std::istream & str, bool hashGiven) +std::optional decodeValidPathInfo(const Store & store, std::istream & str, std::optional hashGiven) { std::string path; getline(str, path); if (str.eof()) { return {}; } - ValidPathInfo info(store.parseStorePath(path)); - if (hashGiven) { + if (!hashGiven) { string s; getline(str, s); - info.narHash = Hash::parseAny(s, htSHA256); + auto narHash = Hash::parseAny(s, htSHA256); getline(str, s); - if (!string2Int(s, info.narSize)) throw Error("number expected"); + uint64_t narSize; + if (!string2Int(s, narSize)) throw Error("number expected"); + hashGiven = { narHash, narSize }; } + ValidPathInfo info(store.parseStorePath(path), hashGiven->first); + info.narSize = hashGiven->second; std::string deriver; getline(str, deriver); if (deriver != "") info.deriver = store.parseStorePath(deriver); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index e94d975c5..f6f6acaf5 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -4,7 +4,6 @@ #include "hash.hh" #include "content-address.hh" #include "serialise.hh" -#include "crypto.hh" #include "lru-cache.hh" #include "sync.hh" #include "globals.hh" @@ -761,7 +760,7 @@ string showPaths(const PathSet & paths); std::optional decodeValidPathInfo( const Store & store, std::istream & str, - bool hashGiven = false); + std::optional hashGiven = std::nullopt); /* Split URI into protocol+hierarchy part and its parameter set. */ std::pair splitUriAndParams(const std::string & uri); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index cc104bf35..e08d908e6 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -495,7 +495,10 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise) ValidPathInfos infos; while (1) { - auto info = decodeValidPathInfo(*store, cin, hashGiven); + // We use a dummy value because we'll set it below. FIXME be correct by + // construction and avoid dummy value. + auto hashResultOpt = !hashGiven ? std::optional { {Hash::dummy, -1} } : std::nullopt; + auto info = decodeValidPathInfo(*store, cin, hashResultOpt); if (!info) break; if (!store->isValidPath(info->path) || reregister) { /* !!! races */ @@ -944,11 +947,13 @@ static void opServe(Strings opFlags, Strings opArgs) if (!writeAllowed) throw Error("importing paths is not allowed"); auto path = readString(in); - ValidPathInfo info(store->parseStorePath(path)); auto deriver = readString(in); + ValidPathInfo info { + store->parseStorePath(path), + Hash::parseAny(readString(in), htSHA256), + }; if (deriver != "") info.deriver = store->parseStorePath(deriver); - 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/add-to-store.cc b/src/nix/add-to-store.cc index e183cb8b5..713155840 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -60,8 +60,10 @@ struct CmdAddToStore : MixDryRun, StoreCommand hash = hsink.finish().first; } - ValidPathInfo info(store->makeFixedOutputPath(ingestionMethod, hash, *namePart)); - info.narHash = narHash; + ValidPathInfo info { + store->makeFixedOutputPath(ingestionMethod, hash, *namePart), + narHash, + }; info.narSize = sink.s->size(); info.ca = std::optional { FixedOutputHash { .method = ingestionMethod, diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index 712043978..38b60fc38 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -77,10 +77,12 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON auto narHash = hashModuloSink.finish().first; - ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference)); + ValidPathInfo info { + store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference), + narHash, + }; info.references = std::move(references); if (hasSelfReference) info.references.insert(info.path); - info.narHash = narHash; info.narSize = sink.s->size(); info.ca = FixedOutputHash { .method = FileIngestionMethod::Recursive, diff --git a/src/nix/profile.cc b/src/nix/profile.cc index c6cd88c49..cffc9ee44 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -129,9 +129,11 @@ struct ProfileManifest auto narHash = hashString(htSHA256, *sink.s); - ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, "profile", references)); + ValidPathInfo info { + store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, "profile", references), + narHash, + }; info.references = std::move(references); - info.narHash = narHash; info.narSize = sink.s->size(); info.ca = FixedOutputHash { .method = FileIngestionMethod::Recursive, .hash = info.narHash }; From 641c95070162595c71a1f775a2364cd8533e1c4b Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Thu, 6 Aug 2020 18:13:14 -0500 Subject: [PATCH 08/14] Add hashed-mirrors back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some users have their own hashed-mirrors setup, that is used to mirror things in addition to what’s available on tarballs.nixos.org. Although this should be feasable to do with a Binary Cache, it’s not always easy, since you have to remember what "name" each of the tarballs has. Continuing to support hashed-mirrors is cheap, so it’s best to leave support in Nix. Note that NIX_HASHED_MIRRORS is also supported in Nixpkgs through fetchurl.nix. Note that this excludes tarballs.nixos.org from the default, as in \#3689. All of these are available on cache.nixos.org. --- doc/manual/command-ref/conf-file.xml | 27 +++++++++++++++++++++++++++ src/libstore/builtins/fetchurl.cc | 14 ++++++++++++++ src/libstore/globals.hh | 3 +++ 3 files changed, 44 insertions(+) diff --git a/doc/manual/command-ref/conf-file.xml b/doc/manual/command-ref/conf-file.xml index 9c55526a3..d0f1b09ca 100644 --- a/doc/manual/command-ref/conf-file.xml +++ b/doc/manual/command-ref/conf-file.xml @@ -370,6 +370,33 @@ false. + hashed-mirrors + + A list of web servers used by + builtins.fetchurl to obtain files by hash. + Given a hash type ht and a base-16 hash + h, Nix will try to download the file + from + hashed-mirror/ht/h. + This allows files to be downloaded even if they have disappeared + from their original URI. For example, given the hashed mirror + http://tarballs.example.com/, when building the + derivation + + +builtins.fetchurl { + url = "https://example.org/foo-1.2.3.tar.xz"; + sha256 = "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae"; +} + + + Nix will attempt to download this file from + http://tarballs.example.com/sha256/2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae + first. If it is not available there, if will try the original URI. + + + + http-connections The maximum number of parallel TCP connections diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 6585a480d..2048f8f87 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -58,6 +58,20 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) } }; + /* Try the hashed mirrors first. */ + if (getAttr("outputHashMode") == "flat") + for (auto hashedMirror : settings.hashedMirrors.get()) + try { + if (!hasSuffix(hashedMirror, "/")) hashedMirror += '/'; + auto ht = parseHashType(getAttr("outputHashAlgo")); + auto h = Hash(getAttr("outputHash"), ht); + fetch(hashedMirror + printHashType(h.type) + "/" + h.to_string(Base16, false)); + return; + } catch (Error & e) { + debug(e.what()); + } + + /* Otherwise try the specified URL. */ fetch(mainUrl); } diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 3406a9331..e3bb4cf84 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -335,6 +335,9 @@ public: "setuid/setgid bits or with file capabilities."}; #endif + Setting hashedMirrors{this, {}, "hashed-mirrors", + "A list of servers used by builtins.fetchurl to fetch files by hash."}; + Setting minFree{this, 0, "min-free", "Automatically run the garbage collector when free disk space drops below the specified amount."}; From 96c158d6e1c6c3bed6451a3c6447d2b0bd0337ad Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Thu, 6 Aug 2020 19:00:33 -0500 Subject: [PATCH 09/14] Fix build --- src/libstore/builtins/fetchurl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 2048f8f87..4fb5d8a06 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -63,8 +63,8 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) for (auto hashedMirror : settings.hashedMirrors.get()) try { if (!hasSuffix(hashedMirror, "/")) hashedMirror += '/'; - auto ht = parseHashType(getAttr("outputHashAlgo")); - auto h = Hash(getAttr("outputHash"), ht); + std::optional ht = parseHashTypeOpt(getAttr("outputHashAlgo")); + Hash h = newHashAllowEmpty(getAttr("outputHash"), ht); fetch(hashedMirror + printHashType(h.type) + "/" + h.to_string(Base16, false)); return; } catch (Error & e) { From 5f8ae16c8b08485e7625283ccafb99727cd47693 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Mon, 10 Aug 2020 17:44:17 +0200 Subject: [PATCH 10/14] Always reset ANSI colors in progress-bar line When having a message like `waiting for a machine to build X` and building with `nix build -L`, the log-prefix is always colored yellow[1] on a small terminal-width as everything (including the ANSI color-reset) is stripped away. To work around that problem, this patch explicitly adds an `ANSI_NORMAL` to the end of the line. [1] https://imgur.com/a/FjtJOk3 --- src/libmain/progress-bar.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 3f7d99a1d..be3c06a38 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -362,7 +362,7 @@ public: auto width = getWindowSize().second; if (width <= 0) width = std::numeric_limits::max(); - writeToStderr("\r" + filterANSIEscapes(line, false, width) + "\e[K"); + writeToStderr("\r" + filterANSIEscapes(line, false, width) + ANSI_NORMAL + "\e[K"); } std::string getStatus(State & state) From 1a281ec07f33baa0d1b10d2760edcc7019caec83 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Tue, 11 Aug 2020 10:29:43 -0600 Subject: [PATCH 11/14] demote remote build message to Info --- src/build-remote/build-remote.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 5247cefa6..cec7b369b 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -201,7 +201,7 @@ static int _main(int argc, char * * argv) % concatStringsSep(", ", m.mandatoryFeatures); } - logError({ + logErrorInfo(lvlInfo, { .name = "Remote build", .description = "Failed to find a machine for remote build!", .hint = hint From 8d4162ff9e940ea9e2f97b07f3030a722695901a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 12 Aug 2020 15:14:56 +0000 Subject: [PATCH 12/14] Separate auth and logic for the daemon Before, processConnection wanted to know a user name and user id, and `nix-daemon --stdio`, when it isn't proxying to an underlying daemon, would just assume "root" and 0. But `nix-daemon --stdio` (no proxying) shouldn't make guesses about who holds the other end of its standard streams. Now processConnection takes an "auth hook", so `nix-daemon` can provide the appropriate policy and daemon.cc doesn't need to know or care what it is. --- src/libstore/build.cc | 3 ++- src/libstore/daemon.cc | 13 ++----------- src/libstore/daemon.hh | 7 +++++-- src/nix-daemon/nix-daemon.cc | 15 +++++++++++++-- tests/remote-store.sh | 3 +++ 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 76baa1a6e..3fb052f00 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2920,7 +2920,8 @@ void DerivationGoal::startDaemon() FdSink to(remote.get()); try { daemon::processConnection(store, from, to, - daemon::NotTrusted, daemon::Recursive, "nobody", 65535); + daemon::NotTrusted, daemon::Recursive, + [&](Store & store) { store.createUser("nobody", 65535); }); debug("terminated daemon connection"); } catch (SysError &) { ignoreException(); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 5e568fc94..7a6eb99be 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -817,8 +817,7 @@ void processConnection( FdSink & to, TrustedFlag trusted, RecursiveFlag recursive, - const std::string & userName, - uid_t userId) + std::function authHook) { auto monitor = !recursive ? std::make_unique(from.fd) : nullptr; @@ -859,15 +858,7 @@ void processConnection( /* If we can't accept clientVersion, then throw an error *here* (not above). */ - -#if 0 - /* Prevent users from doing something very dangerous. */ - if (geteuid() == 0 && - querySetting("build-users-group", "") == "") - throw Error("if you run 'nix-daemon' as root, then you MUST set 'build-users-group'!"); -#endif - - store->createUser(userName, userId); + authHook(*store); tunnelLogger->stopWork(); to.flush(); diff --git a/src/libstore/daemon.hh b/src/libstore/daemon.hh index 266932013..841ace316 100644 --- a/src/libstore/daemon.hh +++ b/src/libstore/daemon.hh @@ -12,7 +12,10 @@ void processConnection( FdSink & to, TrustedFlag trusted, RecursiveFlag recursive, - const std::string & userName, - uid_t userId); + /* Arbitrary hook to check authorization / initialize user data / whatever + after the protocol has been negotiated. The idea is that this function + and everything it calls doesn't know about this stuff, and the + `nix-daemon` handles that instead. */ + std::function authHook); } diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index bcb86cbce..cfa634a44 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -239,7 +239,15 @@ static void daemonLoop(char * * argv) // Handle the connection. FdSource from(remote.get()); FdSink to(remote.get()); - processConnection(openUncachedStore(), from, to, trusted, NotRecursive, user, peer.uid); + processConnection(openUncachedStore(), from, to, trusted, NotRecursive, [&](Store & store) { +#if 0 + /* Prevent users from doing something very dangerous. */ + if (geteuid() == 0 && + querySetting("build-users-group", "") == "") + throw Error("if you run 'nix-daemon' as root, then you MUST set 'build-users-group'!"); +#endif + store.createUser(user, peer.uid); + }); exit(0); }, options); @@ -324,7 +332,10 @@ static int _main(int argc, char * * argv) } else { FdSource from(STDIN_FILENO); FdSink to(STDOUT_FILENO); - processConnection(openUncachedStore(), from, to, Trusted, NotRecursive, "root", 0); + /* Auth hook is empty because in this mode we blindly trust the + standard streams. Limitting access to thoses is explicitly + not `nix-daemon`'s responsibility. */ + processConnection(openUncachedStore(), from, to, Trusted, NotRecursive, [&](Store & _){}); } } else { daemonLoop(argv); diff --git a/tests/remote-store.sh b/tests/remote-store.sh index 4cc73465a..3a61946f9 100644 --- a/tests/remote-store.sh +++ b/tests/remote-store.sh @@ -2,6 +2,9 @@ source common.sh clearStore +# Ensure "fake ssh" remote store works just as legacy fake ssh would. +nix --store ssh-ng://localhost?remote-store=$TEST_ROOT/other-store doctor + startDaemon storeCleared=1 NIX_REMOTE_=$NIX_REMOTE $SHELL ./user-envs.sh From 4720853129b6866775edd9f90ad6f10701f98a3c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 12 Aug 2020 16:32:36 +0000 Subject: [PATCH 13/14] Make `system-features` a store setting This seems more correct. It also means one can specify the features a store should support with --store and remote-store=..., which is useful. I use this to clean up the build remotes test. --- src/build-remote/build-remote.cc | 16 ++++----------- src/libstore/build.cc | 8 ++++---- src/libstore/machines.cc | 24 ++++++++++++++++++++++ src/libstore/machines.hh | 4 ++++ src/libstore/parsed-derivations.cc | 8 ++++---- src/libstore/parsed-derivations.hh | 4 ++-- src/libstore/store-api.hh | 4 ++++ tests/build-hook.nix | 3 ++- tests/build-remote.sh | 33 +++++++++++++++++------------- tests/local.mk | 2 +- 10 files changed, 68 insertions(+), 38 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index cec7b369b..ce5127113 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -38,9 +38,9 @@ static AutoCloseFD openSlotLock(const Machine & m, uint64_t slot) return openLockFile(fmt("%s/%s-%d", currentLoad, escapeUri(m.storeUri), slot), true); } -static bool allSupportedLocally(const std::set& requiredFeatures) { +static bool allSupportedLocally(Store & store, const std::set& requiredFeatures) { for (auto & feature : requiredFeatures) - if (!settings.systemFeatures.get().count(feature)) return false; + if (!store.systemFeatures.get().count(feature)) return false; return true; } @@ -106,7 +106,7 @@ static int _main(int argc, char * * argv) auto canBuildLocally = amWilling && ( neededSystem == settings.thisSystem || settings.extraPlatforms.get().count(neededSystem) > 0) - && allSupportedLocally(requiredFeatures); + && allSupportedLocally(*store, requiredFeatures); /* Error ignored here, will be caught later */ mkdir(currentLoad.c_str(), 0777); @@ -224,15 +224,7 @@ static int _main(int argc, char * * argv) Activity act(*logger, lvlTalkative, actUnknown, fmt("connecting to '%s'", bestMachine->storeUri)); - Store::Params storeParams; - if (hasPrefix(bestMachine->storeUri, "ssh://")) { - storeParams["max-connections"] = "1"; - storeParams["log-fd"] = "4"; - if (bestMachine->sshKey != "") - storeParams["ssh-key"] = bestMachine->sshKey; - } - - sshStore = openStore(bestMachine->storeUri, storeParams); + sshStore = bestMachine->openStore(); sshStore->connect(); storeUri = bestMachine->storeUri; diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 76baa1a6e..d6e6ad6a9 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1475,7 +1475,7 @@ void DerivationGoal::tryToBuild() /* Don't do a remote build if the derivation has the attribute `preferLocalBuild' set. Also, check and repair modes are only supported for local builds. */ - bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally(); + bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally(worker.store); /* Is the build hook willing to accept this job? */ if (!buildLocally) { @@ -1964,13 +1964,13 @@ void linkOrCopy(const Path & from, const Path & to) void DerivationGoal::startBuilder() { /* Right platform? */ - if (!parsedDrv->canBuildLocally()) + if (!parsedDrv->canBuildLocally(worker.store)) throw Error("a '%s' with features {%s} is required to build '%s', but I am a '%s' with features {%s}", drv->platform, concatStringsSep(", ", parsedDrv->getRequiredSystemFeatures()), worker.store.printStorePath(drvPath), settings.thisSystem, - concatStringsSep(", ", settings.systemFeatures)); + concatStringsSep(", ", worker.store.systemFeatures)); if (drv->isBuiltin()) preloadNSS(); @@ -3179,7 +3179,7 @@ void DerivationGoal::runChild() createDirs(chrootRootDir + "/dev/shm"); createDirs(chrootRootDir + "/dev/pts"); ss.push_back("/dev/full"); - if (settings.systemFeatures.get().count("kvm") && pathExists("/dev/kvm")) + if (worker.store.systemFeatures.get().count("kvm") && pathExists("/dev/kvm")) ss.push_back("/dev/kvm"); ss.push_back("/dev/null"); ss.push_back("/dev/random"); diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc index f848582da..7db2556f4 100644 --- a/src/libstore/machines.cc +++ b/src/libstore/machines.cc @@ -1,6 +1,7 @@ #include "machines.hh" #include "util.hh" #include "globals.hh" +#include "store-api.hh" #include @@ -48,6 +49,29 @@ bool Machine::mandatoryMet(const std::set & features) const { }); } +ref Machine::openStore() const { + Store::Params storeParams; + if (hasPrefix(storeUri, "ssh://")) { + storeParams["max-connections"] = "1"; + storeParams["log-fd"] = "4"; + if (sshKey != "") + storeParams["ssh-key"] = sshKey; + } + { + auto & fs = storeParams["system-features"]; + auto append = [&](auto feats) { + for (auto & f : feats) { + if (fs.size() > 0) fs += ' '; + fs += f; + } + }; + append(supportedFeatures); + append(mandatoryFeatures); + } + + return nix::openStore(storeUri, storeParams); +} + void parseMachines(const std::string & s, Machines & machines) { for (auto line : tokenizeString>(s, "\n;")) { diff --git a/src/libstore/machines.hh b/src/libstore/machines.hh index de92eb924..341d9bd97 100644 --- a/src/libstore/machines.hh +++ b/src/libstore/machines.hh @@ -4,6 +4,8 @@ namespace nix { +class Store; + struct Machine { const string storeUri; @@ -28,6 +30,8 @@ struct Machine { decltype(supportedFeatures) supportedFeatures, decltype(mandatoryFeatures) mandatoryFeatures, decltype(sshPublicHostKey) sshPublicHostKey); + + ref openStore() const; }; typedef std::vector Machines; diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index 24f848e46..e7b7202d4 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -94,7 +94,7 @@ StringSet ParsedDerivation::getRequiredSystemFeatures() const return res; } -bool ParsedDerivation::canBuildLocally() const +bool ParsedDerivation::canBuildLocally(Store & localStore) const { if (drv.platform != settings.thisSystem.get() && !settings.extraPlatforms.get().count(drv.platform) @@ -102,14 +102,14 @@ bool ParsedDerivation::canBuildLocally() const return false; for (auto & feature : getRequiredSystemFeatures()) - if (!settings.systemFeatures.get().count(feature)) return false; + if (!localStore.systemFeatures.get().count(feature)) return false; return true; } -bool ParsedDerivation::willBuildLocally() const +bool ParsedDerivation::willBuildLocally(Store & localStore) const { - return getBoolAttr("preferLocalBuild") && canBuildLocally(); + return getBoolAttr("preferLocalBuild") && canBuildLocally(localStore); } bool ParsedDerivation::substitutesAllowed() const diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh index 6ee172d81..3fa09f34f 100644 --- a/src/libstore/parsed-derivations.hh +++ b/src/libstore/parsed-derivations.hh @@ -29,9 +29,9 @@ public: StringSet getRequiredSystemFeatures() const; - bool canBuildLocally() const; + bool canBuildLocally(Store & localStore) const; - bool willBuildLocally() const; + bool willBuildLocally(Store & localStore) const; bool substitutesAllowed() const; }; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index e94d975c5..1e940e6a8 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -164,6 +164,10 @@ public: Setting wantMassQuery{this, false, "want-mass-query", "whether this substituter can be queried efficiently for path validity"}; + Setting systemFeatures{this, settings.systemFeatures, + "system-features", + "Optional features that the system this store builds on implements (like \"kvm\")."}; + protected: struct PathInfoCacheValue { diff --git a/tests/build-hook.nix b/tests/build-hook.nix index a19c10dde..1bd0b759f 100644 --- a/tests/build-hook.nix +++ b/tests/build-hook.nix @@ -23,6 +23,7 @@ let shell = busybox; name = "build-remote-input-2"; buildCommand = "echo BAR > $out"; + requiredSystemFeatures = ["bar"]; }; in @@ -34,6 +35,6 @@ in '' read x < ${input1} read y < ${input2} - echo $x$y > $out + echo "$x $y" > $out ''; } diff --git a/tests/build-remote.sh b/tests/build-remote.sh index 4dfb753e1..7638f536f 100644 --- a/tests/build-remote.sh +++ b/tests/build-remote.sh @@ -1,31 +1,36 @@ source common.sh -clearStore - if ! canUseSandbox; then exit; fi if ! [[ $busybox =~ busybox ]]; then exit; fi -chmod -R u+w $TEST_ROOT/machine0 || true -chmod -R u+w $TEST_ROOT/machine1 || true -chmod -R u+w $TEST_ROOT/machine2 || true -rm -rf $TEST_ROOT/machine0 $TEST_ROOT/machine1 $TEST_ROOT/machine2 -rm -f $TEST_ROOT/result - unset NIX_STORE_DIR unset NIX_STATE_DIR +function join_by { local d=$1; shift; echo -n "$1"; shift; printf "%s" "${@/#/$d}"; } + +builders=( + # system-features will automatically be added to the outer URL, but not inner + # remote-store URL. + "ssh://localhost?remote-store=$TEST_ROOT/machine1?system-features=foo - - 1 1 foo" + "$TEST_ROOT/machine2 - - 1 1 bar" +) + # Note: ssh://localhost bypasses ssh, directly invoking nix-store as a # child process. This allows us to test LegacySSHStore::buildDerivation(). +# ssh-ng://... likewise allows us to test RemoteStore::buildDerivation(). nix build -L -v -f build-hook.nix -o $TEST_ROOT/result --max-jobs 0 \ --arg busybox $busybox \ --store $TEST_ROOT/machine0 \ - --builders "ssh://localhost?remote-store=$TEST_ROOT/machine1; $TEST_ROOT/machine2 - - 1 1 foo" \ - --system-features foo + --builders "$(join_by '; ' "${builders[@]}")" outPath=$(readlink -f $TEST_ROOT/result) -cat $TEST_ROOT/machine0/$outPath | grep FOOBAR +grep 'FOO BAR' $TEST_ROOT/machine0/$outPath -# Ensure that input1 was built on store2 due to the required feature. -(! nix path-info --store $TEST_ROOT/machine1 --all | grep builder-build-remote-input-1.sh) -nix path-info --store $TEST_ROOT/machine2 --all | grep builder-build-remote-input-1.sh +# Ensure that input1 was built on store1 due to the required feature. +(! nix path-info --store $TEST_ROOT/machine2 --all | grep builder-build-remote-input-1.sh) +nix path-info --store $TEST_ROOT/machine1 --all | grep builder-build-remote-input-1.sh + +# Ensure that input2 was built on store2 due to the required feature. +(! nix path-info --store $TEST_ROOT/machine1 --all | grep builder-build-remote-input-2.sh) +nix path-info --store $TEST_ROOT/machine2 --all | grep builder-build-remote-input-2.sh diff --git a/tests/local.mk b/tests/local.mk index 0f3bfe606..5c77b9bb7 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -1,5 +1,5 @@ nix_tests = \ - init.sh hash.sh lang.sh add.sh simple.sh dependencies.sh \ + hash.sh lang.sh add.sh simple.sh dependencies.sh \ config.sh \ gc.sh \ gc-concurrent.sh \ From 85aacbee64680e60b22e8a729756b0a3882f8b8f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 13 Aug 2020 14:47:53 +0000 Subject: [PATCH 14/14] Use `TeeSink` and `TeeSouce` in a few more places --- src/libstore/binary-cache-store.cc | 12 ++++-------- src/libstore/local-store.cc | 6 +----- src/libstore/store-api.cc | 6 +++--- src/libutil/archive.cc | 6 +----- src/libutil/serialise.hh | 11 +++++++++++ 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 86877dd1a..c360b9dda 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -312,14 +312,10 @@ void BinaryCacheStore::narFromPath(const StorePath & storePath, Sink & sink) { auto info = queryPathInfo(storePath).cast(); - uint64_t narSize = 0; + LengthSink narSize; + TeeSink tee { sink, narSize }; - LambdaSink wrapperSink([&](const unsigned char * data, size_t len) { - sink(data, len); - narSize += len; - }); - - auto decompressor = makeDecompressionSink(info->compression, wrapperSink); + auto decompressor = makeDecompressionSink(info->compression, tee); try { getFile(info->url, *decompressor); @@ -331,7 +327,7 @@ void BinaryCacheStore::narFromPath(const StorePath & storePath, Sink & sink) stats.narRead++; //stats.narReadCompressedBytes += nar->size(); // FIXME - stats.narReadBytes += narSize; + stats.narReadBytes += narSize.length; } void BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath, diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 3c66a4dfd..e552bdc59 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1021,11 +1021,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, else hashSink = std::make_unique(htSHA256, std::string(info.path.hashPart())); - LambdaSource wrapperSource([&](unsigned char * data, size_t len) -> size_t { - size_t n = source.read(data, len); - (*hashSink)(data, n); - return n; - }); + TeeSource wrapperSource { source, *hashSink }; restorePath(realPath, wrapperSource); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 2837910b9..3d07e2d38 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -746,12 +746,12 @@ void copyStorePath(ref srcStore, ref dstStore, } auto source = sinkToSource([&](Sink & sink) { - LambdaSink wrapperSink([&](const unsigned char * data, size_t len) { - sink(data, len); + LambdaSink progressSink([&](const unsigned char * data, size_t len) { total += len; act.progress(total, info->narSize); }); - srcStore->narFromPath(storePath, wrapperSink); + TeeSink tee { sink, progressSink }; + srcStore->narFromPath(storePath, tee); }, [&]() { throw EndOfFile("NAR for '%s' fetched from '%s' is incomplete", srcStore->printStorePath(storePath), srcStore->getUri()); }); diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index ce7cf9754..14399dea3 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -366,11 +366,7 @@ void copyNAR(Source & source, Sink & sink) ParseSink parseSink; /* null sink; just parse the NAR */ - LambdaSource wrapper([&](unsigned char * data, size_t len) { - auto n = source.read(data, len); - sink(data, n); - return n; - }); + TeeSource wrapper { source, sink }; parseDump(parseSink, wrapper); } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index c29c6b29b..69ae0874a 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -225,6 +225,17 @@ struct SizedSource : Source } }; +/* A sink that that just counts the number of bytes given to it */ +struct LengthSink : Sink +{ + uint64_t length = 0; + + virtual void operator () (const unsigned char * _, size_t len) + { + length += len; + } +}; + /* Convert a function into a sink. */ struct LambdaSink : Sink {