From edfd676e059578fb574ce78d1a2cc66d018d3b16 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 7 Aug 2020 21:18:29 +0200 Subject: [PATCH 1/3] Fix .ls file names in binary caches These are not supposed to include the 'name' part of the store path. This was broken by 759947bf72. --- src/libstore/binary-cache-store.cc | 2 +- tests/binary-cache.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 9682db730..86877dd1a 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -219,7 +219,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource } } - upsertFile(std::string(info.path.to_string()) + ".ls", jsonOut.str(), "application/json"); + upsertFile(std::string(info.path.hashPart()) + ".ls", jsonOut.str(), "application/json"); } /* Optionally maintain an index of DWARF debug info files diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh index b05a7d167..fe4ddec8d 100644 --- a/tests/binary-cache.sh +++ b/tests/binary-cache.sh @@ -219,7 +219,7 @@ outPath=$(nix-build --no-out-link -E ' nix copy --to file://$cacheDir?write-nar-listing=1 $outPath diff -u \ - <(jq -S < $cacheDir/$(basename $outPath).ls) \ + <(jq -S < $cacheDir/$(basename $outPath | cut -c1-32).ls) \ <(echo '{"version":1,"root":{"type":"directory","entries":{"bar":{"type":"regular","size":4,"narOffset":232},"link":{"type":"symlink","target":"xyzzy"}}}}' | jq -S) From bcd0629c2e499788e4b3a9d6e380143a8f807419 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 9 Aug 2020 20:32:35 +0000 Subject: [PATCH 2/3] Remove name parameter from `writeDerivation` The name is now stored with the derivation itself. --- src/libexpr/primops.cc | 2 +- src/libstore/derivations.cc | 4 ++-- src/libstore/derivations.hh | 2 +- src/nix/develop.cc | 9 +++------ 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 65d36ca0e..3955287b7 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -839,7 +839,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * } /* Write the resulting term into the Nix store directory. */ - auto drvPath = writeDerivation(state.store, drv, drvName, state.repair); + auto drvPath = writeDerivation(state.store, drv, state.repair); auto drvPathS = state.store->printStorePath(drvPath); printMsg(lvlChatty, "instantiated '%1%' -> '%2%'", drvName, drvPathS); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 68b081058..f7e764a80 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -62,7 +62,7 @@ bool BasicDerivation::isBuiltin() const StorePath writeDerivation(ref store, - const Derivation & drv, std::string_view name, RepairFlag repair) + const Derivation & drv, RepairFlag repair) { auto references = drv.inputSrcs; for (auto & i : drv.inputDrvs) @@ -70,7 +70,7 @@ StorePath writeDerivation(ref store, /* Note that the outputs of a derivation are *not* references (that can be missing (of course) and should not necessarily be held during a garbage collection). */ - auto suffix = std::string(name) + drvExtension; + auto suffix = std::string(drv.name) + drvExtension; auto contents = drv.unparse(*store, false); return settings.readOnlyMode ? store->computeStorePathForText(suffix, contents, references) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 14e0e947a..53dfc7f13 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -128,7 +128,7 @@ enum RepairFlag : bool { NoRepair = false, Repair = true }; /* Write a derivation to the Nix store, and return its path. */ StorePath writeDerivation(ref store, - const Derivation & drv, std::string_view name, RepairFlag repair = NoRepair); + const Derivation & drv, RepairFlag repair = NoRepair); /* Read a derivation from a file. */ Derivation readDerivation(const Store & store, const Path & drvPath, std::string_view name); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 12658078a..434088da7 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -124,10 +124,7 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) /* Rehash and write the derivation. FIXME: would be nice to use 'buildDerivation', but that's privileged. */ - auto drvName = std::string(drvPath.name()); - assert(hasSuffix(drvName, ".drv")); - drvName.resize(drvName.size() - 4); - drvName += "-env"; + drv.name += "-env"; for (auto & output : drv.outputs) drv.env.erase(output.first); drv.outputs = {{"out", DerivationOutput { .output = DerivationOutputInputAddressed { .path = StorePath::dummy }}}}; @@ -136,12 +133,12 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) drv.env["outputs"] = "out"; drv.inputSrcs.insert(std::move(getEnvShPath)); Hash h = std::get<0>(hashDerivationModulo(*store, drv, true)); - auto shellOutPath = store->makeOutputPath("out", h, drvName); + auto shellOutPath = store->makeOutputPath("out", h, drv.name); drv.outputs.insert_or_assign("out", DerivationOutput { .output = DerivationOutputInputAddressed { .path = shellOutPath } }); drv.env["out"] = store->printStorePath(shellOutPath); - auto shellDrvPath2 = writeDerivation(store, drv, drvName); + auto shellDrvPath2 = writeDerivation(store, drv); /* Build the derivation. */ store->buildPaths({{shellDrvPath2}}); From 581183d4d57d5382e3a375f760f3323ef451f555 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 9 Aug 2020 20:51:34 +0000 Subject: [PATCH 3/3] Deduplicate parsing and reading derivations --- src/libstore/derivations.cc | 54 +++++++++++-------------------------- 1 file changed, 16 insertions(+), 38 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index f7e764a80..e9e4b5173 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -139,18 +139,14 @@ static StringSet parseStrings(std::istream & str, bool arePaths) } -static DerivationOutput parseDerivationOutput(const Store & store, std::istringstream & str) +static DerivationOutput parseDerivationOutput(const Store & store, + StorePath path, std::string_view hashAlgo, std::string_view hash) { - expect(str, ","); auto path = store.parseStorePath(parsePath(str)); - expect(str, ","); auto hashAlgo = parseString(str); - expect(str, ","); const auto hash = parseString(str); - expect(str, ")"); - if (hashAlgo != "") { auto method = FileIngestionMethod::Flat; if (string(hashAlgo, 0, 2) == "r:") { method = FileIngestionMethod::Recursive; - hashAlgo = string(hashAlgo, 2); + hashAlgo = hashAlgo.substr(2); } const HashType hashType = parseHashType(hashAlgo); @@ -178,6 +174,16 @@ static DerivationOutput parseDerivationOutput(const Store & store, std::istrings }; } +static DerivationOutput parseDerivationOutput(const Store & store, std::istringstream & str) +{ + expect(str, ","); auto path = store.parseStorePath(parsePath(str)); + expect(str, ","); const auto hashAlgo = parseString(str); + expect(str, ","); const auto hash = parseString(str); + expect(str, ")"); + + return parseDerivationOutput(store, std::move(path), hashAlgo, hash); +} + static Derivation parseDerivation(const Store & store, std::string && s, std::string_view name) { @@ -541,38 +547,10 @@ StorePathSet BasicDerivation::outputPaths(const Store & store) const static DerivationOutput readDerivationOutput(Source & in, const Store & store) { auto path = store.parseStorePath(readString(in)); - auto hashAlgo = readString(in); - auto hash = readString(in); + const auto hashAlgo = readString(in); + const auto hash = readString(in); - if (hashAlgo != "") { - auto method = FileIngestionMethod::Flat; - if (string(hashAlgo, 0, 2) == "r:") { - method = FileIngestionMethod::Recursive; - hashAlgo = string(hashAlgo, 2); - } - auto hashType = parseHashType(hashAlgo); - return hash != "" - ? DerivationOutput { - .output = DerivationOutputCAFixed { - .hash = FixedOutputHash { - .method = std::move(method), - .hash = Hash::parseNonSRIUnprefixed(hash, hashType), - }, - } - } - : (settings.requireExperimentalFeature("ca-derivations"), - DerivationOutput { - .output = DerivationOutputCAFloating { - .method = std::move(method), - .hashType = std::move(hashType), - }, - }); - } else - return DerivationOutput { - .output = DerivationOutputInputAddressed { - .path = std::move(path), - } - }; + return parseDerivationOutput(store, std::move(path), hashAlgo, hash); } StringSet BasicDerivation::outputNames() const