nixos/nix
1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-12-13 20:41:04 +01:00
Code Activity

Parse deriving paths in DerivationOptions

Browse source 
This is an example of "Parse, don't validate" principle [1].

Before, we had a number of `StringSet`s in `DerivationOptions` that
were not *actually* allowed to be arbitrary sets of strings. Instead,
each set member had to be one of:

- a store path

- a CA "downstream placeholder"

- an output name

Only later, in the code that checks outputs, would these strings be
further parsed to match these cases. (Actually, only 2 by that point,
because the placeholders must be rewritten away by then.)

Now, we fully parse everything up front, and have an "honest" data type
that reflects these invariants:

- store paths are parsed, stored as (opaque) deriving paths

- CA "downstream placeholders" are rewritten to the output deriving
  paths they denote

- output names are the only arbitrary strings left

Since the first two cases both become deriving paths, that leaves us
with a `std::variant<SingleDerivedPath, String>` data type, which we use
in our sets instead.

Getting rid of placeholders is especially nice because we are replacing
them with something much more internally-structured / transparent.

[1]: https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/

Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
This commit is contained in:
John Ericson 2025-04-14 14:26:13 -04:00
parent 72dbd43882
commit 76bd600302
23 changed files with 731 additions and 260 deletions
Download patch file Download diff file Expand all files Collapse all files

57
src/libstore/build/derivation-building-goal.cc
View file

