Skip to content

Commit cf6da92

Browse files
Mic92Mic92
authored
Merge pull request #12410 from obsidiansystems/derivation-options-2
Scrap `ParsedDerivation` for parts
2 parents 99a16c5 + d8be4f6 commit cf6da92

10 files changed

Lines changed: 223 additions & 216 deletions

File tree

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

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

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

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

114-
EXPECT_TRUE(!parsedDrv.hasStructuredAttrs());
114+
EXPECT_TRUE(!parsedDrv);
115115

116116
EXPECT_EQ(options.additionalSandboxProfile, "");
117117
EXPECT_EQ(options.noChroot, false);
@@ -143,8 +143,8 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_defaults)
143143

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

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

149149
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{});
150150
});
@@ -157,8 +157,8 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_defaults)
157157

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

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

163163
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{"ca-derivations"});
164164
});
@@ -171,10 +171,10 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes)
171171

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

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

177-
EXPECT_TRUE(!parsedDrv.hasStructuredAttrs());
177+
EXPECT_TRUE(!parsedDrv);
178178

179179
EXPECT_EQ(options.additionalSandboxProfile, "sandcastle");
180180
EXPECT_EQ(options.noChroot, true);
@@ -195,8 +195,8 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes)
195195

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

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

201201
EXPECT_EQ(
202202
options.exportReferencesGraph,
@@ -243,8 +243,8 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes)
243243

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

246-
ParsedDerivation parsedDrv(got);
247-
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
246+
auto parsedDrv = StructuredAttrs::tryParse(got.env);
247+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
248248

249249
EXPECT_EQ(
250250
options.exportReferencesGraph,
@@ -296,10 +296,10 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs_d
296296

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

299-
ParsedDerivation parsedDrv(got);
300-
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
299+
auto parsedDrv = StructuredAttrs::tryParse(got.env);
300+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
301301

302-
EXPECT_TRUE(parsedDrv.hasStructuredAttrs());
302+
EXPECT_TRUE(parsedDrv);
303303

304304
EXPECT_EQ(options.additionalSandboxProfile, "");
305305
EXPECT_EQ(options.noChroot, false);
@@ -330,8 +330,8 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_defaults)
330330

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

333-
ParsedDerivation parsedDrv(got);
334-
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
333+
auto parsedDrv = StructuredAttrs::tryParse(got.env);
334+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
335335

336336
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{});
337337
});
@@ -344,8 +344,8 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_default
344344

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

347-
ParsedDerivation parsedDrv(got);
348-
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
347+
auto parsedDrv = StructuredAttrs::tryParse(got.env);
348+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
349349

350350
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{"ca-derivations"});
351351
});
@@ -358,10 +358,10 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs)
358358

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

361-
ParsedDerivation parsedDrv(got);
362-
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
361+
auto parsedDrv = StructuredAttrs::tryParse(got.env);
362+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
363363

364-
EXPECT_TRUE(parsedDrv.hasStructuredAttrs());
364+
EXPECT_TRUE(parsedDrv);
365365

366366
EXPECT_EQ(options.additionalSandboxProfile, "sandcastle");
367367
EXPECT_EQ(options.noChroot, true);
@@ -392,8 +392,8 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs)
392392

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

395-
ParsedDerivation parsedDrv(got);
396-
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
395+
auto parsedDrv = StructuredAttrs::tryParse(got.env);
396+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
397397

398398
EXPECT_EQ(
399399
options.exportReferencesGraph,
@@ -445,8 +445,8 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs)
445445

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

448-
ParsedDerivation parsedDrv(got);
449-
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
448+
auto parsedDrv = StructuredAttrs::tryParse(got.env);
449+
DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, parsedDrv ? &*parsedDrv : nullptr);
450450

