Skip to content

Commit 917b8b2

Browse files
Ericson2314haenoe
Ericson2314
and
haenoe
committed
Introduce DerivationOptions
This is a first step towards PR #10760, and the issues it addresses. See the Doxygen for details. Thanks to these changes, we are able to drastically restrict how the rest of the code-base uses `ParseDerivation`. Co-Authored-By: HaeNoe <[email protected]>
1 parent f0dbfad commit 917b8b2

File tree

14 files changed

+661
-276
lines changed

14 files changed

+661
-276
lines changed

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

Lines changed: 97 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33

44
#include "experimental-features.hh"
55
#include "derivations.hh"
6-
7-
#include "tests/libstore.hh"
8-
#include "tests/characterization.hh"
6+
#include "derivations.hh"
7+
#include "derivation-options.hh"
98
#include "parsed-derivations.hh"
109
#include "types.hh"
1110
#include "json-utils.hh"
1211

12+
#include "tests/libstore.hh"
13+
#include "tests/characterization.hh"
14+
1315
namespace nix {
1416

1517
using nlohmann::json;
@@ -80,21 +82,30 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_defaults)
8082
auto drvPath = writeDerivation(*store, got, NoRepair, true);
8183

8284
ParsedDerivation parsedDrv(drvPath, got);
85+
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
86+
87+
EXPECT_TRUE(!parsedDrv.hasStructuredAttrs());
8388

84-
EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "");
85-
EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), false);
86-
EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings());
87-
EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings());
88-
EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), false);
89-
EXPECT_EQ(parsedDrv.getStringsAttr("allowedReferences"), std::nullopt);
90-
EXPECT_EQ(parsedDrv.getStringsAttr("allowedRequisites"), std::nullopt);
91-
EXPECT_EQ(parsedDrv.getStringsAttr("disallowedReferences"), std::nullopt);
92-
EXPECT_EQ(parsedDrv.getStringsAttr("disallowedRequisites"), std::nullopt);
93-
EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), StringSet());
94-
EXPECT_EQ(parsedDrv.canBuildLocally(*store), false);
95-
EXPECT_EQ(parsedDrv.willBuildLocally(*store), false);
96-
EXPECT_EQ(parsedDrv.substitutesAllowed(), true);
97-
EXPECT_EQ(parsedDrv.useUidRange(), false);
89+
EXPECT_EQ(options.additionalSandboxProfile, "");
90+
EXPECT_EQ(options.noChroot, false);
91+
EXPECT_EQ(options.impureHostDeps, StringSet{});
92+
EXPECT_EQ(options.impureEnvVars, StringSet{});
93+
EXPECT_EQ(options.allowLocalNetworking, false);
94+
{
95+
auto * checksForAllOutputs_ = std::get_if<0>(&options.outputChecks);
96+
ASSERT_TRUE(checksForAllOutputs_ != nullptr);
97+
auto & checksForAllOutputs = *checksForAllOutputs_;
98+
99+
EXPECT_EQ(checksForAllOutputs.allowedReferences, std::nullopt);
100+
EXPECT_EQ(checksForAllOutputs.allowedRequisites, std::nullopt);
101+
EXPECT_EQ(checksForAllOutputs.disallowedReferences, StringSet{});
102+
EXPECT_EQ(checksForAllOutputs.disallowedRequisites, StringSet{});
103+
}
104+
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet());
105+
EXPECT_EQ(options.canBuildLocally(*store, got), false);
106+
EXPECT_EQ(options.willBuildLocally(*store, got), false);
107+
EXPECT_EQ(options.substitutesAllowed(), true);
108+
EXPECT_EQ(options.useUidRange(got), false);
98109
});
99110
};
100111

@@ -106,29 +117,36 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes)
106117
auto drvPath = writeDerivation(*store, got, NoRepair, true);
107118

108119
ParsedDerivation parsedDrv(drvPath, got);
120+
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
109121

110122
StringSet systemFeatures{"rainbow", "uid-range"};
111123