@ -32,14 +32,6 @@ DerivationBuildingGoal::DerivationBuildingGoal(
, drv{std::make_unique<Derivation>(drv)}
, buildMode(buildMode)
{
try {
drvOptions =
std::make_unique<DerivationOptions>(DerivationOptions::fromStructuredAttrs(drv.env, drv.structuredAttrs));
} catch (Error & e) {
e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath));
throw;
}
name = fmt("building derivation '%s'", worker.store.printStorePath(drvPath));
trace("created");
@ -206,6 +198,38 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation)
Goal::Co DerivationBuildingGoal::tryToBuild()
{
auto drvOptions = [&] {
DerivationOptions<SingleDerivedPath> temp;
try {
temp =
derivationOptionsFromStructuredAttrs(worker.store, drv->inputDrvs, drv->env, get(drv->structuredAttrs));
} catch (Error & e) {
e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath));
throw;
}
auto res = tryResolve(
temp,
[&](ref<const SingleDerivedPath> drvPath, const std::string & outputName) -> std::optional<StorePath> {
try {
return resolveDerivedPath(
worker.store, SingleDerivedPath::Built{drvPath, outputName}, &worker.evalStore);
} catch (Error &) {
return std::nullopt;
}
});
/* The derivation must have all of its inputs gotten this point,
so the resolution will surely succeed.
(Actually, we shouldn't even enter this goal until we have a
resolved derivation, or derivation with only input addressed
transitive inputs, so this should be a no-opt anyways.)
*/
assert(res);
return *res;
}();
std::map<std::string, InitialOutput> initialOutputs;
/* Recheck at this point. In particular, whereas before we were
@ -344,13 +368,13 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
/* Don't do a remote build if the derivation has the attribute
`preferLocalBuild' set. Also, check and repair modes are only
supported for local builds. */
bool buildLocally = (buildMode != bmNormal || drvOptions->willBuildLocally(worker.store, *drv))
bool buildLocally = (buildMode != bmNormal || drvOptions.willBuildLocally(worker.store, *drv))
&& settings.maxBuildJobs.get() != 0;
if (buildLocally) {
useHook = false;
} else {
switch (tryBuildHook(initialOutputs)) {
switch (tryBuildHook(initialOutputs, drvOptions)) {
case rpAccept:
/* Yes, it has started doing so. Wait until we get
EOF from the hook. */
@ -379,7 +403,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
externalBuilder = settings.findExternalDerivationBuilderIfSupported(*drv);
if (!externalBuilder && !drvOptions->canBuildLocally(worker.store, *drv)) {
if (!externalBuilder && !drvOptions.canBuildLocally(worker.store, *drv)) {
auto msg =
fmt("Cannot build '%s'.\n"
"Reason: " ANSI_RED "required system or feature not available" ANSI_NORMAL
@ -388,7 +412,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
"Current system: '%s' with features {%s}",
Magenta(worker.store.printStorePath(drvPath)),
Magenta(drv->platform),
concatStringsSep(", ", drvOptions->getRequiredSystemFeatures(*drv)),
concatStringsSep(", ", drvOptions.getRequiredSystemFeatures(*drv)),
Magenta(settings.thisSystem),
concatStringsSep<StringSet>(", ", worker.store.Store::config.systemFeatures));
@ -586,7 +610,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
}
try {
desugaredEnv = DesugaredEnv::create(worker.store, *drv, *drvOptions, inputPaths);
desugaredEnv = DesugaredEnv::create(worker.store, *drv, drvOptions, inputPaths);
} catch (BuildError & e) {
outputLocks.unlock();
worker.permanentFailure = true;
@ -597,7 +621,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
.drvPath = drvPath,
.buildResult = buildResult,
.drv = *drv,
.drvOptions = *drvOptions,
.drvOptions = drvOptions,
.inputPaths = inputPaths,
.initialOutputs = initialOutputs,
.buildMode = buildMode,
@ -803,7 +827,8 @@ BuildError DerivationBuildingGoal::fixupBuilderFailureErrorMessage(BuilderFailur
return BuildError{e.status, msg};
}
HookReply DerivationBuildingGoal::tryBuildHook(const std::map<std::string, InitialOutput> & initialOutputs)
HookReply DerivationBuildingGoal::tryBuildHook(
const std::map<std::string, InitialOutput> & initialOutputs, const DerivationOptions<StorePath> & drvOptions)
{
#ifdef _WIN32 // TODO enable build hook on Windows
return rpDecline;
@ -820,7 +845,7 @@ HookReply DerivationBuildingGoal::tryBuildHook(const std::map<std::string, Initi
/* Send the request to the hook. */
worker.hook->sink << "try" << (worker.getNrLocalBuilds() < settings.maxBuildJobs ? 1 : 0) << drv->platform
<< worker.store.printStorePath(drvPath) << drvOptions->getRequiredSystemFeatures(*drv);
<< worker.store.printStorePath(drvPath) << drvOptions.getRequiredSystemFeatures(*drv);
worker.hook->sink.flush();
/* Read the first line of input, which should be a word indicating

47
src/libstore/build/derivation-check.cc
View file

@ -11,7 +11,7 @@ void checkOutputs(
Store & store,
const StorePath & drvPath,
const decltype(Derivation::outputs) & drvOutputs,
const decltype(DerivationOptions::outputChecks) & outputChecks,
const decltype(DerivationOptions<StorePath>::outputChecks) & outputChecks,
const std::map<std::string, ValidPathInfo> & outputs)
{
std::map<Path, const ValidPathInfo &> outputsByPath;
@ -85,7 +85,7 @@ void checkOutputs(
return std::make_pair(std::move(pathsDone), closureSize);
};
auto applyChecks = [&](const DerivationOptions::OutputChecks & checks) {
auto applyChecks = [&](const DerivationOptions<StorePath>::OutputChecks & checks) {
if (checks.maxSize && info.narSize > *checks.maxSize)
throw BuildError(
BuildResult::Failure::OutputRejected,
@ -105,28 +105,33 @@ void checkOutputs(
*checks.maxClosureSize);
}
auto checkRefs = [&](const StringSet & value, bool allowed, bool recursive) {
auto checkRefs = [&](const std::set<DrvRef<StorePath>> & value, bool allowed, bool recursive) {
/* Parse a list of reference specifiers. Each element must
either be a store path, or the symbolic name of the output
of the derivation (such as `out'). */
StorePathSet spec;
for (auto & i : value) {
if (store.isStorePath(i))
spec.insert(store.parseStorePath(i));
else if (auto output = get(outputs, i))
spec.insert(output->path);
else {
std::string outputsListing =
concatMapStringsSep(", ", outputs, [](auto & o) { return o.first; });
throw BuildError(
BuildResult::Failure::OutputRejected,
"derivation '%s' output check for '%s' contains an illegal reference specifier '%s',"
" expected store path or output name (one of [%s])",
store.printStorePath(drvPath),
outputName,
i,
outputsListing);
}
std::visit(
overloaded{
[&](const StorePath & path) { spec.insert(path); },
[&](const OutputName & refOutputName) {
if (auto output = get(outputs, refOutputName))
spec.insert(output->path);
else {
std::string outputsListing =
concatMapStringsSep(", ", outputs, [](auto & o) { return o.first; });
throw BuildError(
BuildResult::Failure::OutputRejected,
"derivation '%s' output check for '%s' contains output name '%s',"
" but this is not a valid output of this derivation."
" (Valid outputs are [%s].)",
store.printStorePath(drvPath),
outputName,
refOutputName,
outputsListing);
}
}},
i);
}
auto used = recursive ? getClosure(info.path).first : info.references;
@ -180,8 +185,8 @@ void checkOutputs(
std::visit(
overloaded{
[&](const DerivationOptions::OutputChecks & checks) { applyChecks(checks); },
[&](const std::map<std::string, DerivationOptions::OutputChecks> & checksPerOutput) {
[&](const DerivationOptions<StorePath>::OutputChecks & checks) { applyChecks(checks); },
[&](const std::map<std::string, DerivationOptions<StorePath>::OutputChecks> & checksPerOutput) {
if (auto outputChecks = get(checksPerOutput, outputName))
applyChecks(*outputChecks);

2
src/libstore/build/derivation-check.hh
View file

@ -21,7 +21,7 @@ void checkOutputs(
Store & store,
const StorePath & drvPath,
const decltype(Derivation::outputs) & drvOutputs,
const decltype(DerivationOptions::outputChecks) & drvOptions,
const decltype(DerivationOptions<StorePath>::outputChecks) & drvOptions,
const std::map<std::string, ValidPathInfo> & outputs);
} // namespace nix

7
src/libstore/build/derivation-env-desugar.cc
View file

@ -18,7 +18,10 @@ std::string & DesugaredEnv::atFileEnvPair(std::string_view name, std::string fil
}
DesugaredEnv DesugaredEnv::create(
Store & store, const Derivation & drv, const DerivationOptions & drvOptions, const StorePathSet & inputPaths)
Store & store,
const Derivation & drv,
const DerivationOptions<StorePath> & drvOptions,
const StorePathSet & inputPaths)
{
DesugaredEnv res;
@ -46,7 +49,7 @@ DesugaredEnv DesugaredEnv::create(
}
/* Handle exportReferencesGraph(), if set. */
for (auto & [fileName, storePaths] : drvOptions.getParsedExportReferencesGraph(store)) {
for (auto & [fileName, storePaths] : drvOptions.exportReferencesGraph) {
/* Write closure info to <fileName>. */
res.extraFiles.insert_or_assign(
fileName, store.makeValidityRegistration(store.exportReferences(storePaths, inputPaths), false, false));

5
src/libstore/build/derivation-goal.cc
View file

@ -64,9 +64,10 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation)
{
trace("have derivation");
auto drvOptions = [&]() -> DerivationOptions {
auto drvOptions = [&]() -> DerivationOptions<SingleDerivedPath> {
try {
return DerivationOptions::fromStructuredAttrs(drv->env, drv->structuredAttrs);
return derivationOptionsFromStructuredAttrs(
worker.store, drv->inputDrvs, drv->env, get(drv->structuredAttrs));
} catch (Error & e) {
e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath));
throw;