From 6b70bda2485de5bde5fd5814a9f43402075a016b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 3 Feb 2025 11:41:33 -0500 Subject: [PATCH] Limit `ParsedDerivation` just to the derivation's environment This moves us towards getting rid of `ParsedDerivation` and just having `DerivationOptions`. Co-Authored-By: HaeNoe --- .../derivation-advanced-attrs.cc | 8 ++--- src/libstore/build/derivation-goal.cc | 2 +- src/libstore/derivation-options.cc | 4 +-- .../include/nix/store/parsed-derivations.hh | 11 ++++--- src/libstore/misc.cc | 2 +- src/libstore/parsed-derivations.cc | 31 ++++++++++--------- .../unix/build/local-derivation-goal.cc | 7 ++++- src/nix-build/nix-build.cc | 9 ++++-- 8 files changed, 44 insertions(+), 30 deletions(-) diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc index 4f381ab33..dd53c91a6 100644 --- a/src/libstore-tests/derivation-advanced-attrs.cc +++ b/src/libstore-tests/derivation-advanced-attrs.cc @@ -81,7 +81,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_defaults) auto drvPath = writeDerivation(*store, got, NoRepair, true); - ParsedDerivation parsedDrv(got); + ParsedDerivation parsedDrv(got.env); DerivationOptions options = DerivationOptions::fromParsedDerivation(*store, parsedDrv); EXPECT_TRUE(!parsedDrv.hasStructuredAttrs()); @@ -116,7 +116,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes) auto drvPath = writeDerivation(*store, got, NoRepair, true); - ParsedDerivation parsedDrv(got); + ParsedDerivation parsedDrv(got.env); DerivationOptions options = DerivationOptions::fromParsedDerivation(*store, parsedDrv); StringSet systemFeatures{"rainbow", "uid-range"}; @@ -157,7 +157,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr auto drvPath = writeDerivation(*store, got, NoRepair, true); - ParsedDerivation parsedDrv(got); + ParsedDerivation parsedDrv(got.env); DerivationOptions options = DerivationOptions::fromParsedDerivation(*store, parsedDrv); EXPECT_TRUE(parsedDrv.hasStructuredAttrs()); @@ -191,7 +191,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr auto drvPath = writeDerivation(*store, got, NoRepair, true); - ParsedDerivation parsedDrv(got); + ParsedDerivation parsedDrv(got.env); DerivationOptions options = DerivationOptions::fromParsedDerivation(*store, parsedDrv); StringSet systemFeatures{"rainbow", "uid-range"}; diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 729b52d63..c521ab4d1 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -180,7 +180,7 @@ Goal::Co DerivationGoal::haveDerivation() { trace("have derivation"); - parsedDrv = std::make_unique(*drv); + parsedDrv = std::make_unique(drv->env); try { drvOptions = std::make_unique( DerivationOptions::fromParsedDerivation(worker.store, *parsedDrv)); diff --git a/src/libstore/derivation-options.cc b/src/libstore/derivation-options.cc index 841b91716..068c64626 100644 --- a/src/libstore/derivation-options.cc +++ b/src/libstore/derivation-options.cc @@ -117,7 +117,7 @@ DerivationOptions::fromParsedDerivation(const StoreDirConfig & store, const Pars .passAsFile = [&] { StringSet res; - if (auto * passAsFileString = get(parsed.drv.env, "passAsFile")) { + if (auto * passAsFileString = get(parsed.env, "passAsFile")) { if (parsed.hasStructuredAttrs()) { if (shouldWarn) { warn( @@ -144,7 +144,7 @@ DerivationOptions::fromParsedDerivation(const StoreDirConfig & store, const Pars ret.insert_or_assign(key, std::move(storePaths)); } } else { - auto s = getOr(parsed.drv.env, "exportReferencesGraph", ""); + auto s = getOr(parsed.env, "exportReferencesGraph", ""); Strings ss = tokenizeString(s); if (ss.size() % 2 != 0) throw BuildError("odd number of tokens in 'exportReferencesGraph': '%1%'", s); diff --git a/src/libstore/include/nix/store/parsed-derivations.hh b/src/libstore/include/nix/store/parsed-derivations.hh index e23eab1bb..30a294187 100644 --- a/src/libstore/include/nix/store/parsed-derivations.hh +++ b/src/libstore/include/nix/store/parsed-derivations.hh @@ -12,7 +12,7 @@ struct DerivationOptions; class ParsedDerivation { - BasicDerivation & drv; + const StringPairs & env; std::unique_ptr structuredAttrs; std::optional getStringAttr(const std::string & name) const; @@ -33,7 +33,7 @@ class ParsedDerivation public: - ParsedDerivation(BasicDerivation & drv); + ParsedDerivation(const StringPairs & env); ~ParsedDerivation(); @@ -42,8 +42,11 @@ public: return static_cast(structuredAttrs); } - std::optional - prepareStructuredAttrs(Store & store, const DerivationOptions & drvOptions, const StorePathSet & inputPaths); + std::optional prepareStructuredAttrs( + Store & store, + const DerivationOptions & drvOptions, + const StorePathSet & inputPaths, + const DerivationOutputs & outputs); }; std::string writeStructuredAttrsShell(const nlohmann::json & json); diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 20b25b4ca..5f97590a2 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -222,7 +222,7 @@ void Store::queryMissing(const std::vector & targets, if (knownOutputPaths && invalid.empty()) return; auto drv = make_ref(derivationFromPath(drvPath)); - ParsedDerivation parsedDrv(*drv); + ParsedDerivation parsedDrv(drv->env); DerivationOptions drvOptions; try { drvOptions = DerivationOptions::fromParsedDerivation(*this, parsedDrv); diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index c4d4ff0d6..698e218d7 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -6,12 +6,12 @@ namespace nix { -ParsedDerivation::ParsedDerivation(BasicDerivation & drv) - : drv(drv) +ParsedDerivation::ParsedDerivation(const StringPairs & env) + : env(env) { /* Parse the __json attribute, if any. */ - auto jsonAttr = drv.env.find("__json"); - if (jsonAttr != drv.env.end()) { + auto jsonAttr = env.find("__json"); + if (jsonAttr != env.end()) { try { structuredAttrs = std::make_unique(nlohmann::json::parse(jsonAttr->second)); } catch (std::exception & e) { @@ -34,8 +34,8 @@ std::optional ParsedDerivation::getStringAttr(const std::string & n return i->get(); } } else { - auto i = drv.env.find(name); - if (i == drv.env.end()) + auto i = env.find(name); + if (i == env.end()) return {}; else return i->second; @@ -54,8 +54,8 @@ bool ParsedDerivation::getBoolAttr(const std::string & name, bool def) const return i->get(); } } else { - auto i = drv.env.find(name); - if (i == drv.env.end()) + auto i = env.find(name); + if (i == env.end()) return def; else return i->second == "1"; @@ -80,8 +80,8 @@ std::optional ParsedDerivation::getStringsAttr(const std::string & name return res; } } else { - auto i = drv.env.find(name); - if (i == drv.env.end()) + auto i = env.find(name); + if (i == env.end()) return {}; else return tokenizeString(i->second); @@ -155,17 +155,18 @@ static nlohmann::json pathInfoToJSON( std::optional ParsedDerivation::prepareStructuredAttrs( Store & store, const DerivationOptions & drvOptions, - const StorePathSet & inputPaths) + const StorePathSet & inputPaths, + const DerivationOutputs & outputs) { if (!structuredAttrs) return std::nullopt; auto json = *structuredAttrs; /* Add an "outputs" object containing the output paths. */ - nlohmann::json outputs; - for (auto & i : drv.outputs) - outputs[i.first] = hashPlaceholder(i.first); - json["outputs"] = outputs; + nlohmann::json outputsJson; + for (auto & i : outputs) + outputsJson[i.first] = hashPlaceholder(i.first); + json["outputs"] = std::move(outputsJson); /* Handle exportReferencesGraph. */ for (auto & [key, storePaths] : drvOptions.exportReferencesGraph) { diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index b306c3ccd..161dac8d7 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -1570,7 +1570,12 @@ void LocalDerivationGoal::initEnv() void LocalDerivationGoal::writeStructuredAttrs() { - if (auto structAttrsJson = parsedDrv->prepareStructuredAttrs(worker.store, *drvOptions, inputPaths)) { + if (auto structAttrsJson = parsedDrv->prepareStructuredAttrs( + worker.store, + *drvOptions, + inputPaths, + drv->outputs)) + { auto json = structAttrsJson.value(); nlohmann::json rewritten; for (auto & [i, v] : json["outputs"].get()) { diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index e07233356..9d5677faa 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -544,7 +544,7 @@ static void main_nix_build(int argc, char * * argv) env["NIX_STORE"] = store->storeDir; env["NIX_BUILD_CORES"] = std::to_string(settings.buildCores); - ParsedDerivation parsedDrv(drv); + ParsedDerivation parsedDrv(drv.env); DerivationOptions drvOptions; try { drvOptions = DerivationOptions::fromParsedDerivation(*store, parsedDrv); @@ -584,7 +584,12 @@ static void main_nix_build(int argc, char * * argv) for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map) accumInputClosure(inputDrv, inputNode); - if (auto structAttrs = parsedDrv.prepareStructuredAttrs(*store, drvOptions, inputs)) { + if (auto structAttrs = parsedDrv.prepareStructuredAttrs( + *store, + drvOptions, + inputs, + drv.outputs)) + { auto json = structAttrs.value(); structuredAttrsRC = writeStructuredAttrsShell(json);