From 9daef9cca23c8d7f1658c3941eae59f1735c391e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 4 Nov 2025 02:21:24 -0500 Subject: [PATCH 1/2] Clean up `DerivationOptions` unit tests We now test equality on whole strucks much more often, which avoids forgetting to test for specific fields. --- .../derivation-advanced-attrs.cc | 534 +++++++++--------- 1 file changed, 252 insertions(+), 282 deletions(-) diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc index 02bc8fa24..c994b26d5 100644 --- a/src/libstore-tests/derivation-advanced-attrs.cc +++ b/src/libstore-tests/derivation-advanced-attrs.cc @@ -32,6 +32,33 @@ public: * to worry about race conditions if the tests run concurrently. */ ExperimentalFeatureSettings mockXpSettings; + + /** + * Helper function to test getRequiredSystemFeatures for a given derivation file + */ + void testRequiredSystemFeatures(const std::string & fileName, const StringSet & expectedFeatures) + { + this->readTest(fileName, [&](auto encoded) { + auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); + EXPECT_EQ(options.getRequiredSystemFeatures(got), expectedFeatures); + }); + } + + /** + * Helper function to test DerivationOptions parsing and comparison + */ + void testDerivationOptions( + const std::string & fileName, const DerivationOptions & expected, const StringSet & expectedSystemFeatures) + { + this->readTest(fileName, [&](auto encoded) { + auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); + DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); + + EXPECT_EQ(options, expected); + EXPECT_EQ(options.getRequiredSystemFeatures(got), expectedSystemFeatures); + }); + } }; class CaDerivationAdvancedAttrsTest : public DerivationAdvancedAttrsTest @@ -100,33 +127,35 @@ TEST_ATERM_JSON(advancedAttributes_structuredAttrs_defaults, "advanced-attribute using ExportReferencesMap = decltype(DerivationOptions::exportReferencesGraph); +static const DerivationOptions advancedAttributes_defaults = { + .outputChecks = + DerivationOptions::OutputChecks{ + .ignoreSelfRefs = true, + }, + .unsafeDiscardReferences = {}, + .passAsFile = {}, + .exportReferencesGraph = {}, + .additionalSandboxProfile = "", + .noChroot = false, + .impureHostDeps = {}, + .impureEnvVars = {}, + .allowLocalNetworking = false, + .requiredSystemFeatures = {}, + .preferLocalBuild = false, + .allowSubstitutes = true, +}; + TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_defaults) { this->readTest("advanced-attributes-defaults.drv", [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_TRUE(!got.structuredAttrs); - EXPECT_EQ(options.additionalSandboxProfile, ""); - EXPECT_EQ(options.noChroot, false); - EXPECT_EQ(options.impureHostDeps, StringSet{}); - EXPECT_EQ(options.impureEnvVars, StringSet{}); - EXPECT_EQ(options.allowLocalNetworking, false); - EXPECT_EQ(options.exportReferencesGraph, ExportReferencesMap{}); - { - auto * checksForAllOutputs_ = std::get_if<0>(&options.outputChecks); - ASSERT_TRUE(checksForAllOutputs_ != nullptr); - auto & checksForAllOutputs = *checksForAllOutputs_; + EXPECT_EQ(options, advancedAttributes_defaults); - EXPECT_EQ(checksForAllOutputs.allowedReferences, std::nullopt); - EXPECT_EQ(checksForAllOutputs.allowedRequisites, std::nullopt); - EXPECT_EQ(checksForAllOutputs.disallowedReferences, StringSet{}); - EXPECT_EQ(checksForAllOutputs.disallowedRequisites, StringSet{}); - } EXPECT_EQ(options.canBuildLocally(*this->store, got), false); EXPECT_EQ(options.willBuildLocally(*this->store, got), false); EXPECT_EQ(options.substitutesAllowed(), true); @@ -136,152 +165,124 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_defaults) TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_defaults) { - this->readTest("advanced-attributes-defaults.drv", [&](auto encoded) { - auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - - EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{}); - }); + testRequiredSystemFeatures("advanced-attributes-defaults.drv", {}); }; TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_defaults) { - this->readTest("advanced-attributes-defaults.drv", [&](auto encoded) { - auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - - EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{"ca-derivations"}); - }); + testRequiredSystemFeatures("advanced-attributes-defaults.drv", {"ca-derivations"}); }; TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes) { + DerivationOptions expected = { + .outputChecks = + DerivationOptions::OutputChecks{ + .ignoreSelfRefs = true, + }, + .unsafeDiscardReferences = {}, + .passAsFile = {}, + .additionalSandboxProfile = "sandcastle", + .noChroot = true, + .impureHostDeps = {"/usr/bin/ditto"}, + .impureEnvVars = {"UNICORN"}, + .allowLocalNetworking = true, + .requiredSystemFeatures = {"rainbow", "uid-range"}, + .preferLocalBuild = true, + .allowSubstitutes = false, + }; + this->readTest("advanced-attributes.drv", [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_TRUE(!got.structuredAttrs); - EXPECT_EQ(options.additionalSandboxProfile, "sandcastle"); - EXPECT_EQ(options.noChroot, true); - EXPECT_EQ(options.impureHostDeps, StringSet{"/usr/bin/ditto"}); - EXPECT_EQ(options.impureEnvVars, StringSet{"UNICORN"}); - EXPECT_EQ(options.allowLocalNetworking, true); - EXPECT_EQ(options.canBuildLocally(*this->store, got), false); - EXPECT_EQ(options.willBuildLocally(*this->store, got), false); + // Reset fields that vary between test cases to enable whole-object comparison + options.outputChecks = DerivationOptions::OutputChecks{.ignoreSelfRefs = true}; + options.exportReferencesGraph = {}; + + EXPECT_EQ(options, expected); + EXPECT_EQ(options.substitutesAllowed(), false); EXPECT_EQ(options.useUidRange(got), true); }); }; -TEST_F(DerivationAdvancedAttrsTest, advancedAttributes) +DerivationOptions advancedAttributes_ia = { + .outputChecks = + DerivationOptions::OutputChecks{ + .ignoreSelfRefs = true, + .allowedReferences = StringSet{"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}, + .disallowedReferences = StringSet{"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar"}, + .allowedRequisites = StringSet{"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev"}, + .disallowedRequisites = StringSet{"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev"}, + }, + .unsafeDiscardReferences = {}, + .passAsFile = {}, + .exportReferencesGraph{ + {"refs1", {"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}}, + {"refs2", {"/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"}}, + }, + .additionalSandboxProfile = "sandcastle", + .noChroot = true, + .impureHostDeps = {"/usr/bin/ditto"}, + .impureEnvVars = {"UNICORN"}, + .allowLocalNetworking = true, + .requiredSystemFeatures = {"rainbow", "uid-range"}, + .preferLocalBuild = true, + .allowSubstitutes = false, +}; + +TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_ia) { - this->readTest("advanced-attributes.drv", [&](auto encoded) { - auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); + testDerivationOptions("advanced-attributes.drv", advancedAttributes_ia, {"rainbow", "uid-range"}); +}; - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - - EXPECT_EQ( - options.exportReferencesGraph, - (ExportReferencesMap{ - { - "refs1", - { - "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo", - }, - }, - { - "refs2", - { - "/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv", - }, - }, - })); - - { - auto * checksForAllOutputs_ = std::get_if<0>(&options.outputChecks); - ASSERT_TRUE(checksForAllOutputs_ != nullptr); - auto & checksForAllOutputs = *checksForAllOutputs_; - - EXPECT_EQ( - checksForAllOutputs.allowedReferences, StringSet{"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}); - EXPECT_EQ( - checksForAllOutputs.allowedRequisites, - StringSet{"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev"}); - EXPECT_EQ( - checksForAllOutputs.disallowedReferences, StringSet{"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar"}); - EXPECT_EQ( - checksForAllOutputs.disallowedRequisites, - StringSet{"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev"}); - } - - StringSet systemFeatures{"rainbow", "uid-range"}; - - EXPECT_EQ(options.getRequiredSystemFeatures(got), systemFeatures); - }); +DerivationOptions advancedAttributes_ca = { + .outputChecks = + DerivationOptions::OutputChecks{ + .ignoreSelfRefs = true, + .allowedReferences = StringSet{"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}, + .disallowedReferences = StringSet{"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g"}, + .allowedRequisites = StringSet{"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z"}, + .disallowedRequisites = StringSet{"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"}, + }, + .unsafeDiscardReferences = {}, + .passAsFile = {}, + .exportReferencesGraph{ + {"refs1", {"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}}, + {"refs2", {"/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"}}, + }, + .additionalSandboxProfile = "sandcastle", + .noChroot = true, + .impureHostDeps = {"/usr/bin/ditto"}, + .impureEnvVars = {"UNICORN"}, + .allowLocalNetworking = true, + .requiredSystemFeatures = {"rainbow", "uid-range"}, + .preferLocalBuild = true, + .allowSubstitutes = false, }; TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes) { - this->readTest("advanced-attributes.drv", [&](auto encoded) { - auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); + testDerivationOptions("advanced-attributes.drv", advancedAttributes_ca, {"rainbow", "uid-range", "ca-derivations"}); +}; - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - - EXPECT_EQ( - options.exportReferencesGraph, - (ExportReferencesMap{ - { - "refs1", - { - "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9", - }, - }, - { - "refs2", - { - "/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv", - }, - }, - })); - - { - auto * checksForAllOutputs_ = std::get_if<0>(&options.outputChecks); - ASSERT_TRUE(checksForAllOutputs_ != nullptr); - auto & checksForAllOutputs = *checksForAllOutputs_; - - EXPECT_EQ( - checksForAllOutputs.allowedReferences, - StringSet{"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}); - EXPECT_EQ( - checksForAllOutputs.allowedRequisites, - StringSet{"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z"}); - EXPECT_EQ( - checksForAllOutputs.disallowedReferences, - StringSet{"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g"}); - EXPECT_EQ( - checksForAllOutputs.disallowedRequisites, - StringSet{"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"}); - } - - StringSet systemFeatures{"rainbow", "uid-range"}; - systemFeatures.insert("ca-derivations"); - - EXPECT_EQ(options.getRequiredSystemFeatures(got), systemFeatures); - }); +DerivationOptions advancedAttributes_structuredAttrs_defaults = { + .outputChecks = std::map{}, + .unsafeDiscardReferences = {}, + .passAsFile = {}, + .exportReferencesGraph = {}, + .additionalSandboxProfile = "", + .noChroot = false, + .impureHostDeps = {}, + .impureEnvVars = {}, + .allowLocalNetworking = false, + .requiredSystemFeatures = {}, + .preferLocalBuild = false, + .allowSubstitutes = true, }; TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs_defaults) @@ -289,26 +290,11 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs_d this->readTest("advanced-attributes-structured-attrs-defaults.drv", [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_TRUE(got.structuredAttrs); - EXPECT_EQ(options.additionalSandboxProfile, ""); - EXPECT_EQ(options.noChroot, false); - EXPECT_EQ(options.impureHostDeps, StringSet{}); - EXPECT_EQ(options.impureEnvVars, StringSet{}); - EXPECT_EQ(options.allowLocalNetworking, false); - EXPECT_EQ(options.exportReferencesGraph, ExportReferencesMap{}); - - { - auto * checksPerOutput_ = std::get_if<1>(&options.outputChecks); - ASSERT_TRUE(checksPerOutput_ != nullptr); - auto & checksPerOutput = *checksPerOutput_; - - EXPECT_EQ(checksPerOutput.size(), 0u); - } + EXPECT_EQ(options, advancedAttributes_structuredAttrs_defaults); EXPECT_EQ(options.canBuildLocally(*this->store, got), false); EXPECT_EQ(options.willBuildLocally(*this->store, got), false); @@ -319,55 +305,61 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs_d TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_defaults) { - this->readTest("advanced-attributes-structured-attrs-defaults.drv", [&](auto encoded) { - auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - - EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{}); - }); + testRequiredSystemFeatures("advanced-attributes-structured-attrs-defaults.drv", {}); }; TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_defaults) { - this->readTest("advanced-attributes-structured-attrs-defaults.drv", [&](auto encoded) { - auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - - EXPECT_EQ(options.getRequiredSystemFeatures(got), StringSet{"ca-derivations"}); - }); + testRequiredSystemFeatures("advanced-attributes-structured-attrs-defaults.drv", {"ca-derivations"}); }; TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) { + DerivationOptions expected = { + .outputChecks = + std::map{ + {"dev", + DerivationOptions::OutputChecks{ + .maxSize = 789, + .maxClosureSize = 5909, + }}, + }, + .unsafeDiscardReferences = {}, + .passAsFile = {}, + .exportReferencesGraph = {}, + .additionalSandboxProfile = "sandcastle", + .noChroot = true, + .impureHostDeps = {"/usr/bin/ditto"}, + .impureEnvVars = {"UNICORN"}, + .allowLocalNetworking = true, + .requiredSystemFeatures = {"rainbow", "uid-range"}, + .preferLocalBuild = true, + .allowSubstitutes = false, + }; + this->readTest("advanced-attributes-structured-attrs.drv", [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); EXPECT_TRUE(got.structuredAttrs); - EXPECT_EQ(options.additionalSandboxProfile, "sandcastle"); - EXPECT_EQ(options.noChroot, true); - EXPECT_EQ(options.impureHostDeps, StringSet{"/usr/bin/ditto"}); - EXPECT_EQ(options.impureEnvVars, StringSet{"UNICORN"}); - EXPECT_EQ(options.allowLocalNetworking, true); - + // Reset fields that vary between test cases to enable whole-object comparison { - auto output_ = get(std::get<1>(options.outputChecks), "dev"); - ASSERT_TRUE(output_); - auto & output = *output_; - - EXPECT_EQ(output.maxSize, 789); - EXPECT_EQ(output.maxClosureSize, 5909); + // Delete all keys but "dev" in options.outputChecks + auto * outputChecksMapP = + std::get_if>(&options.outputChecks); + ASSERT_TRUE(outputChecksMapP); + auto & outputChecksMap = *outputChecksMapP; + auto devEntry = outputChecksMap.find("dev"); + ASSERT_TRUE(devEntry != outputChecksMap.end()); + auto devChecks = devEntry->second; + outputChecksMap.clear(); + outputChecksMap.emplace("dev", std::move(devChecks)); } + options.exportReferencesGraph = {}; + + EXPECT_EQ(options, expected); EXPECT_EQ(options.canBuildLocally(*this->store, got), false); EXPECT_EQ(options.willBuildLocally(*this->store, got), false); @@ -376,112 +368,90 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) }); }; +DerivationOptions advancedAttributes_structuredAttrs_ia = { + .outputChecks = + std::map{ + {"out", + DerivationOptions::OutputChecks{ + .allowedReferences = StringSet{"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}, + .allowedRequisites = StringSet{"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev"}, + }}, + {"bin", + DerivationOptions::OutputChecks{ + .disallowedReferences = StringSet{"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar"}, + .disallowedRequisites = StringSet{"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev"}, + }}, + {"dev", + DerivationOptions::OutputChecks{ + .maxSize = 789, + .maxClosureSize = 5909, + }}, + }, + .unsafeDiscardReferences = {}, + .passAsFile = {}, + .exportReferencesGraph = + { + {"refs1", {"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}}, + {"refs2", {"/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"}}, + }, + .additionalSandboxProfile = "sandcastle", + .noChroot = true, + .impureHostDeps = {"/usr/bin/ditto"}, + .impureEnvVars = {"UNICORN"}, + .allowLocalNetworking = true, + .requiredSystemFeatures = {"rainbow", "uid-range"}, + .preferLocalBuild = true, + .allowSubstitutes = false, +}; + TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs) { - this->readTest("advanced-attributes-structured-attrs.drv", [&](auto encoded) { - auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - - EXPECT_EQ( - options.exportReferencesGraph, - (ExportReferencesMap{ - { - "refs1", - { - "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo", - }, - }, - { - "refs2", - { - "/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv", - }, - }, - })); + testDerivationOptions( + "advanced-attributes-structured-attrs.drv", advancedAttributes_structuredAttrs_ia, {"rainbow", "uid-range"}); +}; +DerivationOptions advancedAttributes_structuredAttrs_ca = { + .outputChecks = + std::map{ + {"out", + DerivationOptions::OutputChecks{ + .allowedReferences = StringSet{"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}, + .allowedRequisites = StringSet{"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z"}, + }}, + {"bin", + DerivationOptions::OutputChecks{ + .disallowedReferences = StringSet{"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g"}, + .disallowedRequisites = StringSet{"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"}, + }}, + {"dev", + DerivationOptions::OutputChecks{ + .maxSize = 789, + .maxClosureSize = 5909, + }}, + }, + .unsafeDiscardReferences = {}, + .passAsFile = {}, + .exportReferencesGraph = { - { - auto output_ = get(std::get<1>(options.outputChecks), "out"); - ASSERT_TRUE(output_); - auto & output = *output_; - - EXPECT_EQ(output.allowedReferences, StringSet{"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}); - EXPECT_EQ(output.allowedRequisites, StringSet{"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev"}); - } - - { - auto output_ = get(std::get<1>(options.outputChecks), "bin"); - ASSERT_TRUE(output_); - auto & output = *output_; - - EXPECT_EQ(output.disallowedReferences, StringSet{"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar"}); - EXPECT_EQ( - output.disallowedRequisites, StringSet{"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev"}); - } - } - - StringSet systemFeatures{"rainbow", "uid-range"}; - - EXPECT_EQ(options.getRequiredSystemFeatures(got), systemFeatures); - }); + {"refs1", {"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}}, + {"refs2", {"/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"}}, + }, + .additionalSandboxProfile = "sandcastle", + .noChroot = true, + .impureHostDeps = {"/usr/bin/ditto"}, + .impureEnvVars = {"UNICORN"}, + .allowLocalNetworking = true, + .requiredSystemFeatures = {"rainbow", "uid-range"}, + .preferLocalBuild = true, + .allowSubstitutes = false, }; TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs) { - this->readTest("advanced-attributes-structured-attrs.drv", [&](auto encoded) { - auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - - auto drvPath = writeDerivation(*this->store, got, NoRepair, true); - - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); - - EXPECT_EQ( - options.exportReferencesGraph, - (ExportReferencesMap{ - { - "refs1", - { - "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9", - }, - }, - { - "refs2", - { - "/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv", - }, - }, - })); - - { - { - auto output_ = get(std::get<1>(options.outputChecks), "out"); - ASSERT_TRUE(output_); - auto & output = *output_; - - EXPECT_EQ(output.allowedReferences, StringSet{"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}); - EXPECT_EQ(output.allowedRequisites, StringSet{"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z"}); - } - - { - auto output_ = get(std::get<1>(options.outputChecks), "bin"); - ASSERT_TRUE(output_); - auto & output = *output_; - - EXPECT_EQ( - output.disallowedReferences, StringSet{"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g"}); - EXPECT_EQ( - output.disallowedRequisites, StringSet{"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"}); - } - } - - StringSet systemFeatures{"rainbow", "uid-range"}; - systemFeatures.insert("ca-derivations"); - - EXPECT_EQ(options.getRequiredSystemFeatures(got), systemFeatures); - }); + testDerivationOptions( + "advanced-attributes-structured-attrs.drv", + advancedAttributes_structuredAttrs_ca, + {"rainbow", "uid-range", "ca-derivations"}); }; } // namespace nix From d05e85e5befbc7fcc19a28d459ed17f5e98c6f23 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 4 Nov 2025 01:22:20 -0500 Subject: [PATCH 2/2] Fix `DerivationOptions` JSON implementation and test --- .../data/derivation/ca/all_set.json | 46 +++++++++++++ .../ca/structuredAttrs_all_set.json | 66 +++++++++++++++++++ .../data/derivation/ia/all_set.json | 46 +++++++++++++ .../data/derivation/ia/defaults.json | 24 +++++++ .../ia/structuredAttrs_all_set.json | 66 +++++++++++++++++++ .../ia/structuredAttrs_defaults.json | 16 +++++ .../derivation-advanced-attrs.cc | 25 ++++++- src/libstore/derivation-options.cc | 33 ++++++++-- src/libstore/derivations.cc | 2 +- src/libutil/include/nix/util/json-utils.hh | 11 ++++ src/libutil/json-utils.cc | 9 +-- 11 files changed, 326 insertions(+), 18 deletions(-) create mode 100644 src/libstore-tests/data/derivation/ca/all_set.json create mode 100644 src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json create mode 100644 src/libstore-tests/data/derivation/ia/all_set.json create mode 100644 src/libstore-tests/data/derivation/ia/defaults.json create mode 100644 src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json create mode 100644 src/libstore-tests/data/derivation/ia/structuredAttrs_defaults.json diff --git a/src/libstore-tests/data/derivation/ca/all_set.json b/src/libstore-tests/data/derivation/ca/all_set.json new file mode 100644 index 000000000..e06eada01 --- /dev/null +++ b/src/libstore-tests/data/derivation/ca/all_set.json @@ -0,0 +1,46 @@ +{ + "additionalSandboxProfile": "sandcastle", + "allowLocalNetworking": true, + "allowSubstitutes": false, + "exportReferencesGraph": { + "refs1": [ + "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + ], + "refs2": [ + "/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv" + ] + }, + "impureEnvVars": [ + "UNICORN" + ], + "impureHostDeps": [ + "/usr/bin/ditto" + ], + "noChroot": true, + "outputChecks": { + "forAllOutputs": { + "allowedReferences": [ + "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + ], + "allowedRequisites": [ + "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z" + ], + "disallowedReferences": [ + "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g" + ], + "disallowedRequisites": [ + "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8" + ], + "ignoreSelfRefs": true, + "maxClosureSize": null, + "maxSize": null + } + }, + "passAsFile": [], + "preferLocalBuild": true, + "requiredSystemFeatures": [ + "rainbow", + "uid-range" + ], + "unsafeDiscardReferences": {} +} diff --git a/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json b/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json new file mode 100644 index 000000000..2a321897c --- /dev/null +++ b/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json @@ -0,0 +1,66 @@ +{ + "additionalSandboxProfile": "sandcastle", + "allowLocalNetworking": true, + "allowSubstitutes": false, + "exportReferencesGraph": { + "refs1": [ + "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + ], + "refs2": [ + "/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv" + ] + }, + "impureEnvVars": [ + "UNICORN" + ], + "impureHostDeps": [ + "/usr/bin/ditto" + ], + "noChroot": true, + "outputChecks": { + "perOutput": { + "bin": { + "allowedReferences": null, + "allowedRequisites": null, + "disallowedReferences": [ + "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g" + ], + "disallowedRequisites": [ + "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8" + ], + "ignoreSelfRefs": false, + "maxClosureSize": null, + "maxSize": null + }, + "dev": { + "allowedReferences": null, + "allowedRequisites": null, + "disallowedReferences": [], + "disallowedRequisites": [], + "ignoreSelfRefs": false, + "maxClosureSize": 5909, + "maxSize": 789 + }, + "out": { + "allowedReferences": [ + "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + ], + "allowedRequisites": [ + "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z" + ], + "disallowedReferences": [], + "disallowedRequisites": [], + "ignoreSelfRefs": false, + "maxClosureSize": null, + "maxSize": null + } + } + }, + "passAsFile": [], + "preferLocalBuild": true, + "requiredSystemFeatures": [ + "rainbow", + "uid-range" + ], + "unsafeDiscardReferences": {} +} diff --git a/src/libstore-tests/data/derivation/ia/all_set.json b/src/libstore-tests/data/derivation/ia/all_set.json new file mode 100644 index 000000000..62b6cdf97 --- /dev/null +++ b/src/libstore-tests/data/derivation/ia/all_set.json @@ -0,0 +1,46 @@ +{ + "additionalSandboxProfile": "sandcastle", + "allowLocalNetworking": true, + "allowSubstitutes": false, + "exportReferencesGraph": { + "refs1": [ + "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + ], + "refs2": [ + "/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv" + ] + }, + "impureEnvVars": [ + "UNICORN" + ], + "impureHostDeps": [ + "/usr/bin/ditto" + ], + "noChroot": true, + "outputChecks": { + "forAllOutputs": { + "allowedReferences": [ + "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + ], + "allowedRequisites": [ + "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev" + ], + "disallowedReferences": [ + "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar" + ], + "disallowedRequisites": [ + "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" + ], + "ignoreSelfRefs": true, + "maxClosureSize": null, + "maxSize": null + } + }, + "passAsFile": [], + "preferLocalBuild": true, + "requiredSystemFeatures": [ + "rainbow", + "uid-range" + ], + "unsafeDiscardReferences": {} +} diff --git a/src/libstore-tests/data/derivation/ia/defaults.json b/src/libstore-tests/data/derivation/ia/defaults.json new file mode 100644 index 000000000..79a68989c --- /dev/null +++ b/src/libstore-tests/data/derivation/ia/defaults.json @@ -0,0 +1,24 @@ +{ + "additionalSandboxProfile": "", + "allowLocalNetworking": false, + "allowSubstitutes": true, + "exportReferencesGraph": {}, + "impureEnvVars": [], + "impureHostDeps": [], + "noChroot": false, + "outputChecks": { + "forAllOutputs": { + "allowedReferences": null, + "allowedRequisites": null, + "disallowedReferences": [], + "disallowedRequisites": [], + "ignoreSelfRefs": true, + "maxClosureSize": null, + "maxSize": null + } + }, + "passAsFile": [], + "preferLocalBuild": false, + "requiredSystemFeatures": [], + "unsafeDiscardReferences": {} +} diff --git a/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json b/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json new file mode 100644 index 000000000..0fa383589 --- /dev/null +++ b/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json @@ -0,0 +1,66 @@ +{ + "additionalSandboxProfile": "sandcastle", + "allowLocalNetworking": true, + "allowSubstitutes": false, + "exportReferencesGraph": { + "refs1": [ + "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + ], + "refs2": [ + "/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv" + ] + }, + "impureEnvVars": [ + "UNICORN" + ], + "impureHostDeps": [ + "/usr/bin/ditto" + ], + "noChroot": true, + "outputChecks": { + "perOutput": { + "bin": { + "allowedReferences": null, + "allowedRequisites": null, + "disallowedReferences": [ + "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar" + ], + "disallowedRequisites": [ + "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" + ], + "ignoreSelfRefs": false, + "maxClosureSize": null, + "maxSize": null + }, + "dev": { + "allowedReferences": null, + "allowedRequisites": null, + "disallowedReferences": [], + "disallowedRequisites": [], + "ignoreSelfRefs": false, + "maxClosureSize": 5909, + "maxSize": 789 + }, + "out": { + "allowedReferences": [ + "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + ], + "allowedRequisites": [ + "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev" + ], + "disallowedReferences": [], + "disallowedRequisites": [], + "ignoreSelfRefs": false, + "maxClosureSize": null, + "maxSize": null + } + } + }, + "passAsFile": [], + "preferLocalBuild": true, + "requiredSystemFeatures": [ + "rainbow", + "uid-range" + ], + "unsafeDiscardReferences": {} +} diff --git a/src/libstore-tests/data/derivation/ia/structuredAttrs_defaults.json b/src/libstore-tests/data/derivation/ia/structuredAttrs_defaults.json new file mode 100644 index 000000000..d898d85f5 --- /dev/null +++ b/src/libstore-tests/data/derivation/ia/structuredAttrs_defaults.json @@ -0,0 +1,16 @@ +{ + "additionalSandboxProfile": "", + "allowLocalNetworking": false, + "allowSubstitutes": true, + "exportReferencesGraph": {}, + "impureEnvVars": [], + "impureHostDeps": [], + "noChroot": false, + "outputChecks": { + "perOutput": {} + }, + "passAsFile": [], + "preferLocalBuild": false, + "requiredSystemFeatures": [], + "unsafeDiscardReferences": {} +} diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc index c994b26d5..41538cdcc 100644 --- a/src/libstore-tests/derivation-advanced-attrs.cc +++ b/src/libstore-tests/derivation-advanced-attrs.cc @@ -10,13 +10,15 @@ #include "nix/util/json-utils.hh" #include "nix/store/tests/libstore.hh" -#include "nix/util/tests/characterization.hh" +#include "nix/util/tests/json-characterization.hh" namespace nix { using namespace nlohmann; -class DerivationAdvancedAttrsTest : public CharacterizationTest, public LibStoreTest +class DerivationAdvancedAttrsTest : public JsonCharacterizationTest, + public JsonCharacterizationTest, + public LibStoreTest { protected: std::filesystem::path unitTestData = getUnitTestData() / "derivation" / "ia"; @@ -454,4 +456,23 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs) {"rainbow", "uid-range", "ca-derivations"}); }; +#define TEST_JSON_OPTIONS(FIXUTURE, VAR, VAR2) \ + TEST_F(FIXUTURE, DerivationOptions_##VAR##_from_json) \ + { \ + this->JsonCharacterizationTest::readJsonTest(#VAR, advancedAttributes_##VAR2); \ + } \ + TEST_F(FIXUTURE, DerivationOptions_##VAR##_to_json) \ + { \ + this->JsonCharacterizationTest::writeJsonTest(#VAR, advancedAttributes_##VAR2); \ + } + +TEST_JSON_OPTIONS(DerivationAdvancedAttrsTest, defaults, defaults) +TEST_JSON_OPTIONS(DerivationAdvancedAttrsTest, all_set, ia) +TEST_JSON_OPTIONS(CaDerivationAdvancedAttrsTest, all_set, ca) +TEST_JSON_OPTIONS(DerivationAdvancedAttrsTest, structuredAttrs_defaults, structuredAttrs_defaults) +TEST_JSON_OPTIONS(DerivationAdvancedAttrsTest, structuredAttrs_all_set, structuredAttrs_ia) +TEST_JSON_OPTIONS(CaDerivationAdvancedAttrsTest, structuredAttrs_all_set, structuredAttrs_ca) + +#undef TEST_JSON_OPTIONS + } // namespace nix diff --git a/src/libstore/derivation-options.cc b/src/libstore/derivation-options.cc index bd9704b44..75313841c 100644 --- a/src/libstore/derivation-options.cc +++ b/src/libstore/derivation-options.cc @@ -176,13 +176,26 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt return {}; }; - checks.allowedReferences = get_("allowedReferences"); - checks.allowedRequisites = get_("allowedRequisites"); - checks.disallowedReferences = get_("disallowedReferences").value_or(StringSet{}); - checks.disallowedRequisites = get_("disallowedRequisites").value_or(StringSet{}); - ; - - res.insert_or_assign(outputName, std::move(checks)); + res.insert_or_assign( + outputName, + OutputChecks{ + .maxSize = [&]() -> std::optional { + if (auto maxSize = get(output, "maxSize")) + return maxSize->get(); + else + return std::nullopt; + }(), + .maxClosureSize = [&]() -> std::optional { + if (auto maxClosureSize = get(output, "maxClosureSize")) + return maxClosureSize->get(); + else + return std::nullopt; + }(), + .allowedReferences = get_("allowedReferences"), + .disallowedReferences = get_("disallowedReferences").value_or(StringSet{}), + .allowedRequisites = get_("allowedRequisites"), + .disallowedRequisites = get_("disallowedRequisites").value_or(StringSet{}), + }); } } return res; @@ -364,6 +377,7 @@ DerivationOptions adl_serializer::from_json(const json & json .unsafeDiscardReferences = valueAt(json, "unsafeDiscardReferences"), .passAsFile = getStringSet(valueAt(json, "passAsFile")), + .exportReferencesGraph = getMap(getObject(valueAt(json, "exportReferencesGraph")), getStringSet), .additionalSandboxProfile = getString(valueAt(json, "additionalSandboxProfile")), .noChroot = getBoolean(valueAt(json, "noChroot")), @@ -396,6 +410,7 @@ void adl_serializer::to_json(json & json, const DerivationOpt json["unsafeDiscardReferences"] = o.unsafeDiscardReferences; json["passAsFile"] = o.passAsFile; + json["exportReferencesGraph"] = o.exportReferencesGraph; json["additionalSandboxProfile"] = o.additionalSandboxProfile; json["noChroot"] = o.noChroot; @@ -423,6 +438,8 @@ DerivationOptions::OutputChecks adl_serializer: return { .ignoreSelfRefs = getBoolean(valueAt(json, "ignoreSelfRefs")), + .maxSize = ptrToOwned(getNullable(valueAt(json, "maxSize"))), + .maxClosureSize = ptrToOwned(getNullable(valueAt(json, "maxClosureSize"))), .allowedReferences = ptrToOwned(getNullable(valueAt(json, "allowedReferences"))), .disallowedReferences = getStringSet(valueAt(json, "disallowedReferences")), .allowedRequisites = ptrToOwned(getNullable(valueAt(json, "allowedRequisites"))), @@ -433,6 +450,8 @@ DerivationOptions::OutputChecks adl_serializer: void adl_serializer::to_json(json & json, const DerivationOptions::OutputChecks & c) { json["ignoreSelfRefs"] = c.ignoreSelfRefs; + json["maxSize"] = c.maxSize; + json["maxClosureSize"] = c.maxClosureSize; json["allowedReferences"] = c.allowedReferences; json["disallowedReferences"] = c.disallowedReferences; json["allowedRequisites"] = c.allowedRequisites; diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 726143db2..e6ac08fd9 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -1404,7 +1404,7 @@ void adl_serializer::to_json(json & res, const Derivation & d) { auto & inputsList = res["inputSrcs"]; - inputsList = nlohmann::json ::array(); + inputsList = nlohmann::json::array(); for (auto & input : d.inputSrcs) inputsList.emplace_back(input); } diff --git a/src/libutil/include/nix/util/json-utils.hh b/src/libutil/include/nix/util/json-utils.hh index 51ebb2b6c..7a3fe4f36 100644 --- a/src/libutil/include/nix/util/json-utils.hh +++ b/src/libutil/include/nix/util/json-utils.hh @@ -59,6 +59,17 @@ auto getInteger(const nlohmann::json & value) -> std::enable_if_t +std::map getMap(const nlohmann::json::object_t & jsonObject, auto && f) +{ + std::map map; + + for (const auto & [key, value] : jsonObject) + map.insert_or_assign(key, f(value)); + + return map; +} + const nlohmann::json::boolean_t & getBoolean(const nlohmann::json & value); Strings getStringList(const nlohmann::json & value); StringMap getStringMap(const nlohmann::json & value); diff --git a/src/libutil/json-utils.cc b/src/libutil/json-utils.cc index 1502384e9..80779541e 100644 --- a/src/libutil/json-utils.cc +++ b/src/libutil/json-utils.cc @@ -91,14 +91,7 @@ Strings getStringList(const nlohmann::json & value) StringMap getStringMap(const nlohmann::json & value) { - auto & jsonObject = getObject(value); - - StringMap stringMap; - - for (const auto & [key, value] : jsonObject) - stringMap[getString(key)] = getString(value); - - return stringMap; + return getMap>(getObject(value), getString); } StringSet getStringSet(const nlohmann::json & value)