112-
EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "sandcastle");
113-
EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), true);
114-
EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings{"/usr/bin/ditto"});
115-
EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings{"UNICORN"});
116-
EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), true);
117-
EXPECT_EQ(
118-
parsedDrv.getStringsAttr("allowedReferences"), Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
119-
EXPECT_EQ(
120-
parsedDrv.getStringsAttr("allowedRequisites"), Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
121-
EXPECT_EQ(
122-
parsedDrv.getStringsAttr("disallowedReferences"),
123-
Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
124-
EXPECT_EQ(
125-
parsedDrv.getStringsAttr("disallowedRequisites"),
126-
Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
127-
EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), systemFeatures);
128-
EXPECT_EQ(parsedDrv.canBuildLocally(*store), false);
129-
EXPECT_EQ(parsedDrv.willBuildLocally(*store), false);
130-
EXPECT_EQ(parsedDrv.substitutesAllowed(), false);
131-
EXPECT_EQ(parsedDrv.useUidRange(), true);
124+
EXPECT_TRUE(!parsedDrv.hasStructuredAttrs());
125+
126+
EXPECT_EQ(options.additionalSandboxProfile, "sandcastle");
127+
EXPECT_EQ(options.noChroot, true);
128+
EXPECT_EQ(options.impureHostDeps, StringSet{"/usr/bin/ditto"});
129+
EXPECT_EQ(options.impureEnvVars, StringSet{"UNICORN"});
130+
EXPECT_EQ(options.allowLocalNetworking, true);
131+
{
132+
auto * checksForAllOutputs_ = std::get_if<0>(&options.outputChecks);
133+
ASSERT_TRUE(checksForAllOutputs_ != nullptr);
134+
auto & checksForAllOutputs = *checksForAllOutputs_;
135+
136+
EXPECT_EQ(
137+
checksForAllOutputs.allowedReferences, StringSet{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
138+
EXPECT_EQ(
139+
checksForAllOutputs.allowedRequisites, StringSet{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
140+
EXPECT_EQ(
141+
checksForAllOutputs.disallowedReferences, StringSet{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
142+
EXPECT_EQ(
143+
checksForAllOutputs.disallowedRequisites, StringSet{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
144+
}
145+
EXPECT_EQ(options.getRequiredSystemFeatures(got), systemFeatures);
146+
EXPECT_EQ(options.canBuildLocally(*store, got), false);
147+
EXPECT_EQ(options.willBuildLocally(*store, got), false);
148+
EXPECT_EQ(options.substitutesAllowed(), false);
149+
EXPECT_EQ(options.useUidRange(got), true);
132150
});
133151
};
134152

@@ -140,27 +158,29 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr
140158
auto drvPath = writeDerivation(*store, got, NoRepair, true);
141159

142160
ParsedDerivation parsedDrv(drvPath, got);
161+
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
143162

144-
EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "");
145-
EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), false);
146-
EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings());
147-
EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings());
148-
EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), false);
163+
EXPECT_TRUE(parsedDrv.hasStructuredAttrs());
164+
165+
EXPECT_EQ(options.additionalSandboxProfile, "");
166+
EXPECT_EQ(options.noChroot, false);
167+
EXPECT_EQ(options.impureHostDeps, StringSet{});
168+
EXPECT_EQ(options.impureEnvVars, StringSet{});
169+
EXPECT_EQ(options.allowLocalNetworking, false);
149170

150171
{
151-
auto structuredAttrs_ = parsedDrv.getStructuredAttrs();
152-
ASSERT_TRUE(structuredAttrs_);
153-
auto & structuredAttrs = *structuredAttrs_;
172+
auto * checksPerOutput_ = std::get_if<1>(&options.outputChecks);
173+
ASSERT_TRUE(checksPerOutput_ != nullptr);
174+
auto & checksPerOutput = *checksPerOutput_;
154175

155-
auto outputChecks_ = get(structuredAttrs, "outputChecks");
156-
ASSERT_FALSE(outputChecks_);
176+
EXPECT_EQ(checksPerOutput.size(), 0);
157177
}
158178

159-
EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), StringSet());
160-
EXPECT_EQ(parsedDrv.canBuildLocally(*store), false);
161-
EXPECT_EQ(parsedDrv.willBuildLocally(*store), false);
162-
EXPECT_EQ(parsedDrv.substitutesAllowed(), true);
163-
EXPECT_EQ(parsedDrv.useUidRange(), false);
179+
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet());
180+
EXPECT_EQ(options.canBuildLocally(*store, got), false);
181+
EXPECT_EQ(options.willBuildLocally(*store, got), false);
182+
EXPECT_EQ(options.substitutesAllowed(), true);
183+
EXPECT_EQ(options.useUidRange(got), false);
164184
});
165185
};
166186

@@ -172,62 +192,52 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr
172192
auto drvPath = writeDerivation(*store, got, NoRepair, true);
173193

174194
ParsedDerivation parsedDrv(drvPath, got);
195+
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);
175196

176197
StringSet systemFeatures{"rainbow", "uid-range"};
177198

