From fcca702a96a8ca0e73f6d035052c30121776aeba Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 28 Jun 2017 18:11:01 +0200 Subject: [PATCH] Replace a few bool flags with enums Functions like copyClosure() had 3 bool arguments, which creates a severe risk of mixing up arguments. Also, implement copyClosure() using copyPaths(). --- src/build-remote/build-remote.cc | 4 +- src/libexpr/eval.cc | 1 + src/libexpr/eval.hh | 3 +- src/libstore/binary-cache-store.cc | 10 +-- src/libstore/binary-cache-store.hh | 6 +- src/libstore/build.cc | 18 ++-- src/libstore/derivations.cc | 2 +- src/libstore/derivations.hh | 3 +- src/libstore/download.cc | 4 +- src/libstore/export-import.cc | 4 +- src/libstore/legacy-ssh-store.cc | 9 +- src/libstore/local-store.cc | 16 ++-- src/libstore/local-store.hh | 15 ++-- src/libstore/remote-store.cc | 12 +-- src/libstore/remote-store.hh | 11 +-- src/libstore/store-api.cc | 103 +++++++++-------------- src/libstore/store-api.hh | 38 ++++++--- src/nix-copy-closure/nix-copy-closure.cc | 6 +- src/nix-daemon/nix-daemon.cc | 10 ++- src/nix-env/nix-env.cc | 4 +- src/nix-instantiate/nix-instantiate.cc | 4 +- src/nix-store/nix-store.cc | 8 +- 22 files changed, 144 insertions(+), 147 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 7ffbdca7c..8719959f0 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -201,7 +201,7 @@ connected: printError("somebody is hogging the upload lock for ‘%s’, continuing..."); alarm(0); signal(SIGALRM, old); - copyPaths(store, ref(sshStore), inputs, false, true); + copyPaths(store, ref(sshStore), inputs, NoRepair, NoCheckSigs); uploadLock = -1; BasicDerivation drv(readDerivation(drvPath)); @@ -219,7 +219,7 @@ connected: if (!missing.empty()) { setenv("NIX_HELD_LOCKS", concatStringsSep(" ", missing).c_str(), 1); /* FIXME: ugly */ - copyPaths(ref(sshStore), store, missing, false, true); + copyPaths(ref(sshStore), store, missing, NoRepair, NoCheckSigs); } return; diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 0cdce602d..ca4c9a373 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -293,6 +293,7 @@ EvalState::EvalState(const Strings & _searchPath, ref store) , sWrong(symbols.create("wrong")) , sStructuredAttrs(symbols.create("__structuredAttrs")) , sBuilder(symbols.create("builder")) + , repair(NoRepair) , store(store) , baseEnv(allocEnv(128)) , staticBaseEnv(false, 0) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 46d5a1cc8..1e32db1e8 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -14,6 +14,7 @@ namespace nix { class Store; class EvalState; +enum RepairFlag : bool; typedef void (* PrimOpFun) (EvalState & state, const Pos & pos, Value * * args, Value & v); @@ -73,7 +74,7 @@ public: /* If set, force copying files to the Nix store even if they already exist there. */ - bool repair = false; + RepairFlag repair; /* If set, don't allow access to files outside of the Nix search path or to environment variables. */ diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 46c5aa21b..8ce5f5bbc 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -134,7 +134,7 @@ Path BinaryCacheStore::narInfoFileFor(const Path & storePath) } void BinaryCacheStore::addToStore(const ValidPathInfo & info, const ref & nar, - bool repair, bool dontCheckSigs, std::shared_ptr accessor) + RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) { if (!repair && isValidPath(info.path)) return; @@ -328,7 +328,7 @@ void BinaryCacheStore::queryPathInfoUncached(const Path & storePath, } Path BinaryCacheStore::addToStore(const string & name, const Path & srcPath, - bool recursive, HashType hashAlgo, PathFilter & filter, bool repair) + bool recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) { // FIXME: some cut&paste from LocalStore::addToStore(). @@ -349,13 +349,13 @@ Path BinaryCacheStore::addToStore(const string & name, const Path & srcPath, ValidPathInfo info; info.path = makeFixedOutputPath(recursive, h, name); - addToStore(info, sink.s, repair, false, 0); + addToStore(info, sink.s, repair, CheckSigs, nullptr); return info.path; } Path BinaryCacheStore::addTextToStore(const string & name, const string & s, - const PathSet & references, bool repair) + const PathSet & references, RepairFlag repair) { ValidPathInfo info; info.path = computeStorePathForText(name, s, references); @@ -364,7 +364,7 @@ Path BinaryCacheStore::addTextToStore(const string & name, const string & s, if (repair || !isValidPath(info.path)) { StringSink sink; dumpString(s, sink); - addToStore(info, sink.s, repair, false, 0); + addToStore(info, sink.s, repair, CheckSigs, nullptr); } return info.path; diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 87d4aa438..bf5a56ab4 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -85,15 +85,15 @@ public: bool wantMassQuery() override { return wantMassQuery_; } void addToStore(const ValidPathInfo & info, const ref & nar, - bool repair, bool dontCheckSigs, + RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) override; Path addToStore(const string & name, const Path & srcPath, bool recursive, HashType hashAlgo, - PathFilter & filter, bool repair) override; + PathFilter & filter, RepairFlag repair) override; Path addTextToStore(const string & name, const string & s, - const PathSet & references, bool repair) override; + const PathSet & references, RepairFlag repair) override; void narFromPath(const Path & path, Sink & sink) override; diff --git a/src/libstore/build.cc b/src/libstore/build.cc index c34083d2e..6c740d99c 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -262,7 +262,7 @@ public: GoalPtr makeDerivationGoal(const Path & drvPath, const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); std::shared_ptr makeBasicDerivationGoal(const Path & drvPath, const BasicDerivation & drv, BuildMode buildMode = bmNormal); - GoalPtr makeSubstitutionGoal(const Path & storePath, bool repair = false); + GoalPtr makeSubstitutionGoal(const Path & storePath, RepairFlag repair = NoRepair); /* Remove a dead goal. */ void removeGoal(GoalPtr goal); @@ -1087,7 +1087,7 @@ void DerivationGoal::haveDerivation() them. */ if (settings.useSubstitutes && drv->substitutesAllowed()) for (auto & i : invalidOutputs) - addWaitee(worker.makeSubstitutionGoal(i, buildMode == bmRepair)); + addWaitee(worker.makeSubstitutionGoal(i, buildMode == bmRepair ? Repair : NoRepair)); if (waitees.empty()) /* to prevent hang (no wake-up event) */ outputsSubstituted(); @@ -1195,7 +1195,7 @@ void DerivationGoal::repairClosure() printError(format("found corrupted or missing path ‘%1%’ in the output closure of ‘%2%’") % i % drvPath); Path drvPath2 = outputsToDrv[i]; if (drvPath2 == "") - addWaitee(worker.makeSubstitutionGoal(i, true)); + addWaitee(worker.makeSubstitutionGoal(i, Repair)); else addWaitee(worker.makeDerivationGoal(drvPath2, PathSet(), bmRepair)); } @@ -3291,7 +3291,7 @@ private: std::promise promise; /* Whether to try to repair a valid path. */ - bool repair; + RepairFlag repair; /* Location where we're downloading the substitute. Differs from storePath when doing a repair. */ @@ -3301,7 +3301,7 @@ private: GoalState state; public: - SubstitutionGoal(const Path & storePath, Worker & worker, bool repair = false); + SubstitutionGoal(const Path & storePath, Worker & worker, RepairFlag repair = NoRepair); ~SubstitutionGoal(); void timedOut() override { abort(); }; @@ -3337,7 +3337,7 @@ public: }; -SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker, bool repair) +SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker, RepairFlag repair) : Goal(worker) , hasSubstitute(false) , repair(repair) @@ -3600,7 +3600,7 @@ std::shared_ptr Worker::makeBasicDerivationGoal(const Path & drv } -GoalPtr Worker::makeSubstitutionGoal(const Path & path, bool repair) +GoalPtr Worker::makeSubstitutionGoal(const Path & path, RepairFlag repair) { GoalPtr goal = substitutionGoals[path].lock(); if (!goal) { @@ -3953,7 +3953,7 @@ void LocalStore::buildPaths(const PathSet & drvPaths, BuildMode buildMode) if (isDerivation(i2.first)) goals.insert(worker.makeDerivationGoal(i2.first, i2.second, buildMode)); else - goals.insert(worker.makeSubstitutionGoal(i, buildMode)); + goals.insert(worker.makeSubstitutionGoal(i, buildMode == bmRepair ? Repair : NoRepair)); } worker.run(goals); @@ -4011,7 +4011,7 @@ void LocalStore::ensurePath(const Path & path) void LocalStore::repairPath(const Path & path) { Worker worker(*this); - GoalPtr goal = worker.makeSubstitutionGoal(path, true); + GoalPtr goal = worker.makeSubstitutionGoal(path, Repair); Goals goals = {goal}; worker.run(goals); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 0c6ceb9f6..bb7b8fe62 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -71,7 +71,7 @@ bool BasicDerivation::canBuildLocally() const Path writeDerivation(ref store, - const Derivation & drv, const string & name, bool repair) + const Derivation & drv, const string & name, RepairFlag repair) { PathSet references; references.insert(drv.inputSrcs.begin(), drv.inputSrcs.end()); diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 9717a81e4..7b97730d3 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -2,6 +2,7 @@ #include "types.hh" #include "hash.hh" +#include "store-api.hh" #include @@ -85,7 +86,7 @@ class Store; /* Write a derivation to the Nix store, and return its path. */ Path writeDerivation(ref store, - const Derivation & drv, const string & name, bool repair = false); + const Derivation & drv, const string & name, RepairFlag repair = NoRepair); /* Read a derivation from a file. */ Derivation readDerivation(const Path & drvPath); diff --git a/src/libstore/download.cc b/src/libstore/download.cc index 33ab1f027..4f3bf2d14 100644 --- a/src/libstore/download.cc +++ b/src/libstore/download.cc @@ -631,7 +631,7 @@ Path Downloader::downloadCached(ref store, const string & url_, bool unpa info.narHash = hashString(htSHA256, *sink.s); info.narSize = sink.s->size(); info.ca = makeFixedOutputCA(false, hash); - store->addToStore(info, sink.s, false, true); + store->addToStore(info, sink.s, NoRepair, NoCheckSigs); storePath = info.path; } @@ -660,7 +660,7 @@ Path Downloader::downloadCached(ref store, const string & url_, bool unpa AutoDelete autoDelete(tmpDir, true); // FIXME: this requires GNU tar for decompression. runProgram("tar", true, {"xf", storePath, "-C", tmpDir, "--strip-components", "1"}); - unpackedStorePath = store->addToStore(name, tmpDir, true, htSHA256, defaultPathFilter, false); + unpackedStorePath = store->addToStore(name, tmpDir, true, htSHA256, defaultPathFilter, NoRepair); } replaceSymlink(unpackedStorePath, unpackedLink); storePath = unpackedStorePath; diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc index 6e8bc692c..1b3a43df3 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -61,7 +61,7 @@ void Store::exportPath(const Path & path, Sink & sink) hashAndWriteSink << exportMagic << path << info->references << info->deriver << 0; } -Paths Store::importPaths(Source & source, std::shared_ptr accessor, bool dontCheckSigs) +Paths Store::importPaths(Source & source, std::shared_ptr accessor, CheckSigsFlag checkSigs) { Paths res; while (true) { @@ -95,7 +95,7 @@ Paths Store::importPaths(Source & source, std::shared_ptr accessor, if (readInt(source) == 1) readString(source); - addToStore(info, tee.source.data, false, dontCheckSigs, accessor); + addToStore(info, tee.source.data, NoRepair, checkSigs, accessor); res.push_back(info.path); } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index e09932e3d..a84f85c1b 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -113,7 +113,7 @@ struct LegacySSHStore : public Store } void addToStore(const ValidPathInfo & info, const ref & nar, - bool repair, bool dontCheckSigs, + RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) override { debug("adding path ‘%s’ to remote host ‘%s’", info.path, host); @@ -168,11 +168,11 @@ struct LegacySSHStore : public Store Path addToStore(const string & name, const Path & srcPath, bool recursive, HashType hashAlgo, - PathFilter & filter, bool repair) override + PathFilter & filter, RepairFlag repair) override { unsupported(); } Path addTextToStore(const string & name, const string & s, - const PathSet & references, bool repair) override + const PathSet & references, RepairFlag repair) override { unsupported(); } BuildResult buildDerivation(const Path & drvPath, const BasicDerivation & drv, @@ -249,7 +249,8 @@ struct LegacySSHStore : public Store out.insert(res.begin(), res.end()); } - PathSet queryValidPaths(const PathSet & paths, bool maybeSubstitute = false) override + PathSet queryValidPaths(const PathSet & paths, + SubstituteFlag maybeSubstitute = NoSubstitute) override { auto conn(connections->get()); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index c76294dcc..a7a94a8b9 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -718,7 +718,7 @@ bool LocalStore::isValidPathUncached(const Path & path) } -PathSet LocalStore::queryValidPaths(const PathSet & paths, bool maybeSubstitute) +PathSet LocalStore::queryValidPaths(const PathSet & paths, SubstituteFlag maybeSubstitute) { PathSet res; for (auto & i : paths) @@ -961,7 +961,7 @@ void LocalStore::invalidatePath(State & state, const Path & path) void LocalStore::addToStore(const ValidPathInfo & info, const ref & nar, - bool repair, bool dontCheckSigs, std::shared_ptr accessor) + RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) { assert(info.narHash); @@ -974,7 +974,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, const ref & throw Error("size mismatch importing path ‘%s’; expected %s, got %s", info.path, info.narSize, nar->size()); - if (requireSigs && !dontCheckSigs && !info.checkSignatures(*this, publicKeys)) + if (requireSigs && checkSigs && !info.checkSignatures(*this, publicKeys)) throw Error("cannot add path ‘%s’ because it lacks a valid signature", info.path); addTempRoot(info.path); @@ -1012,7 +1012,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, const ref & Path LocalStore::addToStoreFromDump(const string & dump, const string & name, - bool recursive, HashType hashAlgo, bool repair) + bool recursive, HashType hashAlgo, RepairFlag repair) { Hash h = hashString(hashAlgo, dump); @@ -1070,7 +1070,7 @@ Path LocalStore::addToStoreFromDump(const string & dump, const string & name, Path LocalStore::addToStore(const string & name, const Path & _srcPath, - bool recursive, HashType hashAlgo, PathFilter & filter, bool repair) + bool recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) { Path srcPath(absPath(_srcPath)); @@ -1088,7 +1088,7 @@ Path LocalStore::addToStore(const string & name, const Path & _srcPath, Path LocalStore::addTextToStore(const string & name, const string & s, - const PathSet & references, bool repair) + const PathSet & references, RepairFlag repair) { auto hash = hashString(htSHA256, s); auto dstPath = makeTextPath(name, hash, references); @@ -1170,7 +1170,7 @@ void LocalStore::invalidatePathChecked(const Path & path) } -bool LocalStore::verifyStore(bool checkContents, bool repair) +bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) { printError(format("reading the Nix store...")); @@ -1255,7 +1255,7 @@ bool LocalStore::verifyStore(bool checkContents, bool repair) void LocalStore::verifyPath(const Path & path, const PathSet & store, - PathSet & done, PathSet & validPaths, bool repair, bool & errors) + PathSet & done, PathSet & validPaths, RepairFlag repair, bool & errors) { checkInterrupt(); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index f2c40e964..551c6b506 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -98,7 +98,8 @@ public: bool isValidPathUncached(const Path & path) override; - PathSet queryValidPaths(const PathSet & paths, bool maybeSubstitute = false) override; + PathSet queryValidPaths(const PathSet & paths, + SubstituteFlag maybeSubstitute = NoSubstitute) override; PathSet queryAllValidPaths() override; @@ -122,22 +123,22 @@ public: SubstitutablePathInfos & infos) override; void addToStore(const ValidPathInfo & info, const ref & nar, - bool repair, bool dontCheckSigs, + RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) override; Path addToStore(const string & name, const Path & srcPath, bool recursive, HashType hashAlgo, - PathFilter & filter, bool repair) override; + PathFilter & filter, RepairFlag repair) override; /* Like addToStore(), but the contents of the path are contained in `dump', which is either a NAR serialisation (if recursive == true) or simply the contents of a regular file (if recursive == false). */ Path addToStoreFromDump(const string & dump, const string & name, - bool recursive = true, HashType hashAlgo = htSHA256, bool repair = false); + bool recursive = true, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair); Path addTextToStore(const string & name, const string & s, - const PathSet & references, bool repair) override; + const PathSet & references, RepairFlag repair) override; void buildPaths(const PathSet & paths, BuildMode buildMode) override; @@ -174,7 +175,7 @@ public: /* Optimise a single store path. */ void optimisePath(const Path & path); - bool verifyStore(bool checkContents, bool repair) override; + bool verifyStore(bool checkContents, RepairFlag repair) override; /* Register the validity of a path, i.e., that `path' exists, that the paths referenced by it exists, and in the case of an output @@ -212,7 +213,7 @@ private: void invalidatePathChecked(const Path & path); void verifyPath(const Path & path, const PathSet & store, - PathSet & done, PathSet & validPaths, bool repair, bool & errors); + PathSet & done, PathSet & validPaths, RepairFlag repair, bool & errors); void updatePathInfo(State & state, const ValidPathInfo & info); diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index be8819bbc..7337e406d 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -185,7 +185,7 @@ bool RemoteStore::isValidPathUncached(const Path & path) } -PathSet RemoteStore::queryValidPaths(const PathSet & paths, bool maybeSubstitute) +PathSet RemoteStore::queryValidPaths(const PathSet & paths, SubstituteFlag maybeSubstitute) { auto conn(connections->get()); if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) { @@ -357,7 +357,7 @@ Path RemoteStore::queryPathFromHashPart(const string & hashPart) void RemoteStore::addToStore(const ValidPathInfo & info, const ref & nar, - bool repair, bool dontCheckSigs, std::shared_ptr accessor) + RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) { auto conn(connections->get()); @@ -390,7 +390,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, const ref << info.path << info.deriver << printHash(info.narHash) << info.references << info.registrationTime << info.narSize << info.ultimate << info.sigs << info.ca - << repair << dontCheckSigs; + << repair << !checkSigs; conn->to(*nar); conn->processStderr(); } @@ -398,7 +398,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, const ref Path RemoteStore::addToStore(const string & name, const Path & _srcPath, - bool recursive, HashType hashAlgo, PathFilter & filter, bool repair) + bool recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) { if (repair) throw Error("repairing is not supported when building through the Nix daemon"); @@ -434,7 +434,7 @@ Path RemoteStore::addToStore(const string & name, const Path & _srcPath, Path RemoteStore::addTextToStore(const string & name, const string & s, - const PathSet & references, bool repair) + const PathSet & references, RepairFlag repair) { if (repair) throw Error("repairing is not supported when building through the Nix daemon"); @@ -570,7 +570,7 @@ void RemoteStore::optimiseStore() } -bool RemoteStore::verifyStore(bool checkContents, bool repair) +bool RemoteStore::verifyStore(bool checkContents, RepairFlag repair) { auto conn(connections->get()); conn->to << wopVerifyStore << checkContents << repair; diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index ed430e4ca..e370e4797 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -31,7 +31,8 @@ public: bool isValidPathUncached(const Path & path) override; - PathSet queryValidPaths(const PathSet & paths, bool maybeSubstitute = false) override; + PathSet queryValidPaths(const PathSet & paths, + SubstituteFlag maybeSubstitute = NoSubstitute) override; PathSet queryAllValidPaths() override; @@ -55,15 +56,15 @@ public: SubstitutablePathInfos & infos) override; void addToStore(const ValidPathInfo & info, const ref & nar, - bool repair, bool dontCheckSigs, + RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) override; Path addToStore(const string & name, const Path & srcPath, bool recursive = true, HashType hashAlgo = htSHA256, - PathFilter & filter = defaultPathFilter, bool repair = false) override; + PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) override; Path addTextToStore(const string & name, const string & s, - const PathSet & references, bool repair = false) override; + const PathSet & references, RepairFlag repair) override; void buildPaths(const PathSet & paths, BuildMode buildMode) override; @@ -84,7 +85,7 @@ public: void optimiseStore() override; - bool verifyStore(bool checkContents, bool repair) override; + bool verifyStore(bool checkContents, RepairFlag repair) override; void addSignatures(const Path & storePath, const StringSet & sigs) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 76ed99422..39b946616 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -378,7 +378,7 @@ void Store::queryPathInfo(const Path & storePath, } -PathSet Store::queryValidPaths(const PathSet & paths, bool maybeSubstitute) +PathSet Store::queryValidPaths(const PathSet & paths, SubstituteFlag maybeSubstitute) { struct State { @@ -537,14 +537,14 @@ void Store::buildPaths(const PathSet & paths, BuildMode buildMode) void copyStorePath(ref srcStore, ref dstStore, - const Path & storePath, bool repair, bool dontCheckSigs) + const Path & storePath, RepairFlag repair, CheckSigsFlag checkSigs) { auto info = srcStore->queryPathInfo(storePath); StringSink sink; srcStore->narFromPath({storePath}, sink); - if (!info->narHash && dontCheckSigs) { + if (!info->narHash && !checkSigs) { auto info2 = make_ref(*info); info2->narHash = hashString(htSHA256, *sink.s); if (!info->narSize) info2->narSize = sink.s->size(); @@ -561,33 +561,47 @@ void copyStorePath(ref srcStore, ref dstStore, assert(info->narHash); - dstStore->addToStore(*info, sink.s, repair, dontCheckSigs); + dstStore->addToStore(*info, sink.s, repair, checkSigs); +} + + +void copyPaths(ref srcStore, ref dstStore, const PathSet & storePaths, + RepairFlag repair, CheckSigsFlag checkSigs, SubstituteFlag substitute) +{ + PathSet valid = dstStore->queryValidPaths(storePaths, substitute); + + PathSet missing; + for (auto & path : storePaths) + if (!valid.count(path)) missing.insert(path); + + ThreadPool pool; + + processGraph(pool, + PathSet(missing.begin(), missing.end()), + + [&](const Path & storePath) { + if (dstStore->isValidPath(storePath)) return PathSet(); + return srcStore->queryPathInfo(storePath)->references; + }, + + [&](const Path & storePath) { + checkInterrupt(); + + if (!dstStore->isValidPath(storePath)) { + printError("copying ‘%s’...", storePath); + copyStorePath(srcStore, dstStore, storePath, repair, checkSigs); + } + }); } void copyClosure(ref srcStore, ref dstStore, - const PathSet & storePaths, bool repair, bool dontCheckSigs) + const PathSet & storePaths, RepairFlag repair, CheckSigsFlag checkSigs, + SubstituteFlag substitute) { PathSet closure; - for (auto & path : storePaths) - srcStore->computeFSClosure(path, closure); - - // FIXME: use copyStorePaths() - - PathSet valid = dstStore->queryValidPaths(closure); - - if (valid.size() == closure.size()) return; - - Paths sorted = srcStore->topoSortPaths(closure); - - Paths missing; - for (auto i = sorted.rbegin(); i != sorted.rend(); ++i) - if (!valid.count(*i)) missing.push_back(*i); - - printMsg(lvlDebug, format("copying %1% missing paths") % missing.size()); - - for (auto & i : missing) - copyStorePath(srcStore, dstStore, i, repair, dontCheckSigs); + srcStore->computeFSClosure({storePaths}, closure); + copyPaths(srcStore, dstStore, closure, repair, checkSigs, substitute); } @@ -812,45 +826,4 @@ std::list> getDefaultSubstituters() } -void copyPaths(ref from, ref to, const PathSet & storePaths, - bool substitute, bool dontCheckSigs) -{ - PathSet valid = to->queryValidPaths(storePaths, substitute); - - PathSet missing; - for (auto & path : storePaths) - if (!valid.count(path)) missing.insert(path); - - std::string copiedLabel = "copied"; - - //logger->setExpected(copiedLabel, missing.size()); - - ThreadPool pool; - - processGraph(pool, - PathSet(missing.begin(), missing.end()), - - [&](const Path & storePath) { - if (to->isValidPath(storePath)) return PathSet(); - return from->queryPathInfo(storePath)->references; - }, - - [&](const Path & storePath) { - checkInterrupt(); - - if (!to->isValidPath(storePath)) { - //Activity act(*logger, lvlInfo, format("copying ‘%s’...") % storePath); - - copyStorePath(from, to, storePath, false, dontCheckSigs); - - //logger->incProgress(copiedLabel); - } else - ; - //logger->incExpected(copiedLabel, -1); - }); - - pool.process(); -} - - } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 929c95a0f..c625a3630 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -32,6 +32,11 @@ class Store; class JSONPlaceholder; +enum RepairFlag : bool { NoRepair = false, Repair = true }; +enum CheckSigsFlag : bool { NoCheckSigs = false, CheckSigs = true }; +enum SubstituteFlag : bool { NoSubstitute = false, Substitute = true }; + + /* Size of the hash part of store paths, in base-32 characters. */ const size_t storePathHashLen = 32; // i.e. 160 bits @@ -332,7 +337,7 @@ public: /* Query which of the given paths is valid. Optionally, try to substitute missing paths. */ virtual PathSet queryValidPaths(const PathSet & paths, - bool maybeSubstitute = false); + SubstituteFlag maybeSubstitute = NoSubstitute); /* Query the set of all valid paths. Note that for some store backends, the name part of store paths may be omitted @@ -392,7 +397,7 @@ public: /* Import a path into the store. */ virtual void addToStore(const ValidPathInfo & info, const ref & nar, - bool repair = false, bool dontCheckSigs = false, + RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs, std::shared_ptr accessor = 0) = 0; /* Copy the contents of a path to the store and register the @@ -401,12 +406,12 @@ public: libutil/archive.hh). */ virtual Path addToStore(const string & name, const Path & srcPath, bool recursive = true, HashType hashAlgo = htSHA256, - PathFilter & filter = defaultPathFilter, bool repair = false) = 0; + PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) = 0; /* Like addToStore, but the contents written to the output path is a regular file containing the given string. */ virtual Path addTextToStore(const string & name, const string & s, - const PathSet & references, bool repair = false) = 0; + const PathSet & references, RepairFlag repair = NoRepair) = 0; /* Write a NAR dump of a store path. */ virtual void narFromPath(const Path & path, Sink & sink) = 0; @@ -496,7 +501,7 @@ public: /* Check the integrity of the Nix store. Returns true if errors remain. */ - virtual bool verifyStore(bool checkContents, bool repair) { return false; }; + virtual bool verifyStore(bool checkContents, RepairFlag repair = NoRepair) { return false; }; /* Return an object to access files in the Nix store. */ virtual ref getFSAccessor() = 0; @@ -548,7 +553,7 @@ public: preloaded into the specified FS accessor to speed up subsequent access. */ Paths importPaths(Source & source, std::shared_ptr accessor, - bool dontCheckSigs = false); + CheckSigsFlag checkSigs = CheckSigs); struct Stats { @@ -650,12 +655,26 @@ void checkStoreName(const string & name); /* Copy a path from one store to another. */ void copyStorePath(ref srcStore, ref dstStore, - const Path & storePath, bool repair = false, bool dontCheckSigs = false); + const Path & storePath, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs); + + +/* Copy store paths from one store to another. The paths may be copied + in parallel. They are copied in a topologically sorted order + (i.e. if A is a reference of B, then A is copied before B), but + the set of store paths is not automatically closed; use + copyClosure() for that. */ +void copyPaths(ref srcStore, ref dstStore, const PathSet & storePaths, + RepairFlag repair = NoRepair, + CheckSigsFlag checkSigs = CheckSigs, + SubstituteFlag substitute = NoSubstitute); /* Copy the closure of the specified paths from one store to another. */ void copyClosure(ref srcStore, ref dstStore, - const PathSet & storePaths, bool repair = false, bool dontCheckSigs = false); + const PathSet & storePaths, + RepairFlag repair = NoRepair, + CheckSigsFlag checkSigs = CheckSigs, + SubstituteFlag substitute = NoSubstitute); /* Remove the temporary roots file for this process. Any temporary @@ -694,9 +713,6 @@ ref openStore(const std::string & uri = getEnv("NIX_REMOTE"), const Store::Params & extraParams = Store::Params()); -void copyPaths(ref from, ref to, const PathSet & storePaths, - bool substitute = false, bool dontCheckSigs = false); - enum StoreType { tDaemon, tLocal, diff --git a/src/nix-copy-closure/nix-copy-closure.cc b/src/nix-copy-closure/nix-copy-closure.cc index dc324abcb..0c69bd413 100755 --- a/src/nix-copy-closure/nix-copy-closure.cc +++ b/src/nix-copy-closure/nix-copy-closure.cc @@ -12,7 +12,7 @@ int main(int argc, char ** argv) auto toMode = true; auto includeOutputs = false; auto dryRun = false; - auto useSubstitutes = false; + auto useSubstitutes = NoSubstitute; std::string sshHost; PathSet storePaths; @@ -36,7 +36,7 @@ int main(int argc, char ** argv) else if (*arg == "--dry-run") dryRun = true; else if (*arg == "--use-substitutes" || *arg == "-s") - useSubstitutes = true; + useSubstitutes = Substitute; else if (sshHost.empty()) sshHost = *arg; else @@ -58,6 +58,6 @@ int main(int argc, char ** argv) PathSet closure; from->computeFSClosure(storePaths2, closure, false, includeOutputs); - copyPaths(from, to, closure, useSubstitutes, true); + copyPaths(from, to, closure, NoRepair, NoCheckSigs, useSubstitutes); }); } diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index 44127635d..c9c167766 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -304,7 +304,7 @@ static void performOp(ref store, bool trusted, unsigned int clientVe string s = readString(from); PathSet refs = readStorePaths(*store, from); startWork(); - Path path = store->addTextToStore(suffix, s, refs, false); + Path path = store->addTextToStore(suffix, s, refs, NoRepair); stopWork(); to << path; break; @@ -324,7 +324,8 @@ static void performOp(ref store, bool trusted, unsigned int clientVe case wopImportPaths: { startWork(); TunnelSource source(from); - Paths paths = store->importPaths(source, 0, trusted); + Paths paths = store->importPaths(source, nullptr, + trusted ? NoCheckSigs : CheckSigs); stopWork(); to << paths; break; @@ -576,7 +577,7 @@ static void performOp(ref store, bool trusted, unsigned int clientVe startWork(); if (repair && !trusted) throw Error("you are not privileged to repair paths"); - bool errors = store->verifyStore(checkContents, repair); + bool errors = store->verifyStore(checkContents, (RepairFlag) repair); stopWork(); to << errors; break; @@ -623,7 +624,8 @@ static void performOp(ref store, bool trusted, unsigned int clientVe parseDump(tee, tee.source); startWork(); - store->addToStore(info, tee.source.data, repair, dontCheckSigs, nullptr); + store->addToStore(info, tee.source.data, (RepairFlag) repair, + dontCheckSigs ? NoCheckSigs : CheckSigs, nullptr); stopWork(); break; } diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 464bcee4a..10100d6a6 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1310,7 +1310,7 @@ int main(int argc, char * * argv) Strings opFlags, opArgs, searchPath; std::map autoArgs_; Operation op = 0; - bool repair = false; + RepairFlag repair = NoRepair; string file; Globals globals; @@ -1372,7 +1372,7 @@ int main(int argc, char * * argv) else if (*arg == "--prebuilt-only" || *arg == "-b") globals.prebuiltOnly = true; else if (*arg == "--repair") - repair = true; + repair = Repair; else if (*arg != "" && arg->at(0) == '-') { opFlags.push_back(*arg); /* FIXME: hacky */ diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index 25f0b1bd6..a5d12c146 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -108,7 +108,7 @@ int main(int argc, char * * argv) Strings attrPaths; bool wantsReadWrite = false; std::map autoArgs_; - bool repair = false; + RepairFlag repair = NoRepair; parseCmdLine(argc, argv, [&](Strings::iterator & arg, const Strings::iterator & end) { if (*arg == "--help") @@ -146,7 +146,7 @@ int main(int argc, char * * argv) else if (*arg == "--strict") strict = true; else if (*arg == "--repair") - repair = true; + repair = Repair; else if (*arg == "--dry-run") settings.readOnlyMode = true; else if (*arg != "" && arg->at(0) == '-') diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 950222812..314c94239 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -677,7 +677,7 @@ static void opImport(Strings opFlags, Strings opArgs) if (!opArgs.empty()) throw UsageError("no arguments expected"); FdSource source(STDIN_FILENO); - Paths paths = store->importPaths(source, nullptr, true); + Paths paths = store->importPaths(source, nullptr, NoCheckSigs); for (auto & i : paths) cout << format("%1%\n") % i << std::flush; @@ -702,11 +702,11 @@ static void opVerify(Strings opFlags, Strings opArgs) throw UsageError("no arguments expected"); bool checkContents = false; - bool repair = false; + RepairFlag repair = NoRepair; for (auto & i : opFlags) if (i == "--check-contents") checkContents = true; - else if (i == "--repair") repair = true; + else if (i == "--repair") repair = Repair; else throw UsageError(format("unknown flag ‘%1%’") % i); if (store->verifyStore(checkContents, repair)) { @@ -871,7 +871,7 @@ static void opServe(Strings opFlags, Strings opArgs) case cmdImportPaths: { if (!writeAllowed) throw Error("importing paths is not allowed"); - store->importPaths(in, 0, true); // FIXME: should we skip sig checking? + store->importPaths(in, nullptr, NoCheckSigs); // FIXME: should we skip sig checking? out << 1; // indicate success break; }