Skip to content

Commit e8800da

Browse files
Ericson2314Ericson2314
committed
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 = <JSON dump>` 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.
1 parent a4713ed commit e8800da

21 files changed

+334
-104
lines changed

‎src/libexpr/primops.cc

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,14 +1258,17 @@ static void derivationStrictInternal(
12581258

12591259
/* Check whether attributes should be passed as a JSON file. */
12601260
using nlohmann::json;
1261-
std::optional<json> jsonObject;
1261+
std::optional<StructuredAttrs> jsonObject;
1262+
std::optional<StructuredAttrs> jsonObjectHack;
12621263
auto pos = v.determinePos(noPos);
12631264
auto attr = attrs->find(state.sStructuredAttrs);
12641265
if (attr != attrs->end() &&
12651266
state.forceBool(*attr->value, pos,
12661267
"while evaluating the `__structuredAttrs` "
12671268
"attribute passed to builtins.derivationStrict"))
1268-
jsonObject = json::object();
1269+
jsonObject = StructuredAttrs{
1270+
.structuredAttrs = json::object()
1271+
};
12691272

12701273
/* Check whether null attributes should be ignored. */
12711274
bool ignoreNulls = false;
@@ -1374,7 +1377,7 @@ static void derivationStrictInternal(
13741377

13751378
if (i->name == state.sStructuredAttrs) continue;
13761379

1377-
jsonObject->emplace(key, printValueAsJSON(state, true, *i->value, pos, context));
1380+
jsonObject->structuredAttrs.emplace(key, printValueAsJSON(state, true, *i->value, pos, context));
13781381

13791382
if (i->name == state.sBuilder)
13801383
drv.builder = state.forceString(*i->value, context, pos, context_below);
@@ -1411,16 +1414,19 @@ static void derivationStrictInternal(
14111414

14121415
} else {
14131416
auto s = state.coerceToString(pos, *i->value, context, context_below, true).toOwned();
1414-
drv.env.emplace(key, s);
1415-
if (i->name == state.sBuilder) drv.builder = std::move(s);
1416-
else if (i->name == state.sSystem) drv.platform = std::move(s);
1417-
else if (i->name == state.sOutputHash) outputHash = std::move(s);
1418-
else if (i->name == state.sOutputHashAlgo) outputHashAlgo = parseHashAlgoOpt(s);
1419-
else if (i->name == state.sOutputHashMode) handleHashMode(s);
1420-
else if (i->name == state.sOutputs)
1421-
handleOutputs(tokenizeString<Strings>(s));
1422-
else if (i->name == state.sJson)
1417+
if (i->name == state.sJson) {
14231418
warn("In a derivation named '%s', setting structured attrs via '__json' is deprecated, and may be removed in a future version of Nix", drvName);
1419+
jsonObjectHack = StructuredAttrs::parse(s);
1420+
} else {
1421+
drv.env.emplace(key, s);
1422+
if (i->name == state.sBuilder) drv.builder = std::move(s);
1423+
else if (i->name == state.sSystem) drv.platform = std::move(s);
1424+
else if (i->name == state.sOutputHash) outputHash = std::move(s);
1425+
else if (i->name == state.sOutputHashAlgo) outputHashAlgo = parseHashAlgoOpt(s);
1426+
else if (i->name == state.sOutputHashMode) handleHashMode(s);
1427+
else if (i->name == state.sOutputs)
1428+
handleOutputs(tokenizeString<Strings>(s));
1429+
}
14241430
}
14251431

14261432
}
@@ -1432,9 +1438,15 @@ static void derivationStrictInternal(
14321438
}
14331439
}
14341440

1441+
/* We should only set `jsonObjectHack` when `jsonObject` is not set. */
1442+
assert(!(jsonObject && jsonObjectHack));
1443+
14351444
if (jsonObject) {
1436-
drv.env.emplace("__json", jsonObject->dump());
1437-
jsonObject.reset();
1445+
drv.structuredAttrs = std::move(*jsonObject);
1446+
}
1447+
1448+
if (jsonObjectHack) {
1449+
drv.structuredAttrs = std::move(*jsonObjectHack);
14381450
}
14391451

14401452
/* Everything in the context of the strings in the derivation

‎src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults.json

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
],
66
"builder": "/bin/bash",
77
"env": {
8-
"__json": "{\"builder\":\"/bin/bash\",\"name\":\"advanced-attributes-structured-attrs-defaults\",\"outputHashAlgo\":\"sha256\",\"outputHashMode\":\"recursive\",\"outputs\":[\"out\",\"dev\"],\"system\":\"my-system\"}",
98
"dev": "/02qcpld1y6xhs5gz9bchpxaw0xdhmsp5dv88lh25r2ss44kh8dxz",
109
"out": "/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9"
1110
},
@@ -22,5 +21,16 @@
2221
"method": "nar"
2322
}
2423
},
24+
"structuredAttrs": {
25+
"builder": "/bin/bash",
26+
"name": "advanced-attributes-structured-attrs-defaults",
27+
"outputHashAlgo": "sha256",
28+
"outputHashMode": "recursive",
29+
"outputs": [
30+
"out",
31+
"dev"
32+
],
33+
"system": "my-system"
34+
},
2535
"system": "my-system"
2636
}

‎src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
],
66
"builder": "/bin/bash",
77
"env": {
8-
"__json": "{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"exportReferencesGraph\":{\"refs1\":[\"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9\"],\"refs2\":[\"/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv\"]},\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g\"],\"disallowedRequisites\":[\"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9\"],\"allowedRequisites\":[\"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z\"]}},\"outputHashAlgo\":\"sha256\",\"outputHashMode\":\"recursive\",\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}",
98
"bin": "/04f3da1kmbr67m3gzxikmsl4vjz5zf777sv6m14ahv22r65aac9m",
109
"dev": "/02qcpld1y6xhs5gz9bchpxaw0xdhmsp5dv88lh25r2ss44kh8dxz",
1110
"out": "/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9"
@@ -44,5 +43,62 @@
4443
"method": "nar"
4544
}
4645
},
46+
"structuredAttrs": {
47+
"__darwinAllowLocalNetworking": true,
48+
"__impureHostDeps": [
49+
"/usr/bin/ditto"
50+
],
51+
"__noChroot": true,
52+
"__sandboxProfile": "sandcastle",
53+
"allowSubstitutes": false,
54+
"builder": "/bin/bash",
55+
"exportReferencesGraph": {
56+
"refs1": [
57+
"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"
58+
],
59+
"refs2": [
60+
"/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"
61+
]
62+
},
63+
"impureEnvVars": [
64+
"UNICORN"
65+
],
66+
"name": "advanced-attributes-structured-attrs",
67+
"outputChecks": {
68+
"bin": {
69+
"disallowedReferences": [
70+
"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g"
71+
],
72+
"disallowedRequisites": [
73+
"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"
74+
]
75+
},
76+
"dev": {
77+
"maxClosureSize": 5909,
78+
"maxSize": 789
79+
},
80+
"out": {
81+
"allowedReferences": [
82+
"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"
83+
],
84+
"allowedRequisites": [
85+
"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z"
86+
]
87+
}
88+
},
89+
"outputHashAlgo": "sha256",
90+
"outputHashMode": "recursive",
91+
"outputs": [
92+
"out",
93+
"bin",
94+
"dev"
95+
],
96+
"preferLocalBuild": true,
97+
"requiredSystemFeatures": [
98+
"rainbow",
99+
"uid-range"
100+
],
101+
"system": "my-system"
102+
},
47103
"system": "my-system"
48104
}

‎src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults.json

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
],
66
"builder": "/bin/bash",
77
"env": {
8-
"__json": "{\"builder\":\"/bin/bash\",\"name\":\"advanced-attributes-structured-attrs-defaults\",\"outputs\":[\"out\",\"dev\"],\"system\":\"my-system\"}",
98
"dev": "/nix/store/8bazivnbipbyi569623skw5zm91z6kc2-advanced-attributes-structured-attrs-defaults-dev",
109
"out": "/nix/store/f8f8nvnx32bxvyxyx2ff7akbvwhwd9dw-advanced-attributes-structured-attrs-defaults"
1110
},
@@ -20,5 +19,14 @@
2019
"path": "/nix/store/f8f8nvnx32bxvyxyx2ff7akbvwhwd9dw-advanced-attributes-structured-attrs-defaults"
2120
}
2221
},
22+
"structuredAttrs": {
23+
"builder": "/bin/bash",
24+
"name": "advanced-attributes-structured-attrs-defaults",
25+
"outputs": [
26+
"out",
27+
"dev"
28+
],
29+
"system": "my-system"
30+
},
2331
"system": "my-system"
2432
}

‎src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
],
66
"builder": "/bin/bash",
77
"env": {
8-
"__json": "{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"exportReferencesGraph\":{\"refs1\":[\"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo\"],\"refs2\":[\"/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv\"]},\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar\"],\"disallowedRequisites\":[\"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo\"],\"allowedRequisites\":[\"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev\"]}},\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}",
98
"bin": "/nix/store/33qms3h55wlaspzba3brlzlrm8m2239g-advanced-attributes-structured-attrs-bin",
109
"dev": "/nix/store/wyfgwsdi8rs851wmy1xfzdxy7y5vrg5l-advanced-attributes-structured-attrs-dev",
1110
"out": "/nix/store/7cxy4zx1vqc885r4jl2l64pymqbdmhii-advanced-attributes-structured-attrs"
@@ -41,5 +40,60 @@
4140
"path": "/nix/store/7cxy4zx1vqc885r4jl2l64pymqbdmhii-advanced-attributes-structured-attrs"
4241
}
4342
},
43+
"structuredAttrs": {
44+
"__darwinAllowLocalNetworking": true,
45+
"__impureHostDeps": [
46+
"/usr/bin/ditto"
47+
],
48+
"__noChroot": true,
49+
"__sandboxProfile": "sandcastle",
50+
"allowSubstitutes": false,
51+
"builder": "/bin/bash",
52+
"exportReferencesGraph": {
53+
"refs1": [
54+
"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"
55+
],
56+
"refs2": [
57+
"/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"
58+
]
59+
},
60+
"impureEnvVars": [
61+
"UNICORN"
62+
],
63+
"name": "advanced-attributes-structured-attrs",
64+
"outputChecks": {
65+
"bin": {
66+
"disallowedReferences": [
67+
"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar"
68+
],
69+
"disallowedRequisites": [
70+
"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev"
71+
]
72+
},
73+
"dev": {
74+
"maxClosureSize": 5909,
75+
"maxSize": 789
76+
},
77+
"out": {
78+
"allowedReferences": [
79+
"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"
80+
],
81+
"allowedRequisites": [
82+
"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev"
83+
]
84+
}
85+
},
86+
"outputs": [
87+
"out",
88+
"bin",
89+
"dev"
90+
],
91+
"preferLocalBuild": true,
92+
"requiredSystemFeatures": [
93+
"rainbow",
94+
"uid-range"
95+
],
96+
"system": "my-system"
97+
},
4498
"system": "my-system"
4599
}

‎src/libstore-tests/derivation-advanced-attrs.cc

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,9 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_defaults)
108108

109109
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
110110

111-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
112-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
111+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
113112

114-
EXPECT_TRUE(!parsedDrv);
113+
EXPECT_TRUE(!got.structuredAttrs);
115114

116115
EXPECT_EQ(options.additionalSandboxProfile, "");
117116
EXPECT_EQ(options.noChroot, false);
@@ -143,8 +142,7 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_defaults)
143142

144143
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
145144

146-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
147-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
145+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
148146

149147
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{});
150148
});
@@ -157,8 +155,7 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_defaults)
157155

158156
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
159157

160-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
161-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
158+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
162159

163160
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{"ca-derivations"});
164161
});
@@ -171,10 +168,9 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes)
171168

172169
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
173170

174-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
175-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
171+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
176172

177-
EXPECT_TRUE(!parsedDrv);
173+
EXPECT_TRUE(!got.structuredAttrs);
178174

179175
EXPECT_EQ(options.additionalSandboxProfile, "sandcastle");
180176
EXPECT_EQ(options.noChroot, true);
@@ -195,8 +191,7 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes)
195191

196192
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
197193

198-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
199-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
194+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
200195

201196
EXPECT_EQ(
202197
options.exportReferencesGraph,
@@ -245,8 +240,7 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes)
245240

246241
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
247242

248-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
249-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
243+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
250244

251245
EXPECT_EQ(
252246
options.exportReferencesGraph,
@@ -298,10 +292,9 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs_d
298292

299293
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
300294

301-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
302-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
295+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
303296

304-
EXPECT_TRUE(parsedDrv);
297+
EXPECT_TRUE(got.structuredAttrs);
305298

306299
EXPECT_EQ(options.additionalSandboxProfile, "");
307300
EXPECT_EQ(options.noChroot, false);
@@ -332,8 +325,7 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_defaults)
332325

333326
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
334327

335-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
336-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
328+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
337329

338330
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{});
339331
});
@@ -346,8 +338,7 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_default
346338

347339
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
348340

349-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
350-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
341+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
351342

352343
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{"ca-derivations"});
353344
});
@@ -360,10 +351,9 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs)
360351

361352
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
362353

363-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
364-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
354+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
365355

366-
EXPECT_TRUE(parsedDrv);
356+
EXPECT_TRUE(got.structuredAttrs);
367357

368358
EXPECT_EQ(options.additionalSandboxProfile, "sandcastle");
369359
EXPECT_EQ(options.noChroot, true);
@@ -394,8 +384,7 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs)
394384

395385
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
396386

397-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
398-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
387+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
399388

400389
EXPECT_EQ(
401390
options.exportReferencesGraph,
@@ -448,8 +437,7 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs)
448437

449438
auto drvPath = writeDerivation(*this->store, got, NoRepair, true);
450439

451-
auto parsedDrv = StructuredAttrs::tryParse(got.env);
452-
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
440+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs);
453441

454442
EXPECT_EQ(
455443
options.exportReferencesGraph,

0 commit comments

Comments
 (0)