Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 24 additions & 19 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1363,7 +1363,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName

/* Check whether attributes should be passed as a JSON file. */
using nlohmann::json;
std::optional<json> jsonObject;
std::optional<StructuredAttrs> jsonObject;
auto pos = v.determinePos(noPos);
auto attr = attrs->find(state.sStructuredAttrs);
if (attr != attrs->end()
Expand All @@ -1372,7 +1372,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
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;
Expand Down Expand Up @@ -1484,7 +1484,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
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);
Expand Down Expand Up @@ -1532,23 +1532,26 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName

} 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<Strings>(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<Strings>(s));
}
}
}

Expand All @@ -1560,8 +1563,10 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
}

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
Expand Down
44 changes: 16 additions & 28 deletions src/libstore-tests/derivation-advanced-attrs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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{});
});
Expand All @@ -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"});
});
Expand All @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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{});
});
Expand All @@ -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"});
});
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/libstore-tests/derivation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ Derivation makeSimpleDrv(const Store & store)
"bar",
"baz",
};
drv.env = {
drv.env = StringPairs{
{
"BIG_BAD",
"WOLF",
Expand Down Expand Up @@ -284,7 +284,7 @@ Derivation makeDynDepDerivation(const Store & store)
"bar",
"baz",
};
drv.env = {
drv.env = StringPairs{
{
"BIG_BAD",
"WOLF",
Expand Down
6 changes: 1 addition & 5 deletions src/libstore/build/derivation-building-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,9 @@ DerivationBuildingGoal::DerivationBuildingGoal(
{
drv = std::make_unique<Derivation>(drv_);

if (auto parsedOpt = StructuredAttrs::tryParse(drv->env)) {
parsedDrv = std::make_unique<StructuredAttrs>(*parsedOpt);
}
try {
drvOptions =
std::make_unique<DerivationOptions>(DerivationOptions::fromStructuredAttrs(drv->env, parsedDrv.get()));
std::make_unique<DerivationOptions>(DerivationOptions::fromStructuredAttrs(drv->env, drv->structuredAttrs));
} catch (Error & e) {
e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath));
throw;
Expand Down Expand Up @@ -661,7 +658,6 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
buildMode,
buildResult,
*drv,
parsedDrv.get(),
*drvOptions,
inputPaths,
initialOutputs,
Expand Down
3 changes: 1 addition & 2 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,8 @@ Goal::Co DerivationGoal::haveDerivation()
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;
Expand Down
6 changes: 6 additions & 0 deletions src/libstore/derivation-options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ using OutputChecks = DerivationOptions::OutputChecks;

using OutputChecksVariant = std::variant<OutputChecks, std::map<std::string, OutputChecks>>;

DerivationOptions DerivationOptions::fromStructuredAttrs(
const StringMap & env, const std::optional<StructuredAttrs> & parsed, bool shouldWarn)
{
return fromStructuredAttrs(env, parsed ? &*parsed : nullptr);
}

DerivationOptions
DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAttrs * parsed, bool shouldWarn)
{
Expand Down
69 changes: 52 additions & 17 deletions src/libstore/derivations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,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;
Expand Down Expand Up @@ -685,16 +686,28 @@ std::string Derivation::unparse(

s += ",[";
first = true;
for (auto & i : env) {
if (first)
first = false;
else

auto unparseEnv = [&](const StringPairs atermEnv) {
for (auto & i : atermEnv) {
if (first)
first = false;
else
s += ',';
s += '(';
printString(s, i.first);
s += ',';
s += '(';
printString(s, i.first);
s += ',';
printString(s, maskOutputs && outputs.count(i.first) ? "" : i.second);
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 += "])";
Expand Down Expand Up @@ -948,6 +961,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;
}
Expand Down Expand Up @@ -983,9 +997,21 @@ void writeDerivation(Sink & out, const StoreDirConfig & store, const BasicDeriva
}
CommonProto::write(store, 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);
}
}

std::string hashPlaceholder(const OutputNameView outputName)
Expand Down Expand Up @@ -1017,6 +1043,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::tryExtract(newEnv);
assert(newStructuredAttrs);
structuredAttrs = std::move(*newStructuredAttrs);
}
}

static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites)
Expand Down Expand Up @@ -1338,10 +1375,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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
Loading
Loading