178-
EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "sandcastle");
179-
EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), true);
180-
EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings{"/usr/bin/ditto"});
181-
EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings{"UNICORN"});
182-
EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), true);
183-
184-
{
185-
auto structuredAttrs_ = parsedDrv.getStructuredAttrs();
186-
ASSERT_TRUE(structuredAttrs_);
187-
auto & structuredAttrs = *structuredAttrs_;
199+
EXPECT_TRUE(parsedDrv.hasStructuredAttrs());
188200

189-
auto outputChecks_ = get(structuredAttrs, "outputChecks");
190-
ASSERT_TRUE(outputChecks_);
191-
auto & outputChecks = *outputChecks_;
201+
EXPECT_EQ(options.additionalSandboxProfile, "sandcastle");
202+
EXPECT_EQ(options.noChroot, true);
203+
EXPECT_EQ(options.impureHostDeps, StringSet{"/usr/bin/ditto"});
204+
EXPECT_EQ(options.impureEnvVars, StringSet{"UNICORN"});
205+
EXPECT_EQ(options.allowLocalNetworking, true);
192206

207+
{
193208
{
194-
auto output_ = get(outputChecks, "out");
209+
auto output_ = get(std::get<1>(options.outputChecks), "out");
195210
ASSERT_TRUE(output_);
196211
auto & output = *output_;
197-
EXPECT_EQ(
198-
get(output, "allowedReferences")->get<Strings>(),
199-
Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
200-
EXPECT_EQ(
201-
get(output, "allowedRequisites")->get<Strings>(),
202-
Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
212+
213+
EXPECT_EQ(output.allowedReferences, StringSet{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
214+
EXPECT_EQ(output.allowedRequisites, StringSet{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
203215
}
204216

205217
{
206-
auto output_ = get(outputChecks, "bin");
218+
auto output_ = get(std::get<1>(options.outputChecks), "bin");
207219
ASSERT_TRUE(output_);
208220
auto & output = *output_;
209-
EXPECT_EQ(
210-
get(output, "disallowedReferences")->get<Strings>(),
211-
Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
212-
EXPECT_EQ(
213-
get(output, "disallowedRequisites")->get<Strings>(),
214-
Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
221+
222+
EXPECT_EQ(output.disallowedReferences, StringSet{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
223+
EXPECT_EQ(output.disallowedRequisites, StringSet{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
215224
}
216225

217226
{
218-
auto output_ = get(outputChecks, "dev");
227+
auto output_ = get(std::get<1>(options.outputChecks), "dev");
219228
ASSERT_TRUE(output_);
220229
auto & output = *output_;
221-
EXPECT_EQ(get(output, "maxSize")->get<uint64_t>(), 789);
222-
EXPECT_EQ(get(output, "maxClosureSize")->get<uint64_t>(), 5909);
230+
231+
EXPECT_EQ(output.maxSize, 789);
232+
EXPECT_EQ(output.maxClosureSize, 5909);
223233
}
224234
}
225235

226-
EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), systemFeatures);
227-
EXPECT_EQ(parsedDrv.canBuildLocally(*store), false);
228-
EXPECT_EQ(parsedDrv.willBuildLocally(*store), false);
229-
EXPECT_EQ(parsedDrv.substitutesAllowed(), false);
230-
EXPECT_EQ(parsedDrv.useUidRange(), true);
236+
EXPECT_EQ(options.getRequiredSystemFeatures(got), systemFeatures);
237+
EXPECT_EQ(options.canBuildLocally(*store, got), false);
238+
EXPECT_EQ(options.willBuildLocally(*store, got), false);
239+
EXPECT_EQ(options.substitutesAllowed(), false);
240+
EXPECT_EQ(options.useUidRange(got), true);
231241
});
232242
};
233243

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ Goal::Co DerivationGoal::haveDerivation()
168168
trace("have derivation");
169169

170170
parsedDrv = std::make_unique<ParsedDerivation>(drvPath, *drv);
171+
drvOptions = std::make_unique<DerivationOptions>(DerivationOptions::fromParsedDerivation(*parsedDrv));
171172

172173
if (!drv->type().hasKnownOutputPaths())
173174
experimentalFeatureSettings.require(Xp::CaDerivations);
@@ -224,7 +225,7 @@ Goal::Co DerivationGoal::haveDerivation()
224225
/* We are first going to try to create the invalid output paths
225226
through substitutes. If that doesn't work, we'll build
226227
them. */
227-
if (settings.useSubstitutes && parsedDrv->substitutesAllowed())
228+
if (settings.useSubstitutes && drvOptions->substitutesAllowed())
228229
for (auto & [outputName, status] : initialOutputs) {
229230
if (!status.wanted) continue;
230231
if (!status.known)
@@ -614,7 +615,7 @@ Goal::Co DerivationGoal::tryToBuild()
614615
`preferLocalBuild' set. Also, check and repair modes are only
615616
supported for local builds. */
616617
bool buildLocally =
617-
(buildMode != bmNormal || parsedDrv->willBuildLocally(worker.store))
618+
(buildMode != bmNormal || drvOptions->willBuildLocally(worker.store, *drv))
618619
&& settings.maxBuildJobs.get() != 0;
619620

620621
if (!buildLocally) {
@@ -1110,7 +1111,7 @@ HookReply DerivationGoal::tryBuildHook()
11101111
<< (worker.getNrLocalBuilds() < settings.maxBuildJobs ? 1 : 0)
11111112
<< drv->platform
11121113
<< worker.store.printStorePath(drvPath)
1113-
<< parsedDrv->getRequiredSystemFeatures();
1114+
<< drvOptions->getRequiredSystemFeatures(*drv);
11141115
worker.hook->sink.flush();
11151116

11161117
/* Read the first line of input, which should be a word indicating

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
///@file
33

44
#include "parsed-derivations.hh"
5+
#include "derivation-options.hh"
56
#ifndef _WIN32
67
# include "user-lock.hh"
78
#endif
@@ -147,6 +148,7 @@ struct DerivationGoal : public Goal
147148
std::unique_ptr<Derivation> drv;
148149

149150
std::unique_ptr<ParsedDerivation> parsedDrv;
151+
std::unique_ptr<DerivationOptions> drvOptions;
150152

151153
/**
152154
* The remainder is state held during the build.

0 commit comments

Comments
 (0)