From 79c169d1c615211af69c1cbc6218f7465a4f81ed Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 12 Jun 2020 09:49:09 -0500 Subject: [PATCH 01/37] 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 80851b591..6385453cc 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 c1e75390c..5a976e3e1 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 9c47fe524..1b0a055d3 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 5c36693e6..956980f49 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 3c86b4524..867d650bc 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 095363d0c..40000da01 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 b1e25fc7d..6f961329c 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/37] 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 1fa74a143..9c55526a3 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 486babf14..254cb4fce 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 da95fd3ae..2e1e405b3 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/37] 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 6385453cc..4cecf0992 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/37] 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 f43f774c1..a5a77f17c 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/37] 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 4cecf0992..bf005a3b3 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)); - try { - auto info = sub->queryPathInfo(path); + for (auto & path : paths) { + auto subPath(path.clone()); - 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 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(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/37] 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 40000da01..1b340bf6c 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); - if (!dstStore->isValidPath(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)) { 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/37] 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 5f25d04cb..5f4997e28 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 2535651b0..13447d774 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/37] =?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 1b340bf6c..afa557e16 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/37] 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 afa557e16..c8dc4744f 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/37] 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 a5a77f17c..26edcf95a 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/37] 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 c8dc4744f..7cb48b293 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 13447d774..510f98843 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/37] 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 510f98843..f4b86d251 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/37] 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 bf005a3b3..9158a52e0 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 7cb48b293..bc9a6c4f5 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 6f961329c..9412785b4 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/37] 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) \ From 9ac0d98a59ed042f698685234109d4b69bccf29c Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Sat, 13 Jun 2020 00:14:30 -0500 Subject: [PATCH 15/37] 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 aaa38f2ed..0f2044342 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/37] 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 bc9a6c4f5..2593b245c 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 26edcf95a..7abb82556 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/37] 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 9753db7b1..bc67ae31e 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 a80026258..d08c7dc6a 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 a28895086..80a69717e 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/37] 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 bc67ae31e..63cb70ef2 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 d08c7dc6a..3230312c9 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 80a69717e..c8097da52 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/37] 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 e370e278c..f7dc16948 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 3230312c9..3901d532a 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 183d27c80..ab9fbfbe4 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 6902dfa0c..dc991e227 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 aaebd3ec3..91962d114 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 396629a32..4538ccd4f 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 498a5c6a0..07ed2b51a 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 65954574f..12a6b6939 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 0c9c1b8826395a9023606cd7749aa16ef6771941 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 19 Jun 2020 14:48:57 -0400 Subject: [PATCH 20/37] Return map of StorePaths in copyPaths This allows the caller to know what values were actually added to the store. --- src/libstore/store-api.cc | 12 ++++++++++-- src/libstore/store-api.hh | 17 ++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 661dea141..0458fd9a6 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -638,7 +638,7 @@ void copyStorePath(ref srcStore, ref dstStore, } -void copyPaths(ref srcStore, ref dstStore, const StorePathSet & storePaths, +std::map copyPaths(ref srcStore, ref dstStore, const StorePathSet & storePaths, RepairFlag repair, CheckSigsFlag checkSigs, SubstituteFlag substitute) { auto valid = dstStore->queryValidPaths(storePaths, substitute); @@ -647,7 +647,11 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st for (auto & path : storePaths) if (!valid.count(path)) missing.insert(srcStore->printStorePath(path)); - if (missing.empty()) return; + std::map pathsMap; + for (auto & path : storePaths) + pathsMap.insert_or_assign(path, path); + + if (missing.empty()) return pathsMap; Activity act(*logger, lvlInfo, actCopyPaths, fmt("copying %d paths", missing.size())); @@ -677,6 +681,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st if (storePathForDst != storePath) debug("replaced path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } + pathsMap.insert_or_assign(storePath, storePathForDst); if (dstStore->isValidPath(storePathForDst)) { nrDone++; @@ -704,6 +709,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st if (storePathForDst != storePath) debug("replaced path '%s' to '%s' for substituter '%s'", srcStore->printStorePath(storePath), dstStore->printStorePath(storePathForDst), dstStore->getUri()); } + pathsMap.insert_or_assign(storePath, storePathForDst); if (!dstStore->isValidPath(storePathForDst)) { MaintainCount mc(nrRunning); @@ -723,6 +729,8 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st nrDone++; showProgress(); }); + + return pathsMap; } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index fd3652588..2b12879e3 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -444,8 +444,9 @@ public: virtual StorePathSet querySubstitutablePaths(const StorePathSet & paths) { return {}; }; /* 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. */ + sizes) of a map of paths to their optional ca values. If a path + does not have substitute info, it's omitted from the resulting + ‘infos’ map. */ virtual void querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos) { return; }; @@ -743,11 +744,13 @@ void copyStorePath(ref srcStore, ref dstStore, /* Copy store paths from one store to another. The paths may be copied - in parallel. They are copied in a topologically sorted order - (i.e. if A is a reference of B, then A is copied before B), but - the set of store paths is not automatically closed; use - copyClosure() for that. */ -void copyPaths(ref srcStore, ref dstStore, const StorePathSet & storePaths, + in parallel. They are copied in a topologically sorted order (i.e. + if A is a reference of B, then A is copied before B), but the set + of store paths is not automatically closed; use copyClosure() for + that. Returns a map of what each path was copied to the dstStore + as. */ +std::map copyPaths(ref srcStore, ref dstStore, + const StorePathSet & storePaths, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs, SubstituteFlag substitute = NoSubstitute); From e8e1f5282ff213be010cbe45772b9e725c01f285 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 19 Jun 2020 15:19:09 -0400 Subject: [PATCH 21/37] Replace error message in getDerivationCA --- src/libstore/misc.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 128c3c987..a01e81b78 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -126,7 +126,7 @@ std::optional getDerivationCA(const BasicDerivation & drv) else if (outputHashMode->second == "flat") ingestionMethod = FileIngestionMethod::Flat; else - throw Error("unknown ingestion method: '%s'", outputHashMode->second); + throw Error("unknown outputHashMode: '%s'", outputHashMode->second); return makeFixedOutputCA(ingestionMethod, h); } From f2a6cee334255ced72a70f527cf3c283b4586e42 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 19 Jun 2020 18:06:19 -0400 Subject: [PATCH 22/37] Update worker protocol to support sending storepath maps We need to also send the ca to daemon in addition to the path. --- src/libstore/daemon.cc | 11 +++++++---- src/libstore/remote-store.cc | 32 ++++++++++++++++++++++++++++---- src/libstore/worker-protocol.hh | 6 +++++- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index f7dc16948..c613ab47d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -609,12 +609,15 @@ static void performOp(TunnelLogger * logger, ref store, } case wopQuerySubstitutablePathInfos: { - auto paths = readStorePaths(*store, from); - logger->startWork(); SubstitutablePathInfos infos; StorePathCAMap pathsMap = {}; - for (auto & path : paths) - pathsMap.emplace(path, std::nullopt); + if (GET_PROTOCOL_MINOR(clientVersion) < 22) { + auto paths = readStorePaths(*store, from); + for (auto & path : paths) + pathsMap.emplace(path, std::nullopt); + } else + pathsMap = readStorePathCAMap(*store, from); + logger->startWork(); store->querySubstitutablePathInfos(pathsMap, infos); logger->stopWork(); to << infos.size(); diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 4538ccd4f..f6158685c 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -38,6 +38,27 @@ void writeStorePaths(const Store & store, Sink & out, const StorePathSet & paths out << store.printStorePath(i); } +std::map> readStorePathCAMap(const Store & store, Source & from) +{ + StorePathCAMap paths; + auto count = readNum(from); + while (count--) { + auto path = readString(from); + auto ca = readString(from); + paths.insert_or_assign(store.parseStorePath(path), !ca.empty() ? std::optional(ca) : std::nullopt); + } + return paths; +} + +void writeStorePathCAMap(const Store & store, Sink & out, const StorePathCAMap & paths) +{ + out << paths.size(); + for (auto & i : paths) { + out << store.printStorePath(i.first); + out << (i.second ? *i.second : ""); + } +} + /* TODO: Separate these store impls into different files, give them better names */ RemoteStore::RemoteStore(const Params & params) @@ -334,10 +355,13 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S } else { conn->to << wopQuerySubstitutablePathInfos; - StorePathSet paths; - for (auto & path : pathsMap) - paths.insert(path.first); - writeStorePaths(*this, conn->to, paths); + if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 22) { + StorePathSet paths; + for (auto & path : pathsMap) + paths.insert(path.first); + writeStorePaths(*this, conn->to, paths); + } else + writeStorePathCAMap(*this, conn->to, pathsMap); conn.processStderr(); size_t count = readNum(conn->from); for (size_t n = 0; n < count; n++) { diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index ac42457fc..d2944674f 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -6,7 +6,7 @@ namespace nix { #define WORKER_MAGIC_1 0x6e697863 #define WORKER_MAGIC_2 0x6478696f -#define PROTOCOL_VERSION 0x115 +#define PROTOCOL_VERSION 0x116 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) @@ -69,5 +69,9 @@ template T readStorePaths(const Store & store, Source & from); void writeStorePaths(const Store & store, Sink & out, const StorePathSet & paths); +std::map> readStorePathCAMap(const Store & store, Source & from); + +void writeStorePathCAMap(const Store & store, Sink & out, const StorePathCAMap & paths); + } From 1f9cb06db23f357f06404969820be1d3d4118a18 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Thu, 2 Jul 2020 10:59:24 -0400 Subject: [PATCH 23/37] Try next when no ca exists and have different store dirs --- src/libstore/build.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 95280ef1e..20d3c7387 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4421,6 +4421,9 @@ void SubstitutionGoal::tryNext() subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca); if (sub->storeDir == worker.store.storeDir) assert(subPath == storePath); + } else if (sub->storeDir != worker.store.storeDir) { + tryNext(); + return; } try { From d2e8b9ff0e8048d7b3914811b2d4bd535feac49d Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Thu, 2 Jul 2020 11:12:05 -0400 Subject: [PATCH 24/37] Store subPath in SubstitutionGoal --- src/libstore/build.cc | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 20d3c7387..12e4b6c3b 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4270,6 +4270,10 @@ private: /* The store path that should be realised through a substitute. */ StorePath storePath; + /* The path the substituter refers to the path as. This will be + * different when the stores have different names. */ + std::optional subPath; + /* The remaining substituters. */ std::list> subs; @@ -4416,7 +4420,6 @@ void SubstitutionGoal::tryNext() sub = subs.front(); subs.pop_front(); - auto subPath = storePath; if (ca) { subPath = sub->makeFixedOutputPathFromCA(storePath.name(), *ca); if (sub->storeDir == worker.store.storeDir) @@ -4428,7 +4431,7 @@ void SubstitutionGoal::tryNext() try { // FIXME: make async - info = sub->queryPathInfo(subPath); + info = sub->queryPathInfo(subPath ? *subPath : storePath); } catch (InvalidPath &) { tryNext(); return; @@ -4548,15 +4551,8 @@ void SubstitutionGoal::tryToRun() Activity act(*logger, actSubstitute, Logger::Fields{worker.store.printStorePath(storePath), sub->getUri()}); PushActivity pact(act.id); - auto subPath = storePath; - if (ca) { - 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); + subPath ? *subPath : storePath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs); promise.set_value(); } catch (...) { From f74243846512ffabf082985bca395890c97643e0 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 29 Apr 2020 22:39:58 +0200 Subject: [PATCH 25/37] Merge legacy `fetchGit`-builtin with the generic `fetchTree`-function The original idea was to implement a git-fetcher in Nix's core that supports content hashes[1]. In #3549[2] it has been suggested to actually use `fetchTree` for this since it's a fairly generic wrapper over the new fetcher-API[3] and already supports content-hashes. This patch implements a new git-fetcher based on `fetchTree` by incorporating the following changes: * Removed the original `fetchGit`-implementation and replaced it with an alias on the `fetchTree` implementation. * Ensured that the `git`-fetcher from `libfetchers` always computes a content-hash and returns an "empty" revision on dirty trees (the latter one is needed to retain backwards-compatibility). * The hash-mismatch error in the fetcher-API exits with code 102 as it usually happens whenever a hash-mismatch is detected by Nix. * Removed the `flakes`-feature-flag: I didn't see a reason why this API is so tightly coupled to the flakes-API and at least `fetchGit` should remain usable without any feature-flags. * It's only possible to specify a `narHash` for a `git`-tree if either a `ref` or a `rev` is given[4]. * It's now possible to specify an URL without a protocol. If it's missing, `file://` is automatically added as it was the case in the original `fetchGit`-implementation. [1] https://github.com/NixOS/nix/pull/3216 [2] https://github.com/NixOS/nix/pull/3549#issuecomment-625194383 [3] https://github.com/NixOS/nix/pull/3459 [4] https://github.com/NixOS/nix/pull/3216#issuecomment-553956703 --- src/libexpr/primops/fetchGit.cc | 91 -------------------------------- src/libexpr/primops/fetchTree.cc | 62 +++++++++++++++++++--- src/libfetchers/fetchers.cc | 2 +- tests/fetchGit.sh | 12 +++-- tests/tarball.sh | 11 ++-- 5 files changed, 73 insertions(+), 105 deletions(-) delete mode 100644 src/libexpr/primops/fetchGit.cc diff --git a/src/libexpr/primops/fetchGit.cc b/src/libexpr/primops/fetchGit.cc deleted file mode 100644 index 5013e74f0..000000000 --- a/src/libexpr/primops/fetchGit.cc +++ /dev/null @@ -1,91 +0,0 @@ -#include "primops.hh" -#include "eval-inline.hh" -#include "store-api.hh" -#include "hash.hh" -#include "fetchers.hh" -#include "url.hh" - -namespace nix { - -static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Value & v) -{ - std::string url; - std::optional ref; - std::optional rev; - std::string name = "source"; - bool fetchSubmodules = false; - PathSet context; - - state.forceValue(*args[0]); - - if (args[0]->type == tAttrs) { - - state.forceAttrs(*args[0], pos); - - for (auto & attr : *args[0]->attrs) { - string n(attr.name); - if (n == "url") - url = state.coerceToString(*attr.pos, *attr.value, context, false, false); - else if (n == "ref") - ref = state.forceStringNoCtx(*attr.value, *attr.pos); - else if (n == "rev") - rev = Hash(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA1); - else if (n == "name") - name = state.forceStringNoCtx(*attr.value, *attr.pos); - else if (n == "submodules") - fetchSubmodules = state.forceBool(*attr.value, *attr.pos); - else - throw EvalError({ - .hint = hintfmt("unsupported argument '%s' to 'fetchGit'", attr.name), - .errPos = *attr.pos - }); - } - - if (url.empty()) - throw EvalError({ - .hint = hintfmt("'url' argument required"), - .errPos = pos - }); - - } else - url = state.coerceToString(pos, *args[0], context, false, false); - - // FIXME: git externals probably can be used to bypass the URI - // whitelist. Ah well. - state.checkURI(url); - - if (evalSettings.pureEval && !rev) - throw Error("in pure evaluation mode, 'fetchGit' requires a Git revision"); - - fetchers::Attrs attrs; - attrs.insert_or_assign("type", "git"); - attrs.insert_or_assign("url", url.find("://") != std::string::npos ? url : "file://" + url); - if (ref) attrs.insert_or_assign("ref", *ref); - if (rev) attrs.insert_or_assign("rev", rev->gitRev()); - if (fetchSubmodules) attrs.insert_or_assign("submodules", fetchers::Explicit{true}); - auto input = fetchers::Input::fromAttrs(std::move(attrs)); - - // FIXME: use name? - auto [tree, input2] = input.fetch(state.store); - - state.mkAttrs(v, 8); - auto storePath = state.store->printStorePath(tree.storePath); - mkString(*state.allocAttr(v, state.sOutPath), storePath, PathSet({storePath})); - // Backward compatibility: set 'rev' to - // 0000000000000000000000000000000000000000 for a dirty tree. - auto rev2 = input2.getRev().value_or(Hash(htSHA1)); - mkString(*state.allocAttr(v, state.symbols.create("rev")), rev2.gitRev()); - mkString(*state.allocAttr(v, state.symbols.create("shortRev")), rev2.gitShortRev()); - // Backward compatibility: set 'revCount' to 0 for a dirty tree. - mkInt(*state.allocAttr(v, state.symbols.create("revCount")), - input2.getRevCount().value_or(0)); - mkBool(*state.allocAttr(v, state.symbols.create("submodules")), fetchSubmodules); - v.attrs->sort(); - - if (state.allowedPaths) - state.allowedPaths->insert(tree.actualPath); -} - -static RegisterPrimOp r("fetchGit", 1, prim_fetchGit); - -} diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 5f480d919..887d2d14f 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -36,6 +36,9 @@ void emitTreeAttrs( mkString(*state.allocAttr(v, state.symbols.create("shortRev")), rev->gitShortRev()); } + if (input.getType() == "git") + mkBool(*state.allocAttr(v, state.symbols.create("submodules")), maybeGetBoolAttr(input.attrs, "submodules").value_or(false)); + if (auto revCount = input.getRevCount()) mkInt(*state.allocAttr(v, state.symbols.create("revCount")), *revCount); @@ -48,10 +51,25 @@ void emitTreeAttrs( v.attrs->sort(); } -static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, Value & v) +std::string fixURI(std::string uri, EvalState &state) { - settings.requireExperimentalFeature("flakes"); + state.checkURI(uri); + return uri.find("://") != std::string::npos ? uri : "file://" + uri; +} +void addURI(EvalState &state, fetchers::Attrs &attrs, Symbol name, std::string v) +{ + string n(name); + attrs.emplace(name, n == "url" ? fixURI(v, state) : v); +} + +static void fetchTree( + EvalState &state, + const Pos &pos, + Value **args, + Value &v, + const std::optional type +) { fetchers::Input input; PathSet context; @@ -64,8 +82,15 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V for (auto & attr : *args[0]->attrs) { state.forceValue(*attr.value); - if (attr.value->type == tString) - attrs.emplace(attr.name, attr.value->string.s); + if (attr.value->type == tPath || attr.value->type == tString) + addURI( + state, + attrs, + attr.name, + state.coerceToString(*attr.pos, *attr.value, context, false, false) + ); + else if (attr.value->type == tString) + addURI(state, attrs, attr.name, attr.value->string.s); else if (attr.value->type == tBool) attrs.emplace(attr.name, fetchers::Explicit{attr.value->boolean}); else if (attr.value->type == tInt) @@ -75,6 +100,9 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V attr.name, showType(*attr.value)); } + if (type) + attrs.emplace("type", type.value()); + if (!attrs.count("type")) throw Error({ .hint = hintfmt("attribute 'type' is missing in call to 'fetchTree'"), @@ -82,8 +110,18 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V }); input = fetchers::Input::fromAttrs(std::move(attrs)); - } else - input = fetchers::Input::fromURL(state.coerceToString(pos, *args[0], context, false, false)); + } else { + auto url = fixURI(state.coerceToString(pos, *args[0], context, false, false), state); + + if (type == "git") { + fetchers::Attrs attrs; + attrs.emplace("type", "git"); + attrs.emplace("url", url); + input = fetchers::Input::fromAttrs(std::move(attrs)); + } else { + input = fetchers::Input::fromURL(url); + } + } if (!evalSettings.pureEval && !input.isDirect()) input = lookupInRegistries(state.store, input).first; @@ -99,6 +137,12 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V emitTreeAttrs(state, tree, input2, v); } +static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, Value & v) +{ + settings.requireExperimentalFeature("flakes"); + fetchTree(state, pos, args, v, std::nullopt); +} + static RegisterPrimOp r("fetchTree", 1, prim_fetchTree); static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v, @@ -178,7 +222,13 @@ static void prim_fetchTarball(EvalState & state, const Pos & pos, Value * * args fetch(state, pos, args, v, "fetchTarball", true, "source"); } +static void prim_fetchGit(EvalState &state, const Pos &pos, Value **args, Value &v) +{ + fetchTree(state, pos, args, v, "git"); +} + static RegisterPrimOp r2("__fetchurl", 1, prim_fetchurl); static RegisterPrimOp r3("fetchTarball", 1, prim_fetchTarball); +static RegisterPrimOp r4("fetchGit", 1, prim_fetchGit); } diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index c3ee9bf43..28db8aa9c 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -134,7 +134,7 @@ std::pair Input::fetch(ref store) const if (auto prevNarHash = getNarHash()) { if (narHash != *prevNarHash) - throw Error("NAR hash mismatch in input '%s' (%s), expected '%s', got '%s'", + throw Error((unsigned int) 102, "NAR hash mismatch in input '%s' (%s), expected '%s', got '%s'", to_string(), tree.actualPath, prevNarHash->to_string(SRI, true), narHash->to_string(SRI, true)); } diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index 9faa5d9f6..1b9364e60 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -32,6 +32,8 @@ rev2=$(git -C $repo rev-parse HEAD) # Fetch a worktree unset _NIX_FORCE_HTTP path0=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$TEST_ROOT/worktree).outPath") +path0_=$(nix eval --impure --raw --expr "(builtins.fetchTree { type = \"git\"; url = file://$TEST_ROOT/worktree; }).outPath") +[[ $path0 = $path0_ ]] export _NIX_FORCE_HTTP=1 [[ $(tail -n 1 $path0/hello) = "hello" ]] @@ -87,8 +89,6 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") [ ! -e $path2/.git ] [[ $(cat $path2/dir1/foo) = foo ]] -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] - # ... unless we're using an explicit ref or rev. path3=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"master\"; }).outPath") [[ $path = $path3 ]] @@ -102,6 +102,12 @@ git -C $repo commit -m 'Bla3' -a path4=$(nix eval --impure --refresh --raw --expr "(builtins.fetchGit file://$repo).outPath") [[ $path2 = $path4 ]] +nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-B5yIPHhEm0eysJKEsO7nqxprh9vcblFxpJG11gXJus1=\"; }).outPath" || status=$? +[[ "$status" = "102" ]] + +path5=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-Hr8g6AqANb3xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath") +[[ $path = $path5 ]] + # tarball-ttl should be ignored if we specify a rev echo delft > $repo/hello git -C $repo add hello @@ -123,7 +129,7 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath # Using local path with branch other than 'master' should work when clean or dirty path3=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") # (check dirty-tree handling was used) -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] +[[ $(nix eval --impure --expr "(builtins.fetchGit $repo).rev or null") = null ]] # Committing shouldn't change store path, or switch to using 'master' git -C $repo commit -m 'Bla5' -a diff --git a/tests/tarball.sh b/tests/tarball.sh index b3ec16d40..88a1a07a0 100644 --- a/tests/tarball.sh +++ b/tests/tarball.sh @@ -27,10 +27,13 @@ test_tarball() { nix-build -o $TEST_ROOT/result -E "import (fetchTarball file://$tarball)" - nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree file://$tarball)" - nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; })" - nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })" - nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"sha256-xdKv2pq/IiwLSnBBJXW8hNowI4MrdZfW+SYqDQs7Tzc=\"; })" 2>&1 | grep 'NAR hash mismatch in input' + nix-build -o $TEST_ROOT/result -E "import (fetchTree file://$tarball)" + nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; })" + nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })" + nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"sha256-xdKv2pq/IiwLSnBBJXW8hNowI4MrdZfW+SYqDQs7Tzc=\"; })" 2>&1 | grep 'NAR hash mismatch in input' + + nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" >&2 + nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" 2>&1 | grep 'true' nix-instantiate --eval -E '1 + 2' -I fnord=file://no-such-tarball.tar$ext nix-instantiate --eval -E 'with ; 1 + 2' -I fnord=file://no-such-tarball$ext From 189e6f5e1d949f50ab0b6e5acd25e230d206692d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 24 Jul 2020 20:51:16 +0200 Subject: [PATCH 26/37] Bump version to 3.0 Since there are some incompatible changes, it's better to bump the major version number. --- .version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.version b/.version index 7208c2182..f398a2061 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -2.4 \ No newline at end of file +3.0 \ No newline at end of file From b8eea7e81af53905be7845dffc6d0a83ea8edc97 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 28 Jul 2020 13:27:56 +0200 Subject: [PATCH 27/37] Remove putBytes istream->tellg() returns -1 so we can't get the number of bytes written. Fixes 'uploaded 's3://nix-cache/nar/00819r9lp5kajr6baxfw5dhhc0cx8ndxaz43qmd2f0gn1hk1ynlp.nar.xz' (-1 bytes) in 11620 ms' messages. --- src/libstore/s3-binary-cache-store.cc | 7 ++----- src/libstore/s3-binary-cache-store.hh | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 1b7dff085..67935f3ba 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -343,13 +343,10 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore std::chrono::duration_cast(now2 - now1) .count(); - auto size = istream->tellg(); - - printInfo("uploaded 's3://%s/%s' (%d bytes) in %d ms", - bucketName, path, size, duration); + printInfo("uploaded 's3://%s/%s' in %d ms", + bucketName, path, duration); stats.putTimeMs += duration; - stats.putBytes += size; stats.put++; } diff --git a/src/libstore/s3-binary-cache-store.hh b/src/libstore/s3-binary-cache-store.hh index 4d43fe4d2..b2b75d498 100644 --- a/src/libstore/s3-binary-cache-store.hh +++ b/src/libstore/s3-binary-cache-store.hh @@ -19,7 +19,6 @@ public: struct Stats { std::atomic put{0}; - std::atomic putBytes{0}; std::atomic putTimeMs{0}; std::atomic get{0}; std::atomic getBytes{0}; From e4940e90f399c5cde5f44f099ca5cdf5340d22b8 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 28 Jul 2020 22:46:39 +0200 Subject: [PATCH 28/37] Restore backwards-compat for current `builtins.fetchGit` If a repo is dirty, it used to return a `rev` object with an "empty" sha1 (0000000000000000000000000000000000000000). Please note that this only applies for `builtins.fetchGit` and *not* for `builtins.fetchTree{ type = "git"; }`. --- src/libexpr/flake/flake.hh | 2 +- src/libexpr/primops/fetchTree.cc | 17 +++++++++++++---- tests/fetchGit.sh | 4 +++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh index 77f3abdeb..c2bb2888b 100644 --- a/src/libexpr/flake/flake.hh +++ b/src/libexpr/flake/flake.hh @@ -106,6 +106,6 @@ void emitTreeAttrs( EvalState & state, const fetchers::Tree & tree, const fetchers::Input & input, - Value & v); + Value & v, bool emptyRevFallback = false); } diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 887d2d14f..cddcf0e59 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -14,7 +14,8 @@ void emitTreeAttrs( EvalState & state, const fetchers::Tree & tree, const fetchers::Input & input, - Value & v) + Value & v, + bool emptyRevFallback) { assert(input.isImmutable()); @@ -34,6 +35,11 @@ void emitTreeAttrs( if (auto rev = input.getRev()) { mkString(*state.allocAttr(v, state.symbols.create("rev")), rev->gitRev()); mkString(*state.allocAttr(v, state.symbols.create("shortRev")), rev->gitShortRev()); + } else if (emptyRevFallback) { + // Backwards compat for `builtins.fetchGit`: dirty repos return an empty sha1 as rev + auto emptyHash = Hash(htSHA1); + mkString(*state.allocAttr(v, state.symbols.create("rev")), emptyHash.gitRev()); + mkString(*state.allocAttr(v, state.symbols.create("shortRev")), emptyHash.gitRev()); } if (input.getType() == "git") @@ -41,6 +47,8 @@ void emitTreeAttrs( if (auto revCount = input.getRevCount()) mkInt(*state.allocAttr(v, state.symbols.create("revCount")), *revCount); + else if (emptyRevFallback) + mkInt(*state.allocAttr(v, state.symbols.create("revCount")), 0); if (auto lastModified = input.getLastModified()) { mkInt(*state.allocAttr(v, state.symbols.create("lastModified")), *lastModified); @@ -68,7 +76,8 @@ static void fetchTree( const Pos &pos, Value **args, Value &v, - const std::optional type + const std::optional type, + bool emptyRevFallback = false ) { fetchers::Input input; PathSet context; @@ -134,7 +143,7 @@ static void fetchTree( if (state.allowedPaths) state.allowedPaths->insert(tree.actualPath); - emitTreeAttrs(state, tree, input2, v); + emitTreeAttrs(state, tree, input2, v, emptyRevFallback); } static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, Value & v) @@ -224,7 +233,7 @@ static void prim_fetchTarball(EvalState & state, const Pos & pos, Value * * args static void prim_fetchGit(EvalState &state, const Pos &pos, Value **args, Value &v) { - fetchTree(state, pos, args, v, "git"); + fetchTree(state, pos, args, v, "git", true); } static RegisterPrimOp r2("__fetchurl", 1, prim_fetchurl); diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index 1b9364e60..cedd796f7 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -89,6 +89,8 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") [ ! -e $path2/.git ] [[ $(cat $path2/dir1/foo) = foo ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] + # ... unless we're using an explicit ref or rev. path3=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"master\"; }).outPath") [[ $path = $path3 ]] @@ -129,7 +131,7 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath # Using local path with branch other than 'master' should work when clean or dirty path3=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") # (check dirty-tree handling was used) -[[ $(nix eval --impure --expr "(builtins.fetchGit $repo).rev or null") = null ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] # Committing shouldn't change store path, or switch to using 'master' git -C $repo commit -m 'Bla5' -a From c159f48a39835d5b2fe7c1ddd4467bc093ee251f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 29 Jul 2020 00:24:55 +0200 Subject: [PATCH 29/37] Cleanup --- src/libstore/store-api.cc | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b6e6952b7..c804399d2 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -729,9 +729,9 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st { auto valid = dstStore->queryValidPaths(storePaths, substitute); - PathSet missing; + StorePathSet missing; for (auto & path : storePaths) - if (!valid.count(path)) missing.insert(srcStore->printStorePath(path)); + if (!valid.count(path)) missing.insert(path); if (missing.empty()) return; @@ -748,29 +748,27 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st ThreadPool pool; - processGraph(pool, - PathSet(missing.begin(), missing.end()), + processGraph(pool, + StorePathSet(missing.begin(), missing.end()), - [&](const Path & storePath) { - if (dstStore->isValidPath(dstStore->parseStorePath(storePath))) { + [&](const StorePath & storePath) { + if (dstStore->isValidPath(storePath)) { nrDone++; showProgress(); - return PathSet(); + return StorePathSet(); } - auto info = srcStore->queryPathInfo(srcStore->parseStorePath(storePath)); + auto info = srcStore->queryPathInfo(storePath); bytesExpected += info->narSize; act.setExpected(actCopyPath, bytesExpected); - return srcStore->printStorePathSet(info->references); + return info->references; }, - [&](const Path & storePathS) { + [&](const StorePath & storePath) { checkInterrupt(); - auto storePath = dstStore->parseStorePath(storePathS); - if (!dstStore->isValidPath(storePath)) { MaintainCount mc(nrRunning); showProgress(); @@ -780,7 +778,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st nrFailed++; if (!settings.keepGoing) throw e; - logger->log(lvlError, fmt("could not copy %s: %s", storePathS, e.what())); + logger->log(lvlError, fmt("could not copy %s: %s", dstStore->printStorePath(storePath), e.what())); showProgress(); return; } From 4c0077a07d2ee5362bd8ffb0ea9ee053c759303d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 29 Jul 2020 00:48:39 +0200 Subject: [PATCH 30/37] Fix RemoteStore::addToStore() latency Since 6185d25e523a3cd223dd6f6aca10cf6ff15b4823, this was very latency-bound since it required a round-trip for every 32 KiB. So for example copying a 514 MiB closure over a virtual ethernet device with a articial delay of just 1 ms took 343s. Now it takes 2.7s. Fixes #3372. --- src/libstore/daemon.cc | 90 +++++++++++++++++++++++++++------ src/libstore/remote-store.cc | 81 +++++++++++++++++++++++++++-- src/libstore/worker-protocol.hh | 2 +- 3 files changed, 154 insertions(+), 19 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 503e04f92..478ae39ca 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -86,7 +86,7 @@ struct TunnelLogger : public Logger } /* startWork() means that we're starting an operation for which we - want to send out stderr to the client. */ + want to send out stderr to the client. */ void startWork() { auto state(state_.lock()); @@ -703,24 +703,84 @@ static void performOp(TunnelLogger * logger, ref store, if (!trusted) info.ultimate = false; - std::unique_ptr source; - if (GET_PROTOCOL_MINOR(clientVersion) >= 21) - source = std::make_unique(from, to); - else { - StringSink saved; - TeeSource tee { from, saved }; - ParseSink ether; - parseDump(ether, tee); - source = std::make_unique(std::move(*saved.s)); + if (GET_PROTOCOL_MINOR(clientVersion) >= 23) { + + struct FramedSource : Source + { + Source & from; + bool eof = false; + std::vector pending; + size_t pos = 0; + + FramedSource(Source & from) : from(from) + { } + + ~FramedSource() + { + if (!eof) { + while (true) { + auto n = readInt(from); + if (!n) break; + std::vector data(n); + from(data.data(), n); + } + } + } + + size_t read(unsigned char * data, size_t len) override + { + if (eof) throw EndOfFile("reached end of FramedSource"); + + if (pos >= pending.size()) { + size_t len = readInt(from); + if (!len) { + eof = true; + return 0; + } + pending = std::vector(len); + pos = 0; + from(pending.data(), len); + } + + auto n = std::min(len, pending.size() - pos); + memcpy(data, pending.data() + pos, n); + pos += n; + return n; + } + }; + + logger->startWork(); + + { + FramedSource source(from); + store->addToStore(info, source, (RepairFlag) repair, + dontCheckSigs ? NoCheckSigs : CheckSigs); + } + + logger->stopWork(); } - logger->startWork(); + else { + std::unique_ptr source; + if (GET_PROTOCOL_MINOR(clientVersion) >= 21) + source = std::make_unique(from, to); + else { + StringSink saved; + TeeSource tee { from, saved }; + ParseSink ether; + parseDump(ether, tee); + source = std::make_unique(std::move(*saved.s)); + } - // FIXME: race if addToStore doesn't read source? - store->addToStore(info, *source, (RepairFlag) repair, - dontCheckSigs ? NoCheckSigs : CheckSigs); + logger->startWork(); + + // FIXME: race if addToStore doesn't read source? + store->addToStore(info, *source, (RepairFlag) repair, + dontCheckSigs ? NoCheckSigs : CheckSigs); + + logger->stopWork(); + } - logger->stopWork(); break; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 8d01c6667..4ed81d9d8 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -503,9 +503,84 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, conn->to << info.registrationTime << info.narSize << info.ultimate << info.sigs << renderContentAddress(info.ca) << repair << !checkSigs; - bool tunnel = GET_PROTOCOL_MINOR(conn->daemonVersion) >= 21; - if (!tunnel) copyNAR(source, conn->to); - conn.processStderr(0, tunnel ? &source : nullptr); + + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) { + + std::exception_ptr ex; + + struct FramedSink : BufferedSink + { + ConnectionHandle & conn; + std::exception_ptr & ex; + + FramedSink(ConnectionHandle & conn, std::exception_ptr & ex) : conn(conn), ex(ex) + { } + + ~FramedSink() + { + try { + conn->to << 0; + conn->to.flush(); + } catch (...) { + ignoreException(); + } + } + + void write(const unsigned char * data, size_t len) override + { + /* Don't send more data if the remote has + encountered an error. */ + if (ex) { + auto ex2 = ex; + ex = nullptr; + std::rethrow_exception(ex2); + } + conn->to << len; + conn->to(data, len); + }; + }; + + /* Handle log messages / exceptions from the remote on a + separate thread. */ + std::thread stderrThread([&]() + { + try { + conn.processStderr(0, nullptr); + } catch (...) { + ex = std::current_exception(); + } + }); + + Finally joinStderrThread([&]() + { + if (stderrThread.joinable()) { + stderrThread.join(); + if (ex) { + try { + std::rethrow_exception(ex); + } catch (...) { + ignoreException(); + } + } + } + }); + + { + FramedSink sink(conn, ex); + copyNAR(source, sink); + sink.flush(); + } + + stderrThread.join(); + if (ex) + std::rethrow_exception(ex); + + } else if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 21) { + conn.processStderr(0, &source); + } else { + copyNAR(source, conn->to); + conn.processStderr(0, nullptr); + } } } diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index 8b538f6da..dcba73116 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -6,7 +6,7 @@ namespace nix { #define WORKER_MAGIC_1 0x6e697863 #define WORKER_MAGIC_2 0x6478696f -#define PROTOCOL_VERSION 0x116 +#define PROTOCOL_VERSION 0x117 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) From fa8515d7ec160938b0287f405a43aeeca971777e Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 29 Jul 2020 00:57:20 -0500 Subject: [PATCH 31/37] Set regex_constants::match_continuous for quicker search match_continuous limits the search to the current start position, instead of searching the entire file. On libc++, this improves performance dramatically: $ time /nix/store/70ai68dfm6xbzwn26j5n4li9di52ylia-nix-3.0pre20200728_c159f48/bin/nix print-dev-env >/dev/null /nix/store/70ai68dfm6xbzwn26j5n4li9di52ylia-nix-3.0pre20200728_c159f48/bin/ni 2.39s user 0.19s system 64% cpu 4.032 total $ time /nix/store/cwjfxxlp83zln4mfyy1d2dbsx7f6s962-nix-3.0pre20200728_dirty/bin/nix print-dev-env >/dev/null /nix/store/cwjfxxlp83zln4mfyy1d2dbsx7f6s962-nix-3.0pre20200728_dirty/bin/nix 0.09s user 0.05s system 65% cpu 0.204 total Fixes #3874 --- src/nix/develop.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 2f590f53f..a0c119e43 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -68,22 +68,22 @@ BuildEnvironment readEnvironment(const Path & path) std::smatch match; - if (std::regex_search(pos, file.cend(), match, declareRegex)) { + if (std::regex_search(pos, file.cend(), match, declareRegex, std::regex_constants::match_continuous)) { pos = match[0].second; exported.insert(match[1]); } - else if (std::regex_search(pos, file.cend(), match, varRegex)) { + else if (std::regex_search(pos, file.cend(), match, varRegex, std::regex_constants::match_continuous)) { pos = match[0].second; res.env.insert({match[1], Var { .exported = exported.count(match[1]) > 0, .value = match[2] }}); } - else if (std::regex_search(pos, file.cend(), match, assocArrayRegex)) { + else if (std::regex_search(pos, file.cend(), match, assocArrayRegex, std::regex_constants::match_continuous)) { pos = match[0].second; res.env.insert({match[1], Var { .associative = true, .value = match[2] }}); } - else if (std::regex_search(pos, file.cend(), match, functionRegex)) { + else if (std::regex_search(pos, file.cend(), match, functionRegex, std::regex_constants::match_continuous)) { res.bashFunctions = std::string(pos, file.cend()); break; } From f63839bfa402f73e001a5ee412c3f6a6b95d1a38 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 29 Jul 2020 18:04:35 +0200 Subject: [PATCH 32/37] Cleanup --- src/libstore/remote-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 4ed81d9d8..616081aa1 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -545,7 +545,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, std::thread stderrThread([&]() { try { - conn.processStderr(0, nullptr); + conn.processStderr(); } catch (...) { ex = std::current_exception(); } From e4f6f8da77f8de0f19e81f1cbb60052ca92dd371 Mon Sep 17 00:00:00 2001 From: Mat Marini Date: Wed, 29 Jul 2020 20:50:06 -0400 Subject: [PATCH 33/37] Prevent nix-build from accepting --packages --- src/nix-build/nix-build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index f77de56ea..d8f2fe6d7 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -192,7 +192,7 @@ static void _main(int argc, char * * argv) else if (*arg == "--pure") pure = true; else if (*arg == "--impure") pure = false; - else if (*arg == "--packages" || *arg == "-p") + else if (runEnv && (*arg == "--packages" || *arg == "-p")) packages = true; else if (inShebang && *arg == "-i") { From d9e23bfee2f1995b3047d67e578e2cfd4e94dea6 Mon Sep 17 00:00:00 2001 From: Mat Marini Date: Wed, 29 Jul 2020 20:53:02 -0400 Subject: [PATCH 34/37] Prevent nix-build from accepting --run/--command --- src/nix-build/nix-build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index d8f2fe6d7..723db71fd 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -174,7 +174,7 @@ static void _main(int argc, char * * argv) else if (*arg == "--run-env") // obsolete runEnv = true; - else if (*arg == "--command" || *arg == "--run") { + else if (runEnv && (*arg == "--command" || *arg == "--run")) { if (*arg == "--run") interactive = false; envCommand = getArg(*arg, arg, end) + "\nexit"; From ebee2b7852a8c0441e9470a0df8ec3c71d322109 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 30 Jul 2020 13:00:30 +0200 Subject: [PATCH 35/37] receiveContents(): unsigned int -> size_t --- src/libstore/nar-accessor.cc | 2 +- src/libutil/archive.cc | 2 +- src/libutil/archive.hh | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstore/nar-accessor.cc b/src/libstore/nar-accessor.cc index d884a131e..e438b5710 100644 --- a/src/libstore/nar-accessor.cc +++ b/src/libstore/nar-accessor.cc @@ -86,7 +86,7 @@ struct NarAccessor : public FSAccessor parents.top()->start = pos; } - void receiveContents(unsigned char * data, unsigned int len) override + void receiveContents(unsigned char * data, size_t len) override { } void createSymlink(const Path & path, const string & target) override diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 51c88537e..c07e710cd 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -338,7 +338,7 @@ struct RestoreSink : ParseSink #endif } - void receiveContents(unsigned char * data, unsigned int len) + void receiveContents(unsigned char * data, size_t len) { writeFull(fd.get(), data, len); } diff --git a/src/libutil/archive.hh b/src/libutil/archive.hh index 57780d16a..b6e1188c4 100644 --- a/src/libutil/archive.hh +++ b/src/libutil/archive.hh @@ -58,7 +58,7 @@ struct ParseSink virtual void createRegularFile(const Path & path) { }; virtual void isExecutable() { }; virtual void preallocateContents(unsigned long long size) { }; - virtual void receiveContents(unsigned char * data, unsigned int len) { }; + virtual void receiveContents(unsigned char * data, size_t len) { }; virtual void createSymlink(const Path & path, const string & target) { }; }; @@ -77,7 +77,7 @@ struct RetrieveRegularNARSink : ParseSink regular = false; } - void receiveContents(unsigned char * data, unsigned int len) + void receiveContents(unsigned char * data, size_t len) { sink(data, len); } From 3f6e88a5527dcc4d58e3147f78388a88eb8896e0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 30 Jul 2020 13:10:49 +0200 Subject: [PATCH 36/37] unsigned long long -> uint64_t --- src/build-remote/build-remote.cc | 8 ++++---- src/libexpr/primops.cc | 2 +- src/libmain/shared.cc | 6 +++--- src/libmain/shared.hh | 4 ++-- src/libstore/build.cc | 10 +++++----- src/libstore/daemon.cc | 2 +- src/libstore/gc.cc | 14 +++++++------- src/libstore/local-store.hh | 4 ++-- src/libstore/misc.cc | 6 +++--- src/libstore/nar-accessor.cc | 2 +- src/libstore/optimise-store.cc | 2 +- src/libstore/remote-store.cc | 2 +- src/libstore/remote-store.hh | 2 +- src/libstore/store-api.hh | 10 +++++----- src/libutil/archive.cc | 8 ++++---- src/libutil/archive.hh | 2 +- src/libutil/config.cc | 2 +- src/libutil/hash.hh | 4 ++-- src/libutil/json.cc | 2 +- src/libutil/serialise.hh | 16 ++++++++-------- src/libutil/util.cc | 8 ++++---- src/libutil/util.hh | 2 +- src/nix-build/nix-build.cc | 2 +- src/nix-collect-garbage/nix-collect-garbage.cc | 6 ++---- src/nix-store/nix-store.cc | 10 ++++------ src/nix/path-info.cc | 2 +- 26 files changed, 67 insertions(+), 71 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index e07117496..3579d8fff 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -33,7 +33,7 @@ std::string escapeUri(std::string uri) static string currentLoad; -static AutoCloseFD openSlotLock(const Machine & m, unsigned long long slot) +static AutoCloseFD openSlotLock(const Machine & m, uint64_t slot) { return openLockFile(fmt("%s/%s-%d", currentLoad, escapeUri(m.storeUri), slot), true); } @@ -119,7 +119,7 @@ static int _main(int argc, char * * argv) bool rightType = false; Machine * bestMachine = nullptr; - unsigned long long bestLoad = 0; + uint64_t bestLoad = 0; for (auto & m : machines) { debug("considering building on remote machine '%s'", m.storeUri); @@ -130,8 +130,8 @@ static int _main(int argc, char * * argv) m.mandatoryMet(requiredFeatures)) { rightType = true; AutoCloseFD free; - unsigned long long load = 0; - for (unsigned long long slot = 0; slot < m.maxJobs; ++slot) { + uint64_t load = 0; + for (uint64_t slot = 0; slot < m.maxJobs; ++slot) { auto slotLock = openSlotLock(m, slot); if (lockFile(slotLock.get(), ltWrite, false)) { if (!free) { diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 5059286b3..a98cadc80 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -65,7 +65,7 @@ void EvalState::realiseContext(const PathSet & context) /* For performance, prefetch all substitute info. */ StorePathSet willBuild, willSubstitute, unknown; - unsigned long long downloadSize, narSize; + uint64_t downloadSize, narSize; store->queryMissing(drvs, willBuild, willSubstitute, unknown, downloadSize, narSize); store->buildPaths(drvs); diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 52718c231..2b1f25ca3 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -36,7 +36,7 @@ void printGCWarning() void printMissing(ref store, const std::vector & paths, Verbosity lvl) { - unsigned long long downloadSize, narSize; + uint64_t downloadSize, narSize; StorePathSet willBuild, willSubstitute, unknown; store->queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize); printMissing(store, willBuild, willSubstitute, unknown, downloadSize, narSize, lvl); @@ -45,7 +45,7 @@ void printMissing(ref store, const std::vector & pa void printMissing(ref store, const StorePathSet & willBuild, const StorePathSet & willSubstitute, const StorePathSet & unknown, - unsigned long long downloadSize, unsigned long long narSize, Verbosity lvl) + uint64_t downloadSize, uint64_t narSize, Verbosity lvl) { if (!willBuild.empty()) { if (willBuild.size() == 1) @@ -384,7 +384,7 @@ RunPager::~RunPager() } -string showBytes(unsigned long long bytes) +string showBytes(uint64_t bytes) { return (format("%.2f MiB") % (bytes / (1024.0 * 1024.0))).str(); } diff --git a/src/libmain/shared.hh b/src/libmain/shared.hh index f558247c0..ffae5d796 100644 --- a/src/libmain/shared.hh +++ b/src/libmain/shared.hh @@ -47,7 +47,7 @@ void printMissing( void printMissing(ref store, const StorePathSet & willBuild, const StorePathSet & willSubstitute, const StorePathSet & unknown, - unsigned long long downloadSize, unsigned long long narSize, Verbosity lvl = lvlInfo); + uint64_t downloadSize, uint64_t narSize, Verbosity lvl = lvlInfo); string getArg(const string & opt, Strings::iterator & i, const Strings::iterator & end); @@ -110,7 +110,7 @@ extern volatile ::sig_atomic_t blockInt; /* GC helpers. */ -string showBytes(unsigned long long bytes); +string showBytes(uint64_t bytes); struct GCResults; diff --git a/src/libstore/build.cc b/src/libstore/build.cc index d26f96b74..05235510d 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1646,13 +1646,13 @@ void DerivationGoal::buildDone() So instead, check if the disk is (nearly) full now. If so, we don't mark this build as a permanent failure. */ #if HAVE_STATVFS - unsigned long long required = 8ULL * 1024 * 1024; // FIXME: make configurable + uint64_t required = 8ULL * 1024 * 1024; // FIXME: make configurable struct statvfs st; if (statvfs(worker.store.realStoreDir.c_str(), &st) == 0 && - (unsigned long long) st.f_bavail * st.f_bsize < required) + (uint64_t) st.f_bavail * st.f_bsize < required) diskFull = true; if (statvfs(tmpDir.c_str(), &st) == 0 && - (unsigned long long) st.f_bavail * st.f_bsize < required) + (uint64_t) st.f_bavail * st.f_bsize < required) diskFull = true; #endif @@ -2851,7 +2851,7 @@ struct RestrictedStore : public LocalFSStore void queryMissing(const std::vector & targets, StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - unsigned long long & downloadSize, unsigned long long & narSize) override + uint64_t & downloadSize, uint64_t & narSize) override { /* This is slightly impure since it leaks information to the client about what paths will be built/substituted or are @@ -5038,7 +5038,7 @@ void Worker::markContentsGood(const StorePath & path) static void primeCache(Store & store, const std::vector & paths) { StorePathSet willBuild, willSubstitute, unknown; - unsigned long long downloadSize, narSize; + uint64_t downloadSize, narSize; store.queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize); if (!willBuild.empty() && 0 == settings.maxBuildJobs && getMachines().empty()) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 478ae39ca..9ac7e7cca 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -790,7 +790,7 @@ static void performOp(TunnelLogger * logger, ref store, targets.push_back(store->parsePathWithOutputs(s)); logger->startWork(); StorePathSet willBuild, willSubstitute, unknown; - unsigned long long downloadSize, narSize; + uint64_t downloadSize, narSize; store->queryMissing(targets, willBuild, willSubstitute, unknown, downloadSize, narSize); logger->stopWork(); writeStorePaths(*store, to, willBuild); diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index aaed5c218..e74382ed2 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -500,7 +500,7 @@ struct LocalStore::GCState StorePathSet alive; bool gcKeepOutputs; bool gcKeepDerivations; - unsigned long long bytesInvalidated; + uint64_t bytesInvalidated; bool moveToTrash = true; bool shouldDelete; GCState(const GCOptions & options, GCResults & results) @@ -518,7 +518,7 @@ bool LocalStore::isActiveTempFile(const GCState & state, void LocalStore::deleteGarbage(GCState & state, const Path & path) { - unsigned long long bytesFreed; + uint64_t bytesFreed; deletePath(path, bytesFreed); state.results.bytesFreed += bytesFreed; } @@ -528,7 +528,7 @@ void LocalStore::deletePathRecursive(GCState & state, const Path & path) { checkInterrupt(); - unsigned long long size = 0; + uint64_t size = 0; auto storePath = maybeParseStorePath(path); if (storePath && isValidPath(*storePath)) { @@ -687,7 +687,7 @@ void LocalStore::removeUnusedLinks(const GCState & state) AutoCloseDir dir(opendir(linksDir.c_str())); if (!dir) throw SysError("opening directory '%1%'", linksDir); - long long actualSize = 0, unsharedSize = 0; + int64_t actualSize = 0, unsharedSize = 0; struct dirent * dirent; while (errno = 0, dirent = readdir(dir.get())) { @@ -717,10 +717,10 @@ void LocalStore::removeUnusedLinks(const GCState & state) struct stat st; if (stat(linksDir.c_str(), &st) == -1) throw SysError("statting '%1%'", linksDir); - long long overhead = st.st_blocks * 512ULL; + auto overhead = st.st_blocks * 512ULL; - printInfo(format("note: currently hard linking saves %.2f MiB") - % ((unsharedSize - actualSize - overhead) / (1024.0 * 1024.0))); + printInfo("note: currently hard linking saves %.2f MiB", + ((unsharedSize - actualSize - overhead) / (1024.0 * 1024.0))); } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 355c2814f..547fe589c 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -29,8 +29,8 @@ struct Derivation; struct OptimiseStats { unsigned long filesLinked = 0; - unsigned long long bytesFreed = 0; - unsigned long long blocksFreed = 0; + uint64_t bytesFreed = 0; + uint64_t blocksFreed = 0; }; diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index c4d22a634..a542a259d 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -110,7 +110,7 @@ void Store::computeFSClosure(const StorePath & startPath, void Store::queryMissing(const std::vector & targets, StorePathSet & willBuild_, StorePathSet & willSubstitute_, StorePathSet & unknown_, - unsigned long long & downloadSize_, unsigned long long & narSize_) + uint64_t & downloadSize_, uint64_t & narSize_) { Activity act(*logger, lvlDebug, actUnknown, "querying info about missing paths"); @@ -122,8 +122,8 @@ void Store::queryMissing(const std::vector & targets, { std::unordered_set done; StorePathSet & unknown, & willSubstitute, & willBuild; - unsigned long long & downloadSize; - unsigned long long & narSize; + uint64_t & downloadSize; + uint64_t & narSize; }; struct DrvState diff --git a/src/libstore/nar-accessor.cc b/src/libstore/nar-accessor.cc index e438b5710..59ec164b6 100644 --- a/src/libstore/nar-accessor.cc +++ b/src/libstore/nar-accessor.cc @@ -79,7 +79,7 @@ struct NarAccessor : public FSAccessor parents.top()->isExecutable = true; } - void preallocateContents(unsigned long long size) override + void preallocateContents(uint64_t size) override { assert(size <= std::numeric_limits::max()); parents.top()->size = (uint64_t) size; diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc index b2b2412a3..e4b4b6213 100644 --- a/src/libstore/optimise-store.cc +++ b/src/libstore/optimise-store.cc @@ -282,7 +282,7 @@ void LocalStore::optimiseStore(OptimiseStats & stats) } } -static string showBytes(unsigned long long bytes) +static string showBytes(uint64_t bytes) { return (format("%.2f MiB") % (bytes / (1024.0 * 1024.0))).str(); } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 616081aa1..611f70783 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -782,7 +782,7 @@ void RemoteStore::addSignatures(const StorePath & storePath, const StringSet & s void RemoteStore::queryMissing(const std::vector & targets, StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - unsigned long long & downloadSize, unsigned long long & narSize) + uint64_t & downloadSize, uint64_t & narSize) { { auto conn(getConnection()); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 3c1b78b6a..eb6f225b1 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -94,7 +94,7 @@ public: void queryMissing(const std::vector & targets, StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - unsigned long long & downloadSize, unsigned long long & narSize) override; + uint64_t & downloadSize, uint64_t & narSize) override; void connect() override; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a7288d0cc..667dc4117 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -85,7 +85,7 @@ struct GCOptions StorePathSet pathsToDelete; /* Stop after at least `maxFreed' bytes have been freed. */ - unsigned long long maxFreed{std::numeric_limits::max()}; + uint64_t maxFreed{std::numeric_limits::max()}; }; @@ -97,7 +97,7 @@ struct GCResults /* For `gcReturnDead', `gcDeleteDead' and `gcDeleteSpecific', the number of bytes that would be or was freed. */ - unsigned long long bytesFreed = 0; + uint64_t bytesFreed = 0; }; @@ -105,8 +105,8 @@ struct SubstitutablePathInfo { std::optional deriver; StorePathSet references; - unsigned long long downloadSize; /* 0 = unknown or inapplicable */ - unsigned long long narSize; /* 0 = unknown */ + uint64_t downloadSize; /* 0 = unknown or inapplicable */ + uint64_t narSize; /* 0 = unknown */ }; typedef std::map SubstitutablePathInfos; @@ -610,7 +610,7 @@ public: that will be substituted. */ virtual void queryMissing(const std::vector & targets, StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - unsigned long long & downloadSize, unsigned long long & narSize); + uint64_t & downloadSize, uint64_t & narSize); /* Sort a set of paths topologically under the references relation. If p refers to q, then p precedes q in this list. */ diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index c07e710cd..ce7cf9754 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -150,17 +150,17 @@ static void skipGeneric(Source & source) static void parseContents(ParseSink & sink, Source & source, const Path & path) { - unsigned long long size = readLongLong(source); + uint64_t size = readLongLong(source); sink.preallocateContents(size); - unsigned long long left = size; + uint64_t left = size; std::vector buf(65536); while (left) { checkInterrupt(); auto n = buf.size(); - if ((unsigned long long)n > left) n = left; + if ((uint64_t)n > left) n = left; source(buf.data(), n); sink.receiveContents(buf.data(), n); left -= n; @@ -323,7 +323,7 @@ struct RestoreSink : ParseSink throw SysError("fchmod"); } - void preallocateContents(unsigned long long len) + void preallocateContents(uint64_t len) { #if HAVE_POSIX_FALLOCATE if (len) { diff --git a/src/libutil/archive.hh b/src/libutil/archive.hh index b6e1188c4..5665732d2 100644 --- a/src/libutil/archive.hh +++ b/src/libutil/archive.hh @@ -57,7 +57,7 @@ struct ParseSink virtual void createRegularFile(const Path & path) { }; virtual void isExecutable() { }; - virtual void preallocateContents(unsigned long long size) { }; + virtual void preallocateContents(uint64_t size) { }; virtual void receiveContents(unsigned char * data, size_t len) { }; virtual void createSymlink(const Path & path, const string & target) { }; diff --git a/src/libutil/config.cc b/src/libutil/config.cc index 8fc700a2b..8454037f2 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -284,7 +284,7 @@ template class BaseSetting; template class BaseSetting; template class BaseSetting; template class BaseSetting; -template class BaseSetting; +template class BaseSetting; template class BaseSetting; template class BaseSetting; template class BaseSetting; diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 98ee1bed0..abcd58f24 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -111,7 +111,7 @@ Hash hashFile(HashType ht, const Path & path); /* Compute the hash of the given path. The hash is defined as (essentially) hashString(ht, dumpPath(path)). */ -typedef std::pair HashResult; +typedef std::pair HashResult; HashResult hashPath(HashType ht, const Path & path, PathFilter & filter = defaultPathFilter); @@ -141,7 +141,7 @@ class HashSink : public BufferedSink, public AbstractHashSink private: HashType ht; Ctx * ctx; - unsigned long long bytes; + uint64_t bytes; public: HashSink(HashType ht); diff --git a/src/libutil/json.cc b/src/libutil/json.cc index 01331947e..104b99e6d 100644 --- a/src/libutil/json.cc +++ b/src/libutil/json.cc @@ -29,7 +29,7 @@ template<> void toJSON(std::ostream & str, const unsigned int & n) template<> void toJSON(std::ostream & str, const long & n) { str << n; } template<> void toJSON(std::ostream & str, const unsigned long & n) { str << n; } template<> void toJSON(std::ostream & str, const long long & n) { str << n; } -template<> void toJSON(std::ostream & str, const unsigned long long & n) { str << n; } +template<> void toJSON(std::ostream & str, const uint64_t & n) { str << n; } template<> void toJSON(std::ostream & str, const float & n) { str << n; } template<> void toJSON(std::ostream & str, const double & n) { str << n; } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 5d9acf887..c29c6b29b 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -312,14 +312,14 @@ T readNum(Source & source) source(buf, sizeof(buf)); uint64_t n = - ((unsigned long long) buf[0]) | - ((unsigned long long) buf[1] << 8) | - ((unsigned long long) buf[2] << 16) | - ((unsigned long long) buf[3] << 24) | - ((unsigned long long) buf[4] << 32) | - ((unsigned long long) buf[5] << 40) | - ((unsigned long long) buf[6] << 48) | - ((unsigned long long) buf[7] << 56); + ((uint64_t) buf[0]) | + ((uint64_t) buf[1] << 8) | + ((uint64_t) buf[2] << 16) | + ((uint64_t) buf[3] << 24) | + ((uint64_t) buf[4] << 32) | + ((uint64_t) buf[5] << 40) | + ((uint64_t) buf[6] << 48) | + ((uint64_t) buf[7] << 56); if (n > std::numeric_limits::max()) throw SerialisationError("serialised integer %d is too large for type '%s'", n, typeid(T).name()); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index a0a8ff4d3..97d278581 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -374,7 +374,7 @@ void writeLine(int fd, string s) } -static void _deletePath(int parentfd, const Path & path, unsigned long long & bytesFreed) +static void _deletePath(int parentfd, const Path & path, uint64_t & bytesFreed) { checkInterrupt(); @@ -414,7 +414,7 @@ static void _deletePath(int parentfd, const Path & path, unsigned long long & by } } -static void _deletePath(const Path & path, unsigned long long & bytesFreed) +static void _deletePath(const Path & path, uint64_t & bytesFreed) { Path dir = dirOf(path); if (dir == "") @@ -435,12 +435,12 @@ static void _deletePath(const Path & path, unsigned long long & bytesFreed) void deletePath(const Path & path) { - unsigned long long dummy; + uint64_t dummy; deletePath(path, dummy); } -void deletePath(const Path & path, unsigned long long & bytesFreed) +void deletePath(const Path & path, uint64_t & bytesFreed) { //Activity act(*logger, lvlDebug, format("recursively deleting path '%1%'") % path); bytesFreed = 0; diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 42130f6dc..6850b5a7a 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -125,7 +125,7 @@ void writeLine(int fd, string s); second variant returns the number of bytes and blocks freed. */ void deletePath(const Path & path); -void deletePath(const Path & path, unsigned long long & bytesFreed); +void deletePath(const Path & path, uint64_t & bytesFreed); std::string getUserName(); diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 723db71fd..94412042f 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -325,7 +325,7 @@ static void _main(int argc, char * * argv) auto buildPaths = [&](const std::vector & paths) { /* Note: we do this even when !printMissing to efficiently fetch binary cache data. */ - unsigned long long downloadSize, narSize; + uint64_t downloadSize, narSize; StorePathSet willBuild, willSubstitute, unknown; store->queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize); diff --git a/src/nix-collect-garbage/nix-collect-garbage.cc b/src/nix-collect-garbage/nix-collect-garbage.cc index aa5ada3a6..bcf1d8518 100644 --- a/src/nix-collect-garbage/nix-collect-garbage.cc +++ b/src/nix-collect-garbage/nix-collect-garbage.cc @@ -67,10 +67,8 @@ static int _main(int argc, char * * argv) deleteOlderThan = getArg(*arg, arg, end); } else if (*arg == "--dry-run") dryRun = true; - else if (*arg == "--max-freed") { - long long maxFreed = getIntArg(*arg, arg, end, true); - options.maxFreed = maxFreed >= 0 ? maxFreed : 0; - } + else if (*arg == "--max-freed") + options.maxFreed = std::max(getIntArg(*arg, arg, end, true), (int64_t) 0); else return false; return true; diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 76bf5154a..7b26970ef 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -130,7 +130,7 @@ static void opRealise(Strings opFlags, Strings opArgs) for (auto & i : opArgs) paths.push_back(store->followLinksToStorePathWithOutputs(i)); - unsigned long long downloadSize, narSize; + uint64_t downloadSize, narSize; StorePathSet willBuild, willSubstitute, unknown; store->queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize); @@ -572,10 +572,8 @@ static void opGC(Strings opFlags, Strings opArgs) if (*i == "--print-roots") printRoots = true; else if (*i == "--print-live") options.action = GCOptions::gcReturnLive; else if (*i == "--print-dead") options.action = GCOptions::gcReturnDead; - else if (*i == "--max-freed") { - long long maxFreed = getIntArg(*i, i, opFlags.end(), true); - options.maxFreed = maxFreed >= 0 ? maxFreed : 0; - } + else if (*i == "--max-freed") + options.maxFreed = std::max(getIntArg(*i, i, opFlags.end(), true), (int64_t) 0); else throw UsageError("bad sub-operation '%1%' in GC", *i); if (!opArgs.empty()) throw UsageError("no arguments expected"); @@ -831,7 +829,7 @@ static void opServe(Strings opFlags, Strings opArgs) for (auto & path : paths) if (!path.isDerivation()) paths2.push_back({path}); - unsigned long long downloadSize, narSize; + uint64_t downloadSize, narSize; StorePathSet willBuild, willSubstitute, unknown; store->queryMissing(paths2, willBuild, willSubstitute, unknown, downloadSize, narSize); diff --git a/src/nix/path-info.cc b/src/nix/path-info.cc index 65f73cd94..0c12efaf0 100644 --- a/src/nix/path-info.cc +++ b/src/nix/path-info.cc @@ -61,7 +61,7 @@ struct CmdPathInfo : StorePathsCommand, MixJSON }; } - void printSize(unsigned long long value) + void printSize(uint64_t value) { if (!humanReadable) { std::cout << fmt("\t%11d", value); From a785b3eddf8c02750b1715939069d20980bd5125 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 30 Jul 2020 15:27:28 +0200 Subject: [PATCH 37/37] Fix build --- src/libutil/config.cc | 2 +- src/libutil/json.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/config.cc b/src/libutil/config.cc index 8454037f2..8fc700a2b 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -284,7 +284,7 @@ template class BaseSetting; template class BaseSetting; template class BaseSetting; template class BaseSetting; -template class BaseSetting; +template class BaseSetting; template class BaseSetting; template class BaseSetting; template class BaseSetting; diff --git a/src/libutil/json.cc b/src/libutil/json.cc index 104b99e6d..01331947e 100644 --- a/src/libutil/json.cc +++ b/src/libutil/json.cc @@ -29,7 +29,7 @@ template<> void toJSON(std::ostream & str, const unsigned int & n) template<> void toJSON(std::ostream & str, const long & n) { str << n; } template<> void toJSON(std::ostream & str, const unsigned long & n) { str << n; } template<> void toJSON(std::ostream & str, const long long & n) { str << n; } -template<> void toJSON(std::ostream & str, const uint64_t & n) { str << n; } +template<> void toJSON(std::ostream & str, const unsigned long long & n) { str << n; } template<> void toJSON(std::ostream & str, const float & n) { str << n; } template<> void toJSON(std::ostream & str, const double & n) { str << n; }