From e7c1113a37e6a8fd0dc2dde0d070dbef276a0481 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 17 May 2023 17:31:33 -0400 Subject: [PATCH 1/2] Add test for `downstreamPlaceholder` This is good in general, but in particular ensures when we heavily refactor it in the next commit there is less likelihood for an unintentional change in behavior to sneak in. --- src/libstore/tests/downstream-placeholder.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 src/libstore/tests/downstream-placeholder.cc diff --git a/src/libstore/tests/downstream-placeholder.cc b/src/libstore/tests/downstream-placeholder.cc new file mode 100644 index 000000000..7b8e2c649 --- /dev/null +++ b/src/libstore/tests/downstream-placeholder.cc @@ -0,0 +1,16 @@ +#include + +#include "derivations.hh" + +namespace nix { + +TEST(Derivation, downstreamPlaceholder) { + ASSERT_EQ( + downstreamPlaceholder( + (const Store &)*(const Store *)nullptr, // argument is unused + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, + "out"), + "/0c6rn30q4frawknapgwq386zq358m8r6msvywcvc89n6m5p2dgbz"); +} + +} From b9e5ce4a27f4a8bbee1a2eeb6fddbf569cbfdd7a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 11 May 2023 18:01:41 -0400 Subject: [PATCH 2/2] Upgrade `downstreamPlaceholder` to a type with methods This gets us ready for dynamic derivation dependencies (part of RFC 92). --- src/libexpr/eval.cc | 5 +- src/libexpr/eval.hh | 4 +- src/libexpr/primops.cc | 3 +- src/libstore/derivations.cc | 11 +-- src/libstore/derivations.hh | 12 +--- src/libstore/downstream-placeholder.cc | 39 ++++++++++ src/libstore/downstream-placeholder.hh | 75 ++++++++++++++++++++ src/libstore/tests/downstream-placeholder.cc | 27 +++++-- src/libutil/experimental-features.cc | 3 + src/nix/app.cc | 3 +- 10 files changed, 152 insertions(+), 30 deletions(-) create mode 100644 src/libstore/downstream-placeholder.cc create mode 100644 src/libstore/downstream-placeholder.hh diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 740a5e677..585670e69 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -4,6 +4,7 @@ #include "util.hh" #include "store-api.hh" #include "derivations.hh" +#include "downstream-placeholder.hh" #include "globals.hh" #include "eval-inline.hh" #include "filetransfer.hh" @@ -1058,7 +1059,7 @@ void EvalState::mkOutputString( ? store->printStorePath(*std::move(optOutputPath)) /* Downstream we would substitute this for an actual path once we build the floating CA derivation */ - : downstreamPlaceholder(*store, drvPath, outputName), + : DownstreamPlaceholder::unknownCaOutput(drvPath, outputName).render(), NixStringContext { NixStringContextElem::Built { .drvPath = drvPath, @@ -2380,7 +2381,7 @@ DerivedPath EvalState::coerceToDerivedPath(const PosIdx pos, Value & v, std::str // This is testing for the case of CA derivations auto sExpected = optOutputPath ? store->printStorePath(*optOutputPath) - : downstreamPlaceholder(*store, b.drvPath, output); + : DownstreamPlaceholder::unknownCaOutput(b.drvPath, output).render(); if (s != sExpected) error( "string '%s' has context with the output '%s' from derivation '%s', but the string is not the right placeholder for this derivation output. It should be '%s'", diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index a90ff34c0..62b380929 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -483,7 +483,7 @@ public: * Coerce to `DerivedPath`. * * Must be a string which is either a literal store path or a - * "placeholder (see `downstreamPlaceholder()`). + * "placeholder (see `DownstreamPlaceholder`). * * Even more importantly, the string context must be exactly one * element, which is either a `NixStringContextElem::Opaque` or @@ -622,7 +622,7 @@ public: * @param optOutputPath Optional output path for that string. Must * be passed if and only if output store object is input-addressed. * Will be printed to form string if passed, otherwise a placeholder - * will be used (see `downstreamPlaceholder()`). + * will be used (see `DownstreamPlaceholder`). */ void mkOutputString( Value & value, diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 6fbd66389..cfae1e5f8 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1,5 +1,6 @@ #include "archive.hh" #include "derivations.hh" +#include "downstream-placeholder.hh" #include "eval-inline.hh" #include "eval.hh" #include "globals.hh" @@ -87,7 +88,7 @@ StringMap EvalState::realiseContext(const NixStringContext & context) auto outputs = resolveDerivedPath(*store, drv); for (auto & [outputName, outputPath] : outputs) { res.insert_or_assign( - downstreamPlaceholder(*store, drv.drvPath, outputName), + DownstreamPlaceholder::unknownCaOutput(drv.drvPath, outputName).render(), store->printStorePath(outputPath) ); } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index d56dc727b..56a3df66d 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -1,4 +1,5 @@ #include "derivations.hh" +#include "downstream-placeholder.hh" #include "store-api.hh" #include "globals.hh" #include "util.hh" @@ -810,13 +811,7 @@ std::string hashPlaceholder(const std::string_view outputName) return "/" + hashString(htSHA256, concatStrings("nix-output:", outputName)).to_string(Base32, false); } -std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath, std::string_view outputName) -{ - auto drvNameWithExtension = drvPath.name(); - auto drvName = drvNameWithExtension.substr(0, drvNameWithExtension.size() - 4); - auto clearText = "nix-upstream-output:" + std::string { drvPath.hashPart() } + ":" + outputPathName(drvName, outputName); - return "/" + hashString(htSHA256, clearText).to_string(Base32, false); -} + static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) @@ -880,7 +875,7 @@ std::optional Derivation::tryResolve( for (auto & outputName : inputOutputs) { if (auto actualPath = get(inputDrvOutputs, { inputDrv, outputName })) { inputRewrites.emplace( - downstreamPlaceholder(store, inputDrv, outputName), + DownstreamPlaceholder::unknownCaOutput(inputDrv, outputName).render(), store.printStorePath(*actualPath)); resolved.inputSrcs.insert(*actualPath); } else { diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 1e2143f31..fa79f77fd 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -6,6 +6,7 @@ #include "hash.hh" #include "content-address.hh" #include "repair-flag.hh" +#include "derived-path.hh" #include "sync.hh" #include "comparator.hh" @@ -495,17 +496,6 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr */ std::string hashPlaceholder(const std::string_view outputName); -/** - * This creates an opaque and almost certainly unique string - * deterministically from a derivation path and output name. - * - * It is used as a placeholder to allow derivations to refer to - * content-addressed paths whose content --- and thus the path - * themselves --- isn't yet known. This occurs when a derivation has a - * dependency which is a CA derivation. - */ -std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath, std::string_view outputName); - extern const Hash impureOutputHash; } diff --git a/src/libstore/downstream-placeholder.cc b/src/libstore/downstream-placeholder.cc new file mode 100644 index 000000000..1752738f2 --- /dev/null +++ b/src/libstore/downstream-placeholder.cc @@ -0,0 +1,39 @@ +#include "downstream-placeholder.hh" +#include "derivations.hh" + +namespace nix { + +std::string DownstreamPlaceholder::render() const +{ + return "/" + hash.to_string(Base32, false); +} + + +DownstreamPlaceholder DownstreamPlaceholder::unknownCaOutput( + const StorePath & drvPath, + std::string_view outputName) +{ + auto drvNameWithExtension = drvPath.name(); + auto drvName = drvNameWithExtension.substr(0, drvNameWithExtension.size() - 4); + auto clearText = "nix-upstream-output:" + std::string { drvPath.hashPart() } + ":" + outputPathName(drvName, outputName); + return DownstreamPlaceholder { + hashString(htSHA256, clearText) + }; +} + +DownstreamPlaceholder DownstreamPlaceholder::unknownDerivation( + const DownstreamPlaceholder & placeholder, + std::string_view outputName, + const ExperimentalFeatureSettings & xpSettings) +{ + xpSettings.require(Xp::DynamicDerivations); + auto compressed = compressHash(placeholder.hash, 20); + auto clearText = "nix-computed-output:" + + compressed.to_string(Base32, false) + + ":" + std::string { outputName }; + return DownstreamPlaceholder { + hashString(htSHA256, clearText) + }; +} + +} diff --git a/src/libstore/downstream-placeholder.hh b/src/libstore/downstream-placeholder.hh new file mode 100644 index 000000000..f0c0dee77 --- /dev/null +++ b/src/libstore/downstream-placeholder.hh @@ -0,0 +1,75 @@ +#pragma once +///@file + +#include "hash.hh" +#include "path.hh" + +namespace nix { + +/** + * Downstream Placeholders are opaque and almost certainly unique values + * used to allow derivations to refer to store objects which are yet to + * be built and for we do not yet have store paths for. + * + * They correspond to `DerivedPaths` that are not `DerivedPath::Opaque`, + * except for the cases involving input addressing or fixed outputs + * where we do know a store path for the derivation output in advance. + * + * Unlike `DerivationPath`, however, `DownstreamPlaceholder` is + * purposefully opaque and obfuscated. This is so they are hard to + * create by accident, and so substituting them (once we know what the + * path to store object is) is unlikely to capture other stuff it + * shouldn't. + * + * We use them with `Derivation`: the `render()` method is called to + * render an opaque string which can be used in the derivation, and the + * resolving logic can substitute those strings for store paths when + * resolving `Derivation.inputDrvs` to `BasicDerivation.inputSrcs`. + */ +class DownstreamPlaceholder +{ + /** + * `DownstreamPlaceholder` is just a newtype of `Hash`. + * This its only field. + */ + Hash hash; + + /** + * Newtype constructor + */ + DownstreamPlaceholder(Hash hash) : hash(hash) { } + +public: + /** + * This creates an opaque and almost certainly unique string + * deterministically from the placeholder. + */ + std::string render() const; + + /** + * Create a placeholder for an unknown output of a content-addressed + * derivation. + * + * The derivation itself is known (we have a store path for it), but + * the output doesn't yet have a known store path. + */ + static DownstreamPlaceholder unknownCaOutput( + const StorePath & drvPath, + std::string_view outputName); + + /** + * Create a placehold for the output of an unknown derivation. + * + * The derivation is not yet known because it is a dynamic + * derivaiton --- it is itself an output of another derivation --- + * and we just have (another) placeholder for it. + * + * @param xpSettings Stop-gap to avoid globals during unit tests. + */ + static DownstreamPlaceholder unknownDerivation( + const DownstreamPlaceholder & drvPlaceholder, + std::string_view outputName, + const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); +}; + +} diff --git a/src/libstore/tests/downstream-placeholder.cc b/src/libstore/tests/downstream-placeholder.cc index 7b8e2c649..ec3e1000f 100644 --- a/src/libstore/tests/downstream-placeholder.cc +++ b/src/libstore/tests/downstream-placeholder.cc @@ -1,16 +1,33 @@ #include -#include "derivations.hh" +#include "downstream-placeholder.hh" namespace nix { -TEST(Derivation, downstreamPlaceholder) { +TEST(DownstreamPlaceholder, unknownCaOutput) { ASSERT_EQ( - downstreamPlaceholder( - (const Store &)*(const Store *)nullptr, // argument is unused + DownstreamPlaceholder::unknownCaOutput( StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, - "out"), + "out").render(), "/0c6rn30q4frawknapgwq386zq358m8r6msvywcvc89n6m5p2dgbz"); } +TEST(DownstreamPlaceholder, unknownDerivation) { + /** + * We set these in tests rather than the regular globals so we don't have + * to worry about race conditions if the tests run concurrently. + */ + ExperimentalFeatureSettings mockXpSettings; + mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations"); + + ASSERT_EQ( + DownstreamPlaceholder::unknownDerivation( + DownstreamPlaceholder::unknownCaOutput( + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv.drv" }, + "out"), + "out", + mockXpSettings).render(), + "/0gn6agqxjyyalf0dpihgyf49xq5hqxgw100f0wydnj6yqrhqsb3w"); +} + } diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index ad0ec0427..5aae0347b 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -207,6 +207,9 @@ constexpr std::array xpFeatureDetails = {{ - "text hashing" derivation outputs, so we can build .drv files. + + - dependencies in derivations on the outputs of + derivations that are themselves derivations outputs. )", }, }}; diff --git a/src/nix/app.cc b/src/nix/app.cc index fd4569bb4..e678b54f0 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -7,6 +7,7 @@ #include "names.hh" #include "command.hh" #include "derivations.hh" +#include "downstream-placeholder.hh" namespace nix { @@ -23,7 +24,7 @@ StringPairs resolveRewrites( if (auto drvDep = std::get_if(&dep.path)) for (auto & [ outputName, outputPath ] : drvDep->outputs) res.emplace( - downstreamPlaceholder(store, drvDep->drvPath, outputName), + DownstreamPlaceholder::unknownCaOutput(drvDep->drvPath, outputName).render(), store.printStorePath(outputPath) ); return res;