From c214cda9401cf50d0419038746428260b0dfdd63 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Sat, 13 Jun 2020 00:07:42 -0500 Subject: [PATCH] Correctly substitute from different storeDir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Originally, the test was only checking for different “real” storeDir. That’s an easy case to handle, but the much harder one is if different virtual store dirs are used. To do this, we need the SubstitutionGoal to know about the ca, so it can recalculate the path to copy it over. An important note here is that the store path passed to copyStorePath needs to be one for srcStore - so that queryPathInfo works properly. This also adds an error message when the store path from queryPathInfo is different from the one we requested. --- src/libstore/build.cc | 44 ++++++++++++++++++++++++++++----------- src/libstore/misc.cc | 12 +++++------ src/libstore/store-api.hh | 2 ++ tests/fetchurl.sh | 4 ++-- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index f5c132a83..f3f6f01cc 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -305,7 +305,7 @@ public: GoalPtr makeDerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); std::shared_ptr makeBasicDerivationGoal(StorePath && drvPath, const BasicDerivation & drv, BuildMode buildMode = bmNormal); - GoalPtr makeSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair); + GoalPtr makeSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); /* Remove a dead goal. */ void removeGoal(GoalPtr goal); @@ -1195,7 +1195,7 @@ void DerivationGoal::haveDerivation() them. */ if (settings.useSubstitutes && parsedDrv->substitutesAllowed()) for (auto & i : invalidOutputs) - addWaitee(worker.makeSubstitutionGoal(i, buildMode == bmRepair ? Repair : NoRepair)); + addWaitee(worker.makeSubstitutionGoal(i, buildMode == bmRepair ? Repair : NoRepair, getDerivationCA(*drv))); if (waitees.empty()) /* to prevent hang (no wake-up event) */ outputsSubstituted(); @@ -4268,8 +4268,11 @@ private: typedef void (SubstitutionGoal::*GoalState)(); GoalState state; + /* Content address for recomputing store path */ + std::optional ca; + public: - SubstitutionGoal(StorePath && storePath, Worker & worker, RepairFlag repair = NoRepair); + SubstitutionGoal(StorePath && storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); ~SubstitutionGoal(); void timedOut() override { abort(); }; @@ -4304,10 +4307,11 @@ public: }; -SubstitutionGoal::SubstitutionGoal(StorePath && storePath, Worker & worker, RepairFlag repair) +SubstitutionGoal::SubstitutionGoal(StorePath && storePath, Worker & worker, RepairFlag repair, std::optional ca) : Goal(worker) , storePath(std::move(storePath)) , repair(repair) + , ca(ca) { state = &SubstitutionGoal::init; name = fmt("substitution of '%s'", worker.store.printStorePath(this->storePath)); @@ -4382,14 +4386,13 @@ void SubstitutionGoal::tryNext() sub = subs.front(); subs.pop_front(); - if (sub->storeDir != worker.store.storeDir) { - tryNext(); - return; - } + auto subPath = storePath.clone(); + if (ca && (hasPrefix(*ca, "fixed:") || hasPrefix(*ca, "text:"))) + subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca); try { // FIXME: make async - info = sub->queryPathInfo(storePath); + info = sub->queryPathInfo(subPath); } catch (InvalidPath &) { tryNext(); return; @@ -4408,6 +4411,19 @@ void SubstitutionGoal::tryNext() throw; } + if (info->path != storePath) { + if (info->isContentAddressed(*sub)) { + auto info2 = std::const_pointer_cast(std::shared_ptr(info)); + info2->path = storePath.clone(); + info = info2; + } else { + printError("asked '%s' for '%s' but got '%s'", + sub->getUri(), worker.store.printStorePath(storePath), sub->printStorePath(info->path)); + tryNext(); + return; + } + } + /* Update the total expected download size. */ auto narInfo = std::dynamic_pointer_cast(info); @@ -4493,8 +4509,12 @@ void SubstitutionGoal::tryToRun() Activity act(*logger, actSubstitute, Logger::Fields{worker.store.printStorePath(storePath), sub->getUri()}); PushActivity pact(act.id); + auto subPath = storePath.clone(); + if (ca && (hasPrefix(*ca, "fixed:") || hasPrefix(*ca, "text:"))) + subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca); + copyStorePath(ref(sub), ref(worker.store.shared_from_this()), - storePath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs); + subPath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs); promise.set_value(); } catch (...) { @@ -4628,11 +4648,11 @@ std::shared_ptr Worker::makeBasicDerivationGoal(StorePath && drv } -GoalPtr Worker::makeSubstitutionGoal(const StorePath & path, RepairFlag repair) +GoalPtr Worker::makeSubstitutionGoal(const StorePath & path, RepairFlag repair, std::optional ca) { GoalPtr goal = substitutionGoals[path.clone()].lock(); // FIXME if (!goal) { - goal = std::make_shared(path.clone(), *this, repair); + goal = std::make_shared(path.clone(), *this, repair, ca); substitutionGoals.insert_or_assign(path.clone(), goal); wakeUp(goal); } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 1b0a055d3..109e9e473 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -108,12 +108,12 @@ void Store::computeFSClosure(const StorePath & startPath, } -static std::optional getDerivationCA(ref drv) +std::optional getDerivationCA(const BasicDerivation & drv) { - auto outputHashMode = drv->env.find("outputHashMode"); - auto outputHashAlgo = drv->env.find("outputHashAlgo"); - auto outputHash = drv->env.find("outputHash"); - if (outputHashMode != drv->env.end() && outputHashAlgo != drv->env.end() && outputHash != drv->env.end()) { + auto outputHashMode = drv.env.find("outputHashMode"); + auto outputHashAlgo = drv.env.find("outputHashAlgo"); + auto outputHash = drv.env.find("outputHash"); + if (outputHashMode != drv.env.end() && outputHashAlgo != drv.env.end() && outputHash != drv.env.end()) { auto ht = parseHashType(outputHashAlgo->second); auto h = Hash(outputHash->second, ht); FileIngestionMethod ingestionMethod; @@ -182,7 +182,7 @@ void Store::queryMissing(const std::vector & targets, paths.insert(outPath.clone()); std::map pathsCA = {}; - if (auto ca = getDerivationCA(drv)) + if (auto ca = getDerivationCA(*drv)) pathsCA.insert({outPathS, *ca}); querySubstitutablePathInfos(paths, infos, pathsCA); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 9412785b4..9918a36c6 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -855,4 +855,6 @@ std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash); /* Split URI into protocol+hierarchy part and its parameter set. */ std::pair splitUriAndParams(const std::string & uri); +std::optional getDerivationCA(const BasicDerivation & drv); + } diff --git a/tests/fetchurl.sh b/tests/fetchurl.sh index f4b86d251..aaa38f2ed 100644 --- a/tests/fetchurl.sh +++ b/tests/fetchurl.sh @@ -32,13 +32,13 @@ cmp $outPath fetchurl.sh # Test that we can substitute from a different store dir. clearStore -other_store=file://$TEST_ROOT/other_store +other_store=file://$TEST_ROOT/other_store?store=/fnord/store hash=$(nix hash-file --type sha256 --base16 ./fetchurl.sh) storePath=$(nix --store $other_store add-to-store --flat ./fetchurl.sh) -outPath=$(nix-build '' --argstr url file:///no-such-dir/fetchurl.sh --argstr sha256 $hash --no-out-link --substituters $other_store) +outPath=$(nix-build -vvvvv '' --argstr url file:///no-such-dir/fetchurl.sh --argstr sha256 $hash --no-out-link --substituters $other_store) # Test hashed mirrors with an SRI hash. nix-build '' --argstr url file:///no-such-dir/fetchurl.sh --argstr hash $(nix to-sri --type sha256 $hash) \