From d850983f1dfebf12aea361ca7aeb584ac33e8f68 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 25 May 2025 20:01:20 -0400 Subject: [PATCH] Store `StructuredAttrs` directly in `Derivation` Instead of parsing a structured attrs at some later point, we parsed it right away when parsing the A-Term format, and likewise serialize it to `__json = ` when serializing a derivation to A-Term. The JSON format can directly contain the JSON structured attrs without so encoding it, so we just do that. --- src/libexpr/primops.cc | 35 ++++++----- .../derivation-advanced-attrs.cc | 44 +++++-------- src/libstore-tests/derivation.cc | 4 +- .../build/derivation-building-goal.cc | 6 +- src/libstore/build/derivation-goal.cc | 3 +- src/libstore/derivation-options.cc | 6 ++ src/libstore/derivations.cc | 61 +++++++++++++++---- .../store/build/derivation-building-goal.hh | 3 +- .../include/nix/store/derivation-options.hh | 3 + src/libstore/include/nix/store/derivations.hh | 6 ++ .../include/nix/store/parsed-derivations.hh | 29 +++++++++ src/libstore/misc.cc | 5 +- src/libstore/parsed-derivations.cc | 51 +++++++++++++--- src/libstore/unix/build/derivation-builder.cc | 8 +-- .../nix/store/build/derivation-builder.hh | 11 ---- src/nix-build/nix-build.cc | 9 +-- 16 files changed, 183 insertions(+), 101 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 60f44ca62..fe1cbb143 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1266,14 +1266,16 @@ static void derivationStrictInternal( /* Check whether attributes should be passed as a JSON file. */ using nlohmann::json; - std::optional jsonObject; + std::optional jsonObject; auto pos = v.determinePos(noPos); auto attr = attrs->find(state.sStructuredAttrs); if (attr != attrs->end() && state.forceBool(*attr->value, pos, "while evaluating the `__structuredAttrs` " "attribute passed to builtins.derivationStrict")) - jsonObject = json::object(); + jsonObject = StructuredAttrs{ + .structuredAttrs = json::object() + }; /* Check whether null attributes should be ignored. */ bool ignoreNulls = false; @@ -1382,7 +1384,7 @@ static void derivationStrictInternal( if (i->name == state.sStructuredAttrs) continue; - jsonObject->emplace(key, printValueAsJSON(state, true, *i->value, pos, context)); + jsonObject->structuredAttrs.emplace(key, printValueAsJSON(state, true, *i->value, pos, context)); if (i->name == state.sBuilder) drv.builder = state.forceString(*i->value, context, pos, context_below); @@ -1419,16 +1421,19 @@ static void derivationStrictInternal( } else { auto s = state.coerceToString(pos, *i->value, context, context_below, true).toOwned(); - drv.env.emplace(key, s); - if (i->name == state.sBuilder) drv.builder = std::move(s); - else if (i->name == state.sSystem) drv.platform = std::move(s); - else if (i->name == state.sOutputHash) outputHash = std::move(s); - else if (i->name == state.sOutputHashAlgo) outputHashAlgo = parseHashAlgoOpt(s); - else if (i->name == state.sOutputHashMode) handleHashMode(s); - else if (i->name == state.sOutputs) - handleOutputs(tokenizeString(s)); - else if (i->name == state.sJson) + if (i->name == state.sJson) { warn("In derivation '%s': setting structured attributes via '__json' is deprecated, and may be disallowed in future versions of Nix. Set '__structuredAttrs = true' instead.", drvName); + drv.structuredAttrs = StructuredAttrs::parse(s); + } else { + drv.env.emplace(key, s); + if (i->name == state.sBuilder) drv.builder = std::move(s); + else if (i->name == state.sSystem) drv.platform = std::move(s); + else if (i->name == state.sOutputHash) outputHash = std::move(s); + else if (i->name == state.sOutputHashAlgo) outputHashAlgo = parseHashAlgoOpt(s); + else if (i->name == state.sOutputHashMode) handleHashMode(s); + else if (i->name == state.sOutputs) + handleOutputs(tokenizeString(s)); + } } } @@ -1441,8 +1446,10 @@ static void derivationStrictInternal( } if (jsonObject) { - drv.env.emplace("__json", jsonObject->dump()); - jsonObject.reset(); + /* The only other way `drv.structuredAttrs` can be set is when + `jsonObject` is not set. */ + assert(!drv.structuredAttrs); + drv.structuredAttrs = std::move(*jsonObject); } /* Everything in the context of the strings in the derivation diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc index b68134cd1..6f7dae15f 100644 --- a/src/libstore-tests/derivation-advanced-attrs.cc +++ b/src/libstore-tests/derivation-advanced-attrs.cc @@ -108,10 +108,9 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_defaults) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - EXPECT_TRUE(!parsedDrv); + EXPECT_TRUE(!got.structuredAttrs); EXPECT_EQ(options.additionalSandboxProfile, ""); EXPECT_EQ(options.noChroot, false); @@ -143,8 +142,7 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_defaults) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{}); }); @@ -157,8 +155,7 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_defaults) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{"ca-derivations"}); }); @@ -171,10 +168,9 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - EXPECT_TRUE(!parsedDrv); + EXPECT_TRUE(!got.structuredAttrs); EXPECT_EQ(options.additionalSandboxProfile, "sandcastle"); EXPECT_EQ(options.noChroot, true); @@ -195,8 +191,7 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_EQ( options.exportReferencesGraph, @@ -245,8 +240,7 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_EQ( options.exportReferencesGraph, @@ -298,10 +292,9 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs_d auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - EXPECT_TRUE(parsedDrv); + EXPECT_TRUE(got.structuredAttrs); EXPECT_EQ(options.additionalSandboxProfile, ""); EXPECT_EQ(options.noChroot, false); @@ -332,8 +325,7 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_defaults) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{}); }); @@ -346,8 +338,7 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_default auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{"ca-derivations"}); }); @@ -360,10 +351,9 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - EXPECT_TRUE(parsedDrv); + EXPECT_TRUE(got.structuredAttrs); EXPECT_EQ(options.additionalSandboxProfile, "sandcastle"); EXPECT_EQ(options.noChroot, true); @@ -394,8 +384,7 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_EQ( options.exportReferencesGraph, @@ -448,8 +437,7 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs) auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - auto parsedDrv = StructuredAttrs::tryParse(got.env); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_EQ( options.exportReferencesGraph, diff --git a/src/libstore-tests/derivation.cc b/src/libstore-tests/derivation.cc index fa6711d40..103ff369f 100644 --- a/src/libstore-tests/derivation.cc +++ b/src/libstore-tests/derivation.cc @@ -220,7 +220,7 @@ Derivation makeSimpleDrv(const Store & store) { "bar", "baz", }; - drv.env = { + drv.env = StringPairs{ { "BIG_BAD", "WOLF", @@ -278,7 +278,7 @@ Derivation makeDynDepDerivation(const Store & store) { "bar", "baz", }; - drv.env = { + drv.env = StringPairs{ { "BIG_BAD", "WOLF", diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 9a91b3592..51ba94467 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -32,12 +32,9 @@ DerivationBuildingGoal::DerivationBuildingGoal(const StorePath & drvPath, const { drv = std::make_unique(drv_); - if (auto parsedOpt = StructuredAttrs::tryParse(drv->env)) { - parsedDrv = std::make_unique(*parsedOpt); - } try { drvOptions = std::make_unique( - DerivationOptions::fromStructuredAttrs(drv->env, parsedDrv.get())); + DerivationOptions::fromStructuredAttrs(drv->env, drv->structuredAttrs)); } catch (Error & e) { e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath)); throw; @@ -628,7 +625,6 @@ Goal::Co DerivationBuildingGoal::tryToBuild() buildMode, buildResult, *drv, - parsedDrv.get(), *drvOptions, inputPaths, initialOutputs, diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3fcc376ed..d7ce2c5de 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -173,9 +173,8 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) trace("have derivation"); auto drvOptions = [&]() -> DerivationOptions { - auto parsedOpt = StructuredAttrs::tryParse(drv->env); try { - return DerivationOptions::fromStructuredAttrs(drv->env, parsedOpt ? &*parsedOpt : nullptr); + return DerivationOptions::fromStructuredAttrs(drv->env, drv->structuredAttrs); } catch (Error & e) { e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath)); throw; diff --git a/src/libstore/derivation-options.cc b/src/libstore/derivation-options.cc index f6bac2868..10cc729f4 100644 --- a/src/libstore/derivation-options.cc +++ b/src/libstore/derivation-options.cc @@ -92,6 +92,12 @@ using OutputChecks = DerivationOptions::OutputChecks; using OutputChecksVariant = std::variant>; +DerivationOptions DerivationOptions::fromStructuredAttrs( + const StringMap & env, const std::optional & parsed, bool shouldWarn) +{ + return fromStructuredAttrs(env, parsed ? &*parsed : nullptr); +} + DerivationOptions DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAttrs * parsed, bool shouldWarn) { diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 0657a7499..32aec6b4e 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -460,6 +460,7 @@ Derivation parseDerivation( expect(str, ")"); drv.env.insert_or_assign(std::move(name), std::move(value)); } + drv.structuredAttrs = StructuredAttrs::tryExtract(drv.env); expect(str, ")"); return drv; @@ -650,11 +651,23 @@ std::string Derivation::unparse(const StoreDirConfig & store, bool maskOutputs, s += ",["; first = true; - for (auto & i : env) { - if (first) first = false; else s += ','; - s += '('; printString(s, i.first); - s += ','; printString(s, maskOutputs && outputs.count(i.first) ? "" : i.second); - s += ')'; + + auto unparseEnv = [&](const StringPairs atermEnv) { + for (auto & i : atermEnv) { + if (first) first = false; else s += ','; + s += '('; printString(s, i.first); + s += ','; printString(s, maskOutputs && outputs.count(i.first) ? "" : i.second); + s += ')'; + } + }; + + StructuredAttrs::checkKeyNotInUse(env); + if (structuredAttrs) { + StringPairs scratch = env; + scratch.insert(structuredAttrs->unparse()); + unparseEnv(scratch); + } else { + unparseEnv(env); } s += "])"; @@ -953,6 +966,7 @@ Source & readDerivation(Source & in, const StoreDirConfig & store, BasicDerivati auto value = readString(in); drv.env[key] = value; } + drv.structuredAttrs = StructuredAttrs::tryExtract(drv.env); return in; } @@ -995,9 +1009,21 @@ void writeDerivation(Sink & out, const StoreDirConfig & store, const BasicDeriva CommonProto::WriteConn { .to = out }, drv.inputSrcs); out << drv.platform << drv.builder << drv.args; - out << drv.env.size(); - for (auto & i : drv.env) - out << i.first << i.second; + + auto writeEnv = [&](const StringPairs atermEnv) { + out << atermEnv.size(); + for (auto & [k, v] : atermEnv) + out << k << v; + }; + + StructuredAttrs::checkKeyNotInUse(drv.env); + if (drv.structuredAttrs) { + StringPairs scratch = drv.env; + scratch.insert(drv.structuredAttrs->unparse()); + writeEnv(scratch); + } else { + writeEnv(drv.env); + } } @@ -1027,6 +1053,17 @@ void BasicDerivation::applyRewrites(const StringMap & rewrites) newEnv.emplace(envName, envValue); } env = std::move(newEnv); + + if (structuredAttrs) { + // TODO rewrite the JSON AST properly, rather than dump parse round trip. + auto [k, jsonS] = structuredAttrs->unparse(); + jsonS = rewriteStrings(jsonS, rewrites); + StringPairs newEnv; + newEnv.insert(std::pair{k, std::move(jsonS)}); + auto newStructuredAttrs = StructuredAttrs::tryParse(newEnv); + assert(newStructuredAttrs); + structuredAttrs = std::move(*newStructuredAttrs); + } } static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) @@ -1333,10 +1370,8 @@ nlohmann::json Derivation::toJSON(const StoreDirConfig & store) const res["args"] = args; res["env"] = env; - if (auto it = env.find("__json"); it != env.end()) { - res["env"].erase("__json"); - res["structuredAttrs"] = nlohmann::json::parse(it->second); - } + if (structuredAttrs) + res["structuredAttrs"] = structuredAttrs->structuredAttrs; return res; } @@ -1411,7 +1446,7 @@ Derivation Derivation::fromJSON( } if (auto structuredAttrs = get(json, "structuredAttrs")) - res.env.insert_or_assign("__json", structuredAttrs->dump()); + res.structuredAttrs = StructuredAttrs{*structuredAttrs}; return res; } diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index bff2e7a89..b125fe27f 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -1,8 +1,8 @@ #pragma once ///@file -#include "nix/store/parsed-derivations.hh" #include "nix/store/derivations.hh" +#include "nix/store/parsed-derivations.hh" #include "nix/store/derivation-options.hh" #include "nix/store/build/derivation-building-misc.hh" #include "nix/store/outputs-spec.hh" @@ -41,7 +41,6 @@ struct DerivationBuildingGoal : public Goal */ std::unique_ptr drv; - std::unique_ptr parsedDrv; std::unique_ptr drvOptions; /** diff --git a/src/libstore/include/nix/store/derivation-options.hh b/src/libstore/include/nix/store/derivation-options.hh index f61a43e60..02827b9b9 100644 --- a/src/libstore/include/nix/store/derivation-options.hh +++ b/src/libstore/include/nix/store/derivation-options.hh @@ -176,6 +176,9 @@ struct DerivationOptions static DerivationOptions fromStructuredAttrs(const StringMap & env, const StructuredAttrs * parsed, bool shouldWarn = true); + static DerivationOptions + fromStructuredAttrs(const StringMap & env, const std::optional & parsed, bool shouldWarn = true); + /** * @param drv Must be the same derivation we parsed this from. In * the future we'll flip things around so a `BasicDerivation` has diff --git a/src/libstore/include/nix/store/derivations.hh b/src/libstore/include/nix/store/derivations.hh index a813137bc..54c498372 100644 --- a/src/libstore/include/nix/store/derivations.hh +++ b/src/libstore/include/nix/store/derivations.hh @@ -7,6 +7,7 @@ #include "nix/store/content-address.hh" #include "nix/util/repair-flag.hh" #include "nix/store/derived-path-map.hh" +#include "nix/store/parsed-derivations.hh" #include "nix/util/sync.hh" #include "nix/util/variant-wrapper.hh" @@ -294,7 +295,12 @@ struct BasicDerivation std::string platform; Path builder; Strings args; + /** + * Must not contain the key `__json`, at least in order to serialize to A-Term. + */ StringPairs env; + std::optional structuredAttrs; + std::string name; BasicDerivation() = default; diff --git a/src/libstore/include/nix/store/parsed-derivations.hh b/src/libstore/include/nix/store/parsed-derivations.hh index a7c053a8f..61d22e40b 100644 --- a/src/libstore/include/nix/store/parsed-derivations.hh +++ b/src/libstore/include/nix/store/parsed-derivations.hh @@ -18,8 +18,37 @@ struct StructuredAttrs { nlohmann::json structuredAttrs; + bool operator==(const StructuredAttrs &) const = default; + + /** + * Unconditionally parse from a JSON string. Used by `tryParse`. + */ + static StructuredAttrs parse(const std::string & encoded); + + /** + * Parse structured attrs from one of the env vars, as is used by the ATerm + * format. + */ static std::optional tryParse(const StringPairs & env); + /** + * Like `tryParse`, but removes the env var which encoded the structured + * attrs from the map if one is found. + */ + static std::optional tryExtract(StringPairs & env); + + /** + * Opposite of `tryParse`, at least if one makes a map from this + * single key-value PR. + */ + std::pair unparse() const; + + /** + * Ensures that the structured attrs "env var" is not in used, so we + * are free to use it instead. + */ + static void checkKeyNotInUse(const StringPairs & env); + nlohmann::json prepareStructuredAttrs( Store & store, const DerivationOptions & drvOptions, diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index dabae647f..06429e1d3 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -222,14 +222,11 @@ void Store::queryMissing(const std::vector & targets, if (knownOutputPaths && invalid.empty()) return; auto drv = make_ref(derivationFromPath(drvPath)); - auto parsedDrv = StructuredAttrs::tryParse(drv->env); DerivationOptions drvOptions; try { // FIXME: this is a lot of work just to get the value // of `allowSubstitutes`. - drvOptions = DerivationOptions::fromStructuredAttrs( - drv->env, - parsedDrv ? &*parsedDrv : nullptr); + drvOptions = DerivationOptions::fromStructuredAttrs(drv->env, drv->structuredAttrs); } catch (Error & e) { e.addTrace({}, "while parsing derivation '%s'", printStorePath(drvPath)); throw; diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index d6453c6db..898ec893f 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -8,20 +8,50 @@ namespace nix { +static constexpr std::string_view envVarName = "__json"; + +StructuredAttrs StructuredAttrs::parse(const std::string & encoded) +{ + try { + return StructuredAttrs { + .structuredAttrs = nlohmann::json::parse(encoded), + }; + } catch (std::exception & e) { + throw Error("cannot process __json attribute: %s", e.what()); + } +} + std::optional StructuredAttrs::tryParse(const StringPairs & env) { /* Parse the __json attribute, if any. */ - auto jsonAttr = env.find("__json"); + auto jsonAttr = env.find(envVarName); + if (jsonAttr != env.end()) + return parse(jsonAttr->second); + else + return {}; +} + +std::optional StructuredAttrs::tryExtract(StringPairs & env) +{ + /* Parse the __json attribute, if any. */ + auto jsonAttr = env.find(envVarName); if (jsonAttr != env.end()) { - try { - return StructuredAttrs { - .structuredAttrs = nlohmann::json::parse(jsonAttr->second), - }; - } catch (std::exception & e) { - throw Error("cannot process __json attribute: %s", e.what()); - } - } - return {}; + auto encoded = std::move(jsonAttr->second); + env.erase(jsonAttr); + return parse(encoded); + } else + return {}; +} + +std::pair StructuredAttrs::unparse() const +{ + return {envVarName, structuredAttrs.dump()}; +} + +void StructuredAttrs::checkKeyNotInUse(const StringPairs & env) +{ + if (env.count(envVarName)) + throw Error("Cannot have an environment variable named '__json'. This key is reserved for encoding structured attrs"); } static std::regex shVarName("[A-Za-z_][A-Za-z0-9_]*"); @@ -170,4 +200,5 @@ std::string StructuredAttrs::writeShell(const nlohmann::json & json) return jsonSh; } + } diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 1f15cc9e9..65a9e5d65 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -773,7 +773,7 @@ void DerivationBuilderImpl::startBuilder() writeStructuredAttrs(); /* Handle exportReferencesGraph(), if set. */ - if (!parsedDrv) { + if (!drv.structuredAttrs) { for (auto & [fileName, ss] : drvOptions.exportReferencesGraph) { StorePathSet storePathSet; for (auto & storePathS : ss) { @@ -1042,7 +1042,7 @@ void DerivationBuilderImpl::initEnv() /* In non-structured mode, set all bindings either directory in the environment or via a file, as specified by `DerivationOptions::passAsFile`. */ - if (!parsedDrv) { + if (!drv.structuredAttrs) { for (auto & i : drv.env) { if (drvOptions.passAsFile.find(i.first) == drvOptions.passAsFile.end()) { env[i.first] = i.second; @@ -1113,8 +1113,8 @@ void DerivationBuilderImpl::initEnv() void DerivationBuilderImpl::writeStructuredAttrs() { - if (parsedDrv) { - auto json = parsedDrv->prepareStructuredAttrs( + if (drv.structuredAttrs) { + auto json = drv.structuredAttrs->prepareStructuredAttrs( store, drvOptions, inputPaths, diff --git a/src/libstore/unix/include/nix/store/build/derivation-builder.hh b/src/libstore/unix/include/nix/store/build/derivation-builder.hh index 5ce38e034..856ea0b28 100644 --- a/src/libstore/unix/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/unix/include/nix/store/build/derivation-builder.hh @@ -27,15 +27,6 @@ struct DerivationBuilderParams */ const Derivation & drv; - /** - * The "structured attrs" of `drv`, if it has them. - * - * @todo this should be part of `Derivation`. - * - * @todo this should be renamed from `parsedDrv`. - */ - const StructuredAttrs * parsedDrv; - /** * The derivation options of `drv`. * @@ -63,14 +54,12 @@ struct DerivationBuilderParams const BuildMode & buildMode, BuildResult & buildResult, const Derivation & drv, - const StructuredAttrs * parsedDrv, const DerivationOptions & drvOptions, const StorePathSet & inputPaths, std::map & initialOutputs) : drvPath{drvPath} , buildResult{buildResult} , drv{drv} - , parsedDrv{parsedDrv} , drvOptions{drvOptions} , inputPaths{inputPaths} , initialOutputs{initialOutputs} diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index e302aca67..73662d442 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -544,12 +544,9 @@ static void main_nix_build(int argc, char * * argv) env["NIX_STORE"] = store->storeDir; env["NIX_BUILD_CORES"] = std::to_string(settings.buildCores); - auto parsedDrv = StructuredAttrs::tryParse(drv.env); DerivationOptions drvOptions; try { - drvOptions = DerivationOptions::fromStructuredAttrs( - drv.env, - parsedDrv ? &*parsedDrv : nullptr); + drvOptions = DerivationOptions::fromStructuredAttrs(drv.env, drv.structuredAttrs); } catch (Error & e) { e.addTrace({}, "while parsing derivation '%s'", store->printStorePath(packageInfo.requireDrvPath())); throw; @@ -568,7 +565,7 @@ static void main_nix_build(int argc, char * * argv) std::string structuredAttrsRC; - if (parsedDrv) { + if (drv.structuredAttrs) { StorePathSet inputs; std::function::ChildNode &)> accumInputClosure; @@ -586,7 +583,7 @@ static void main_nix_build(int argc, char * * argv) for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map) accumInputClosure(inputDrv, inputNode); - auto json = parsedDrv->prepareStructuredAttrs( + auto json = drv.structuredAttrs->prepareStructuredAttrs( *store, drvOptions, inputs,