Skip to content

Commit

Permalink
Introduce DerivationOptions
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@Ericson2314 @haenoe
Ericson2314 and haenoe committed Jan 20, 2025
1 parent bcb92a5 commit 6b6529b
Show file tree
Hide file tree
Showing 13 changed files with 542 additions and 255 deletions.
153 changes: 70 additions & 83 deletions src/libstore-tests/derivation-advanced-attrs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@

#include "experimental-features.hh"
#include "derivations.hh"

#include "tests/libstore.hh"
#include "tests/characterization.hh"
#include "derivations.hh"
#include "derivation-options.hh"
#include "parsed-derivations.hh"
#include "types.hh"
#include "json-utils.hh"

#include "tests/libstore.hh"
#include "tests/characterization.hh"

namespace nix {

using nlohmann::json;
Expand Down Expand Up @@ -80,21 +82,22 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_defaults)
auto drvPath = writeDerivation(*store, got, NoRepair, true);

ParsedDerivation parsedDrv(drvPath, got);

EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "");
EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), false);
EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings());
EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings());
EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), false);
EXPECT_EQ(parsedDrv.getStringsAttr("allowedReferences"), std::nullopt);
EXPECT_EQ(parsedDrv.getStringsAttr("allowedRequisites"), std::nullopt);
EXPECT_EQ(parsedDrv.getStringsAttr("disallowedReferences"), std::nullopt);
EXPECT_EQ(parsedDrv.getStringsAttr("disallowedRequisites"), std::nullopt);
EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), StringSet());
EXPECT_EQ(parsedDrv.canBuildLocally(*store), false);
EXPECT_EQ(parsedDrv.willBuildLocally(*store), false);
EXPECT_EQ(parsedDrv.substitutesAllowed(), true);
EXPECT_EQ(parsedDrv.useUidRange(), false);
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);

EXPECT_EQ(options.additionalSandboxProfile, "");
EXPECT_EQ(options.noChroot, false);
EXPECT_EQ(options.impureHostDeps, Strings());
EXPECT_EQ(options.impureEnvVars, Strings());
EXPECT_EQ(options.allowLocalNetworking, false);
EXPECT_EQ(options.checksAllOutputs.allowedReferences, std::nullopt);
EXPECT_EQ(options.checksAllOutputs.allowedRequisites, std::nullopt);
EXPECT_EQ(options.checksAllOutputs.disallowedReferences, std::nullopt);
EXPECT_EQ(options.checksAllOutputs.disallowedRequisites, std::nullopt);
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet());
EXPECT_EQ(options.canBuildLocally(*store, got), false);
EXPECT_EQ(options.willBuildLocally(*store, got), false);
EXPECT_EQ(options.substitutesAllowed(), true);
EXPECT_EQ(options.useUidRange(got), false);
});
};

Expand All @@ -106,29 +109,28 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes)
auto drvPath = writeDerivation(*store, got, NoRepair, true);

ParsedDerivation parsedDrv(drvPath, got);
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);

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

EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "sandcastle");
EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), true);
EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings{"/usr/bin/ditto"});
EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings{"UNICORN"});
EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), true);
EXPECT_EQ(options.additionalSandboxProfile, "sandcastle");
EXPECT_EQ(options.noChroot, true);
EXPECT_EQ(options.impureHostDeps, Strings{"/usr/bin/ditto"});
EXPECT_EQ(options.impureEnvVars, Strings{"UNICORN"});
EXPECT_EQ(options.allowLocalNetworking, true);
EXPECT_EQ(
parsedDrv.getStringsAttr("allowedReferences"), Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
options.checksAllOutputs.allowedReferences, Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
EXPECT_EQ(
parsedDrv.getStringsAttr("allowedRequisites"), Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
options.checksAllOutputs.allowedRequisites, Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
EXPECT_EQ(
parsedDrv.getStringsAttr("disallowedReferences"),
Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
options.checksAllOutputs.disallowedReferences, Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
EXPECT_EQ(
parsedDrv.getStringsAttr("disallowedRequisites"),
Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), systemFeatures);
EXPECT_EQ(parsedDrv.canBuildLocally(*store), false);
EXPECT_EQ(parsedDrv.willBuildLocally(*store), false);
EXPECT_EQ(parsedDrv.substitutesAllowed(), false);
EXPECT_EQ(parsedDrv.useUidRange(), true);
options.checksAllOutputs.disallowedRequisites, Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
EXPECT_EQ(options.getRequiredSystemFeatures(got), systemFeatures);
EXPECT_EQ(options.canBuildLocally(*store, got), false);
EXPECT_EQ(options.willBuildLocally(*store, got), false);
EXPECT_EQ(options.substitutesAllowed(), false);
EXPECT_EQ(options.useUidRange(got), true);
});
};