451451
EXPECT_EQ(
452452
options.exportReferencesGraph,

‎src/libstore/build/derivation-goal.cc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,12 @@ Goal::Co DerivationGoal::haveDerivation()
180180
{
181181
trace("have derivation");
182182

183-
parsedDrv = std::make_unique<ParsedDerivation>(*drv);
183+
if (auto parsedOpt = StructuredAttrs::tryParse(drv->env)) {
184+
parsedDrv = std::make_unique<StructuredAttrs>(*parsedOpt);
185+
}
184186
try {
185-
drvOptions = std::make_unique<DerivationOptions>(DerivationOptions::fromParsedDerivation(*parsedDrv));
187+
drvOptions = std::make_unique<DerivationOptions>(
188+
DerivationOptions::fromStructuredAttrs(drv->env, parsedDrv.get()));
186189
} catch (Error & e) {
187190
e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath));
188191
throw;

‎src/libstore/derivation-options.cc‎

Lines changed: 108 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#include "nix/store/derivation-options.hh"
22
#include "nix/util/json-utils.hh"
33
#include "nix/store/parsed-derivations.hh"
4+
#include "nix/store/derivations.hh"
5+
#include "nix/store/store-api.hh"
46
#include "nix/util/types.hh"
57
#include "nix/util/util.hh"
68

@@ -11,48 +13,122 @@
1113

1214
namespace nix {
1315

16+
static std::optional<std::string>
17+
getStringAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name)
18+
{
19+
if (parsed) {
20+
auto i = parsed->structuredAttrs.find(name);
21+
if (i == parsed->structuredAttrs.end())
22+
return {};
23+
else {
24+
if (!i->is_string())
25+
throw Error("attribute '%s' of must be a string", name);
26+
return i->get<std::string>();
27+
}
28+
} else {
29+
auto i = env.find(name);
30+
if (i == env.end())
31+
return {};
32+
else
33+
return i->second;
34+
}
35+
}
36+
37+
static bool getBoolAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name, bool def)
38+
{
39+
if (parsed) {
40+
auto i = parsed->structuredAttrs.find(name);
41+
if (i == parsed->structuredAttrs.end())
42+
return def;
43+
else {
44+
if (!i->is_boolean())
45+
throw Error("attribute '%s' must be a Boolean", name);
46+
return i->get<bool>();
47+
}
48+
} else {
49+
auto i = env.find(name);
50+
if (i == env.end())
51+
return def;
52+
else
53+
return i->second == "1";
54+
}
55+
}
56+
57+
static std::optional<Strings>
58+
getStringsAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name)
59+
{
60+
if (parsed) {
61+
auto i = parsed->structuredAttrs.find(name);
62+
if (i == parsed->structuredAttrs.end())
63+
return {};
64+
else {
65+
if (!i->is_array())
66+
throw Error("attribute '%s' must be a list of strings", name);
67+
Strings res;
68+
for (auto j = i->begin(); j != i->end(); ++j) {
69+
if (!j->is_string())
70+
throw Error("attribute '%s' must be a list of strings", name);
71+
res.push_back(j->get<std::string>());
72+
}
73+
return res;
74+
}
75+
} else {
76+
auto i = env.find(name);
77+
if (i == env.end())
78+
return {};
79+
else
80+
return tokenizeString<Strings>(i->second);
81+
}
82+
}
83+
84+
static std::optional<StringSet>
85+
getStringSetAttr(const StringMap & env, const StructuredAttrs * parsed, const std::string & name)
86+
{
87+
auto ss = getStringsAttr(env, parsed, name);
88+
return ss ? (std::optional{StringSet{ss->begin(), ss->end()}}) : (std::optional<StringSet>{});
89+
}
90+
1491
using OutputChecks = DerivationOptions::OutputChecks;
1592

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

