From 79c169d1c615211af69c1cbc6218f7465a4f81ed Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 09:49:09 -0500 Subject: [PATCH 01/27] Allow substituting from different storeDir Substituters can substitute from one store dir to another with a little bit of help. The store api just needs to have a CA so it can recompute the store path based on the new store dir. We can only do this for fixed output derivations with no references, though. --- src/libstore/local-store.cc | 16 ++++++++++++---- src/libstore/local-store.hh | 3 ++- src/libstore/misc.cc | 27 ++++++++++++++++++++++++++- src/libstore/remote-store.cc | 2 +- src/libstore/remote-store.hh | 2 +- src/libstore/store-api.cc | 7 +++++++ src/libstore/store-api.hh | 2 +- 7 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 80851b59106..6385453ccbd 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -855,16 +855,24 @@ StorePathSet LocalStore::querySubstitutablePaths(const StorePathSet & paths) void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos) + SubstitutablePathInfos & infos, std::map pathsCA) { if (!settings.useSubstitutes) return; for (auto & sub : getDefaultSubstituters()) { - if (sub->storeDir != storeDir) continue; - for (auto & path : paths) { - if (infos.count(path)) continue; + for (auto & path_ : paths) { + auto path(path_.clone()); debug("checking substituter '%s' for path '%s'", sub->getUri(), printStorePath(path)); try { auto info = sub->queryPathInfo(path); + + auto ca = pathsCA.find(printStorePath(path)); + if (sub->storeDir != storeDir && info->references.empty() && ca != pathsCA.end()) { + if (!hasPrefix(ca->second, "fixed:")) + continue; + // recompute store path so that we can use a fixed output ca + path = sub->makeStorePath("output:out", hashString(htSHA256, ca->second), path.name()); + } else continue; + auto narInfo = std::dynamic_pointer_cast( std::shared_ptr(info)); infos.insert_or_assign(path.clone(), SubstitutablePathInfo{ diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index c1e75390c5d..5a976e3e198 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -142,7 +142,8 @@ public: StorePathSet querySubstitutablePaths(const StorePathSet & paths) override; void querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos) override; + SubstitutablePathInfos & infos, + std::map pathsCA = {}) override; void addToStore(const ValidPathInfo & info, Source & source, RepairFlag repair, CheckSigsFlag checkSigs, diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 9c47fe52454..1b0a055d3c7 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -108,6 +108,27 @@ void Store::computeFSClosure(const StorePath & startPath, } +static std::optional getDerivationCA(ref 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 ht = parseHashType(outputHashAlgo->second); + auto h = Hash(outputHash->second, ht); + FileIngestionMethod ingestionMethod; + if (outputHashMode->second == "recursive") + ingestionMethod = FileIngestionMethod::Recursive; + else if (outputHashMode->second == "flat") + ingestionMethod = FileIngestionMethod::Flat; + else + throw Error("unknown ingestion method: '%s'", outputHashMode->second); + return makeFixedOutputCA(ingestionMethod, h); + } + + return std::nullopt; +} + void Store::queryMissing(const std::vector & targets, StorePathSet & willBuild_, StorePathSet & willSubstitute_, StorePathSet & unknown_, unsigned long long & downloadSize_, unsigned long long & narSize_) @@ -159,7 +180,11 @@ void Store::queryMissing(const std::vector & targets, SubstitutablePathInfos infos; StorePathSet paths; // FIXME paths.insert(outPath.clone()); - querySubstitutablePathInfos(paths, infos); + + std::map pathsCA = {}; + if (auto ca = getDerivationCA(drv)) + pathsCA.insert({outPathS, *ca}); + querySubstitutablePathInfos(paths, infos, pathsCA); if (infos.empty()) { drvState_->lock()->done = true; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 5c36693e699..956980f49f0 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -309,7 +309,7 @@ StorePathSet RemoteStore::querySubstitutablePaths(const StorePathSet & paths) void RemoteStore::querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos) + SubstitutablePathInfos & infos, std::map pathsCA) { if (paths.empty()) return; diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 3c86b4524eb..867d650bc9a 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -58,7 +58,7 @@ public: StorePathSet querySubstitutablePaths(const StorePathSet & paths) override; void querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos) override; + SubstitutablePathInfos & infos, std::map pathsCA = {}) override; void addToStore(const ValidPathInfo & info, Source & nar, RepairFlag repair, CheckSigsFlag checkSigs, diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 095363d0c9b..40000da01bf 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -580,6 +580,13 @@ void copyStorePath(ref srcStore, ref dstStore, uint64_t total = 0; + // recompute store path on the chance dstStore does it differently + if (hasPrefix(info->ca, "fixed:") && info->references.empty()) { + auto info2 = make_ref(*info); + info2->path = dstStore->makeStorePath("output:out", hashString(htSHA256, info->ca), storePath.name()); + info = info2; + } + if (!info->narHash) { StringSink sink; srcStore->narFromPath({storePath}, sink); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index b1e25fc7d66..6f961329cf8 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -445,7 +445,7 @@ public: sizes) of a set of paths. If a path does not have substitute info, it's omitted from the resulting ‘infos’ map. */ virtual void querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos) { return; }; + SubstitutablePathInfos & infos, std::map pathsCA = {}) { return; }; /* Import a path into the store. */ virtual void addToStore(const ValidPathInfo & info, Source & narSource, From 3e3eaa90dd1aaf4684697f27726ec72aec75206c Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 09:51:44 -0500 Subject: [PATCH 02/27] Remove hashed-mirrors --- doc/manual/command-ref/conf-file.xml | 28 ---------------------------- src/libstore/builtins/fetchurl.cc | 14 -------------- src/libstore/globals.hh | 3 --- 3 files changed, 45 deletions(-) diff --git a/doc/manual/command-ref/conf-file.xml b/doc/manual/command-ref/conf-file.xml index 1fa74a14334..9c55526a3c2 100644 --- a/doc/manual/command-ref/conf-file.xml +++ b/doc/manual/command-ref/conf-file.xml @@ -370,34 +370,6 @@ false. - hashed-mirrors - - A list of web servers used by - builtins.fetchurl to obtain files by - hash. The default is - http://tarballs.nixos.org/. 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 default mirror - http://tarballs.nixos.org/, 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.nixos.org/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 486babf1427..254cb4fce2c 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -58,20 +58,6 @@ 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 da95fd3ae91..2e1e405b3d6 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -339,9 +339,6 @@ public: "setuid/setgid bits or with file capabilities."}; #endif - Setting hashedMirrors{this, {"http://tarballs.nixos.org/"}, "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 11c97070f3d845a9313f137c41974c021429e834 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 10:14:03 -0500 Subject: [PATCH 03/27] Fix storeDir != storeDir condition this needs to only continue if the path replacement fails. --- src/libstore/local-store.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 6385453ccbd..4cecf0992df 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -866,12 +866,12 @@ void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths, auto info = sub->queryPathInfo(path); auto ca = pathsCA.find(printStorePath(path)); - if (sub->storeDir != storeDir && info->references.empty() && ca != pathsCA.end()) { + if (info->references.empty() && ca != pathsCA.end()) { if (!hasPrefix(ca->second, "fixed:")) continue; - // recompute store path so that we can use a fixed output ca + // recompute store path so that we can use a different store path path = sub->makeStorePath("output:out", hashString(htSHA256, ca->second), path.name()); - } else continue; + } else if (sub->storeDir != storeDir) continue; auto narInfo = std::dynamic_pointer_cast( std::shared_ptr(info)); From 1c55f16a1648431301f044710644a674c018321d Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 12:26:08 -0500 Subject: [PATCH 04/27] Add --flat to nix add-to-store This can be used to add flat hashes to the nix store. --- src/nix/add-to-store.cc | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index f43f774c1c8..a5a77f17cdb 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -9,6 +9,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand { Path path; std::optional namePart; + FileIngestionMethod ingestionMethod = FileIngestionMethod::Recursive; CmdAddToStore() { @@ -21,6 +22,13 @@ struct CmdAddToStore : MixDryRun, StoreCommand .labels = {"name"}, .handler = {&namePart}, }); + + addFlag({ + .longName = "flat", + .shortName = 0, + .description = "use flat file ingestion", + .handler = {&ingestionMethod, FileIngestionMethod::Flat}, + }); } std::string description() override @@ -45,10 +53,24 @@ struct CmdAddToStore : MixDryRun, StoreCommand auto narHash = hashString(htSHA256, *sink.s); - ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, *namePart)); + ValidPathInfo info(store->makeFixedOutputPath(ingestionMethod, narHash, *namePart)); info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash); + + Hash hash; + switch (ingestionMethod) { + case FileIngestionMethod::Recursive: { + hash = info.narHash; + break; + } + case FileIngestionMethod::Flat: { + HashSink hsink(htSHA256); + readFile(path, hsink); + hash = hsink.finish().first; + break; + } + } + info.ca = makeFixedOutputCA(ingestionMethod, hash); if (!dryRun) { auto source = StringSource { *sink.s }; From 2f2ac850b54110a81e3338468161cf132618c156 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 14:03:17 -0500 Subject: [PATCH 05/27] Compute new store path correctly --- src/libstore/local-store.cc | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 4cecf0992df..bf005a3b36a 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -859,19 +859,28 @@ void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths, { if (!settings.useSubstitutes) return; for (auto & sub : getDefaultSubstituters()) { - for (auto & path_ : paths) { - auto path(path_.clone()); - debug("checking substituter '%s' for path '%s'", sub->getUri(), printStorePath(path)); + for (auto & path : paths) { + auto subPath(path.clone()); + + auto ca_ = pathsCA.find(printStorePath(path)); + // recompute store path so that we can use a different store root + if (ca_ != pathsCA.end()) { + auto ca(ca_->second); + if (!hasPrefix(ca, "fixed:")) + continue; + FileIngestionMethod ingestionMethod { ca.compare(6, 2, "r:") == 0 }; + Hash hash(std::string(ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); + subPath = makeFixedOutputPath(ingestionMethod, hash, path.name()); + if (subPath != path) + debug("replaced path '%s' with '%s' for substituter '%s'", printStorePath(path), sub->printStorePath(subPath), sub->getUri()); + } else if (sub->storeDir != storeDir) continue; + + debug("checking substituter '%s' for path '%s'", sub->getUri(), sub->printStorePath(subPath)); try { - auto info = sub->queryPathInfo(path); - - auto ca = pathsCA.find(printStorePath(path)); - if (info->references.empty() && ca != pathsCA.end()) { - if (!hasPrefix(ca->second, "fixed:")) - continue; - // recompute store path so that we can use a different store path - path = sub->makeStorePath("output:out", hashString(htSHA256, ca->second), path.name()); - } else if (sub->storeDir != storeDir) continue; + auto info = sub->queryPathInfo(subPath); + + if (sub->storeDir != storeDir && !info->isContentAddressed(*sub)) + continue; auto narInfo = std::dynamic_pointer_cast( std::shared_ptr(info)); From 9077b9e6b28b94cdfac80ea4afbe18d13a7b2de6 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 14:27:28 -0500 Subject: [PATCH 06/27] Separate dstStore path from srcStore path --- src/libstore/store-api.cc | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 40000da01bf..1b340bf6c32 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -649,15 +649,24 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st processGraph(pool, PathSet(missing.begin(), missing.end()), - [&](const Path & storePath) { - if (dstStore->isValidPath(dstStore->parseStorePath(storePath))) { + [&](const Path & storePathS) { + auto storePath = srcStore->parseStorePath(storePathS); + + auto info = srcStore->queryPathInfo(storePath); + auto storePathForDst = storePath.clone(); + if (hasPrefix(info->ca, "fixed:")) + { + FileIngestionMethod ingestionMethod { info->ca.compare(6, 2, "r:") == 0 }; + Hash hash(std::string(info->ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); + storePathForDst = dstStore->makeFixedOutputPath(ingestionMethod, hash, storePath.name()); + } + + if (dstStore->isValidPath(storePathForDst)) { nrDone++; showProgress(); return PathSet(); } - auto info = srcStore->queryPathInfo(srcStore->parseStorePath(storePath)); - bytesExpected += info->narSize; act.setExpected(actCopyPath, bytesExpected); @@ -667,9 +676,18 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st [&](const Path & storePathS) { checkInterrupt(); - auto storePath = dstStore->parseStorePath(storePathS); + auto storePath = srcStore->parseStorePath(storePathS); + auto info = srcStore->queryPathInfo(storePath); + + auto storePathForDst = storePath.clone(); + if (hasPrefix(info->ca, "fixed:")) + { + FileIngestionMethod ingestionMethod { info->ca.compare(6, 2, "r:") == 0 }; + Hash hash(std::string(info->ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); + storePathForDst = dstStore->makeFixedOutputPath(ingestionMethod, hash, storePath.name()); + } - if (!dstStore->isValidPath(storePath)) { + if (!dstStore->isValidPath(storePathForDst)) { MaintainCount mc(nrRunning); showProgress(); try { From 1c148a80fe05580f94e624134b923799f6cbc7b3 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 14:39:44 -0500 Subject: [PATCH 07/27] Replace --hashed-mirrors with substituters test --- tests/check.sh | 14 +++++++------- tests/fetchurl.sh | 21 +++++++++------------ 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/tests/check.sh b/tests/check.sh index 5f25d04cb2c..5f4997e287c 100644 --- a/tests/check.sh +++ b/tests/check.sh @@ -61,30 +61,30 @@ nix-build check.nix -A nondeterministic --no-out-link --repeat 1 2> $TEST_ROOT/l [ "$status" = "1" ] grep 'differs from previous round' $TEST_ROOT/log -path=$(nix-build check.nix -A fetchurl --no-out-link --hashed-mirrors '') +path=$(nix-build check.nix -A fetchurl --no-out-link) chmod +w $path echo foo > $path chmod -w $path -nix-build check.nix -A fetchurl --no-out-link --check --hashed-mirrors '' +nix-build check.nix -A fetchurl --no-out-link --check # Note: "check" doesn't repair anything, it just compares to the hash stored in the database. [[ $(cat $path) = foo ]] -nix-build check.nix -A fetchurl --no-out-link --repair --hashed-mirrors '' +nix-build check.nix -A fetchurl --no-out-link --repair [[ $(cat $path) != foo ]] -nix-build check.nix -A hashmismatch --no-out-link --hashed-mirrors '' || status=$? +nix-build check.nix -A hashmismatch --no-out-link || status=$? [ "$status" = "102" ] echo -n > ./dummy -nix-build check.nix -A hashmismatch --no-out-link --hashed-mirrors '' +nix-build check.nix -A hashmismatch --no-out-link echo 'Hello World' > ./dummy -nix-build check.nix -A hashmismatch --no-out-link --check --hashed-mirrors '' || status=$? +nix-build check.nix -A hashmismatch --no-out-link --check || status=$? [ "$status" = "102" ] # Multiple failures with --keep-going nix-build check.nix -A nondeterministic --no-out-link -nix-build check.nix -A nondeterministic -A hashmismatch --no-out-link --check --keep-going --hashed-mirrors '' || status=$? +nix-build check.nix -A nondeterministic -A hashmismatch --no-out-link --check --keep-going || status=$? [ "$status" = "110" ] diff --git a/tests/fetchurl.sh b/tests/fetchurl.sh index 2535651b09a..13447d774d2 100644 --- a/tests/fetchurl.sh +++ b/tests/fetchurl.sh @@ -5,7 +5,7 @@ clearStore # Test fetching a flat file. hash=$(nix-hash --flat --type sha256 ./fetchurl.sh) -outPath=$(nix-build '' --argstr url file://$(pwd)/fetchurl.sh --argstr sha256 $hash --no-out-link --hashed-mirrors '') +outPath=$(nix-build '' --argstr url file://$(pwd)/fetchurl.sh --argstr sha256 $hash --no-out-link) cmp $outPath fetchurl.sh @@ -14,7 +14,7 @@ clearStore hash=$(nix hash-file --type sha512 --base64 ./fetchurl.sh) -outPath=$(nix-build '' --argstr url file://$(pwd)/fetchurl.sh --argstr sha512 $hash --no-out-link --hashed-mirrors '') +outPath=$(nix-build '' --argstr url file://$(pwd)/fetchurl.sh --argstr sha512 $hash --no-out-link) cmp $outPath fetchurl.sh @@ -25,26 +25,23 @@ hash=$(nix hash-file ./fetchurl.sh) [[ $hash =~ ^sha256- ]] -outPath=$(nix-build '' --argstr url file://$(pwd)/fetchurl.sh --argstr hash $hash --no-out-link --hashed-mirrors '') +outPath=$(nix-build '' --argstr url file://$(pwd)/fetchurl.sh --argstr hash $hash --no-out-link) cmp $outPath fetchurl.sh -# Test the hashed mirror feature. +# Test that we can substitute from a different store dir. clearStore -hash=$(nix hash-file --type sha512 --base64 ./fetchurl.sh) -hash32=$(nix hash-file --type sha512 --base16 ./fetchurl.sh) +other_store=file://$TEST_ROOT/other_store -mirror=$TEST_ROOT/hashed-mirror -rm -rf $mirror -mkdir -p $mirror/sha512 -ln -s $(pwd)/fetchurl.sh $mirror/sha512/$hash32 +hash=$(nix hash-file --type sha256 --base16 ./fetchurl.sh) +storePath=$(nix add-to-store --store $other_store ./fetchurl.sh) -outPath=$(nix-build '' --argstr url file:///no-such-dir/fetchurl.sh --argstr sha512 $hash --no-out-link --hashed-mirrors "file://$mirror") +outPath=$(nix-build -vvvvvv '' --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 sha512 $hash) \ - --argstr name bla --no-out-link --hashed-mirrors "file://$mirror" + --argstr name bla --no-out-link --substituters $other_store # Test unpacking a NAR. rm -rf $TEST_ROOT/archive From 85d01e1f658d93a0ea8d0396b1e79d03c2ff1da2 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 14:53:30 -0500 Subject: [PATCH 08/27] =?UTF-8?q?Don=E2=80=99t=20use=20makeStorePath?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/libstore/store-api.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 1b340bf6c32..afa557e1679 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -583,7 +583,9 @@ void copyStorePath(ref srcStore, ref dstStore, // recompute store path on the chance dstStore does it differently if (hasPrefix(info->ca, "fixed:") && info->references.empty()) { auto info2 = make_ref(*info); - info2->path = dstStore->makeStorePath("output:out", hashString(htSHA256, info->ca), storePath.name()); + FileIngestionMethod ingestionMethod { info->ca.compare(6, 2, "r:") == 0 }; + Hash hash(std::string(info->ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); + info2->path = dstStore->makeFixedOutputPath(ingestionMethod, hash, storePath.name()); info = info2; } From 5b386b05f58b1ed996afe47791d85e6d20da91a2 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 14:57:45 -0500 Subject: [PATCH 09/27] Recompute storePath based on isContentAddressed --- src/libstore/store-api.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index afa557e1679..c8dc4744f6b 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -581,7 +581,7 @@ void copyStorePath(ref srcStore, ref dstStore, uint64_t total = 0; // recompute store path on the chance dstStore does it differently - if (hasPrefix(info->ca, "fixed:") && info->references.empty()) { + if (info->isContentAddressed(*srcStore)) { auto info2 = make_ref(*info); FileIngestionMethod ingestionMethod { info->ca.compare(6, 2, "r:") == 0 }; Hash hash(std::string(info->ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); From e3cb536f19920018b522f76ac2661d68627b7c9e Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 15:25:29 -0500 Subject: [PATCH 10/27] Fix add-to-store --flat to put in correct hash --- src/nix/add-to-store.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index a5a77f17cdb..26edcf95ab0 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -53,14 +53,10 @@ struct CmdAddToStore : MixDryRun, StoreCommand auto narHash = hashString(htSHA256, *sink.s); - ValidPathInfo info(store->makeFixedOutputPath(ingestionMethod, narHash, *namePart)); - info.narHash = narHash; - info.narSize = sink.s->size(); - Hash hash; switch (ingestionMethod) { case FileIngestionMethod::Recursive: { - hash = info.narHash; + hash = narHash; break; } case FileIngestionMethod::Flat: { @@ -70,6 +66,10 @@ struct CmdAddToStore : MixDryRun, StoreCommand break; } } + + ValidPathInfo info(store->makeFixedOutputPath(ingestionMethod, hash, *namePart)); + info.narHash = narHash; + info.narSize = sink.s->size(); info.ca = makeFixedOutputCA(ingestionMethod, hash); if (!dryRun) { From 88120442d23a7e00833e8b0d523a6aa8072287b3 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 15:32:52 -0500 Subject: [PATCH 11/27] Debug when storePath changes these rewrites should be transparent, but they are important to know about when debugging --- src/libstore/store-api.cc | 4 ++++ tests/fetchurl.sh | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index c8dc4744f6b..7cb48b29346 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -661,6 +661,8 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st FileIngestionMethod ingestionMethod { info->ca.compare(6, 2, "r:") == 0 }; Hash hash(std::string(info->ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); storePathForDst = dstStore->makeFixedOutputPath(ingestionMethod, hash, storePath.name()); + if (storePathForDst != storePath) + debug("rewriting path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } if (dstStore->isValidPath(storePathForDst)) { @@ -687,6 +689,8 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st FileIngestionMethod ingestionMethod { info->ca.compare(6, 2, "r:") == 0 }; Hash hash(std::string(info->ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); storePathForDst = dstStore->makeFixedOutputPath(ingestionMethod, hash, storePath.name()); + if (storePathForDst != storePath) + debug("rewriting path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } if (!dstStore->isValidPath(storePathForDst)) { diff --git a/tests/fetchurl.sh b/tests/fetchurl.sh index 13447d774d2..510f988436e 100644 --- a/tests/fetchurl.sh +++ b/tests/fetchurl.sh @@ -35,12 +35,13 @@ clearStore other_store=file://$TEST_ROOT/other_store hash=$(nix hash-file --type sha256 --base16 ./fetchurl.sh) -storePath=$(nix add-to-store --store $other_store ./fetchurl.sh) -outPath=$(nix-build -vvvvvv '' --argstr url file:///no-such-dir/fetchurl.sh --argstr sha256 $hash --no-out-link --substituters $other_store) +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) # Test hashed mirrors with an SRI hash. -nix-build '' --argstr url file:///no-such-dir/fetchurl.sh --argstr hash $(nix to-sri --type sha512 $hash) \ +nix-build '' --argstr url file:///no-such-dir/fetchurl.sh --argstr hash $(nix to-sri --type sha256 $hash) \ --argstr name bla --no-out-link --substituters $other_store # Test unpacking a NAR. From 006f1252d2fdb36c7ee66bf79f0df2ab8cd16816 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 15:33:24 -0500 Subject: [PATCH 12/27] Fix SRI test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can’t use custom name here because different names will have different store paths. This is a limitation of the Store API’s reliance on store paths. We might be able to get around the above in the future by using a dummy name for certain fixed output paths. --- tests/fetchurl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fetchurl.sh b/tests/fetchurl.sh index 510f988436e..f4b86d25151 100644 --- a/tests/fetchurl.sh +++ b/tests/fetchurl.sh @@ -42,7 +42,7 @@ outPath=$(nix-build '' --argstr url file:///no-such-dir/fetchu # 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) \ - --argstr name bla --no-out-link --substituters $other_store + --no-out-link --substituters $other_store # Test unpacking a NAR. rm -rf $TEST_ROOT/archive From b2cb288cdd462cbca35ffd3afda9d7ec2d595209 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 16:36:35 -0500 Subject: [PATCH 13/27] Add makeFixedOutputPathFromCA function This puts what we are already doing into a shared method. It just needs a path name and a ca and produces a store path. --- src/libstore/local-store.cc | 11 +++-------- src/libstore/store-api.cc | 31 ++++++++++++++++++------------- src/libstore/store-api.hh | 6 +++++- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index bf005a3b36a..9158a52e088 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -862,15 +862,10 @@ void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths, for (auto & path : paths) { auto subPath(path.clone()); - auto ca_ = pathsCA.find(printStorePath(path)); + auto ca = pathsCA.find(printStorePath(path)); // recompute store path so that we can use a different store root - if (ca_ != pathsCA.end()) { - auto ca(ca_->second); - if (!hasPrefix(ca, "fixed:")) - continue; - FileIngestionMethod ingestionMethod { ca.compare(6, 2, "r:") == 0 }; - Hash hash(std::string(ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); - subPath = makeFixedOutputPath(ingestionMethod, hash, path.name()); + if (ca != pathsCA.end() && (hasPrefix(ca->second, "fixed:") || hasPrefix(ca->second, "text:"))) { + subPath = makeFixedOutputPathFromCA(path.name(), ca->second); if (subPath != path) debug("replaced path '%s' with '%s' for substituter '%s'", printStorePath(path), sub->printStorePath(subPath), sub->getUri()); } else if (sub->storeDir != storeDir) continue; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 7cb48b29346..bc9a6c4f589 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -191,6 +191,19 @@ StorePath Store::makeFixedOutputPath( } } +StorePath Store::makeFixedOutputPathFromCA(std::string_view name, std::string ca, + const StorePathSet & references, bool hasSelfReference) const +{ + if (hasPrefix(ca, "fixed:")) { + FileIngestionMethod ingestionMethod { ca.compare(6, 2, "r:") == 0 }; + Hash hash(std::string(ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); + return makeFixedOutputPath(ingestionMethod, hash, name, references, hasSelfReference); + } else if (hasPrefix(ca, "text:")) { + Hash hash(std::string(ca, 5)); + return makeTextPath(name, hash, references); + } else + throw Error("'%s' is not a valid ca", ca); +} StorePath Store::makeTextPath(std::string_view name, const Hash & hash, const StorePathSet & references) const @@ -583,9 +596,7 @@ void copyStorePath(ref srcStore, ref dstStore, // recompute store path on the chance dstStore does it differently if (info->isContentAddressed(*srcStore)) { auto info2 = make_ref(*info); - FileIngestionMethod ingestionMethod { info->ca.compare(6, 2, "r:") == 0 }; - Hash hash(std::string(info->ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); - info2->path = dstStore->makeFixedOutputPath(ingestionMethod, hash, storePath.name()); + info2->path = dstStore->makeFixedOutputPathFromCA(info->path.name(), info->ca); info = info2; } @@ -656,11 +667,8 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st auto info = srcStore->queryPathInfo(storePath); auto storePathForDst = storePath.clone(); - if (hasPrefix(info->ca, "fixed:")) - { - FileIngestionMethod ingestionMethod { info->ca.compare(6, 2, "r:") == 0 }; - Hash hash(std::string(info->ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); - storePathForDst = dstStore->makeFixedOutputPath(ingestionMethod, hash, storePath.name()); + if (info->isContentAddressed(*srcStore)) { + storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); if (storePathForDst != storePath) debug("rewriting path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } @@ -684,11 +692,8 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st auto info = srcStore->queryPathInfo(storePath); auto storePathForDst = storePath.clone(); - if (hasPrefix(info->ca, "fixed:")) - { - FileIngestionMethod ingestionMethod { info->ca.compare(6, 2, "r:") == 0 }; - Hash hash(std::string(info->ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); - storePathForDst = dstStore->makeFixedOutputPath(ingestionMethod, hash, storePath.name()); + if (info->isContentAddressed(*srcStore)) { + storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); if (storePathForDst != storePath) debug("rewriting path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 6f961329cf8..9412785b4e9 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -352,7 +352,11 @@ public: bool hasSelfReference = false) const; StorePath makeTextPath(std::string_view name, const Hash & hash, - const StorePathSet & references) const; + const StorePathSet & references = {}) const; + + StorePath makeFixedOutputPathFromCA(std::string_view name, std::string ca, + const StorePathSet & references = {}, + bool hasSelfReference = false) const; /* This is the preparatory part of addToStore(); it computes the store path to which srcPath is to be copied. Returns the store From c214cda9401cf50d0419038746428260b0dfdd63 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Sat, 13 Jun 2020 00:07:42 -0500 Subject: [PATCH 14/27] 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 f5c132a8397..f3f6f01cc72 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -305,7 +305,7 @@ class Worker 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 @@ class SubstitutionGoal : public Goal 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 @@ class SubstitutionGoal : public Goal }; -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 1b0a055d3c7..109e9e473a8 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 9412785b4e9..9918a36c6ae 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 f4b86d25151..aaa38f2eda3 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) \ From 9ac0d98a59ed042f698685234109d4b69bccf29c Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Sat, 13 Jun 2020 00:14:30 -0500 Subject: [PATCH 15/27] Remove -vvvvv from tests/fetchurl.sh nix-build call --- tests/fetchurl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fetchurl.sh b/tests/fetchurl.sh index aaa38f2eda3..0f204434215 100644 --- a/tests/fetchurl.sh +++ b/tests/fetchurl.sh @@ -38,7 +38,7 @@ hash=$(nix hash-file --type sha256 --base16 ./fetchurl.sh) storePath=$(nix --store $other_store add-to-store --flat ./fetchurl.sh) -outPath=$(nix-build -vvvvv '' --argstr url file:///no-such-dir/fetchurl.sh --argstr sha256 $hash --no-out-link --substituters $other_store) +outPath=$(nix-build '' --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) \ From 6438ba1e990ddf76602a70d5c1143b73ed31855c Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 17 Jun 2020 12:31:01 -0400 Subject: [PATCH 16/27] Update strings from review comment --- src/libstore/store-api.cc | 4 ++-- src/nix/add-to-store.cc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index bc9a6c4f589..2593b245cbd 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -670,7 +670,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st if (info->isContentAddressed(*srcStore)) { storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); if (storePathForDst != storePath) - debug("rewriting path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); + debug("replaced path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } if (dstStore->isValidPath(storePathForDst)) { @@ -695,7 +695,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st if (info->isContentAddressed(*srcStore)) { storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); if (storePathForDst != storePath) - debug("rewriting path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); + debug("replaced path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } if (!dstStore->isValidPath(storePathForDst)) { diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 26edcf95ab0..7abb82556f9 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -26,7 +26,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand addFlag({ .longName = "flat", .shortName = 0, - .description = "use flat file ingestion", + .description = "add flat file to the Nix store", .handler = {&ingestionMethod, FileIngestionMethod::Flat}, }); } From 8974755d1958824e732640f8131b0ed22ebd703b Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 17 Jun 2020 14:04:46 -0400 Subject: [PATCH 17/27] Add assert for replaced storePath --- src/libstore/build.cc | 10 ++++++++-- src/libstore/local-store.cc | 2 ++ src/libstore/store-api.cc | 6 ++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 9753db7b138..bc67ae31e56 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4409,8 +4409,11 @@ void SubstitutionGoal::tryNext() subs.pop_front(); auto subPath = storePath; - if (ca && (hasPrefix(*ca, "fixed:") || hasPrefix(*ca, "text:"))) + if (ca && (hasPrefix(*ca, "fixed:") || hasPrefix(*ca, "text:"))) { subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca); + if (sub->storeDir == worker.store.storeDir) + assert(subPath == storePath); + } try { // FIXME: make async @@ -4535,8 +4538,11 @@ void SubstitutionGoal::tryToRun() PushActivity pact(act.id); auto subPath = storePath; - if (ca && (hasPrefix(*ca, "fixed:") || hasPrefix(*ca, "text:"))) + if (ca && (hasPrefix(*ca, "fixed:") || hasPrefix(*ca, "text:"))) { subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca); + if (sub->storeDir == worker.store.storeDir) + assert(subPath == storePath); + } copyStorePath(ref(sub), ref(worker.store.shared_from_this()), subPath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index a80026258e0..d08c7dc6a9e 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -853,6 +853,8 @@ void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths, // recompute store path so that we can use a different store root if (ca != pathsCA.end() && (hasPrefix(ca->second, "fixed:") || hasPrefix(ca->second, "text:"))) { subPath = makeFixedOutputPathFromCA(path.name(), ca->second); + if (sub->storeDir == storeDir) + assert(subPath == path); if (subPath != path) debug("replaced path '%s' with '%s' for substituter '%s'", printStorePath(path), sub->printStorePath(subPath), sub->getUri()); } else if (sub->storeDir != storeDir) continue; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index a2889508690..80a69717ee6 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -598,6 +598,8 @@ void copyStorePath(ref srcStore, ref dstStore, if (info->isContentAddressed(*srcStore)) { auto info2 = make_ref(*info); info2->path = dstStore->makeFixedOutputPathFromCA(info->path.name(), info->ca); + if (dstStore->storeDir == srcStore->storeDir) + assert(info->path == info2->path); info = info2; } @@ -670,6 +672,8 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st auto storePathForDst = storePath; if (info->isContentAddressed(*srcStore)) { storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); + if (dstStore->storeDir == srcStore->storeDir) + assert(storePathForDst == storePath); if (storePathForDst != storePath) debug("replaced path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } @@ -695,6 +699,8 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st auto storePathForDst = storePath; if (info->isContentAddressed(*srcStore)) { storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); + if (dstStore->storeDir == srcStore->storeDir) + assert(storePathForDst == storePath); if (storePathForDst != storePath) debug("replaced path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } From be50de1142b0600afebde32130e8561e28b63b41 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 17 Jun 2020 14:14:22 -0400 Subject: [PATCH 18/27] Make sure references are empty for store path replacing also copy info2 instead of casting --- src/libstore/build.cc | 4 ++-- src/libstore/local-store.cc | 2 +- src/libstore/store-api.cc | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index bc67ae31e56..63cb70ef20a 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4437,8 +4437,8 @@ void SubstitutionGoal::tryNext() } if (info->path != storePath) { - if (info->isContentAddressed(*sub)) { - auto info2 = std::const_pointer_cast(std::shared_ptr(info)); + if (info->isContentAddressed(*sub) && info->references.empty()) { + auto info2 = std::make_shared(*info); info2->path = storePath; info = info2; } else { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index d08c7dc6a9e..3230312c91a 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -863,7 +863,7 @@ void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths, try { auto info = sub->queryPathInfo(subPath); - if (sub->storeDir != storeDir && !info->isContentAddressed(*sub)) + if (sub->storeDir != storeDir && !(info->isContentAddressed(*sub) && info->references.empty())) continue; auto narInfo = std::dynamic_pointer_cast( diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 80a69717ee6..c8097da527e 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -595,7 +595,7 @@ void copyStorePath(ref srcStore, ref dstStore, uint64_t total = 0; // recompute store path on the chance dstStore does it differently - if (info->isContentAddressed(*srcStore)) { + if (info->isContentAddressed(*srcStore) && info->references.empty()) { auto info2 = make_ref(*info); info2->path = dstStore->makeFixedOutputPathFromCA(info->path.name(), info->ca); if (dstStore->storeDir == srcStore->storeDir) @@ -670,7 +670,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st auto info = srcStore->queryPathInfo(storePath); auto storePathForDst = storePath; - if (info->isContentAddressed(*srcStore)) { + if (info->isContentAddressed(*srcStore) && info->references.empty()) { storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); if (dstStore->storeDir == srcStore->storeDir) assert(storePathForDst == storePath); @@ -697,7 +697,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st auto info = srcStore->queryPathInfo(storePath); auto storePathForDst = storePath; - if (info->isContentAddressed(*srcStore)) { + if (info->isContentAddressed(*srcStore) && info->references.empty()) { storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); if (dstStore->storeDir == srcStore->storeDir) assert(storePathForDst == storePath); From 5e631e3304aaf00b912c12025debfd35942a0ca9 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 17 Jun 2020 15:03:05 -0400 Subject: [PATCH 19/27] Add StorePathCAMap for querySubstitutablePathInfos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I’m not 100% sure this is wanted since it kind of makes everything have to know about ca even if they don’t really want to. But it also make things easier in dealing with looking up ca. --- src/libstore/daemon.cc | 7 +++++-- src/libstore/local-store.cc | 18 ++++++++---------- src/libstore/local-store.hh | 5 ++--- src/libstore/misc.cc | 7 ++----- src/libstore/path.hh | 2 ++ src/libstore/remote-store.cc | 14 ++++++++------ src/libstore/remote-store.hh | 4 ++-- src/libstore/store-api.hh | 4 ++-- 8 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index e370e278c52..f7dc16948d1 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -593,7 +593,7 @@ static void performOp(TunnelLogger * logger, ref store, auto path = store->parseStorePath(readString(from)); logger->startWork(); SubstitutablePathInfos infos; - store->querySubstitutablePathInfos({path}, infos); + store->querySubstitutablePathInfos({{path, std::nullopt}}, infos); logger->stopWork(); auto i = infos.find(path); if (i == infos.end()) @@ -612,7 +612,10 @@ static void performOp(TunnelLogger * logger, ref store, auto paths = readStorePaths(*store, from); logger->startWork(); SubstitutablePathInfos infos; - store->querySubstitutablePathInfos(paths, infos); + StorePathCAMap pathsMap = {}; + for (auto & path : paths) + pathsMap.emplace(path, std::nullopt); + store->querySubstitutablePathInfos(pathsMap, infos); logger->stopWork(); to << infos.size(); for (auto & i : infos) { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 3230312c91a..3901d532af4 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -841,22 +841,20 @@ StorePathSet LocalStore::querySubstitutablePaths(const StorePathSet & paths) } -void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos, std::map pathsCA) +void LocalStore::querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos) { if (!settings.useSubstitutes) return; for (auto & sub : getDefaultSubstituters()) { for (auto & path : paths) { - auto subPath(path); + auto subPath(path.first); - auto ca = pathsCA.find(printStorePath(path)); // recompute store path so that we can use a different store root - if (ca != pathsCA.end() && (hasPrefix(ca->second, "fixed:") || hasPrefix(ca->second, "text:"))) { - subPath = makeFixedOutputPathFromCA(path.name(), ca->second); + if (path.second && (hasPrefix(*path.second, "fixed:") || hasPrefix(*path.second, "text:"))) { + subPath = makeFixedOutputPathFromCA(path.first.name(), *path.second); if (sub->storeDir == storeDir) - assert(subPath == path); - if (subPath != path) - debug("replaced path '%s' with '%s' for substituter '%s'", printStorePath(path), sub->printStorePath(subPath), sub->getUri()); + assert(subPath == path.first); + if (subPath != path.first) + debug("replaced path '%s' with '%s' for substituter '%s'", printStorePath(path.first), sub->printStorePath(subPath), sub->getUri()); } else if (sub->storeDir != storeDir) continue; debug("checking substituter '%s' for path '%s'", sub->getUri(), sub->printStorePath(subPath)); @@ -868,7 +866,7 @@ void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths, auto narInfo = std::dynamic_pointer_cast( std::shared_ptr(info)); - infos.insert_or_assign(path, SubstitutablePathInfo{ + infos.insert_or_assign(path.first, SubstitutablePathInfo{ info->deriver, info->references, narInfo ? narInfo->fileSize : 0, diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 183d27c8002..ab9fbfbe40c 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -139,9 +139,8 @@ public: StorePathSet querySubstitutablePaths(const StorePathSet & paths) override; - void querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos, - std::map pathsCA = {}) override; + void querySubstitutablePathInfos(const StorePathCAMap & paths, + SubstitutablePathInfos & infos) override; void addToStore(const ValidPathInfo & info, Source & source, RepairFlag repair, CheckSigsFlag checkSigs, diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 6902dfa0c4d..dc991e227d2 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -178,10 +178,7 @@ void Store::queryMissing(const std::vector & targets, auto outPath = parseStorePath(outPathS); SubstitutablePathInfos infos; - std::map pathsCA = {}; - if (auto ca = getDerivationCA(*drv)) - pathsCA.insert({outPathS, *ca}); - querySubstitutablePathInfos({outPath}, infos, pathsCA); + querySubstitutablePathInfos({{outPath, getDerivationCA(*drv)}}, infos); if (infos.empty()) { drvState_->lock()->done = true; @@ -238,7 +235,7 @@ void Store::queryMissing(const std::vector & targets, if (isValidPath(path.path)) return; SubstitutablePathInfos infos; - querySubstitutablePathInfos({path.path}, infos); + querySubstitutablePathInfos({{path.path, std::nullopt}}, infos); if (infos.empty()) { auto state(state_.lock()); diff --git a/src/libstore/path.hh b/src/libstore/path.hh index aaebd3ec369..91962d114c1 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -62,6 +62,8 @@ public: typedef std::set StorePathSet; typedef std::vector StorePaths; +typedef std::map> StorePathCAMap; + /* Extension of derivations in the Nix store. */ const std::string drvExtension = ".drv"; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 396629a324b..4538ccd4fb4 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -308,18 +308,17 @@ StorePathSet RemoteStore::querySubstitutablePaths(const StorePathSet & paths) } -void RemoteStore::querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos, std::map pathsCA) +void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, SubstitutablePathInfos & infos) { - if (paths.empty()) return; + if (pathsMap.empty()) return; auto conn(getConnection()); if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) { - for (auto & i : paths) { + for (auto & i : pathsMap) { SubstitutablePathInfo info; - conn->to << wopQuerySubstitutablePathInfo << printStorePath(i); + conn->to << wopQuerySubstitutablePathInfo << printStorePath(i.first); conn.processStderr(); unsigned int reply = readInt(conn->from); if (reply == 0) continue; @@ -329,12 +328,15 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathSet & paths, info.references = readStorePaths(*this, conn->from); info.downloadSize = readLongLong(conn->from); info.narSize = readLongLong(conn->from); - infos.insert_or_assign(i, std::move(info)); + infos.insert_or_assign(i.first, std::move(info)); } } else { conn->to << wopQuerySubstitutablePathInfos; + StorePathSet paths; + for (auto & path : pathsMap) + paths.insert(path.first); writeStorePaths(*this, conn->to, paths); conn.processStderr(); size_t count = readNum(conn->from); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 498a5c6a002..07ed2b51a1d 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -55,8 +55,8 @@ public: StorePathSet querySubstitutablePaths(const StorePathSet & paths) override; - void querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos, std::map pathsCA = {}) override; + void querySubstitutablePathInfos(const StorePathCAMap & paths, + SubstitutablePathInfos & infos) override; void addToStore(const ValidPathInfo & info, Source & nar, RepairFlag repair, CheckSigsFlag checkSigs, diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 65954574f45..12a6b6939ec 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -445,8 +445,8 @@ public: /* Query substitute info (i.e. references, derivers and download sizes) of a set of paths. If a path does not have substitute info, it's omitted from the resulting ‘infos’ map. */ - virtual void querySubstitutablePathInfos(const StorePathSet & paths, - SubstitutablePathInfos & infos, std::map pathsCA = {}) { return; }; + virtual void querySubstitutablePathInfos(const StorePathCAMap & paths, + SubstitutablePathInfos & infos) { return; }; /* Import a path into the store. */ virtual void addToStore(const ValidPathInfo & info, Source & narSource, From 91445eded9d675d4546b1193ce4a9f0b2ebfbe5b Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 19 Jun 2020 13:55:34 -0400 Subject: [PATCH 20/27] Fix build, use ContentAddress and FixedOutputHash --- src/libstore/build.cc | 14 ++++++------- src/libstore/builtins/fetchurl.cc | 13 ------------ src/libstore/local-store.cc | 2 +- src/libstore/misc.cc | 4 ++-- src/libstore/path.hh | 2 +- src/libstore/store-api.cc | 34 +++++++++++++++---------------- src/libstore/store-api.hh | 4 ++-- 7 files changed, 30 insertions(+), 43 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index ec2cfbed3ad..c57b259f341 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -297,7 +297,7 @@ class Worker GoalPtr makeDerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); std::shared_ptr makeBasicDerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, BuildMode buildMode = bmNormal); - GoalPtr makeSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); + GoalPtr makeSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); /* Remove a dead goal. */ void removeGoal(GoalPtr goal); @@ -4298,10 +4298,10 @@ class SubstitutionGoal : public Goal GoalState state; /* Content address for recomputing store path */ - std::optional ca; + std::optional ca; public: - SubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); + SubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); ~SubstitutionGoal(); void timedOut(Error && ex) override { abort(); }; @@ -4331,7 +4331,7 @@ class SubstitutionGoal : public Goal }; -SubstitutionGoal::SubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair, std::optional ca) +SubstitutionGoal::SubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair, std::optional ca) : Goal(worker) , storePath(storePath) , repair(repair) @@ -4411,7 +4411,7 @@ void SubstitutionGoal::tryNext() subs.pop_front(); auto subPath = storePath; - if (ca && (hasPrefix(*ca, "fixed:") || hasPrefix(*ca, "text:"))) { + if (ca) { subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca); if (sub->storeDir == worker.store.storeDir) assert(subPath == storePath); @@ -4540,7 +4540,7 @@ void SubstitutionGoal::tryToRun() PushActivity pact(act.id); auto subPath = storePath; - if (ca && (hasPrefix(*ca, "fixed:") || hasPrefix(*ca, "text:"))) { + if (ca) { subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca); if (sub->storeDir == worker.store.storeDir) assert(subPath == storePath); @@ -4680,7 +4680,7 @@ std::shared_ptr Worker::makeBasicDerivationGoal(const StorePath } -GoalPtr Worker::makeSubstitutionGoal(const StorePath & path, RepairFlag repair, std::optional ca) +GoalPtr Worker::makeSubstitutionGoal(const StorePath & path, RepairFlag repair, std::optional ca) { GoalPtr goal = substitutionGoals[path].lock(); // FIXME if (!goal) { diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 1cfe4a46a1a..28cebaa95f8 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -58,19 +58,6 @@ 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 = parseHashTypeOpt(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/local-store.cc b/src/libstore/local-store.cc index adbc521685d..d07f9ac43d0 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -851,7 +851,7 @@ void LocalStore::querySubstitutablePathInfos(const StorePathCAMap & paths, Subst auto subPath(path.first); // recompute store path so that we can use a different store root - if (path.second && (hasPrefix(*path.second, "fixed:") || hasPrefix(*path.second, "text:"))) { + if (path.second) { subPath = makeFixedOutputPathFromCA(path.first.name(), *path.second); if (sub->storeDir == storeDir) assert(subPath == path.first); diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index dc991e227d2..53e7c6feb5f 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -108,7 +108,7 @@ void Store::computeFSClosure(const StorePath & startPath, } -std::optional getDerivationCA(const BasicDerivation & drv) +std::optional getDerivationCA(const BasicDerivation & drv) { auto outputHashMode = drv.env.find("outputHashMode"); auto outputHashAlgo = drv.env.find("outputHashAlgo"); @@ -123,7 +123,7 @@ std::optional getDerivationCA(const BasicDerivation & drv) ingestionMethod = FileIngestionMethod::Flat; else throw Error("unknown ingestion method: '%s'", outputHashMode->second); - return makeFixedOutputCA(ingestionMethod, h); + return FixedOutputHash{ingestionMethod, h}; } return std::nullopt; diff --git a/src/libstore/path.hh b/src/libstore/path.hh index 80c36b7f07a..a07186323ad 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -63,7 +63,7 @@ public: typedef std::set StorePathSet; typedef std::vector StorePaths; -typedef std::map> StorePathCAMap; +typedef std::map> StorePathCAMap; /* Extension of derivations in the Nix store. */ const std::string drvExtension = ".drv"; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index cfbdeb794ae..31610b0b45c 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -192,18 +192,22 @@ StorePath Store::makeFixedOutputPath( } } -StorePath Store::makeFixedOutputPathFromCA(std::string_view name, std::string ca, +// FIXME Put this somewhere? +template struct overloaded : Ts... { using Ts::operator()...; }; +template overloaded(Ts...) -> overloaded; + +StorePath Store::makeFixedOutputPathFromCA(std::string_view name, ContentAddress ca, const StorePathSet & references, bool hasSelfReference) const { - if (hasPrefix(ca, "fixed:")) { - FileIngestionMethod ingestionMethod { ca.compare(6, 2, "r:") == 0 }; - Hash hash(std::string(ca, ingestionMethod == FileIngestionMethod::Recursive ? 8 : 6)); - return makeFixedOutputPath(ingestionMethod, hash, name, references, hasSelfReference); - } else if (hasPrefix(ca, "text:")) { - Hash hash(std::string(ca, 5)); - return makeTextPath(name, hash, references); - } else - throw Error("'%s' is not a valid ca", ca); + // New template + return std::visit(overloaded { + [=](TextHash th) { + return makeTextPath(name, th.hash, references); + }, + [=](FixedOutputHash fsh) { + return makeFixedOutputPath(fsh.method, fsh.hash, name, references, hasSelfReference); + } + }, ca); } StorePath Store::makeTextPath(std::string_view name, const Hash & hash, @@ -597,7 +601,7 @@ void copyStorePath(ref srcStore, ref dstStore, // recompute store path on the chance dstStore does it differently if (info->isContentAddressed(*srcStore) && info->references.empty()) { auto info2 = make_ref(*info); - info2->path = dstStore->makeFixedOutputPathFromCA(info->path.name(), info->ca); + info2->path = dstStore->makeFixedOutputPathFromCA(info->path.name(), *info->ca); if (dstStore->storeDir == srcStore->storeDir) assert(info->path == info2->path); info = info2; @@ -671,7 +675,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st auto info = srcStore->queryPathInfo(storePath); auto storePathForDst = storePath; if (info->isContentAddressed(*srcStore) && info->references.empty()) { - storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); + storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), *info->ca); if (dstStore->storeDir == srcStore->storeDir) assert(storePathForDst == storePath); if (storePathForDst != storePath) @@ -698,7 +702,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st auto storePathForDst = storePath; if (info->isContentAddressed(*srcStore) && info->references.empty()) { - storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), info->ca); + storePathForDst = dstStore->makeFixedOutputPathFromCA(storePath.name(), *info->ca); if (dstStore->storeDir == srcStore->storeDir) assert(storePathForDst == storePath); if (storePathForDst != storePath) @@ -799,10 +803,6 @@ void ValidPathInfo::sign(const Store & store, const SecretKey & secretKey) sigs.insert(secretKey.signDetached(fingerprint(store))); } -// FIXME Put this somewhere? -template struct overloaded : Ts... { using Ts::operator()...; }; -template overloaded(Ts...) -> overloaded; - bool ValidPathInfo::isContentAddressed(const Store & store) const { if (! ca) return false; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 91abfdf14bc..c2682f3cc1c 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -345,7 +345,7 @@ public: StorePath makeTextPath(std::string_view name, const Hash & hash, const StorePathSet & references = {}) const; - StorePath makeFixedOutputPathFromCA(std::string_view name, std::string ca, + StorePath makeFixedOutputPathFromCA(std::string_view name, ContentAddress ca, const StorePathSet & references = {}, bool hasSelfReference = false) const; @@ -835,6 +835,6 @@ std::optional decodeValidPathInfo( /* Split URI into protocol+hierarchy part and its parameter set. */ std::pair splitUriAndParams(const std::string & uri); -std::optional getDerivationCA(const BasicDerivation & drv); +std::optional getDerivationCA(const BasicDerivation & drv); } From fb51cf5b1de55eb5a82382aa87c62a1f314257f1 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 19 Jun 2020 15:25:01 -0400 Subject: [PATCH 21/27] Just use the derivation outputs --- src/libstore/misc.cc | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 53e7c6feb5f..9c9a4e0e796 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -110,20 +110,9 @@ void Store::computeFSClosure(const StorePath & startPath, 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 ht = parseHashType(outputHashAlgo->second); - auto h = Hash(outputHash->second, ht); - FileIngestionMethod ingestionMethod; - if (outputHashMode->second == "recursive") - ingestionMethod = FileIngestionMethod::Recursive; - else if (outputHashMode->second == "flat") - ingestionMethod = FileIngestionMethod::Flat; - else - throw Error("unknown ingestion method: '%s'", outputHashMode->second); - return FixedOutputHash{ingestionMethod, h}; + if (drv.outputs.count("out") > 0) { + auto res = drv.outputs.find("out"); + return res->second.hash; } return std::nullopt; From 41e97f5ffcab8b01068c6e1ce20d489a6c03232a Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 19 Jun 2020 15:26:05 -0400 Subject: [PATCH 22/27] WIP print statements and only the test that fails --- src/libstore/build.cc | 1 + src/libstore/store-api.cc | 7 ++-- tests/local.mk | 73 ++++++++++++++++++++------------------- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 697c3fcfff3..1e5ec4b05dd 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4442,6 +4442,7 @@ void SubstitutionGoal::tryNext() } if (info->path != storePath) { + printError("Printing ca: %s", renderContentAddress(info->ca)); if (info->isContentAddressed(*sub) && info->references.empty()) { auto info2 = std::make_shared(*info); info2->path = storePath; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 31610b0b45c..6f381bf7041 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -824,8 +824,11 @@ bool ValidPathInfo::isContentAddressed(const Store & store) const bool res = caPath == path; - if (!res) - printError("warning: path '%s' claims to be content-addressed but isn't", store.printStorePath(path)); + if (!res) { + std::string p1 = store.printStorePath(caPath); + std::string p2 = store.printStorePath(path); + printError("warning: path '%s' claims to be content-addressed but isn't\n caPath: %s\n path: %s\n", store.printStorePath(path), p1, p2); + } return res; } diff --git a/tests/local.mk b/tests/local.mk index 536661af88e..7289d091d72 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -1,38 +1,41 @@ -nix_tests = \ - init.sh hash.sh lang.sh add.sh simple.sh dependencies.sh \ - config.sh \ - gc.sh \ - gc-concurrent.sh \ - gc-auto.sh \ - referrers.sh user-envs.sh logging.sh nix-build.sh misc.sh fixed.sh \ - gc-runtime.sh check-refs.sh filter-source.sh \ - remote-store.sh export.sh export-graph.sh \ - timeout.sh secure-drv-outputs.sh nix-channel.sh \ - multiple-outputs.sh import-derivation.sh fetchurl.sh optimise-store.sh \ - binary-cache.sh nix-profile.sh repair.sh dump-db.sh case-hack.sh \ - check-reqs.sh pass-as-file.sh tarball.sh restricted.sh \ - placeholders.sh nix-shell.sh \ - linux-sandbox.sh \ - build-dry.sh \ - build-remote.sh \ - nar-access.sh \ - structured-attrs.sh \ - fetchGit.sh \ - fetchGitRefs.sh \ - fetchGitSubmodules.sh \ - fetchMercurial.sh \ - signing.sh \ - shell.sh \ - brotli.sh \ - pure-eval.sh \ - check.sh \ - plugins.sh \ - search.sh \ - nix-copy-ssh.sh \ - post-hook.sh \ - function-trace.sh \ - recursive.sh - # parallel.sh +nix_tests = init.sh fetchurl.sh + + +# nix_tests = \ +# init.sh hash.sh lang.sh add.sh simple.sh dependencies.sh \ +# config.sh \ +# gc.sh \ +# gc-concurrent.sh \ +# gc-auto.sh \ +# referrers.sh user-envs.sh logging.sh nix-build.sh misc.sh fixed.sh \ +# gc-runtime.sh check-refs.sh filter-source.sh \ +# remote-store.sh export.sh export-graph.sh \ +# timeout.sh secure-drv-outputs.sh nix-channel.sh \ +# multiple-outputs.sh import-derivation.sh fetchurl.sh optimise-store.sh \ +# binary-cache.sh nix-profile.sh repair.sh dump-db.sh case-hack.sh \ +# check-reqs.sh pass-as-file.sh tarball.sh restricted.sh \ +# placeholders.sh nix-shell.sh \ +# linux-sandbox.sh \ +# build-dry.sh \ +# build-remote.sh \ +# nar-access.sh \ +# structured-attrs.sh \ +# fetchGit.sh \ +# fetchGitRefs.sh \ +# fetchGitSubmodules.sh \ +# fetchMercurial.sh \ +# signing.sh \ +# shell.sh \ +# brotli.sh \ +# pure-eval.sh \ +# check.sh \ +# plugins.sh \ +# search.sh \ +# nix-copy-ssh.sh \ +# post-hook.sh \ +# function-trace.sh \ +# recursive.sh +# # parallel.sh install-tests += $(foreach x, $(nix_tests), tests/$(x)) From 53426121488dc0c7db127bab0ee0ceadee8fdabe Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 19 Jun 2020 16:57:31 -0400 Subject: [PATCH 23/27] Use [&] instead of [=] this avoids a segmentation fault on macos: https://github.com/obsidiansystems/nix/runs/789252835 --- src/libstore/store-api.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 6f381bf7041..857ae65ce26 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -201,10 +201,10 @@ StorePath Store::makeFixedOutputPathFromCA(std::string_view name, ContentAddress { // New template return std::visit(overloaded { - [=](TextHash th) { + [&](TextHash th) { return makeTextPath(name, th.hash, references); }, - [=](FixedOutputHash fsh) { + [&](FixedOutputHash fsh) { return makeFixedOutputPath(fsh.method, fsh.hash, name, references, hasSelfReference); } }, ca); From f53ba6cf2e1a92f387b7575db14f4010444367a3 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 19 Jun 2020 17:09:35 -0400 Subject: [PATCH 24/27] Make add-to-store correctly add flat hash --- src/nix/add-to-store.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 865b24d943f..48a4722aec3 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -71,8 +71,8 @@ struct CmdAddToStore : MixDryRun, StoreCommand info.narHash = narHash; info.narSize = sink.s->size(); info.ca = std::optional { FixedOutputHash { - .method = FileIngestionMethod::Recursive, - .hash = info.narHash, + .method = ingestionMethod, + .hash = hash, } }; if (!dryRun) { From 648951468fce177ac5aef70290915c4967ec03db Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 19 Jun 2020 17:09:56 -0400 Subject: [PATCH 25/27] Revert "WIP print statements and only the test that fails" This reverts commit 41e97f5ffcab8b01068c6e1ce20d489a6c03232a. --- src/libstore/build.cc | 1 - src/libstore/store-api.cc | 7 ++-- tests/local.mk | 73 +++++++++++++++++++-------------------- 3 files changed, 37 insertions(+), 44 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 1e5ec4b05dd..697c3fcfff3 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4442,7 +4442,6 @@ void SubstitutionGoal::tryNext() } if (info->path != storePath) { - printError("Printing ca: %s", renderContentAddress(info->ca)); if (info->isContentAddressed(*sub) && info->references.empty()) { auto info2 = std::make_shared(*info); info2->path = storePath; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 857ae65ce26..3bb10564d93 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -824,11 +824,8 @@ bool ValidPathInfo::isContentAddressed(const Store & store) const bool res = caPath == path; - if (!res) { - std::string p1 = store.printStorePath(caPath); - std::string p2 = store.printStorePath(path); - printError("warning: path '%s' claims to be content-addressed but isn't\n caPath: %s\n path: %s\n", store.printStorePath(path), p1, p2); - } + if (!res) + printError("warning: path '%s' claims to be content-addressed but isn't", store.printStorePath(path)); return res; } diff --git a/tests/local.mk b/tests/local.mk index 7289d091d72..536661af88e 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -1,41 +1,38 @@ -nix_tests = init.sh fetchurl.sh - - -# nix_tests = \ -# init.sh hash.sh lang.sh add.sh simple.sh dependencies.sh \ -# config.sh \ -# gc.sh \ -# gc-concurrent.sh \ -# gc-auto.sh \ -# referrers.sh user-envs.sh logging.sh nix-build.sh misc.sh fixed.sh \ -# gc-runtime.sh check-refs.sh filter-source.sh \ -# remote-store.sh export.sh export-graph.sh \ -# timeout.sh secure-drv-outputs.sh nix-channel.sh \ -# multiple-outputs.sh import-derivation.sh fetchurl.sh optimise-store.sh \ -# binary-cache.sh nix-profile.sh repair.sh dump-db.sh case-hack.sh \ -# check-reqs.sh pass-as-file.sh tarball.sh restricted.sh \ -# placeholders.sh nix-shell.sh \ -# linux-sandbox.sh \ -# build-dry.sh \ -# build-remote.sh \ -# nar-access.sh \ -# structured-attrs.sh \ -# fetchGit.sh \ -# fetchGitRefs.sh \ -# fetchGitSubmodules.sh \ -# fetchMercurial.sh \ -# signing.sh \ -# shell.sh \ -# brotli.sh \ -# pure-eval.sh \ -# check.sh \ -# plugins.sh \ -# search.sh \ -# nix-copy-ssh.sh \ -# post-hook.sh \ -# function-trace.sh \ -# recursive.sh -# # parallel.sh +nix_tests = \ + init.sh hash.sh lang.sh add.sh simple.sh dependencies.sh \ + config.sh \ + gc.sh \ + gc-concurrent.sh \ + gc-auto.sh \ + referrers.sh user-envs.sh logging.sh nix-build.sh misc.sh fixed.sh \ + gc-runtime.sh check-refs.sh filter-source.sh \ + remote-store.sh export.sh export-graph.sh \ + timeout.sh secure-drv-outputs.sh nix-channel.sh \ + multiple-outputs.sh import-derivation.sh fetchurl.sh optimise-store.sh \ + binary-cache.sh nix-profile.sh repair.sh dump-db.sh case-hack.sh \ + check-reqs.sh pass-as-file.sh tarball.sh restricted.sh \ + placeholders.sh nix-shell.sh \ + linux-sandbox.sh \ + build-dry.sh \ + build-remote.sh \ + nar-access.sh \ + structured-attrs.sh \ + fetchGit.sh \ + fetchGitRefs.sh \ + fetchGitSubmodules.sh \ + fetchMercurial.sh \ + signing.sh \ + shell.sh \ + brotli.sh \ + pure-eval.sh \ + check.sh \ + plugins.sh \ + search.sh \ + nix-copy-ssh.sh \ + post-hook.sh \ + function-trace.sh \ + recursive.sh + # parallel.sh install-tests += $(foreach x, $(nix_tests), tests/$(x)) From 1bd1cfd33c872d0859fe589fe59a2dfa03d9d68c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 23:09:38 +0000 Subject: [PATCH 26/27] `addToStore` should always use NAR hash `addToStoreFromDump` would use the CA-specific method. --- src/libstore/local-store.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 7a713fbc488..b2361c7b93d 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1017,11 +1017,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, return n; }); - auto p = info.ca ? std::get_if(&*info.ca) : NULL; - if (p && p->method == FileIngestionMethod::Git) - restoreGit(realPath, wrapperSource, realStoreDir, storeDir); - else - restorePath(realPath, wrapperSource); + restorePath(realPath, wrapperSource); auto hashResult = hashSink->finish(); From 4a73bd046ba6343defdd31022ce4ca47fa5b4b87 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 23:09:38 +0000 Subject: [PATCH 27/27] `addToStore` should always use NAR hash `addToStoreFromDump` would use the CA-specific method. --- src/libstore/local-store.cc | 5 +---- src/nix/add-to-store.cc | 29 ++++++++++++++++++++--------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 69d3997778e..fe083b8e878 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1005,10 +1005,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, return n; }); - if (hasPrefix(info.ca, "fixed:git:")) - restoreGit(realPath, wrapperSource, realStoreDir, storeDir); - else - restorePath(realPath, wrapperSource); + restorePath(realPath, wrapperSource); auto hashResult = hashSink->finish(); diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index b70eeccd8db..9541e5d1ac9 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -10,7 +10,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand { Path path; std::optional namePart; - bool git = false; + FileIngestionMethod ingestionMethod = FileIngestionMethod::Recursive; CmdAddToStore() { @@ -26,8 +26,9 @@ struct CmdAddToStore : MixDryRun, StoreCommand addFlag({ .longName = "git", + .shortName = 0, .description = "treat path as a git object", - .handler = {&this->git, true}, + .handler = {&ingestionMethod, FileIngestionMethod::Git}, }); } @@ -48,13 +49,25 @@ struct CmdAddToStore : MixDryRun, StoreCommand { if (!namePart) namePart = baseNameOf(path); - auto ingestionMethod = git ? FileIngestionMethod::Git : FileIngestionMethod::Recursive; - StringSink sink; dumpPath(path, sink); auto narHash = hashString(htSHA256, *sink.s); - auto hash = git ? dumpGitHash(htSHA1, path) : narHash; + + Hash hash; + switch (ingestionMethod) { + case FileIngestionMethod::Recursive: { + hash = narHash; + break; + } + case FileIngestionMethod::Flat: { + abort(); // not yet supported above + } + case FileIngestionMethod::Git: { + hash = dumpGitHash(htSHA1, path); + break; + } + } ValidPathInfo info(store->makeFixedOutputPath(ingestionMethod, hash, *namePart)); info.narHash = narHash; @@ -62,10 +75,8 @@ struct CmdAddToStore : MixDryRun, StoreCommand info.ca = makeFixedOutputCA(ingestionMethod, hash); if (!dryRun) { - auto addedPath = store->addToStore(*namePart, path, ingestionMethod, git ? htSHA1 : htSHA256); - if (addedPath != info.path) - throw Error("Added path %s does not match calculated path %s; something has changed", - addedPath.to_string(), info.path.to_string()); + auto source = StringSource { *sink.s }; + store->addToStore(info, source); } logger->stdout("%s", store->printStorePath(info.path));