Expand All @@ -140,27 +142,25 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr
auto drvPath = writeDerivation(*store, got, NoRepair, true);

ParsedDerivation parsedDrv(drvPath, got);
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);

EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "");
EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), false);
EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings());
EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings());
EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), false);
EXPECT_EQ(options.additionalSandboxProfile, "");
EXPECT_EQ(options.noChroot, false);
EXPECT_EQ(options.impureHostDeps, Strings());
EXPECT_EQ(options.impureEnvVars, Strings());
EXPECT_EQ(options.allowLocalNetworking, false);

{
auto structuredAttrs_ = parsedDrv.getStructuredAttrs();
ASSERT_TRUE(structuredAttrs_);
auto & structuredAttrs = *structuredAttrs_;
ASSERT_TRUE(parsedDrv.hasStructuredAttrs());

auto outputChecks_ = get(structuredAttrs, "outputChecks");
ASSERT_FALSE(outputChecks_);
ASSERT_EQ(options.checksPerOutput.size(), 0);
}

EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), StringSet());
EXPECT_EQ(parsedDrv.canBuildLocally(*store), false);
EXPECT_EQ(parsedDrv.willBuildLocally(*store), false);
EXPECT_EQ(parsedDrv.substitutesAllowed(), true);
EXPECT_EQ(parsedDrv.useUidRange(), false);
EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet());
EXPECT_EQ(options.canBuildLocally(*store, got), false);
EXPECT_EQ(options.willBuildLocally(*store, got), false);
EXPECT_EQ(options.substitutesAllowed(), true);
EXPECT_EQ(options.useUidRange(got), false);
});
};

Expand All @@ -172,62 +172,49 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr
auto drvPath = writeDerivation(*store, got, NoRepair, true);

ParsedDerivation parsedDrv(drvPath, got);
DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv);

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

EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "sandcastle");
EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), true);
EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings{"/usr/bin/ditto"});
EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings{"UNICORN"});
EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), true);
EXPECT_EQ(options.additionalSandboxProfile, "sandcastle");
EXPECT_EQ(options.noChroot, true);
EXPECT_EQ(options.impureHostDeps, Strings{"/usr/bin/ditto"});
EXPECT_EQ(options.impureEnvVars, Strings{"UNICORN"});
EXPECT_EQ(options.allowLocalNetworking, true);