18-
DerivationOptions DerivationOptions::fromParsedDerivation(const ParsedDerivation & parsed, bool shouldWarn)
95+
DerivationOptions
96+
DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAttrs * parsed, bool shouldWarn)
1997
{
2098
DerivationOptions defaults = {};
2199

22-
auto structuredAttrs = parsed.structuredAttrs.get();
23-
24-
if (shouldWarn && structuredAttrs) {
25-
if (get(*structuredAttrs, "allowedReferences")) {
100+
if (shouldWarn && parsed) {
101+
if (get(parsed->structuredAttrs, "allowedReferences")) {
26102
warn(
27103
"'structuredAttrs' disables the effect of the top-level attribute 'allowedReferences'; use 'outputChecks' instead");
28104
}
29-
if (get(*structuredAttrs, "allowedRequisites")) {
105+
if (get(parsed->structuredAttrs, "allowedRequisites")) {
30106
warn(
31107
"'structuredAttrs' disables the effect of the top-level attribute 'allowedRequisites'; use 'outputChecks' instead");
32108
}
33-
if (get(*structuredAttrs, "disallowedRequisites")) {
109+
if (get(parsed->structuredAttrs, "disallowedRequisites")) {
34110
warn(
35111
"'structuredAttrs' disables the effect of the top-level attribute 'disallowedRequisites'; use 'outputChecks' instead");
36112
}
37-
if (get(*structuredAttrs, "disallowedReferences")) {
113+
if (get(parsed->structuredAttrs, "disallowedReferences")) {
38114
warn(
39115
"'structuredAttrs' disables the effect of the top-level attribute 'disallowedReferences'; use 'outputChecks' instead");
40116
}
41-
if (get(*structuredAttrs, "maxSize")) {
117+
if (get(parsed->structuredAttrs, "maxSize")) {
42118
warn(
43119
"'structuredAttrs' disables the effect of the top-level attribute 'maxSize'; use 'outputChecks' instead");
44120
}
45-
if (get(*structuredAttrs, "maxClosureSize")) {
121+
if (get(parsed->structuredAttrs, "maxClosureSize")) {
46122
warn(
47123
"'structuredAttrs' disables the effect of the top-level attribute 'maxClosureSize'; use 'outputChecks' instead");
48124
}
49125
}
50126

51127
return {
52128
.outputChecks = [&]() -> OutputChecksVariant {
53-
if (auto structuredAttrs = parsed.structuredAttrs.get()) {
129+
if (parsed) {
54130
std::map<std::string, OutputChecks> res;
55-
if (auto outputChecks = get(*structuredAttrs, "outputChecks")) {
131+
if (auto outputChecks = get(parsed->structuredAttrs, "outputChecks")) {
56132
for (auto & [outputName, output] : getObject(*outputChecks)) {
57133
OutputChecks checks;
58134

@@ -90,19 +166,19 @@ DerivationOptions DerivationOptions::fromParsedDerivation(const ParsedDerivation
90166
return OutputChecks{
91167
// legacy non-structured-attributes case
92168
.ignoreSelfRefs = true,
93-
.allowedReferences = parsed.getStringSetAttr("allowedReferences"),
94-
.disallowedReferences = parsed.getStringSetAttr("disallowedReferences").value_or(StringSet{}),
95-
.allowedRequisites = parsed.getStringSetAttr("allowedRequisites"),
96-
.disallowedRequisites = parsed.getStringSetAttr("disallowedRequisites").value_or(StringSet{}),
169+
.allowedReferences = getStringSetAttr(env, parsed, "allowedReferences"),
170+
.disallowedReferences = getStringSetAttr(env, parsed, "disallowedReferences").value_or(StringSet{}),
171+
.allowedRequisites = getStringSetAttr(env, parsed, "allowedRequisites"),
172+
.disallowedRequisites = getStringSetAttr(env, parsed, "disallowedRequisites").value_or(StringSet{}),
97173
};
98174
}
99175
}(),
100176
.unsafeDiscardReferences =
101177
[&] {
102178
std::map<std::string, bool> res;
103179

104-
if (auto structuredAttrs = parsed.structuredAttrs.get()) {
105-
if (auto udr = get(*structuredAttrs, "unsafeDiscardReferences")) {
180+
if (parsed) {
181+
if (auto udr = get(parsed->structuredAttrs, "unsafeDiscardReferences")) {
106182
for (auto & [outputName, output] : getObject(*udr)) {
107183
if (!output.is_boolean())
108184
throw Error("attribute 'unsafeDiscardReferences.\"%s\"' must be a Boolean", outputName);
@@ -116,8 +192,8 @@ DerivationOptions DerivationOptions::fromParsedDerivation(const ParsedDerivation
116192
.passAsFile =
117193
[&] {
118194
StringSet res;
119-
if (auto * passAsFileString = get(parsed.drv.env, "passAsFile")) {
120-
if (parsed.hasStructuredAttrs()) {
195+
if (auto * passAsFileString = get(env, "passAsFile")) {
196+
if (parsed) {
121197
if (shouldWarn) {
122198
warn(
123199
"'structuredAttrs' disables the effect of the top-level attribute 'passAsFile'; because all JSON is always passed via file");
@@ -132,18 +208,18 @@ DerivationOptions DerivationOptions::fromParsedDerivation(const ParsedDerivation
132208
[&] {
133209
std::map<std::string, StringSet> ret;
134210

135-
if (auto structuredAttrs = parsed.structuredAttrs.get()) {
136-
auto e = optionalValueAt(*structuredAttrs, "exportReferencesGraph");
211+
if (parsed) {
212+
auto e = optionalValueAt(parsed->structuredAttrs, "exportReferencesGraph");
137213
if (!e || !e->is_object())
138214
return ret;
139215
for (auto & [key, storePathsJson] : getObject(*e)) {
140216
ret.insert_or_assign(key, storePathsJson);
141217
}
142218
} else {
143-
auto s = getOr(parsed.drv.env, "exportReferencesGraph", "");
219+
auto s = getOr(env, "exportReferencesGraph", "");
144220
Strings ss = tokenizeString<Strings>(s);
145221
if (ss.size() % 2 != 0)
146-
throw BuildError("odd number of tokens in 'exportReferencesGraph': '%1%'", s);
222+
throw Error("odd number of tokens in 'exportReferencesGraph': '%1%'", s);
147223
for (Strings::iterator i = ss.begin(); i != ss.end();) {
148224
auto fileName = std::move(*i++);
149225
static std::regex regex("[A-Za-z_][A-Za-z0-9_.-]*");
@@ -157,15 +233,15 @@ DerivationOptions DerivationOptions::fromParsedDerivation(const ParsedDerivation
157233
return ret;
158234
}(),
159235
.additionalSandboxProfile =
160-
parsed.getStringAttr("__sandboxProfile").value_or(defaults.additionalSandboxProfile),
161-
.noChroot = parsed.getBoolAttr("__noChroot", defaults.noChroot),
162-
.impureHostDeps = parsed.getStringSetAttr("__impureHostDeps").value_or(defaults.impureHostDeps),
163-
.impureEnvVars = parsed.getStringSetAttr("impureEnvVars").value_or(defaults.impureEnvVars),
164-
.allowLocalNetworking = parsed.getBoolAttr("__darwinAllowLocalNetworking", defaults.allowLocalNetworking),
236+
getStringAttr(env, parsed, "__sandboxProfile").value_or(defaults.additionalSandboxProfile),
237+
.noChroot = getBoolAttr(env, parsed, "__noChroot", defaults.noChroot),
238+
.impureHostDeps = getStringSetAttr(env, parsed, "__impureHostDeps").value_or(defaults.impureHostDeps),
239+
.impureEnvVars = getStringSetAttr(env, parsed, "impureEnvVars").value_or(defaults.impureEnvVars),
240+
.allowLocalNetworking = getBoolAttr(env, parsed, "__darwinAllowLocalNetworking", defaults.allowLocalNetworking),
165241
.requiredSystemFeatures =
166-
parsed.getStringSetAttr("requiredSystemFeatures").value_or(defaults.requiredSystemFeatures),
167-
.preferLocalBuild = parsed.getBoolAttr("preferLocalBuild", defaults.preferLocalBuild),
168-
.allowSubstitutes = parsed.getBoolAttr("allowSubstitutes", defaults.allowSubstitutes),
242+
getStringSetAttr(env, parsed, "requiredSystemFeatures").value_or(defaults.requiredSystemFeatures),
243+
.preferLocalBuild = getBoolAttr(env, parsed, "preferLocalBuild", defaults.preferLocalBuild),
244+
.allowSubstitutes = getBoolAttr(env, parsed, "allowSubstitutes", defaults.allowSubstitutes),
169245
};
170246
}
171247

0 commit comments

Comments
 (0)