{
auto structuredAttrs_ = parsedDrv.getStructuredAttrs();
ASSERT_TRUE(structuredAttrs_);
auto & structuredAttrs = *structuredAttrs_;

auto outputChecks_ = get(structuredAttrs, "outputChecks");
ASSERT_TRUE(outputChecks_);
auto & outputChecks = *outputChecks_;
ASSERT_TRUE(parsedDrv.hasStructuredAttrs());

{
auto output_ = get(outputChecks, "out");
auto output_ = get(options.checksPerOutput, "out");
ASSERT_TRUE(output_);
auto & output = *output_;
EXPECT_EQ(
get(output, "allowedReferences")->get<Strings>(),
Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
EXPECT_EQ(
get(output, "allowedRequisites")->get<Strings>(),
Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
EXPECT_EQ(output.allowedReferences, Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
EXPECT_EQ(output.allowedRequisites, Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"});
}

{
auto output_ = get(outputChecks, "bin");
auto output_ = get(options.checksPerOutput, "bin");
ASSERT_TRUE(output_);
auto & output = *output_;
EXPECT_EQ(
get(output, "disallowedReferences")->get<Strings>(),
Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
EXPECT_EQ(
get(output, "disallowedRequisites")->get<Strings>(),
Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
EXPECT_EQ(output.disallowedReferences, Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
EXPECT_EQ(output.disallowedRequisites, Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"});
}

{
auto output_ = get(outputChecks, "dev");
auto output_ = get(options.checksPerOutput, "dev");
ASSERT_TRUE(output_);
auto & output = *output_;
EXPECT_EQ(get(output, "maxSize")->get<uint64_t>(), 789);
EXPECT_EQ(get(output, "maxClosureSize")->get<uint64_t>(), 5909);
EXPECT_EQ(output.maxSize, 789);
EXPECT_EQ(output.maxClosureSize, 5909);
}
}

EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), systemFeatures);
EXPECT_EQ(parsedDrv.canBuildLocally(*store), false);
EXPECT_EQ(parsedDrv.willBuildLocally(*store), false);
EXPECT_EQ(parsedDrv.substitutesAllowed(), false);
EXPECT_EQ(parsedDrv.useUidRange(), true);
EXPECT_EQ(options.getRequiredSystemFeatures(got), systemFeatures);
EXPECT_EQ(options.canBuildLocally(*store, got), false);
EXPECT_EQ(options.willBuildLocally(*store, got), false);
EXPECT_EQ(options.substitutesAllowed(), false);
EXPECT_EQ(options.useUidRange(got), true);
});
};

Expand Down
7 changes: 4 additions & 3 deletions src/libstore/build/derivation-goal.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ Goal::Co DerivationGoal::haveDerivation()
trace("have derivation");

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

if (!drv->type().hasKnownOutputPaths())
experimentalFeatureSettings.require(Xp::CaDerivations);
Expand Down Expand Up @@ -246,7 +247,7 @@ Goal::Co DerivationGoal::haveDerivation()
/* We are first going to try to create the invalid output paths
through substitutes. If that doesn't work, we'll build
them. */
if (settings.useSubstitutes && parsedDrv->substitutesAllowed())
if (settings.useSubstitutes && drvOptions->substitutesAllowed())
for (auto & [outputName, status] : initialOutputs) {
if (!status.wanted) continue;
if (!status.known)
Expand Down Expand Up @@ -718,7 +719,7 @@ Goal::Co DerivationGoal::tryToBuild()
`preferLocalBuild' set. Also, check and repair modes are only
supported for local builds. */
bool buildLocally =
(buildMode != bmNormal || parsedDrv->willBuildLocally(worker.store))
(buildMode != bmNormal || drvOptions->willBuildLocally(worker.store, *drv))
&& settings.maxBuildJobs.get() != 0;

if (!buildLocally) {
Expand Down Expand Up @@ -1147,7 +1148,7 @@ HookReply DerivationGoal::tryBuildHook()
<< (worker.getNrLocalBuilds() < settings.maxBuildJobs ? 1 : 0)
<< drv->platform
<< worker.store.printStorePath(drvPath)
<< parsedDrv->getRequiredSystemFeatures();
<< drvOptions->getRequiredSystemFeatures(*drv);
worker.hook->sink.flush();

/* Read the first line of input, which should be a word indicating
Expand Down
2 changes: 2 additions & 0 deletions src/libstore/build/derivation-goal.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
///@file

#include "parsed-derivations.hh"
#include "derivation-options.hh"
#ifndef _WIN32
# include "user-lock.hh"
#endif
Expand Down Expand Up @@ -143,6 +144,7 @@ struct DerivationGoal : public Goal
std::unique_ptr<Derivation> drv;

std::unique_ptr<ParsedDerivation> parsedDrv;
std::unique_ptr<DerivationOptions> drvOptions;

/**
* The remainder is state held during the build.
Expand Down
Loading

0 comments on commit 6b6529b

Please sign in to comment.