From c5f348db959b87a4b61806830989d5e927921c00 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 7 Nov 2025 00:16:03 -0500 Subject: [PATCH 1/3] Test output checks referring to other outputs `allowedReferences` and friends can, in addition to supporting store paths (and placeholders, but because those will be rewritten to store paths), they also support to refering to other outputs in the derivation by name. We update the tests in order to cover for that. (While we are at it, also introduce some scratch variables for paths and placeholders to make the C++ literalsf for this test more concise.) --- .../advanced-attributes-structured-attrs.json | 6 +- .../derivation/ca/advanced-attributes.json | 4 +- .../data/derivation/ca/all_set.json | 6 +- .../ca/structuredAttrs_all_set.json | 6 +- .../advanced-attributes-structured-attrs.json | 18 +++--- .../derivation/ia/advanced-attributes.json | 8 +-- .../data/derivation/ia/all_set.json | 6 +- .../ia/structuredAttrs_all_set.json | 6 +- .../derivation-advanced-attrs.cc | 63 ++++++++++++------- .../advanced-attributes-structured-attrs.nix | 10 ++- .../derivation/advanced-attributes.nix | 10 ++- .../advanced-attributes-structured-attrs.drv | 2 +- .../derivation/ca/advanced-attributes.drv | 2 +- .../advanced-attributes-structured-attrs.drv | 2 +- .../derivation/ia/advanced-attributes.drv | 2 +- 15 files changed, 95 insertions(+), 56 deletions(-) diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json index 2a4e70558..95122ad41 100644 --- a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json @@ -69,7 +69,8 @@ "outputChecks": { "bin": { "disallowedReferences": [ - "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g" + "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g", + "dev" ], "disallowedRequisites": [ "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8" @@ -84,7 +85,8 @@ "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" ], "allowedRequisites": [ - "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z" + "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z", + "bin" ] } }, diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes.json b/src/libstore-tests/data/derivation/ca/advanced-attributes.json index 55dbe62e0..6b77459bc 100644 --- a/src/libstore-tests/data/derivation/ca/advanced-attributes.json +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes.json @@ -11,9 +11,9 @@ "__sandboxProfile": "sandcastle", "allowSubstitutes": "", "allowedReferences": "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9", - "allowedRequisites": "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z", + "allowedRequisites": "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z bin", "builder": "/bin/bash", - "disallowedReferences": "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g", + "disallowedReferences": "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g dev", "disallowedRequisites": "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8", "exportReferencesGraph": "refs1 /164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9 refs2 /nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv", "impureEnvVars": "UNICORN", diff --git a/src/libstore-tests/data/derivation/ca/all_set.json b/src/libstore-tests/data/derivation/ca/all_set.json index e06eada01..198356c64 100644 --- a/src/libstore-tests/data/derivation/ca/all_set.json +++ b/src/libstore-tests/data/derivation/ca/all_set.json @@ -23,10 +23,12 @@ "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" ], "allowedRequisites": [ - "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z" + "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z", + "bin" ], "disallowedReferences": [ - "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g" + "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g", + "dev" ], "disallowedRequisites": [ "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8" diff --git a/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json b/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json index 2a321897c..f566c48dd 100644 --- a/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json +++ b/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json @@ -23,7 +23,8 @@ "allowedReferences": null, "allowedRequisites": null, "disallowedReferences": [ - "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g" + "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g", + "dev" ], "disallowedRequisites": [ "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8" @@ -46,7 +47,8 @@ "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" ], "allowedRequisites": [ - "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z" + "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z", + "bin" ], "disallowedReferences": [], "disallowedRequisites": [], diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json index e07d1294b..bbd68e087 100644 --- a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json @@ -5,9 +5,9 @@ ], "builder": "/bin/bash", "env": { - "bin": "/nix/store/33qms3h55wlaspzba3brlzlrm8m2239g-advanced-attributes-structured-attrs-bin", - "dev": "/nix/store/wyfgwsdi8rs851wmy1xfzdxy7y5vrg5l-advanced-attributes-structured-attrs-dev", - "out": "/nix/store/7cxy4zx1vqc885r4jl2l64pymqbdmhii-advanced-attributes-structured-attrs" + "bin": "/nix/store/cnpasdljgkhnwaf78cf3qygcp4qbki1c-advanced-attributes-structured-attrs-bin", + "dev": "/nix/store/ijq6mwpa9jbnpnl33qldfqihrr38kprx-advanced-attributes-structured-attrs-dev", + "out": "/nix/store/h1vh648d3p088kdimy0r8ngpfx7c3nzw-advanced-attributes-structured-attrs" }, "inputs": { "drvs": { @@ -33,13 +33,13 @@ "name": "advanced-attributes-structured-attrs", "outputs": { "bin": { - "path": "33qms3h55wlaspzba3brlzlrm8m2239g-advanced-attributes-structured-attrs-bin" + "path": "cnpasdljgkhnwaf78cf3qygcp4qbki1c-advanced-attributes-structured-attrs-bin" }, "dev": { - "path": "wyfgwsdi8rs851wmy1xfzdxy7y5vrg5l-advanced-attributes-structured-attrs-dev" + "path": "ijq6mwpa9jbnpnl33qldfqihrr38kprx-advanced-attributes-structured-attrs-dev" }, "out": { - "path": "7cxy4zx1vqc885r4jl2l64pymqbdmhii-advanced-attributes-structured-attrs" + "path": "h1vh648d3p088kdimy0r8ngpfx7c3nzw-advanced-attributes-structured-attrs" } }, "structuredAttrs": { @@ -66,7 +66,8 @@ "outputChecks": { "bin": { "disallowedReferences": [ - "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar" + "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar", + "dev" ], "disallowedRequisites": [ "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" @@ -81,7 +82,8 @@ "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" ], "allowedRequisites": [ - "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev" + "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev", + "bin" ] } }, diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes.json b/src/libstore-tests/data/derivation/ia/advanced-attributes.json index 372b4fbb9..e2de9431b 100644 --- a/src/libstore-tests/data/derivation/ia/advanced-attributes.json +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes.json @@ -11,14 +11,14 @@ "__sandboxProfile": "sandcastle", "allowSubstitutes": "", "allowedReferences": "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo", - "allowedRequisites": "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev", + "allowedRequisites": "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev bin", "builder": "/bin/bash", - "disallowedReferences": "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar", + "disallowedReferences": "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar dev", "disallowedRequisites": "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev", "exportReferencesGraph": "refs1 /nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo refs2 /nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv", "impureEnvVars": "UNICORN", "name": "advanced-attributes", - "out": "/nix/store/wyhpwd748pns4k7svh48wdrc8kvjk0ra-advanced-attributes", + "out": "/nix/store/ymqmybkq5j4nd1xplw6ccdpbjnfi017v-advanced-attributes", "preferLocalBuild": "1", "requiredSystemFeatures": "rainbow uid-range", "system": "my-system" @@ -47,7 +47,7 @@ "name": "advanced-attributes", "outputs": { "out": { - "path": "wyhpwd748pns4k7svh48wdrc8kvjk0ra-advanced-attributes" + "path": "ymqmybkq5j4nd1xplw6ccdpbjnfi017v-advanced-attributes" } }, "system": "my-system", diff --git a/src/libstore-tests/data/derivation/ia/all_set.json b/src/libstore-tests/data/derivation/ia/all_set.json index 62b6cdf97..8731ca3a2 100644 --- a/src/libstore-tests/data/derivation/ia/all_set.json +++ b/src/libstore-tests/data/derivation/ia/all_set.json @@ -23,10 +23,12 @@ "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" ], "allowedRequisites": [ - "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev" + "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev", + "bin" ], "disallowedReferences": [ - "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar" + "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar", + "dev" ], "disallowedRequisites": [ "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" diff --git a/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json b/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json index 0fa383589..67fa634cf 100644 --- a/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json +++ b/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json @@ -23,7 +23,8 @@ "allowedReferences": null, "allowedRequisites": null, "disallowedReferences": [ - "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar" + "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar", + "dev" ], "disallowedRequisites": [ "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" @@ -46,7 +47,8 @@ "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" ], "allowedRequisites": [ - "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev" + "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev", + "bin" ], "disallowedReferences": [], "disallowedRequisites": [], diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc index 41538cdcc..f44e96cdd 100644 --- a/src/libstore-tests/derivation-advanced-attrs.cc +++ b/src/libstore-tests/derivation-advanced-attrs.cc @@ -127,6 +127,21 @@ TEST_ATERM_JSON(advancedAttributes_structuredAttrs_defaults, "advanced-attribute #undef TEST_ATERM_JSON +/** + * Since these are both repeated and sensative opaque values, it makes + * sense to give them names in this file. + */ +static std::string pathFoo = "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo", + pathFooDev = "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev", + pathBar = "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar", + pathBarDev = "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev", + pathBarDrvIA = "/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv", + pathBarDrvCA = "/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv", + placeholderFoo = "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9", + placeholderFooDev = "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z", + placeholderBar = "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g", + placeholderBarDev = "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"; + using ExportReferencesMap = decltype(DerivationOptions::exportReferencesGraph); static const DerivationOptions advancedAttributes_defaults = { @@ -216,16 +231,16 @@ 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"}, + .allowedReferences = StringSet{pathFoo}, + .disallowedReferences = StringSet{pathBar, "dev"}, + .allowedRequisites = StringSet{pathFooDev, "bin"}, + .disallowedRequisites = StringSet{pathBarDev}, }, .unsafeDiscardReferences = {}, .passAsFile = {}, .exportReferencesGraph{ - {"refs1", {"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}}, - {"refs2", {"/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"}}, + {"refs1", {pathFoo}}, + {"refs2", {pathBarDrvIA}}, }, .additionalSandboxProfile = "sandcastle", .noChroot = true, @@ -246,16 +261,16 @@ DerivationOptions advancedAttributes_ca = { .outputChecks = DerivationOptions::OutputChecks{ .ignoreSelfRefs = true, - .allowedReferences = StringSet{"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}, - .disallowedReferences = StringSet{"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g"}, - .allowedRequisites = StringSet{"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z"}, - .disallowedRequisites = StringSet{"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"}, + .allowedReferences = StringSet{placeholderFoo}, + .disallowedReferences = StringSet{placeholderBar, "dev"}, + .allowedRequisites = StringSet{placeholderFooDev, "bin"}, + .disallowedRequisites = StringSet{placeholderBarDev}, }, .unsafeDiscardReferences = {}, .passAsFile = {}, .exportReferencesGraph{ - {"refs1", {"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}}, - {"refs2", {"/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"}}, + {"refs1", {placeholderFoo}}, + {"refs2", {pathBarDrvCA}}, }, .additionalSandboxProfile = "sandcastle", .noChroot = true, @@ -375,13 +390,13 @@ DerivationOptions advancedAttributes_structuredAttrs_ia = { std::map{ {"out", DerivationOptions::OutputChecks{ - .allowedReferences = StringSet{"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}, - .allowedRequisites = StringSet{"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev"}, + .allowedReferences = StringSet{pathFoo}, + .allowedRequisites = StringSet{pathFooDev, "bin"}, }}, {"bin", DerivationOptions::OutputChecks{ - .disallowedReferences = StringSet{"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar"}, - .disallowedRequisites = StringSet{"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev"}, + .disallowedReferences = StringSet{pathBar, "dev"}, + .disallowedRequisites = StringSet{pathBarDev}, }}, {"dev", DerivationOptions::OutputChecks{ @@ -393,8 +408,8 @@ DerivationOptions advancedAttributes_structuredAttrs_ia = { .passAsFile = {}, .exportReferencesGraph = { - {"refs1", {"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}}, - {"refs2", {"/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"}}, + {"refs1", {pathFoo}}, + {"refs2", {pathBarDrvIA}}, }, .additionalSandboxProfile = "sandcastle", .noChroot = true, @@ -417,13 +432,13 @@ DerivationOptions advancedAttributes_structuredAttrs_ca = { std::map{ {"out", DerivationOptions::OutputChecks{ - .allowedReferences = StringSet{"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}, - .allowedRequisites = StringSet{"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z"}, + .allowedReferences = StringSet{placeholderFoo}, + .allowedRequisites = StringSet{placeholderFooDev, "bin"}, }}, {"bin", DerivationOptions::OutputChecks{ - .disallowedReferences = StringSet{"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g"}, - .disallowedRequisites = StringSet{"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"}, + .disallowedReferences = StringSet{placeholderBar, "dev"}, + .disallowedRequisites = StringSet{placeholderBarDev}, }}, {"dev", DerivationOptions::OutputChecks{ @@ -435,8 +450,8 @@ DerivationOptions advancedAttributes_structuredAttrs_ca = { .passAsFile = {}, .exportReferencesGraph = { - {"refs1", {"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"}}, - {"refs2", {"/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"}}, + {"refs1", {placeholderFoo}}, + {"refs2", {pathBarDrvCA}}, }, .additionalSandboxProfile = "sandcastle", .noChroot = true, diff --git a/tests/functional/derivation/advanced-attributes-structured-attrs.nix b/tests/functional/derivation/advanced-attributes-structured-attrs.nix index 46f619272..b11041303 100644 --- a/tests/functional/derivation/advanced-attributes-structured-attrs.nix +++ b/tests/functional/derivation/advanced-attributes-structured-attrs.nix @@ -66,10 +66,16 @@ derivation' { outputChecks = { out = { allowedReferences = [ foo ]; - allowedRequisites = [ foo.dev ]; + allowedRequisites = [ + foo.dev + "bin" + ]; }; bin = { - disallowedReferences = [ bar ]; + disallowedReferences = [ + bar + "dev" + ]; disallowedRequisites = [ bar.dev ]; }; dev = { diff --git a/tests/functional/derivation/advanced-attributes.nix b/tests/functional/derivation/advanced-attributes.nix index dd0c09e22..19a80f15d 100644 --- a/tests/functional/derivation/advanced-attributes.nix +++ b/tests/functional/derivation/advanced-attributes.nix @@ -58,8 +58,14 @@ derivation' { impureEnvVars = [ "UNICORN" ]; __darwinAllowLocalNetworking = true; allowedReferences = [ foo ]; - allowedRequisites = [ foo.dev ]; - disallowedReferences = [ bar ]; + allowedRequisites = [ + foo.dev + "bin" + ]; + disallowedReferences = [ + bar + "dev" + ]; disallowedRequisites = [ bar.dev ]; requiredSystemFeatures = [ "rainbow" diff --git a/tests/functional/derivation/ca/advanced-attributes-structured-attrs.drv b/tests/functional/derivation/ca/advanced-attributes-structured-attrs.drv index cd02c2f86..eeaba88e6 100644 --- a/tests/functional/derivation/ca/advanced-attributes-structured-attrs.drv +++ b/tests/functional/derivation/ca/advanced-attributes-structured-attrs.drv @@ -1 +1 @@ -Derive([("bin","","r:sha256",""),("dev","","r:sha256",""),("out","","r:sha256","")],[("/nix/store/j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv",["dev","out"]),("/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv",["dev","out"])],["/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"],"my-system","/bin/bash",["-c","echo hello > $out"],[("__json","{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"exportReferencesGraph\":{\"refs1\":[\"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9\"],\"refs2\":[\"/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv\"]},\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g\"],\"disallowedRequisites\":[\"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9\"],\"allowedRequisites\":[\"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z\"]}},\"outputHashAlgo\":\"sha256\",\"outputHashMode\":\"recursive\",\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}"),("bin","/04f3da1kmbr67m3gzxikmsl4vjz5zf777sv6m14ahv22r65aac9m"),("dev","/02qcpld1y6xhs5gz9bchpxaw0xdhmsp5dv88lh25r2ss44kh8dxz"),("out","/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9")]) \ No newline at end of file +Derive([("bin","","r:sha256",""),("dev","","r:sha256",""),("out","","r:sha256","")],[("/nix/store/j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv",["dev","out"]),("/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv",["dev","out"])],["/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"],"my-system","/bin/bash",["-c","echo hello > $out"],[("__json","{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"exportReferencesGraph\":{\"refs1\":[\"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9\"],\"refs2\":[\"/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv\"]},\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g\",\"dev\"],\"disallowedRequisites\":[\"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9\"],\"allowedRequisites\":[\"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z\",\"bin\"]}},\"outputHashAlgo\":\"sha256\",\"outputHashMode\":\"recursive\",\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}"),("bin","/04f3da1kmbr67m3gzxikmsl4vjz5zf777sv6m14ahv22r65aac9m"),("dev","/02qcpld1y6xhs5gz9bchpxaw0xdhmsp5dv88lh25r2ss44kh8dxz"),("out","/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9")]) \ No newline at end of file diff --git a/tests/functional/derivation/ca/advanced-attributes.drv b/tests/functional/derivation/ca/advanced-attributes.drv index 068cb593e..ee5968cdc 100644 --- a/tests/functional/derivation/ca/advanced-attributes.drv +++ b/tests/functional/derivation/ca/advanced-attributes.drv @@ -1 +1 @@ -Derive([("out","","r:sha256","")],[("/nix/store/j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv",["dev","out"]),("/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv",["dev","out"])],["/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"],"my-system","/bin/bash",["-c","echo hello > $out"],[("__darwinAllowLocalNetworking","1"),("__impureHostDeps","/usr/bin/ditto"),("__noChroot","1"),("__sandboxProfile","sandcastle"),("allowSubstitutes",""),("allowedReferences","/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"),("allowedRequisites","/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z"),("builder","/bin/bash"),("disallowedReferences","/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g"),("disallowedRequisites","/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"),("exportReferencesGraph","refs1 /164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9 refs2 /nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"),("impureEnvVars","UNICORN"),("name","advanced-attributes"),("out","/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9"),("outputHashAlgo","sha256"),("outputHashMode","recursive"),("preferLocalBuild","1"),("requiredSystemFeatures","rainbow uid-range"),("system","my-system")]) \ No newline at end of file +Derive([("out","","r:sha256","")],[("/nix/store/j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv",["dev","out"]),("/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv",["dev","out"])],["/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"],"my-system","/bin/bash",["-c","echo hello > $out"],[("__darwinAllowLocalNetworking","1"),("__impureHostDeps","/usr/bin/ditto"),("__noChroot","1"),("__sandboxProfile","sandcastle"),("allowSubstitutes",""),("allowedReferences","/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9"),("allowedRequisites","/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z bin"),("builder","/bin/bash"),("disallowedReferences","/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g dev"),("disallowedRequisites","/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"),("exportReferencesGraph","refs1 /164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9 refs2 /nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"),("impureEnvVars","UNICORN"),("name","advanced-attributes"),("out","/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9"),("outputHashAlgo","sha256"),("outputHashMode","recursive"),("preferLocalBuild","1"),("requiredSystemFeatures","rainbow uid-range"),("system","my-system")]) \ No newline at end of file diff --git a/tests/functional/derivation/ia/advanced-attributes-structured-attrs.drv b/tests/functional/derivation/ia/advanced-attributes-structured-attrs.drv index 1dfcac42d..0aa82e636 100644 --- a/tests/functional/derivation/ia/advanced-attributes-structured-attrs.drv +++ b/tests/functional/derivation/ia/advanced-attributes-structured-attrs.drv @@ -1 +1 @@ -Derive([("bin","/nix/store/33qms3h55wlaspzba3brlzlrm8m2239g-advanced-attributes-structured-attrs-bin","",""),("dev","/nix/store/wyfgwsdi8rs851wmy1xfzdxy7y5vrg5l-advanced-attributes-structured-attrs-dev","",""),("out","/nix/store/7cxy4zx1vqc885r4jl2l64pymqbdmhii-advanced-attributes-structured-attrs","","")],[("/nix/store/afc3vbjbzql750v2lp8gxgaxsajphzih-foo.drv",["dev","out"]),("/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv",["dev","out"])],["/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"],"my-system","/bin/bash",["-c","echo hello > $out"],[("__json","{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"exportReferencesGraph\":{\"refs1\":[\"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo\"],\"refs2\":[\"/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv\"]},\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar\"],\"disallowedRequisites\":[\"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo\"],\"allowedRequisites\":[\"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev\"]}},\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}"),("bin","/nix/store/33qms3h55wlaspzba3brlzlrm8m2239g-advanced-attributes-structured-attrs-bin"),("dev","/nix/store/wyfgwsdi8rs851wmy1xfzdxy7y5vrg5l-advanced-attributes-structured-attrs-dev"),("out","/nix/store/7cxy4zx1vqc885r4jl2l64pymqbdmhii-advanced-attributes-structured-attrs")]) \ No newline at end of file +Derive([("bin","/nix/store/cnpasdljgkhnwaf78cf3qygcp4qbki1c-advanced-attributes-structured-attrs-bin","",""),("dev","/nix/store/ijq6mwpa9jbnpnl33qldfqihrr38kprx-advanced-attributes-structured-attrs-dev","",""),("out","/nix/store/h1vh648d3p088kdimy0r8ngpfx7c3nzw-advanced-attributes-structured-attrs","","")],[("/nix/store/afc3vbjbzql750v2lp8gxgaxsajphzih-foo.drv",["dev","out"]),("/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv",["dev","out"])],["/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"],"my-system","/bin/bash",["-c","echo hello > $out"],[("__json","{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"exportReferencesGraph\":{\"refs1\":[\"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo\"],\"refs2\":[\"/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv\"]},\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar\",\"dev\"],\"disallowedRequisites\":[\"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo\"],\"allowedRequisites\":[\"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev\",\"bin\"]}},\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}"),("bin","/nix/store/cnpasdljgkhnwaf78cf3qygcp4qbki1c-advanced-attributes-structured-attrs-bin"),("dev","/nix/store/ijq6mwpa9jbnpnl33qldfqihrr38kprx-advanced-attributes-structured-attrs-dev"),("out","/nix/store/h1vh648d3p088kdimy0r8ngpfx7c3nzw-advanced-attributes-structured-attrs")]) \ No newline at end of file diff --git a/tests/functional/derivation/ia/advanced-attributes.drv b/tests/functional/derivation/ia/advanced-attributes.drv index c71a88886..4bc7320f5 100644 --- a/tests/functional/derivation/ia/advanced-attributes.drv +++ b/tests/functional/derivation/ia/advanced-attributes.drv @@ -1 +1 @@ -Derive([("out","/nix/store/wyhpwd748pns4k7svh48wdrc8kvjk0ra-advanced-attributes","","")],[("/nix/store/afc3vbjbzql750v2lp8gxgaxsajphzih-foo.drv",["dev","out"]),("/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv",["dev","out"])],["/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"],"my-system","/bin/bash",["-c","echo hello > $out"],[("__darwinAllowLocalNetworking","1"),("__impureHostDeps","/usr/bin/ditto"),("__noChroot","1"),("__sandboxProfile","sandcastle"),("allowSubstitutes",""),("allowedReferences","/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"),("allowedRequisites","/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev"),("builder","/bin/bash"),("disallowedReferences","/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar"),("disallowedRequisites","/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev"),("exportReferencesGraph","refs1 /nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo refs2 /nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"),("impureEnvVars","UNICORN"),("name","advanced-attributes"),("out","/nix/store/wyhpwd748pns4k7svh48wdrc8kvjk0ra-advanced-attributes"),("preferLocalBuild","1"),("requiredSystemFeatures","rainbow uid-range"),("system","my-system")]) \ No newline at end of file +Derive([("out","/nix/store/ymqmybkq5j4nd1xplw6ccdpbjnfi017v-advanced-attributes","","")],[("/nix/store/afc3vbjbzql750v2lp8gxgaxsajphzih-foo.drv",["dev","out"]),("/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv",["dev","out"])],["/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"],"my-system","/bin/bash",["-c","echo hello > $out"],[("__darwinAllowLocalNetworking","1"),("__impureHostDeps","/usr/bin/ditto"),("__noChroot","1"),("__sandboxProfile","sandcastle"),("allowSubstitutes",""),("allowedReferences","/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"),("allowedRequisites","/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev bin"),("builder","/bin/bash"),("disallowedReferences","/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar dev"),("disallowedRequisites","/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev"),("exportReferencesGraph","refs1 /nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo refs2 /nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"),("impureEnvVars","UNICORN"),("name","advanced-attributes"),("out","/nix/store/ymqmybkq5j4nd1xplw6ccdpbjnfi017v-advanced-attributes"),("preferLocalBuild","1"),("requiredSystemFeatures","rainbow uid-range"),("system","my-system")]) \ No newline at end of file From 00d2bf91b23caa20197883d76b47dcce49ba0a2f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 14 Apr 2025 14:26:13 -0400 Subject: [PATCH 2/3] Parse deriving paths in `DerivationOptions` 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` 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 --- .../data/derivation/ca/all_set.json | 37 +- .../ca/structuredAttrs_all_set.json | 37 +- .../data/derivation/ia/all_set.json | 22 +- .../ia/structuredAttrs_all_set.json | 22 +- .../derivation-advanced-attrs.cc | 165 ++++---- .../build/derivation-building-goal.cc | 17 +- src/libstore/build/derivation-check.cc | 47 ++- src/libstore/build/derivation-check.hh | 2 +- src/libstore/build/derivation-env-desugar.cc | 7 +- src/libstore/build/derivation-goal.cc | 4 +- src/libstore/derivation-options.cc | 387 +++++++++++++++--- src/libstore/downstream-placeholder.cc | 43 ++ .../nix/store/build/derivation-builder.hh | 2 +- .../store/build/derivation-building-goal.hh | 2 +- .../nix/store/build/derivation-env-desugar.hh | 6 +- .../include/nix/store/derivation-options.hh | 99 +++-- .../nix/store/downstream-placeholder.hh | 26 ++ .../include/nix/store/parsed-derivations.hh | 3 +- src/libstore/misc.cc | 5 +- src/libstore/parsed-derivations.cc | 6 +- src/nix/nix-build/nix-build.cc | 4 +- tests/functional/check-refs.sh | 2 +- 22 files changed, 701 insertions(+), 244 deletions(-) diff --git a/src/libstore-tests/data/derivation/ca/all_set.json b/src/libstore-tests/data/derivation/ca/all_set.json index 198356c64..8086c752c 100644 --- a/src/libstore-tests/data/derivation/ca/all_set.json +++ b/src/libstore-tests/data/derivation/ca/all_set.json @@ -4,10 +4,13 @@ "allowSubstitutes": false, "exportReferencesGraph": { "refs1": [ - "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + { + "drvPath": "j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv", + "output": "out" + } ], "refs2": [ - "/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv" + "qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv" ] }, "impureEnvVars": [ @@ -20,18 +23,36 @@ "outputChecks": { "forAllOutputs": { "allowedReferences": [ - "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + { + "drvPath": "j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv", + "output": "out" + } ], "allowedRequisites": [ - "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z", - "bin" + { + "drvPath": "self", + "output": "bin" + }, + { + "drvPath": "j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv", + "output": "dev" + } ], "disallowedReferences": [ - "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g", - "dev" + { + "drvPath": "self", + "output": "dev" + }, + { + "drvPath": "qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv", + "output": "out" + } ], "disallowedRequisites": [ - "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8" + { + "drvPath": "qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv", + "output": "dev" + } ], "ignoreSelfRefs": true, "maxClosureSize": null, diff --git a/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json b/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json index f566c48dd..e29447b6a 100644 --- a/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json +++ b/src/libstore-tests/data/derivation/ca/structuredAttrs_all_set.json @@ -4,10 +4,13 @@ "allowSubstitutes": false, "exportReferencesGraph": { "refs1": [ - "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + { + "drvPath": "j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv", + "output": "out" + } ], "refs2": [ - "/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv" + "qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv" ] }, "impureEnvVars": [ @@ -23,11 +26,20 @@ "allowedReferences": null, "allowedRequisites": null, "disallowedReferences": [ - "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g", - "dev" + { + "drvPath": "self", + "output": "dev" + }, + { + "drvPath": "qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv", + "output": "out" + } ], "disallowedRequisites": [ - "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8" + { + "drvPath": "qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv", + "output": "dev" + } ], "ignoreSelfRefs": false, "maxClosureSize": null, @@ -44,11 +56,20 @@ }, "out": { "allowedReferences": [ - "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + { + "drvPath": "j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv", + "output": "out" + } ], "allowedRequisites": [ - "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z", - "bin" + { + "drvPath": "self", + "output": "bin" + }, + { + "drvPath": "j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv", + "output": "dev" + } ], "disallowedReferences": [], "disallowedRequisites": [], diff --git a/src/libstore-tests/data/derivation/ia/all_set.json b/src/libstore-tests/data/derivation/ia/all_set.json index 8731ca3a2..2e1c848da 100644 --- a/src/libstore-tests/data/derivation/ia/all_set.json +++ b/src/libstore-tests/data/derivation/ia/all_set.json @@ -4,10 +4,10 @@ "allowSubstitutes": false, "exportReferencesGraph": { "refs1": [ - "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + "p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" ], "refs2": [ - "/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv" + "vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv" ] }, "impureEnvVars": [ @@ -20,18 +20,24 @@ "outputChecks": { "forAllOutputs": { "allowedReferences": [ - "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + "p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" ], "allowedRequisites": [ - "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev", - "bin" + { + "drvPath": "self", + "output": "bin" + }, + "z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev" ], "disallowedReferences": [ - "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar", - "dev" + { + "drvPath": "self", + "output": "dev" + }, + "r5cff30838majxk5mp3ip2diffi8vpaj-bar" ], "disallowedRequisites": [ - "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" + "9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" ], "ignoreSelfRefs": true, "maxClosureSize": null, diff --git a/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json b/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json index 67fa634cf..a29699b5d 100644 --- a/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json +++ b/src/libstore-tests/data/derivation/ia/structuredAttrs_all_set.json @@ -4,10 +4,10 @@ "allowSubstitutes": false, "exportReferencesGraph": { "refs1": [ - "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + "p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" ], "refs2": [ - "/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv" + "vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv" ] }, "impureEnvVars": [ @@ -23,11 +23,14 @@ "allowedReferences": null, "allowedRequisites": null, "disallowedReferences": [ - "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar", - "dev" + { + "drvPath": "self", + "output": "dev" + }, + "r5cff30838majxk5mp3ip2diffi8vpaj-bar" ], "disallowedRequisites": [ - "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" + "9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" ], "ignoreSelfRefs": false, "maxClosureSize": null, @@ -44,11 +47,14 @@ }, "out": { "allowedReferences": [ - "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + "p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" ], "allowedRequisites": [ - "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev", - "bin" + { + "drvPath": "self", + "output": "bin" + }, + "z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev" ], "disallowedReferences": [], "disallowedRequisites": [], diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc index f44e96cdd..27b8aba16 100644 --- a/src/libstore-tests/derivation-advanced-attrs.cc +++ b/src/libstore-tests/derivation-advanced-attrs.cc @@ -3,7 +3,7 @@ #include "nix/util/experimental-features.hh" #include "nix/store/derivations.hh" -#include "nix/store/derivations.hh" +#include "nix/store/derived-path.hh" #include "nix/store/derivation-options.hh" #include "nix/store/parsed-derivations.hh" #include "nix/util/types.hh" @@ -17,7 +17,7 @@ namespace nix { using namespace nlohmann; class DerivationAdvancedAttrsTest : public JsonCharacterizationTest, - public JsonCharacterizationTest, + public JsonCharacterizationTest>, public LibStoreTest { protected: @@ -42,7 +42,8 @@ public: { this->readTest(fileName, [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); + auto options = derivationOptionsFromStructuredAttrs( + *this->store, got.inputDrvs, got.env, got.structuredAttrs, true, this->mockXpSettings); EXPECT_EQ(options.getRequiredSystemFeatures(got), expectedFeatures); }); } @@ -51,11 +52,14 @@ public: * Helper function to test DerivationOptions parsing and comparison */ void testDerivationOptions( - const std::string & fileName, const DerivationOptions & expected, const StringSet & expectedSystemFeatures) + 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); + auto options = derivationOptionsFromStructuredAttrs( + *this->store, got.inputDrvs, got.env, got.structuredAttrs, true, this->mockXpSettings); EXPECT_EQ(options, expected); EXPECT_EQ(options.getRequiredSystemFeatures(got), expectedSystemFeatures); @@ -131,22 +135,38 @@ TEST_ATERM_JSON(advancedAttributes_structuredAttrs_defaults, "advanced-attribute * Since these are both repeated and sensative opaque values, it makes * sense to give them names in this file. */ -static std::string pathFoo = "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo", - pathFooDev = "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev", - pathBar = "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar", - pathBarDev = "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev", - pathBarDrvIA = "/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv", - pathBarDrvCA = "/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv", - placeholderFoo = "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9", - placeholderFooDev = "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z", - placeholderBar = "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g", - placeholderBarDev = "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8"; +static SingleDerivedPath + pathFoo = SingleDerivedPath::Opaque{StorePath{"p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo"}}, + pathFooDev = SingleDerivedPath::Opaque{StorePath{"z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev"}}, + pathBar = SingleDerivedPath::Opaque{StorePath{"r5cff30838majxk5mp3ip2diffi8vpaj-bar"}}, + pathBarDev = SingleDerivedPath::Opaque{StorePath{"9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev"}}, + pathBarDrvIA = SingleDerivedPath::Opaque{StorePath{"vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv"}}, + pathBarDrvCA = SingleDerivedPath::Opaque{StorePath{"qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"}}, + placeholderFoo = + SingleDerivedPath::Built{ + .drvPath = makeConstantStorePathRef(StorePath{"j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv"}), + .output = "out", + }, + placeholderFooDev = + SingleDerivedPath::Built{ + .drvPath = makeConstantStorePathRef(StorePath{"j56sf12rxpcv5swr14vsjn5cwm6bj03h-foo.drv"}), + .output = "dev", + }, + placeholderBar = + SingleDerivedPath::Built{ + .drvPath = makeConstantStorePathRef(StorePath{"qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"}), + .output = "out", + }, + placeholderBarDev = SingleDerivedPath::Built{ + .drvPath = makeConstantStorePathRef(StorePath{"qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv"}), + .output = "dev", + }; -using ExportReferencesMap = decltype(DerivationOptions::exportReferencesGraph); +using ExportReferencesMap = decltype(DerivationOptions::exportReferencesGraph); -static const DerivationOptions advancedAttributes_defaults = { +static const DerivationOptions advancedAttributes_defaults = { .outputChecks = - DerivationOptions::OutputChecks{ + DerivationOptions::OutputChecks{ .ignoreSelfRefs = true, }, .unsafeDiscardReferences = {}, @@ -167,7 +187,8 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_defaults) this->readTest("advanced-attributes-defaults.drv", [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); + auto options = derivationOptionsFromStructuredAttrs( + *this->store, got.inputDrvs, got.env, got.structuredAttrs, true, this->mockXpSettings); EXPECT_TRUE(!got.structuredAttrs); @@ -192,9 +213,9 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_defaults) TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes) { - DerivationOptions expected = { + DerivationOptions expected = { .outputChecks = - DerivationOptions::OutputChecks{ + DerivationOptions::OutputChecks{ .ignoreSelfRefs = true, }, .unsafeDiscardReferences = {}, @@ -212,12 +233,13 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes) this->readTest("advanced-attributes.drv", [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); + auto options = derivationOptionsFromStructuredAttrs( + *this->store, got.inputDrvs, got.env, got.structuredAttrs, true, this->mockXpSettings); EXPECT_TRUE(!got.structuredAttrs); // Reset fields that vary between test cases to enable whole-object comparison - options.outputChecks = DerivationOptions::OutputChecks{.ignoreSelfRefs = true}; + options.outputChecks = DerivationOptions::OutputChecks{.ignoreSelfRefs = true}; options.exportReferencesGraph = {}; EXPECT_EQ(options, expected); @@ -227,14 +249,14 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes) }); }; -DerivationOptions advancedAttributes_ia = { +DerivationOptions advancedAttributes_ia = { .outputChecks = - DerivationOptions::OutputChecks{ + DerivationOptions::OutputChecks{ .ignoreSelfRefs = true, - .allowedReferences = StringSet{pathFoo}, - .disallowedReferences = StringSet{pathBar, "dev"}, - .allowedRequisites = StringSet{pathFooDev, "bin"}, - .disallowedRequisites = StringSet{pathBarDev}, + .allowedReferences = std::set>{pathFoo}, + .disallowedReferences = std::set>{pathBar, OutputName{"dev"}}, + .allowedRequisites = std::set>{pathFooDev, OutputName{"bin"}}, + .disallowedRequisites = std::set>{pathBarDev}, }, .unsafeDiscardReferences = {}, .passAsFile = {}, @@ -257,14 +279,14 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_ia) testDerivationOptions("advanced-attributes.drv", advancedAttributes_ia, {"rainbow", "uid-range"}); }; -DerivationOptions advancedAttributes_ca = { +DerivationOptions advancedAttributes_ca = { .outputChecks = - DerivationOptions::OutputChecks{ + DerivationOptions::OutputChecks{ .ignoreSelfRefs = true, - .allowedReferences = StringSet{placeholderFoo}, - .disallowedReferences = StringSet{placeholderBar, "dev"}, - .allowedRequisites = StringSet{placeholderFooDev, "bin"}, - .disallowedRequisites = StringSet{placeholderBarDev}, + .allowedReferences = std::set>{placeholderFoo}, + .disallowedReferences = std::set>{placeholderBar, OutputName{"dev"}}, + .allowedRequisites = std::set>{placeholderFooDev, OutputName{"bin"}}, + .disallowedRequisites = std::set>{placeholderBarDev}, }, .unsafeDiscardReferences = {}, .passAsFile = {}, @@ -287,8 +309,8 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes) testDerivationOptions("advanced-attributes.drv", advancedAttributes_ca, {"rainbow", "uid-range", "ca-derivations"}); }; -DerivationOptions advancedAttributes_structuredAttrs_defaults = { - .outputChecks = std::map{}, +DerivationOptions advancedAttributes_structuredAttrs_defaults = { + .outputChecks = std::map::OutputChecks>{}, .unsafeDiscardReferences = {}, .passAsFile = {}, .exportReferencesGraph = {}, @@ -307,7 +329,8 @@ 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); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); + auto options = derivationOptionsFromStructuredAttrs( + *this->store, got.inputDrvs, got.env, got.structuredAttrs, true, this->mockXpSettings); EXPECT_TRUE(got.structuredAttrs); @@ -332,11 +355,11 @@ TEST_F(CaDerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs_default TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) { - DerivationOptions expected = { + DerivationOptions expected = { .outputChecks = - std::map{ + std::map::OutputChecks>{ {"dev", - DerivationOptions::OutputChecks{ + DerivationOptions::OutputChecks{ .maxSize = 789, .maxClosureSize = 5909, }}, @@ -357,7 +380,8 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) this->readTest("advanced-attributes-structured-attrs.drv", [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - DerivationOptions options = DerivationOptions::fromStructuredAttrs(got.env, got.structuredAttrs); + auto options = derivationOptionsFromStructuredAttrs( + *this->store, got.inputDrvs, got.env, got.structuredAttrs, true, this->mockXpSettings); EXPECT_TRUE(got.structuredAttrs); @@ -365,7 +389,8 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) { // Delete all keys but "dev" in options.outputChecks auto * outputChecksMapP = - std::get_if>(&options.outputChecks); + std::get_if::OutputChecks>>( + &options.outputChecks); ASSERT_TRUE(outputChecksMapP); auto & outputChecksMap = *outputChecksMapP; auto devEntry = outputChecksMap.find("dev"); @@ -385,21 +410,21 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) }); }; -DerivationOptions advancedAttributes_structuredAttrs_ia = { +DerivationOptions advancedAttributes_structuredAttrs_ia = { .outputChecks = - std::map{ + std::map::OutputChecks>{ {"out", - DerivationOptions::OutputChecks{ - .allowedReferences = StringSet{pathFoo}, - .allowedRequisites = StringSet{pathFooDev, "bin"}, + DerivationOptions::OutputChecks{ + .allowedReferences = std::set>{pathFoo}, + .allowedRequisites = std::set>{pathFooDev, OutputName{"bin"}}, }}, {"bin", - DerivationOptions::OutputChecks{ - .disallowedReferences = StringSet{pathBar, "dev"}, - .disallowedRequisites = StringSet{pathBarDev}, + DerivationOptions::OutputChecks{ + .disallowedReferences = std::set>{pathBar, OutputName{"dev"}}, + .disallowedRequisites = std::set>{pathBarDev}, }}, {"dev", - DerivationOptions::OutputChecks{ + DerivationOptions::OutputChecks{ .maxSize = 789, .maxClosureSize = 5909, }}, @@ -427,21 +452,21 @@ TEST_F(DerivationAdvancedAttrsTest, advancedAttributes_structuredAttrs) "advanced-attributes-structured-attrs.drv", advancedAttributes_structuredAttrs_ia, {"rainbow", "uid-range"}); }; -DerivationOptions advancedAttributes_structuredAttrs_ca = { +DerivationOptions advancedAttributes_structuredAttrs_ca = { .outputChecks = - std::map{ + std::map::OutputChecks>{ {"out", - DerivationOptions::OutputChecks{ - .allowedReferences = StringSet{placeholderFoo}, - .allowedRequisites = StringSet{placeholderFooDev, "bin"}, + DerivationOptions::OutputChecks{ + .allowedReferences = std::set>{placeholderFoo}, + .allowedRequisites = std::set>{placeholderFooDev, OutputName{"bin"}}, }}, {"bin", - DerivationOptions::OutputChecks{ - .disallowedReferences = StringSet{placeholderBar, "dev"}, - .disallowedRequisites = StringSet{placeholderBarDev}, + DerivationOptions::OutputChecks{ + .disallowedReferences = std::set>{placeholderBar, OutputName{"dev"}}, + .disallowedRequisites = std::set>{placeholderBarDev}, }}, {"dev", - DerivationOptions::OutputChecks{ + DerivationOptions::OutputChecks{ .maxSize = 789, .maxClosureSize = 5909, }}, @@ -471,14 +496,16 @@ 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); \ +#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) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index c72130142..41d76fbba 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -32,14 +32,27 @@ DerivationBuildingGoal::DerivationBuildingGoal( , drv{std::make_unique(drv)} , buildMode(buildMode) { + DerivationOptions temp; try { - drvOptions = - std::make_unique(DerivationOptions::fromStructuredAttrs(drv.env, drv.structuredAttrs)); + temp = derivationOptionsFromStructuredAttrs(worker.store, drv.inputDrvs, drv.env, drv.structuredAttrs); } catch (Error & e) { e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath)); throw; } + auto x = tryResolve( + temp, [&](ref drvPath, const std::string & outputName) -> std::optional { + try { + return resolveDerivedPath( + worker.store, SingleDerivedPath::Built{drvPath, outputName}, &worker.evalStore); + } catch (Error &) { + return std::nullopt; + } + }); + + assert(x); + drvOptions = std::make_unique>(*x); + name = fmt("building derivation '%s'", worker.store.printStorePath(drvPath)); trace("created"); diff --git a/src/libstore/build/derivation-check.cc b/src/libstore/build/derivation-check.cc index 181221ba5..e56b9fe49 100644 --- a/src/libstore/build/derivation-check.cc +++ b/src/libstore/build/derivation-check.cc @@ -11,7 +11,7 @@ void checkOutputs( Store & store, const StorePath & drvPath, const decltype(Derivation::outputs) & drvOutputs, - const decltype(DerivationOptions::outputChecks) & outputChecks, + const decltype(DerivationOptions::outputChecks) & outputChecks, const std::map & outputs) { std::map 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::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> & 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 & checksPerOutput) { + [&](const DerivationOptions::OutputChecks & checks) { applyChecks(checks); }, + [&](const std::map::OutputChecks> & checksPerOutput) { if (auto outputChecks = get(checksPerOutput, outputName)) applyChecks(*outputChecks); diff --git a/src/libstore/build/derivation-check.hh b/src/libstore/build/derivation-check.hh index b425c2ac5..01e6c5d56 100644 --- a/src/libstore/build/derivation-check.hh +++ b/src/libstore/build/derivation-check.hh @@ -21,7 +21,7 @@ void checkOutputs( Store & store, const StorePath & drvPath, const decltype(Derivation::outputs) & drvOutputs, - const decltype(DerivationOptions::outputChecks) & drvOptions, + const decltype(DerivationOptions::outputChecks) & drvOptions, const std::map & outputs); } // namespace nix diff --git a/src/libstore/build/derivation-env-desugar.cc b/src/libstore/build/derivation-env-desugar.cc index 8d552fc4d..75b62c116 100644 --- a/src/libstore/build/derivation-env-desugar.cc +++ b/src/libstore/build/derivation-env-desugar.cc @@ -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 & 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 . */ res.extraFiles.insert_or_assign( fileName, store.makeValidityRegistration(store.exportReferences(storePaths, inputPaths), false, false)); diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 14aa044ea..a44b79f57 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -64,9 +64,9 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) { trace("have derivation"); - auto drvOptions = [&]() -> DerivationOptions { + auto drvOptions = [&]() -> DerivationOptions { try { - return DerivationOptions::fromStructuredAttrs(drv->env, drv->structuredAttrs); + return derivationOptionsFromStructuredAttrs(worker.store, drv->inputDrvs, drv->env, drv->structuredAttrs); } catch (Error & e) { e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath)); throw; diff --git a/src/libstore/derivation-options.cc b/src/libstore/derivation-options.cc index 75313841c..8c71ef4b1 100644 --- a/src/libstore/derivation-options.cc +++ b/src/libstore/derivation-options.cc @@ -2,15 +2,18 @@ #include "nix/util/json-utils.hh" #include "nix/store/parsed-derivations.hh" #include "nix/store/derivations.hh" +#include "nix/store/derived-path.hh" #include "nix/store/store-api.hh" #include "nix/util/types.hh" #include "nix/util/util.hh" #include "nix/store/globals.hh" +#include "nix/util/variant-wrapper.hh" #include #include #include #include +#include namespace nix { @@ -90,14 +93,60 @@ getStringSetAttr(const StringMap & env, const StructuredAttrs * parsed, const st return ss ? (std::optional{StringSet{ss->begin(), ss->end()}}) : (std::optional{}); } -using OutputChecks = DerivationOptions::OutputChecks; +template +using OutputChecks = DerivationOptions::OutputChecks; -using OutputChecksVariant = std::variant>; +template +using OutputChecksVariant = std::variant, std::map>>; -DerivationOptions DerivationOptions::fromStructuredAttrs( - const StringMap & env, const std::optional & parsed, bool shouldWarn) +DerivationOptions derivationOptionsFromStructuredAttrs( + const StoreDirConfig & store, + const StringMap & env, + const StructuredAttrs * parsed, + bool shouldWarn, + const ExperimentalFeatureSettings & mockXpSettings) { - return fromStructuredAttrs(env, parsed ? &*parsed : nullptr); + /* Use the SingleDerivedPath version with empty inputDrvs, then + resolve. */ + DerivedPathMap emptyInputDrvs{}; + auto singleDerivedPathOptions = + derivationOptionsFromStructuredAttrs(store, emptyInputDrvs, env, parsed, shouldWarn, mockXpSettings); + + /* "Resolve" all SingleDerivedPath inputs to StorePath. */ + auto resolved = tryResolve( + singleDerivedPathOptions, + [&](ref drvPath, const std::string & outputName) -> std::optional { + // there should be nothing to resolve + assert(false); + }); + + /* Since we should never need to call the call back, there should be + no way it fails. */ + assert(resolved); + + return *resolved; +} + +DerivationOptions derivationOptionsFromStructuredAttrs( + const StoreDirConfig & store, + const StringMap & env, + const std::optional & parsed, + bool shouldWarn, + const ExperimentalFeatureSettings & mockXpSettings) +{ + return derivationOptionsFromStructuredAttrs(store, env, parsed ? &*parsed : nullptr, shouldWarn, mockXpSettings); +} + +DerivationOptions derivationOptionsFromStructuredAttrs( + const StoreDirConfig & store, + const DerivedPathMap & inputDrvs, + const StringMap & env, + const std::optional & parsed, + bool shouldWarn, + const ExperimentalFeatureSettings & mockXpSettings) +{ + return derivationOptionsFromStructuredAttrs( + store, inputDrvs, env, parsed ? &*parsed : nullptr, shouldWarn, mockXpSettings); } static void flatten(const nlohmann::json & value, StringSet & res) @@ -111,10 +160,63 @@ static void flatten(const nlohmann::json & value, StringSet & res) throw Error("'exportReferencesGraph' value is not an array or a string"); } -DerivationOptions -DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAttrs * parsed, bool shouldWarn) +DerivationOptions derivationOptionsFromStructuredAttrs( + const StoreDirConfig & store, + const DerivedPathMap & inputDrvs, + const StringMap & env, + const StructuredAttrs * parsed, + bool shouldWarn, + const ExperimentalFeatureSettings & mockXpSettings) { - DerivationOptions defaults = {}; + DerivationOptions defaults = {}; + + std::map placeholders; + if (mockXpSettings.isEnabled(Xp::CaDerivations)) { + /* Initialize placeholder map from inputDrvs */ + auto initPlaceholders = [&](this const auto & initPlaceholders, + ref basePath, + const DerivedPathMap::ChildNode & node) -> void { + for (const auto & outputName : node.value) { + auto built = SingleDerivedPath::Built{ + .drvPath = basePath, + .output = outputName, + }; + placeholders.insert_or_assign( + DownstreamPlaceholder::fromSingleDerivedPathBuilt(built, mockXpSettings).render(), + std::move(built)); + } + + for (const auto & [outputName, childNode] : node.childMap) { + initPlaceholders( + make_ref(SingleDerivedPath::Built{ + .drvPath = basePath, + .output = outputName, + }), + childNode); + } + }; + + for (const auto & [drvPath, outputs] : inputDrvs.map) { + auto basePath = make_ref(SingleDerivedPath::Opaque{drvPath}); + initPlaceholders(basePath, outputs); + } + } + + auto parseSingleDerivedPath = [&](const std::string & pathS) -> SingleDerivedPath { + if (auto it = placeholders.find(pathS); it != placeholders.end()) + return it->second; + else + return SingleDerivedPath::Opaque{store.toStorePath(pathS).first}; + }; + + auto parseRef = [&](const std::string & pathS) -> DrvRef { + if (auto it = placeholders.find(pathS); it != placeholders.end()) + return it->second; + if (store.isStorePath(pathS)) + return SingleDerivedPath::Opaque{store.toStorePath(pathS).first}; + else + return pathS; + }; if (shouldWarn && parsed) { auto & structuredAttrs = parsed->structuredAttrs; @@ -146,14 +248,14 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt } return { - .outputChecks = [&]() -> OutputChecksVariant { + .outputChecks = [&]() -> OutputChecksVariant { if (parsed) { auto & structuredAttrs = parsed->structuredAttrs; - std::map res; + std::map> res; if (auto * outputChecks = get(structuredAttrs, "outputChecks")) { for (auto & [outputName, output_] : getObject(*outputChecks)) { - OutputChecks checks; + OutputChecks checks; auto & output = getObject(output_); @@ -163,13 +265,14 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt if (auto maxClosureSize = get(output, "maxClosureSize")) checks.maxClosureSize = maxClosureSize->get(); - auto get_ = [&output = output](const std::string & name) -> std::optional { + auto get_ = + [&](const std::string & name) -> std::optional>> { if (auto i = get(output, name)) { - StringSet res; + std::set> res; for (auto j = i->begin(); j != i->end(); ++j) { if (!j->is_string()) throw Error("attribute '%s' must be a list of strings", name); - res.insert(j->get()); + res.insert(parseRef(j->get())); } return res; } @@ -178,7 +281,7 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt res.insert_or_assign( outputName, - OutputChecks{ + OutputChecks{ .maxSize = [&]() -> std::optional { if (auto maxSize = get(output, "maxSize")) return maxSize->get(); @@ -192,21 +295,32 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt return std::nullopt; }(), .allowedReferences = get_("allowedReferences"), - .disallowedReferences = get_("disallowedReferences").value_or(StringSet{}), + .disallowedReferences = + get_("disallowedReferences").value_or(std::set>{}), .allowedRequisites = get_("allowedRequisites"), - .disallowedRequisites = get_("disallowedRequisites").value_or(StringSet{}), + .disallowedRequisites = + get_("disallowedRequisites").value_or(std::set>{}), }); } } return res; } else { - return OutputChecks{ + auto parseRefSet = [&](const std::optional optionalStringSet) + -> std::optional>> { + if (!optionalStringSet) + return std::nullopt; + auto range = *optionalStringSet | std::views::transform(parseRef); + return std::set>(range.begin(), range.end()); + }; + return OutputChecks{ // legacy non-structured-attributes case .ignoreSelfRefs = true, - .allowedReferences = getStringSetAttr(env, parsed, "allowedReferences"), - .disallowedReferences = getStringSetAttr(env, parsed, "disallowedReferences").value_or(StringSet{}), - .allowedRequisites = getStringSetAttr(env, parsed, "allowedRequisites"), - .disallowedRequisites = getStringSetAttr(env, parsed, "disallowedRequisites").value_or(StringSet{}), + .allowedReferences = parseRefSet(getStringSetAttr(env, parsed, "allowedReferences")), + .disallowedReferences = parseRefSet(getStringSetAttr(env, parsed, "disallowedReferences")) + .value_or(std::set>{}), + .allowedRequisites = parseRefSet(getStringSetAttr(env, parsed, "allowedRequisites")), + .disallowedRequisites = parseRefSet(getStringSetAttr(env, parsed, "disallowedRequisites")) + .value_or(std::set>{}), }; } }(), @@ -245,16 +359,19 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt }(), .exportReferencesGraph = [&] { - std::map ret; + std::map> ret; if (parsed) { auto * e = optionalValueAt(parsed->structuredAttrs, "exportReferencesGraph"); if (!e || !e->is_object()) return ret; - for (auto & [key, value] : getObject(*e)) { + for (auto & [key, storePathsJson] : getObject(*e)) { StringSet ss; - flatten(value, ss); - ret.insert_or_assign(key, std::move(ss)); + flatten(storePathsJson, ss); + std::set storePaths; + for (auto & s : ss) + storePaths.insert(parseSingleDerivedPath(s)); + ret.insert_or_assign(key, std::move(storePaths)); } } else { auto s = getOr(env, "exportReferencesGraph", ""); @@ -268,7 +385,7 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt throw Error("invalid file name '%s' in 'exportReferencesGraph'", fileName); auto & storePathS = *i++; - ret.insert_or_assign(std::move(fileName), StringSet{storePathS}); + ret.insert_or_assign(std::move(fileName), std::set{parseSingleDerivedPath(storePathS)}); } } return ret; @@ -286,28 +403,8 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt }; } -std::map -DerivationOptions::getParsedExportReferencesGraph(const StoreDirConfig & store) const -{ - std::map res; - - for (auto & [fileName, ss] : exportReferencesGraph) { - StorePathSet storePaths; - for (auto & storePathS : ss) { - if (!store.isInStore(storePathS)) - throw BuildError( - BuildResult::Failure::InputRejected, - "'exportReferencesGraph' contains a non-store path '%1%'", - storePathS); - storePaths.insert(store.toStorePath(storePathS).first); - } - res.insert_or_assign(fileName, storePaths); - } - - return res; -} - -StringSet DerivationOptions::getRequiredSystemFeatures(const BasicDerivation & drv) const +template +StringSet DerivationOptions::getRequiredSystemFeatures(const BasicDerivation & drv) const { // FIXME: cache this? StringSet res; @@ -318,7 +415,8 @@ StringSet DerivationOptions::getRequiredSystemFeatures(const BasicDerivation & d return res; } -bool DerivationOptions::canBuildLocally(Store & localStore, const BasicDerivation & drv) const +template +bool DerivationOptions::canBuildLocally(Store & localStore, const BasicDerivation & drv) const { if (drv.platform != settings.thisSystem.get() && !settings.extraPlatforms.get().count(drv.platform) && !drv.isBuiltin()) @@ -334,42 +432,194 @@ bool DerivationOptions::canBuildLocally(Store & localStore, const BasicDerivatio return true; } -bool DerivationOptions::willBuildLocally(Store & localStore, const BasicDerivation & drv) const +template +bool DerivationOptions::willBuildLocally(Store & localStore, const BasicDerivation & drv) const { return preferLocalBuild && canBuildLocally(localStore, drv); } -bool DerivationOptions::substitutesAllowed() const +template +bool DerivationOptions::substitutesAllowed() const { return settings.alwaysAllowSubstitutes ? true : allowSubstitutes; } -bool DerivationOptions::useUidRange(const BasicDerivation & drv) const +template +bool DerivationOptions::useUidRange(const BasicDerivation & drv) const { return getRequiredSystemFeatures(drv).count("uid-range"); } +std::optional> tryResolve( + const DerivationOptions & drvOptions, + std::function(ref drvPath, const std::string & outputName)> + queryResolutionChain) +{ + auto tryResolvePath = [&](const SingleDerivedPath & input) -> std::optional { + return std::visit( + overloaded{ + [](const SingleDerivedPath::Opaque & p) -> std::optional { return p.path; }, + [&](const SingleDerivedPath::Built & p) -> std::optional { + return queryResolutionChain(p.drvPath, p.output); + }}, + input.raw()); + }; + + auto tryResolveRef = [&](const DrvRef & ref) -> std::optional> { + return std::visit( + overloaded{ + [](const OutputName & outputName) -> std::optional> { return outputName; }, + [&](const SingleDerivedPath & input) -> std::optional> { + return tryResolvePath(input); + }}, + ref); + }; + + auto tryResolveRefSet = + [&](const std::set> & refSet) -> std::optional>> { + std::set> resolvedSet; + for (const auto & ref : refSet) { + auto resolvedRef = tryResolveRef(ref); + if (!resolvedRef) + return std::nullopt; + resolvedSet.insert(*resolvedRef); + } + return resolvedSet; + }; + + // Helper function to try resolving OutputChecks using functional style + auto tryResolveOutputChecks = [&](const DerivationOptions::OutputChecks & checks) + -> std::optional::OutputChecks> { + std::optional>> resolvedAllowedReferences; + if (checks.allowedReferences) { + resolvedAllowedReferences = tryResolveRefSet(*checks.allowedReferences); + if (!resolvedAllowedReferences) + return std::nullopt; + } + + std::optional>> resolvedAllowedRequisites; + if (checks.allowedRequisites) { + resolvedAllowedRequisites = tryResolveRefSet(*checks.allowedRequisites); + if (!resolvedAllowedRequisites) + return std::nullopt; + } + + auto resolvedDisallowedReferences = tryResolveRefSet(checks.disallowedReferences); + if (!resolvedDisallowedReferences) + return std::nullopt; + + auto resolvedDisallowedRequisites = tryResolveRefSet(checks.disallowedRequisites); + if (!resolvedDisallowedRequisites) + return std::nullopt; + + return DerivationOptions::OutputChecks{ + .ignoreSelfRefs = checks.ignoreSelfRefs, + .maxSize = checks.maxSize, + .maxClosureSize = checks.maxClosureSize, + .allowedReferences = resolvedAllowedReferences, + .disallowedReferences = *resolvedDisallowedReferences, + .allowedRequisites = resolvedAllowedRequisites, + .disallowedRequisites = *resolvedDisallowedRequisites, + }; + }; + + // Helper function to resolve exportReferencesGraph using functional style + auto tryResolveExportReferencesGraph = [&](const std::map> & exportGraph) + -> std::optional>> { + std::map> resolved; + for (const auto & [name, inputPaths] : exportGraph) { + std::set resolvedPaths; + for (const auto & inputPath : inputPaths) { + auto resolvedPath = tryResolvePath(inputPath); + if (!resolvedPath) + return std::nullopt; + resolvedPaths.insert(*resolvedPath); + } + resolved.emplace(name, std::move(resolvedPaths)); + } + return resolved; + }; + + // Resolve outputChecks using functional style with std::visit + auto resolvedOutputChecks = std::visit( + overloaded{ + [&](const DerivationOptions::OutputChecks & checks) + -> std::optional::OutputChecks, + std::map::OutputChecks>>> { + auto resolved = tryResolveOutputChecks(checks); + if (!resolved) + return std::nullopt; + return std::variant< + DerivationOptions::OutputChecks, + std::map::OutputChecks>>(*resolved); + }, + [&](const std::map::OutputChecks> & checksMap) + -> std::optional::OutputChecks, + std::map::OutputChecks>>> { + std::map::OutputChecks> resolvedMap; + for (const auto & [outputName, checks] : checksMap) { + auto resolved = tryResolveOutputChecks(checks); + if (!resolved) + return std::nullopt; + resolvedMap.emplace(outputName, *resolved); + } + return std::variant< + DerivationOptions::OutputChecks, + std::map::OutputChecks>>(resolvedMap); + }}, + drvOptions.outputChecks); + + if (!resolvedOutputChecks) + return std::nullopt; + + // Resolve exportReferencesGraph + auto resolvedExportGraph = tryResolveExportReferencesGraph(drvOptions.exportReferencesGraph); + if (!resolvedExportGraph) + return std::nullopt; + + // Return resolved DerivationOptions using designated initializers + return DerivationOptions{ + .outputChecks = *resolvedOutputChecks, + .unsafeDiscardReferences = drvOptions.unsafeDiscardReferences, + .passAsFile = drvOptions.passAsFile, + .exportReferencesGraph = *resolvedExportGraph, + .additionalSandboxProfile = drvOptions.additionalSandboxProfile, + .noChroot = drvOptions.noChroot, + .impureHostDeps = drvOptions.impureHostDeps, + .impureEnvVars = drvOptions.impureEnvVars, + .allowLocalNetworking = drvOptions.allowLocalNetworking, + .requiredSystemFeatures = drvOptions.requiredSystemFeatures, + .preferLocalBuild = drvOptions.preferLocalBuild, + .allowSubstitutes = drvOptions.allowSubstitutes, + }; +} + +template struct DerivationOptions; +template struct DerivationOptions; + } // namespace nix namespace nlohmann { using namespace nix; -DerivationOptions adl_serializer::from_json(const json & json_) +DerivationOptions adl_serializer>::from_json(const json & json_) { auto & json = getObject(json_); return { - .outputChecks = [&]() -> OutputChecksVariant { + .outputChecks = [&]() -> OutputChecksVariant { auto outputChecks = getObject(valueAt(json, "outputChecks")); auto forAllOutputsOpt = optionalValueAt(outputChecks, "forAllOutputs"); auto perOutputOpt = optionalValueAt(outputChecks, "perOutput"); if (forAllOutputsOpt && !perOutputOpt) { - return static_cast(*forAllOutputsOpt); + return static_cast>(*forAllOutputsOpt); } else if (perOutputOpt && !forAllOutputsOpt) { - return static_cast>(*perOutputOpt); + return static_cast>>(*perOutputOpt); } else { throw Error("Exactly one of 'perOutput' or 'forAllOutputs' is required"); } @@ -377,7 +627,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), + .exportReferencesGraph = valueAt(json, "exportReferencesGraph"), .additionalSandboxProfile = getString(valueAt(json, "additionalSandboxProfile")), .noChroot = getBoolean(valueAt(json, "noChroot")), @@ -391,16 +641,17 @@ DerivationOptions adl_serializer::from_json(const json & json }; } -void adl_serializer::to_json(json & json, const DerivationOptions & o) +void adl_serializer>::to_json( + json & json, const DerivationOptions & o) { json["outputChecks"] = std::visit( overloaded{ - [&](const OutputChecks & checks) { + [&](const OutputChecks & checks) { nlohmann::json outputChecks; outputChecks["forAllOutputs"] = checks; return outputChecks; }, - [&](const std::map & checksPerOutput) { + [&](const std::map> & checksPerOutput) { nlohmann::json outputChecks; outputChecks["perOutput"] = checksPerOutput; return outputChecks; @@ -432,7 +683,7 @@ static inline std::optional ptrToOwned(const json * ptr) return std::nullopt; } -DerivationOptions::OutputChecks adl_serializer::from_json(const json & json_) +OutputChecks adl_serializer>::from_json(const json & json_) { auto & json = getObject(json_); @@ -440,14 +691,16 @@ DerivationOptions::OutputChecks adl_serializer: .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"))), - .disallowedRequisites = getStringSet(valueAt(json, "disallowedRequisites")), + .allowedReferences = + ptrToOwned>>(getNullable(valueAt(json, "allowedReferences"))), + .disallowedReferences = valueAt(json, "disallowedReferences"), + .allowedRequisites = + ptrToOwned>>(getNullable(valueAt(json, "allowedRequisites"))), + .disallowedRequisites = valueAt(json, "disallowedRequisites"), }; } -void adl_serializer::to_json(json & json, const DerivationOptions::OutputChecks & c) +void adl_serializer>::to_json(json & json, const OutputChecks & c) { json["ignoreSelfRefs"] = c.ignoreSelfRefs; json["maxSize"] = c.maxSize; diff --git a/src/libstore/downstream-placeholder.cc b/src/libstore/downstream-placeholder.cc index 780717a62..73ed2b74a 100644 --- a/src/libstore/downstream-placeholder.cc +++ b/src/libstore/downstream-placeholder.cc @@ -1,5 +1,6 @@ #include "nix/store/downstream-placeholder.hh" #include "nix/store/derivations.hh" +#include "nix/util/json-utils.hh" namespace nix { @@ -49,3 +50,45 @@ DownstreamPlaceholder DownstreamPlaceholder::fromSingleDerivedPathBuilt( } } // namespace nix + +namespace nlohmann { + +using namespace nix; + +template +DrvRef adl_serializer>::from_json(const json & json) +{ + // OutputName case: { "drvPath": "self", "output": } + if (json.type() == nlohmann::json::value_t::object) { + auto & obj = getObject(json); + if (auto * drvPath_ = get(obj, "drvPath")) { + auto & drvPath = *drvPath_; + if (drvPath.type() == nlohmann::json::value_t::string && getString(drvPath) == "self") { + return getString(valueAt(obj, "output")); + } + } + } + + // Input case + return adl_serializer::from_json(json); +} + +template +void adl_serializer>::to_json(json & json, const DrvRef & ref) +{ + std::visit( + overloaded{ + [&](const OutputName & outputName) { + json = nlohmann::json::object(); + json["drvPath"] = "self"; + json["output"] = outputName; + }, + [&](const Item & item) { json = item; }, + }, + ref); +} + +template struct adl_serializer>; +template struct adl_serializer>; + +} // namespace nlohmann diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index 5eed38462..af84661e2 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -69,7 +69,7 @@ struct DerivationBuilderParams * * @todo this should be part of `Derivation`. */ - const DerivationOptions & drvOptions; + const DerivationOptions & drvOptions; // The remainder is state held during the build. diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index 547e533e2..8277d6b57 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -52,7 +52,7 @@ private: */ std::unique_ptr drv; - std::unique_ptr drvOptions; + std::unique_ptr> drvOptions; /** * The remainder is state held during the build. diff --git a/src/libstore/include/nix/store/build/derivation-env-desugar.hh b/src/libstore/include/nix/store/build/derivation-env-desugar.hh index 6e2efa6bb..a10ec9fa8 100644 --- a/src/libstore/include/nix/store/build/derivation-env-desugar.hh +++ b/src/libstore/include/nix/store/build/derivation-env-desugar.hh @@ -8,6 +8,7 @@ namespace nix { class Store; struct Derivation; +template struct DerivationOptions; /** @@ -77,7 +78,10 @@ struct DesugaredEnv * just part of `Derivation`. */ static DesugaredEnv create( - Store & store, const Derivation & drv, const DerivationOptions & drvOptions, const StorePathSet & inputPaths); + Store & store, + const Derivation & drv, + const DerivationOptions & drvOptions, + const StorePathSet & inputPaths); }; } // namespace nix diff --git a/src/libstore/include/nix/store/derivation-options.hh b/src/libstore/include/nix/store/derivation-options.hh index 88694f730..0b79af85d 100644 --- a/src/libstore/include/nix/store/derivation-options.hh +++ b/src/libstore/include/nix/store/derivation-options.hh @@ -8,7 +8,8 @@ #include "nix/util/types.hh" #include "nix/util/json-impls.hh" -#include "nix/store/path.hh" +#include "nix/store/store-dir-config.hh" +#include "nix/store/downstream-placeholder.hh" namespace nix { @@ -17,6 +18,9 @@ struct StoreDirConfig; struct BasicDerivation; struct StructuredAttrs; +template +struct DerivedPathMap; + /** * This represents all the special options on a `Derivation`. * @@ -34,6 +38,7 @@ struct StructuredAttrs; * separately. That would be nice to separate concerns, and not make any * environment variable names magical. */ +template struct DerivationOptions { struct OutputChecks @@ -41,13 +46,15 @@ struct DerivationOptions bool ignoreSelfRefs = false; std::optional maxSize, maxClosureSize; + using DrvRef = nix::DrvRef; + /** * env: allowedReferences * * A value of `nullopt` indicates that the check is skipped. * This means that all references are allowed. */ - std::optional allowedReferences; + std::optional> allowedReferences; /** * env: disallowedReferences @@ -55,21 +62,21 @@ struct DerivationOptions * No needed for `std::optional`, because skipping the check is * the same as disallowing the references. */ - StringSet disallowedReferences; + std::set disallowedReferences; /** * env: allowedRequisites * * See `allowedReferences` */ - std::optional allowedRequisites; + std::optional> allowedRequisites; /** * env: disallowedRequisites * * See `disallowedReferences` */ - StringSet disallowedRequisites; + std::set disallowedRequisites; bool operator==(const OutputChecks &) const = default; }; @@ -116,23 +123,7 @@ struct DerivationOptions * attributes give to the builder. The set of paths in the original JSON * is replaced with a list of `PathInfo` in JSON format. */ - std::map exportReferencesGraph; - - /** - * Once a derivations is resolved, the strings in in - * `exportReferencesGraph` should all be store paths (with possible - * suffix paths, but those are discarded). - * - * @return The parsed path set for for each key in the map. - * - * @todo Ideally, `exportReferencesGraph` would just store - * `StorePath`s for this, but we can't just do that, because for CA - * derivations they is actually in general `DerivedPath`s (via - * placeholder strings) until the derivation is resolved and exact - * inputs store paths are known. We can use better types for that - * too, but that is a longer project. - */ - std::map getParsedExportReferencesGraph(const StoreDirConfig & store) const; + std::map> exportReferencesGraph; /** * env: __sandboxProfile @@ -185,18 +176,6 @@ struct DerivationOptions bool operator==(const DerivationOptions &) const = default; - /** - * Parse this information from its legacy encoding as part of the - * environment. This should not be used with nice greenfield formats - * (e.g. JSON) but is necessary for supporting old formats (e.g. - * ATerm). - */ - static DerivationOptions - fromStructuredAttrs(const StringMap & env, const StructuredAttrs * parsed, bool shouldWarn = true); - - static DerivationOptions - fromStructuredAttrs(const StringMap & env, const std::optional & parsed, bool shouldWarn = true); - /** * @param drv Must be the same derivation we parsed this from. In * the future we'll flip things around so a `BasicDerivation` has @@ -222,7 +201,55 @@ struct DerivationOptions bool useUidRange(const BasicDerivation & drv) const; }; +extern template struct DerivationOptions; +extern template struct DerivationOptions; + +struct DerivationOutput; + +/** + * Parse this information from its legacy encoding as part of the + * environment. This should not be used with nice greenfield formats + * (e.g. JSON) but is necessary for supporting old formats (e.g. + * ATerm). + */ +DerivationOptions derivationOptionsFromStructuredAttrs( + const StoreDirConfig & store, + const DerivedPathMap & inputDrvs, + const StringMap & env, + const StructuredAttrs * parsed, + bool shouldWarn = true, + const ExperimentalFeatureSettings & mockXpSettings = experimentalFeatureSettings); + +DerivationOptions derivationOptionsFromStructuredAttrs( + const StoreDirConfig & store, + const DerivedPathMap & inputDrvs, + const StringMap & env, + const std::optional & parsed, + bool shouldWarn = true, + const ExperimentalFeatureSettings & mockXpSettings = experimentalFeatureSettings); + +DerivationOptions derivationOptionsFromStructuredAttrs( + const StoreDirConfig & store, + const StringMap & env, + const StructuredAttrs * parsed, + bool shouldWarn = true, + const ExperimentalFeatureSettings & mockXpSettings = experimentalFeatureSettings); + +DerivationOptions derivationOptionsFromStructuredAttrs( + const StoreDirConfig & store, + const StringMap & env, + const std::optional & parsed, + bool shouldWarn = true, + const ExperimentalFeatureSettings & mockXpSettings = experimentalFeatureSettings); + +std::optional> tryResolve( + const DerivationOptions & drvOptions, + std::function(ref drvPath, const std::string & outputName)> + queryResolutionChain); + }; // namespace nix -JSON_IMPL(DerivationOptions); -JSON_IMPL(DerivationOptions::OutputChecks) +JSON_IMPL(nix::DerivationOptions); +JSON_IMPL(nix::DerivationOptions); +JSON_IMPL(nix::DerivationOptions::OutputChecks) +JSON_IMPL(nix::DerivationOptions::OutputChecks) diff --git a/src/libstore/include/nix/store/downstream-placeholder.hh b/src/libstore/include/nix/store/downstream-placeholder.hh index ee4d9e3c2..ba3e9faef 100644 --- a/src/libstore/include/nix/store/downstream-placeholder.hh +++ b/src/libstore/include/nix/store/downstream-placeholder.hh @@ -2,11 +2,23 @@ ///@file #include "nix/util/hash.hh" +#include "nix/util/json-impls.hh" #include "nix/store/path.hh" #include "nix/store/derived-path.hh" namespace nix { +/** + * A reference is either to a to-be-registered output (by name), + * or to an already-registered store object (by `Input`). + * + * `Ref +using DrvRef = std::variant; + /** * Downstream Placeholders are opaque and almost certainly unique values * used to allow derivations to refer to store objects which are yet to @@ -92,3 +104,17 @@ public: }; } // namespace nix + +namespace nlohmann { + +template +struct adl_serializer> +{ + static nix::DrvRef from_json(const json & json); + static void to_json(json & json, const nix::DrvRef & t); +}; + +extern template struct adl_serializer>; +extern template struct adl_serializer>; + +} // namespace nlohmann diff --git a/src/libstore/include/nix/store/parsed-derivations.hh b/src/libstore/include/nix/store/parsed-derivations.hh index 52e97b0e7..098591310 100644 --- a/src/libstore/include/nix/store/parsed-derivations.hh +++ b/src/libstore/include/nix/store/parsed-derivations.hh @@ -9,6 +9,7 @@ namespace nix { class Store; +template struct DerivationOptions; struct DerivationOutput; @@ -47,7 +48,7 @@ struct StructuredAttrs nlohmann::json::object_t prepareStructuredAttrs( Store & store, - const DerivationOptions & drvOptions, + const DerivationOptions & drvOptions, const StorePathSet & inputPaths, const DerivationOutputs & outputs) const; diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 8b2a7287e..642ac0978 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -224,11 +224,12 @@ MissingPaths Store::queryMissing(const std::vector & targets) return; auto drv = make_ref(derivationFromPath(drvPath)); - DerivationOptions drvOptions; + DerivationOptions drvOptions; try { // FIXME: this is a lot of work just to get the value // of `allowSubstitutes`. - drvOptions = DerivationOptions::fromStructuredAttrs(drv->env, drv->structuredAttrs); + drvOptions = + derivationOptionsFromStructuredAttrs(*this, drv->inputDrvs, drv->env, drv->structuredAttrs); } catch (Error & e) { e.addTrace({}, "while parsing derivation '%s'", printStorePath(drvPath)); throw; diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index 8d147f65f..95434bd20 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -100,7 +100,7 @@ static nlohmann::json pathInfoToJSON(Store & store, const StorePathSet & storePa nlohmann::json::object_t StructuredAttrs::prepareStructuredAttrs( Store & store, - const DerivationOptions & drvOptions, + const DerivationOptions & drvOptions, const StorePathSet & inputPaths, const DerivationOutputs & outputs) const { @@ -114,8 +114,8 @@ nlohmann::json::object_t StructuredAttrs::prepareStructuredAttrs( json["outputs"] = std::move(outputsJson); /* Handle exportReferencesGraph. */ - for (auto & [key, storePaths] : drvOptions.getParsedExportReferencesGraph(store)) { - json[key] = pathInfoToJSON(store, store.exportReferences(storePaths, storePaths)); + for (auto & [key, storePaths] : drvOptions.exportReferencesGraph) { + json[key] = pathInfoToJSON(store, store.exportReferences(storePaths, inputPaths)); } return json; diff --git a/src/nix/nix-build/nix-build.cc b/src/nix/nix-build/nix-build.cc index 4d876c9eb..00ef32dfa 100644 --- a/src/nix/nix-build/nix-build.cc +++ b/src/nix/nix-build/nix-build.cc @@ -554,9 +554,9 @@ static void main_nix_build(int argc, char ** argv) env["NIX_STORE"] = store->storeDir; env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores ? settings.buildCores : settings.getDefaultCores()); - DerivationOptions drvOptions; + DerivationOptions drvOptions; try { - drvOptions = DerivationOptions::fromStructuredAttrs(drv.env, drv.structuredAttrs); + drvOptions = derivationOptionsFromStructuredAttrs(*store, drv.env, drv.structuredAttrs); } catch (Error & e) { e.addTrace({}, "while parsing derivation '%s'", store->printStorePath(packageInfo.requireDrvPath())); throw; diff --git a/tests/functional/check-refs.sh b/tests/functional/check-refs.sh index 590c3fb53..8eb6aaf68 100755 --- a/tests/functional/check-refs.sh +++ b/tests/functional/check-refs.sh @@ -64,5 +64,5 @@ fi if isDaemonNewer "2.28pre20241225"; then # test12 should fail (syntactically invalid). expectStderr 1 nix-build -vvv -o "$RESULT" check-refs.nix -A test12 >"$TEST_ROOT/test12.stderr" - grepQuiet -F "output check for 'lib' contains an illegal reference specifier 'dev', expected store path or output name (one of [lib, out])" < "$TEST_ROOT/test12.stderr" + grepQuiet -F "output check for 'lib' contains output name 'dev', but this is not a valid output of this derivation. (Valid outputs are [lib, out].)" < "$TEST_ROOT/test12.stderr" fi From a9fdd537acfd8a04d56176f47eaa8c2f6acc526b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 21 May 2024 17:07:23 -0400 Subject: [PATCH 3/3] Store `DerivationOption` instead `Derivation`, put in JSON format This opens the door to derivations that *directly* specify their options, rather than "stealing" environment variables to do so. The A-Term derivation format used on disk didn't change --- we cannot do that for existing derivations, so those derivations will continue to not support separate options. But having a more flexible JSON format opens the door to extending the on-disk format in others, like directly using JSON, using CBOR, etc. Co-authored-by: HaeNoe Co-authored-by: Jonathan Gibbons --- src/libexpr/primops.cc | 2 + .../ca/advanced-attributes-defaults.json | 24 ++++ ...-attributes-structured-attrs-defaults.json | 16 +++ .../advanced-attributes-structured-attrs.json | 66 +++++++++++ .../derivation/ca/advanced-attributes.json | 46 ++++++++ .../data/derivation/dyn-dep-derivation.json | 24 ++++ .../ia/advanced-attributes-defaults.json | 24 ++++ ...-attributes-structured-attrs-defaults.json | 16 +++ .../advanced-attributes-structured-attrs.json | 66 +++++++++++ .../derivation/ia/advanced-attributes.json | 46 ++++++++ .../data/derivation/simple-derivation.json | 24 ++++ .../derivation-advanced-attrs.cc | 105 ++++++++++++------ src/libstore-tests/derivation.cc | 4 + src/libstore-tests/write-derivation.cc | 2 + .../build/derivation-building-goal.cc | 19 +--- src/libstore/build/derivation-goal.cc | 13 +-- .../build/derivation-resolution-goal.cc | 7 +- src/libstore/derivation-options.cc | 12 +- src/libstore/derivations.cc | 29 ++++- .../nix/store/build/derivation-builder.hh | 3 +- .../nix/store/build/derivation-goal.hh | 1 - .../store/build/derivation-resolution-goal.hh | 2 +- .../include/nix/store/derivation-options.hh | 7 ++ src/libstore/include/nix/store/derivations.hh | 16 ++- src/libstore/misc.cc | 14 +-- src/libstore/store-api.cc | 6 +- src/nix/nix-build/nix-build.cc | 18 +-- .../lang/eval-okay-derivation-legacy.err.exp | 6 + 28 files changed, 514 insertions(+), 104 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index d1aae64fa..fac86f0ad 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1605,6 +1605,8 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName drv.structuredAttrs = std::move(*jsonObject); } + drv.options = derivationOptionsFromStructuredAttrs(*state.store, drv.inputDrvs, drv.env, drv.structuredAttrs); + /* Everything in the context of the strings in the derivation attributes should be added as dependencies of the resulting derivation. */ diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes-defaults.json b/src/libstore-tests/data/derivation/ca/advanced-attributes-defaults.json index 781b4cb14..2f041064a 100644 --- a/src/libstore-tests/data/derivation/ca/advanced-attributes-defaults.json +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes-defaults.json @@ -17,6 +17,30 @@ "srcs": [] }, "name": "advanced-attributes-defaults", + "options": { + "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": {} + }, "outputs": { "out": { "hashAlgo": "sha256", diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults.json b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults.json index 7437b51ef..4bb499d50 100644 --- a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults.json +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults.json @@ -13,6 +13,22 @@ "srcs": [] }, "name": "advanced-attributes-structured-attrs-defaults", + "options": { + "additionalSandboxProfile": "", + "allowLocalNetworking": false, + "allowSubstitutes": true, + "exportReferencesGraph": {}, + "impureEnvVars": [], + "impureHostDeps": [], + "noChroot": false, + "outputChecks": { + "perOutput": {} + }, + "passAsFile": [], + "preferLocalBuild": false, + "requiredSystemFeatures": [], + "unsafeDiscardReferences": {} + }, "outputs": { "dev": { "hashAlgo": "sha256", diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json index 95122ad41..0b1992c88 100644 --- a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json @@ -31,6 +31,72 @@ ] }, "name": "advanced-attributes-structured-attrs", + "options": { + "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": {} + }, "outputs": { "bin": { "hashAlgo": "sha256", diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes.json b/src/libstore-tests/data/derivation/ca/advanced-attributes.json index 6b77459bc..9c32e9625 100644 --- a/src/libstore-tests/data/derivation/ca/advanced-attributes.json +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes.json @@ -47,6 +47,52 @@ ] }, "name": "advanced-attributes", + "options": { + "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": {} + }, "outputs": { "out": { "hashAlgo": "sha256", diff --git a/src/libstore-tests/data/derivation/dyn-dep-derivation.json b/src/libstore-tests/data/derivation/dyn-dep-derivation.json index 1793c5f2d..3c30cef7e 100644 --- a/src/libstore-tests/data/derivation/dyn-dep-derivation.json +++ b/src/libstore-tests/data/derivation/dyn-dep-derivation.json @@ -35,6 +35,30 @@ ] }, "name": "dyn-dep-derivation", + "options": { + "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": {} + }, "outputs": {}, "system": "wasm-sel4", "version": 4 diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes-defaults.json b/src/libstore-tests/data/derivation/ia/advanced-attributes-defaults.json index 898762123..c7e3d6cd6 100644 --- a/src/libstore-tests/data/derivation/ia/advanced-attributes-defaults.json +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes-defaults.json @@ -15,6 +15,30 @@ "srcs": [] }, "name": "advanced-attributes-defaults", + "options": { + "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": {} + }, "outputs": { "out": { "path": "1qsc7svv43m4dw2prh6mvyf7cai5czji-advanced-attributes-defaults" diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults.json b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults.json index c51095986..b22235b8b 100644 --- a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults.json +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults.json @@ -13,6 +13,22 @@ "srcs": [] }, "name": "advanced-attributes-structured-attrs-defaults", + "options": { + "additionalSandboxProfile": "", + "allowLocalNetworking": false, + "allowSubstitutes": true, + "exportReferencesGraph": {}, + "impureEnvVars": [], + "impureHostDeps": [], + "noChroot": false, + "outputChecks": { + "perOutput": {} + }, + "passAsFile": [], + "preferLocalBuild": false, + "requiredSystemFeatures": [], + "unsafeDiscardReferences": {} + }, "outputs": { "dev": { "path": "8bazivnbipbyi569623skw5zm91z6kc2-advanced-attributes-structured-attrs-defaults-dev" diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json index bbd68e087..7af3ad069 100644 --- a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json @@ -31,6 +31,72 @@ ] }, "name": "advanced-attributes-structured-attrs", + "options": { + "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": {} + }, "outputs": { "bin": { "path": "cnpasdljgkhnwaf78cf3qygcp4qbki1c-advanced-attributes-structured-attrs-bin" diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes.json b/src/libstore-tests/data/derivation/ia/advanced-attributes.json index e2de9431b..52919d4d9 100644 --- a/src/libstore-tests/data/derivation/ia/advanced-attributes.json +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes.json @@ -45,6 +45,52 @@ ] }, "name": "advanced-attributes", + "options": { + "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": {} + }, "outputs": { "out": { "path": "ymqmybkq5j4nd1xplw6ccdpbjnfi017v-advanced-attributes" diff --git a/src/libstore-tests/data/derivation/simple-derivation.json b/src/libstore-tests/data/derivation/simple-derivation.json index 04129a096..97b42e55a 100644 --- a/src/libstore-tests/data/derivation/simple-derivation.json +++ b/src/libstore-tests/data/derivation/simple-derivation.json @@ -22,6 +22,30 @@ ] }, "name": "simple-derivation", + "options": { + "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": {} + }, "outputs": {}, "system": "wasm-sel4", "version": 4 diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc index 27b8aba16..447212c29 100644 --- a/src/libstore-tests/derivation-advanced-attrs.cc +++ b/src/libstore-tests/derivation-advanced-attrs.cc @@ -187,17 +187,14 @@ 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 options = derivationOptionsFromStructuredAttrs( - *this->store, got.inputDrvs, got.env, got.structuredAttrs, true, this->mockXpSettings); - EXPECT_TRUE(!got.structuredAttrs); - EXPECT_EQ(options, advancedAttributes_defaults); + EXPECT_EQ(got.options, advancedAttributes_defaults); - EXPECT_EQ(options.canBuildLocally(*this->store, got), false); - EXPECT_EQ(options.willBuildLocally(*this->store, got), false); - EXPECT_EQ(options.substitutesAllowed(), true); - EXPECT_EQ(options.useUidRange(got), false); + EXPECT_EQ(got.options.canBuildLocally(*this->store, got), false); + EXPECT_EQ(got.options.willBuildLocally(*this->store, got), false); + EXPECT_EQ(got.options.substitutesAllowed(), true); + EXPECT_EQ(got.options.useUidRange(got), false); }); }; @@ -233,19 +230,16 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes) this->readTest("advanced-attributes.drv", [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - auto options = derivationOptionsFromStructuredAttrs( - *this->store, got.inputDrvs, got.env, got.structuredAttrs, true, this->mockXpSettings); - EXPECT_TRUE(!got.structuredAttrs); // Reset fields that vary between test cases to enable whole-object comparison - options.outputChecks = DerivationOptions::OutputChecks{.ignoreSelfRefs = true}; - options.exportReferencesGraph = {}; + got.options.outputChecks = DerivationOptions::OutputChecks{.ignoreSelfRefs = true}; + got.options.exportReferencesGraph = {}; - EXPECT_EQ(options, expected); + EXPECT_EQ(got.options, expected); - EXPECT_EQ(options.substitutesAllowed(), false); - EXPECT_EQ(options.useUidRange(got), true); + EXPECT_EQ(got.options.substitutesAllowed(), false); + EXPECT_EQ(got.options.useUidRange(got), true); }); }; @@ -334,12 +328,12 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs_d EXPECT_TRUE(got.structuredAttrs); - EXPECT_EQ(options, advancedAttributes_structuredAttrs_defaults); + EXPECT_EQ(got.options, advancedAttributes_structuredAttrs_defaults); - EXPECT_EQ(options.canBuildLocally(*this->store, got), false); - EXPECT_EQ(options.willBuildLocally(*this->store, got), false); - EXPECT_EQ(options.substitutesAllowed(), true); - EXPECT_EQ(options.useUidRange(got), false); + EXPECT_EQ(got.options.canBuildLocally(*this->store, got), false); + EXPECT_EQ(got.options.willBuildLocally(*this->store, got), false); + EXPECT_EQ(got.options.substitutesAllowed(), true); + EXPECT_EQ(got.options.useUidRange(got), false); }); }; @@ -380,9 +374,6 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) this->readTest("advanced-attributes-structured-attrs.drv", [&](auto encoded) { auto got = parseDerivation(*this->store, std::move(encoded), "foo", this->mockXpSettings); - auto options = derivationOptionsFromStructuredAttrs( - *this->store, got.inputDrvs, got.env, got.structuredAttrs, true, this->mockXpSettings); - EXPECT_TRUE(got.structuredAttrs); // Reset fields that vary between test cases to enable whole-object comparison @@ -390,7 +381,7 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) // Delete all keys but "dev" in options.outputChecks auto * outputChecksMapP = std::get_if::OutputChecks>>( - &options.outputChecks); + &got.options.outputChecks); ASSERT_TRUE(outputChecksMapP); auto & outputChecksMap = *outputChecksMapP; auto devEntry = outputChecksMap.find("dev"); @@ -399,14 +390,14 @@ TYPED_TEST(DerivationAdvancedAttrsBothTest, advancedAttributes_structuredAttrs) outputChecksMap.clear(); outputChecksMap.emplace("dev", std::move(devChecks)); } - options.exportReferencesGraph = {}; + got.options.exportReferencesGraph = {}; - EXPECT_EQ(options, expected); + EXPECT_EQ(got.options, expected); - EXPECT_EQ(options.canBuildLocally(*this->store, got), false); - EXPECT_EQ(options.willBuildLocally(*this->store, got), false); - EXPECT_EQ(options.substitutesAllowed(), false); - EXPECT_EQ(options.useUidRange(got), true); + EXPECT_EQ(got.options.canBuildLocally(*this->store, got), false); + EXPECT_EQ(got.options.willBuildLocally(*this->store, got), false); + EXPECT_EQ(got.options.substitutesAllowed(), false); + EXPECT_EQ(got.options.useUidRange(got), true); }); }; @@ -517,4 +508,56 @@ TEST_JSON_OPTIONS(CaDerivationAdvancedAttrsTest, structuredAttrs_all_set, struct #undef TEST_JSON_OPTIONS +#define SYNC_CONFLICT(NAME, VALUE) \ + NAME = VALUE; \ + EXPECT_THROW(got.unparse(*store, false), Error); \ + got.options = options; + +TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_option_syncConflict) +{ + readTest("advanced-attributes-defaults.drv", [&](auto encoded) { + auto got = parseDerivation(*store, std::move(encoded), "foo"); + auto options = got.options; + + SYNC_CONFLICT(got.options.additionalSandboxProfile, "foobar"); + SYNC_CONFLICT(got.options.noChroot, true); + SYNC_CONFLICT(got.options.impureHostDeps, StringSet{"/usr/bin/ditto"}); + SYNC_CONFLICT(got.options.impureEnvVars, StringSet{"HELLO"}); + SYNC_CONFLICT(got.options.allowLocalNetworking, true); + SYNC_CONFLICT(std::get<0>(got.options.outputChecks).allowedReferences, StringSet{"nothing"}); + SYNC_CONFLICT(std::get<0>(got.options.outputChecks).allowedRequisites, StringSet{"hey"}); + SYNC_CONFLICT(std::get<0>(got.options.outputChecks).disallowedReferences, StringSet{"BAR"}); + SYNC_CONFLICT(std::get<0>(got.options.outputChecks).disallowedRequisites, StringSet{"FOO"}); + }); +}; + +#undef SYNC_CONFLICT + +#define SYNC_CONFLICT(NAME, VALUE) \ + got.env[NAME] = VALUE; \ + EXPECT_THROW(got.unparse(*store, false), Error); \ + got.env = env; + +TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_env_syncConflict) +{ + readTest("advanced-attributes-defaults.drv", [&](auto encoded) { + auto got = parseDerivation(*store, std::move(encoded), "foo"); + auto env = got.env; + + // TODO: Is there any way to serialize a boolean/StringSet into an env value (string)? + // Something like `State::coerceToString` + SYNC_CONFLICT("__sandboxProfile", "foobar"); + SYNC_CONFLICT("__noChroot", "1"); + SYNC_CONFLICT("__impureHostDeps", "/usr/bin/ditto"); + SYNC_CONFLICT("impureEnvVars", "FOOBAR"); + SYNC_CONFLICT("__darwinAllowLocalNetworking", "1"); + SYNC_CONFLICT("allowedReferences", "nothing"); + SYNC_CONFLICT("allowedRequisites", "hey"); + SYNC_CONFLICT("disallowedReferences", "BAR"); + SYNC_CONFLICT("disallowedRequisites", "FOO"); + }); +}; + +#undef SYNC_CONFLICT + } // namespace nix diff --git a/src/libstore-tests/derivation.cc b/src/libstore-tests/derivation.cc index 6b33e5442..e3b8fd9e8 100644 --- a/src/libstore-tests/derivation.cc +++ b/src/libstore-tests/derivation.cc @@ -254,6 +254,8 @@ Derivation makeSimpleDrv() "WOLF", }, }; + drv.options = + derivationOptionsFromStructuredAttrs(StoreDirConfig{"/nix/store"}, drv.inputDrvs, drv.env, drv.structuredAttrs); return drv; } @@ -321,6 +323,8 @@ Derivation makeDynDepDerivation() "WOLF", }, }; + drv.options = + derivationOptionsFromStructuredAttrs(StoreDirConfig{"/nix/store"}, drv.inputDrvs, drv.env, drv.structuredAttrs); return drv; } diff --git a/src/libstore-tests/write-derivation.cc b/src/libstore-tests/write-derivation.cc index c320f92fa..154d79da0 100644 --- a/src/libstore-tests/write-derivation.cc +++ b/src/libstore-tests/write-derivation.cc @@ -43,6 +43,8 @@ static Derivation makeSimpleDrv() TEST_F(WriteDerivationTest, addToStoreFromDumpCalledOnce) { auto drv = makeSimpleDrv(); + drv.options = + derivationOptionsFromStructuredAttrs(StoreDirConfig{"/nix/store"}, drv.inputDrvs, drv.env, drv.structuredAttrs); auto path1 = writeDerivation(*store, drv, NoRepair); config->readOnly = true; diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 41d76fbba..7004cb85e 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -32,16 +32,9 @@ DerivationBuildingGoal::DerivationBuildingGoal( , drv{std::make_unique(drv)} , buildMode(buildMode) { - DerivationOptions temp; - try { - temp = derivationOptionsFromStructuredAttrs(worker.store, drv.inputDrvs, drv.env, drv.structuredAttrs); - } catch (Error & e) { - e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath)); - throw; - } - auto x = tryResolve( - temp, [&](ref drvPath, const std::string & outputName) -> std::optional { + drv.options, + [&](ref drvPath, const std::string & outputName) -> std::optional { try { return resolveDerivedPath( worker.store, SingleDerivedPath::Built{drvPath, outputName}, &worker.evalStore); @@ -357,7 +350,7 @@ 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 || drv->options.willBuildLocally(worker.store, *drv)) && settings.maxBuildJobs.get() != 0; if (buildLocally) { @@ -392,7 +385,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild() externalBuilder = settings.findExternalDerivationBuilderIfSupported(*drv); - if (!externalBuilder && !drvOptions->canBuildLocally(worker.store, *drv)) { + if (!externalBuilder && !drv->options.canBuildLocally(worker.store, *drv)) { auto msg = fmt("Cannot build '%s'.\n" "Reason: " ANSI_RED "required system or feature not available" ANSI_NORMAL @@ -401,7 +394,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild() "Current system: '%s' with features {%s}", Magenta(worker.store.printStorePath(drvPath)), Magenta(drv->platform), - concatStringsSep(", ", drvOptions->getRequiredSystemFeatures(*drv)), + concatStringsSep(", ", drv->options.getRequiredSystemFeatures(*drv)), Magenta(settings.thisSystem), concatStringsSep(", ", worker.store.Store::config.systemFeatures)); @@ -833,7 +826,7 @@ HookReply DerivationBuildingGoal::tryBuildHook(const std::mapsink << "try" << (worker.getNrLocalBuilds() < settings.maxBuildJobs ? 1 : 0) << drv->platform - << worker.store.printStorePath(drvPath) << drvOptions->getRequiredSystemFeatures(*drv); + << worker.store.printStorePath(drvPath) << drv->options.getRequiredSystemFeatures(*drv); worker.hook->sink.flush(); /* Read the first line of input, which should be a word indicating diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index a44b79f57..4235c8806 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -64,15 +64,6 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) { trace("have derivation"); - auto drvOptions = [&]() -> DerivationOptions { - try { - return derivationOptionsFromStructuredAttrs(worker.store, drv->inputDrvs, drv->env, drv->structuredAttrs); - } catch (Error & e) { - e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath)); - throw; - } - }(); - if (!drv->type().hasKnownOutputPaths()) experimentalFeatureSettings.require(Xp::CaDerivations); @@ -98,7 +89,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) /* 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 && drvOptions.substitutesAllowed()) { + if (settings.useSubstitutes && drv->options.substitutesAllowed()) { if (!checkResult) waitees.insert(upcast_goal(worker.makeDrvOutputSubstitutionGoal(DrvOutput{outputHash, wantedOutput}))); else { @@ -151,7 +142,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) } if (resolutionGoal->resolvedDrv) { - auto & [pathResolved, drvResolved] = *resolutionGoal->resolvedDrv; + auto & [pathResolved, drvResolved, drvOptionsResolved] = *resolutionGoal->resolvedDrv; auto resolvedDrvGoal = worker.makeDerivationGoal(pathResolved, drvResolved, wantedOutput, buildMode, /*storeDerivation=*/true); diff --git a/src/libstore/build/derivation-resolution-goal.cc b/src/libstore/build/derivation-resolution-goal.cc index 6cb9702f4..4b5d5b30c 100644 --- a/src/libstore/build/derivation-resolution-goal.cc +++ b/src/libstore/build/derivation-resolution-goal.cc @@ -164,7 +164,8 @@ Goal::Co DerivationResolutionGoal::resolveDerivation() } assert(attempt); - auto pathResolved = writeDerivation(worker.store, *attempt, NoRepair, /*readOnly =*/true); + // TODO check options compatibility with ATerm. + auto pathResolved = writeDerivation(worker.store, attempt->first, NoRepair, /*readOnly =*/true); auto msg = fmt("resolved derivation: '%s' -> '%s'", @@ -180,8 +181,8 @@ Goal::Co DerivationResolutionGoal::resolveDerivation() worker.store.printStorePath(pathResolved), }); - resolvedDrv = - std::make_unique>(std::move(pathResolved), *std::move(attempt)); + resolvedDrv = std::make_unique>>( + std::move(pathResolved), std::move(attempt->first), std::move(attempt->second)); } } diff --git a/src/libstore/derivation-options.cc b/src/libstore/derivation-options.cc index 8c71ef4b1..639bb9ea9 100644 --- a/src/libstore/derivation-options.cc +++ b/src/libstore/derivation-options.cc @@ -250,21 +250,11 @@ DerivationOptions derivationOptionsFromStructuredAttrs( return { .outputChecks = [&]() -> OutputChecksVariant { if (parsed) { - auto & structuredAttrs = parsed->structuredAttrs; - std::map> res; - if (auto * outputChecks = get(structuredAttrs, "outputChecks")) { + if (auto * outputChecks = get(parsed->structuredAttrs, "outputChecks")) { for (auto & [outputName, output_] : getObject(*outputChecks)) { - OutputChecks checks; - auto & output = getObject(output_); - if (auto maxSize = get(output, "maxSize")) - checks.maxSize = maxSize->get(); - - if (auto maxClosureSize = get(output, "maxClosureSize")) - checks.maxClosureSize = maxClosureSize->get(); - auto get_ = [&](const std::string & name) -> std::optional>> { if (auto i = get(output, name)) { diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 31ca167f9..8dddcf2c3 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -508,6 +508,9 @@ Derivation parseDerivation( } expect(str, ')'); + + drv.options = derivationOptionsFromStructuredAttrs(store, drv.inputDrvs, drv.env, drv.structuredAttrs); + return drv; } @@ -634,6 +637,14 @@ static bool hasDynamicDrvDep(const Derivation & drv) std::string Derivation::unparse( const StoreDirConfig & store, bool maskOutputs, DerivedPathMap::ChildNode::Map * actualInputs) const { + { + auto optionsFromEnv = derivationOptionsFromStructuredAttrs(store, inputDrvs, env, structuredAttrs); + + if (optionsFromEnv != options) + throw Error( + "'drv.options' and 'drv.env' are out of sync. This is probably an internal error, please open an issue!"); + } + std::string s; s.reserve(65536); @@ -1127,7 +1138,8 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String } } -std::optional Derivation::tryResolve(Store & store, Store * evalStore) const +std::optional>> +Derivation::tryResolve(Store & store, Store * evalStore) const { return tryResolve( store, [&](ref drvPath, const std::string & outputName) -> std::optional { @@ -1184,7 +1196,7 @@ static bool tryResolveInput( return true; } -std::optional Derivation::tryResolve( +std::optional>> Derivation::tryResolve( Store & store, std::function(ref drvPath, const std::string & outputName)> queryResolutionChain) const @@ -1207,7 +1219,12 @@ std::optional Derivation::tryResolve( rewriteDerivation(store, resolved, inputRewrites); - return resolved; + auto resolvedOptions = nix::tryResolve(options, queryResolutionChain); + + if (!resolvedOptions) + return std::nullopt; + + return {{std::move(resolved), *std::move(resolvedOptions)}}; } void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const @@ -1431,6 +1448,7 @@ void adl_serializer::to_json(json & res, const Derivation & d) res["builder"] = d.builder; res["args"] = d.args; res["env"] = d.env; + res["options"] = d.options; if (d.structuredAttrs) res["structuredAttrs"] = d.structuredAttrs->structuredAttrs; @@ -1511,6 +1529,11 @@ Derivation adl_serializer::from_json(const json & _json, const Exper if (auto structuredAttrs = get(json, "structuredAttrs")) res.structuredAttrs = StructuredAttrs{*structuredAttrs}; + if (auto options = get(json, "options")) + res.options = *options; + else + res.options = derivationOptionsFromStructuredAttrs(store, res.inputDrvs, res.env, res.structuredAttrs); + return res; } diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index af84661e2..9977c44c1 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -67,7 +67,8 @@ struct DerivationBuilderParams /** * The derivation options of `drv`. * - * @todo this should be part of `Derivation`. + * @todo this should be part of `Derivation`/`BasicDerivation`, if + * those two were distinguished by type arguments not subtyping. */ const DerivationOptions & drvOptions; diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index 0fe610987..4a1b83bbb 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -1,7 +1,6 @@ #pragma once ///@file -#include "nix/store/parsed-derivations.hh" #include "nix/store/derivations.hh" #include "nix/store/derivation-options.hh" #include "nix/store/build/derivation-building-misc.hh" diff --git a/src/libstore/include/nix/store/build/derivation-resolution-goal.hh b/src/libstore/include/nix/store/build/derivation-resolution-goal.hh index fb4c2a346..5e8036f38 100644 --- a/src/libstore/include/nix/store/build/derivation-resolution-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-resolution-goal.hh @@ -41,7 +41,7 @@ struct DerivationResolutionGoal : public Goal * If the derivation needed to be resolved, this is resulting * resolved derivations and its path. */ - std::unique_ptr> resolvedDrv; + std::unique_ptr>> resolvedDrv; void timedOut(Error && ex) override {} diff --git a/src/libstore/include/nix/store/derivation-options.hh b/src/libstore/include/nix/store/derivation-options.hh index 0b79af85d..63a28c6b7 100644 --- a/src/libstore/include/nix/store/derivation-options.hh +++ b/src/libstore/include/nix/store/derivation-options.hh @@ -247,6 +247,13 @@ std::optional> tryResolve( std::function(ref drvPath, const std::string & outputName)> queryResolutionChain); +template +struct json_avoids_null; + +template +struct json_avoids_null> : std::true_type +{}; + }; // namespace nix JSON_IMPL(nix::DerivationOptions); diff --git a/src/libstore/include/nix/store/derivations.hh b/src/libstore/include/nix/store/derivations.hh index 259314d3f..b669b523d 100644 --- a/src/libstore/include/nix/store/derivations.hh +++ b/src/libstore/include/nix/store/derivations.hh @@ -8,6 +8,7 @@ #include "nix/util/repair-flag.hh" #include "nix/store/derived-path-map.hh" #include "nix/store/parsed-derivations.hh" +#include "nix/store/derivation-options.hh" #include "nix/util/sync.hh" #include "nix/util/variant-wrapper.hh" @@ -332,6 +333,16 @@ struct Derivation : BasicDerivation */ DerivedPathMap>> inputDrvs; + /** + * Derivation options + * + * @todo instead of `BasicDerivation`/`Derivation`, should just have + * `template<...> Derivation`, and then the choice of template + * parameter would control "possibly-unresolved vs definitely + * resolved" and this field would use the overall type parameter. + */ + DerivationOptions options; + /** * Print a derivation. */ @@ -349,14 +360,15 @@ struct Derivation : BasicDerivation * 2. Input placeholders are replaced with realized input store * paths. */ - std::optional tryResolve(Store & store, Store * evalStore = nullptr) const; + std::optional>> + tryResolve(Store & store, Store * evalStore = nullptr) const; /** * Like the above, but instead of querying the Nix database for * realisations, uses a given mapping from input derivation paths + * output names to actual output store paths. */ - std::optional tryResolve( + std::optional>> tryResolve( Store & store, std::function(ref drvPath, const std::string & outputName)> queryResolutionChain) const; diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 642ac0978..0b800dc25 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -224,18 +224,8 @@ MissingPaths Store::queryMissing(const std::vector & targets) return; auto drv = make_ref(derivationFromPath(drvPath)); - DerivationOptions drvOptions; - try { - // FIXME: this is a lot of work just to get the value - // of `allowSubstitutes`. - drvOptions = - derivationOptionsFromStructuredAttrs(*this, drv->inputDrvs, drv->env, drv->structuredAttrs); - } catch (Error & e) { - e.addTrace({}, "while parsing derivation '%s'", printStorePath(drvPath)); - throw; - } - if (!knownOutputPaths && settings.useSubstitutes && drvOptions.substitutesAllowed()) { + if (!knownOutputPaths && settings.useSubstitutes && drv->options.substitutesAllowed()) { experimentalFeatureSettings.require(Xp::CaDerivations); // If there are unknown output paths, attempt to find if the @@ -265,7 +255,7 @@ MissingPaths Store::queryMissing(const std::vector & targets) } } - if (knownOutputPaths && settings.useSubstitutes && drvOptions.substitutesAllowed()) { + if (knownOutputPaths && settings.useSubstitutes && drv->options.substitutesAllowed()) { auto drvState = make_ref>(DrvState(invalid.size())); for (auto & output : invalid) pool.enqueue(std::bind(checkOutput, drvPath, drv, output, drvState)); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index c292e2e43..2846145c6 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1169,8 +1169,10 @@ std::optional Store::getBuildDerivationPath(const StorePath & path) // The build log is actually attached to the corresponding // resolved derivation, so we need to get it first auto resolvedDrv = drv.tryResolve(*this); - if (resolvedDrv) - return ::nix::writeDerivation(*this, *resolvedDrv, NoRepair, true); + if (resolvedDrv) { + // TODO check options compatibility with ATerm. + return ::nix::writeDerivation(*this, resolvedDrv->first, NoRepair, true); + } } return path; diff --git a/src/nix/nix-build/nix-build.cc b/src/nix/nix-build/nix-build.cc index 00ef32dfa..af39cccc6 100644 --- a/src/nix/nix-build/nix-build.cc +++ b/src/nix/nix-build/nix-build.cc @@ -532,9 +532,9 @@ static void main_nix_build(int argc, char ** argv) } if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { - auto resolvedDrv = drv.tryResolve(*store); - assert(resolvedDrv && "Successfully resolved the derivation"); - drv = *resolvedDrv; + auto resolved = drv.tryResolve(*store); + assert(resolved && "Successfully resolved the derivation"); + drv = resolved->first; } // Set the environment. @@ -554,18 +554,10 @@ static void main_nix_build(int argc, char ** argv) env["NIX_STORE"] = store->storeDir; env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores ? settings.buildCores : settings.getDefaultCores()); - DerivationOptions drvOptions; - try { - drvOptions = derivationOptionsFromStructuredAttrs(*store, drv.env, drv.structuredAttrs); - } catch (Error & e) { - e.addTrace({}, "while parsing derivation '%s'", store->printStorePath(packageInfo.requireDrvPath())); - throw; - } - int fileNr = 0; for (auto & var : drv.env) - if (drvOptions.passAsFile.count(var.first)) { + if (drv.options.passAsFile.count(var.first)) { auto fn = ".attr-" + std::to_string(fileNr++); Path p = (tmpDir.path() / fn).string(); writeFile(p, var.second); @@ -594,7 +586,7 @@ static void main_nix_build(int argc, char ** argv) for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map) accumInputClosure(inputDrv, inputNode); - auto json = drv.structuredAttrs->prepareStructuredAttrs(*store, drvOptions, inputs, drv.outputs); + auto json = drv.structuredAttrs->prepareStructuredAttrs(*store, drv.options, inputs, drv.outputs); structuredAttrsRC = StructuredAttrs::writeShell(json); diff --git a/tests/functional/lang/eval-okay-derivation-legacy.err.exp b/tests/functional/lang/eval-okay-derivation-legacy.err.exp index 94f0854dd..eb77358f1 100644 --- a/tests/functional/lang/eval-okay-derivation-legacy.err.exp +++ b/tests/functional/lang/eval-okay-derivation-legacy.err.exp @@ -4,3 +4,9 @@ warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedRequisites'; use 'outputChecks..disallowedRequisites' instead warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' disables the effect of the derivation attribute 'maxClosureSize'; use 'outputChecks..maxClosureSize' instead warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' disables the effect of the derivation attribute 'maxSize'; use 'outputChecks..maxSize' instead +warning: 'structuredAttrs' disables the effect of the top-level attribute 'allowedReferences'; use 'outputChecks' instead +warning: 'structuredAttrs' disables the effect of the top-level attribute 'allowedRequisites'; use 'outputChecks' instead +warning: 'structuredAttrs' disables the effect of the top-level attribute 'disallowedRequisites'; use 'outputChecks' instead +warning: 'structuredAttrs' disables the effect of the top-level attribute 'disallowedReferences'; use 'outputChecks' instead +warning: 'structuredAttrs' disables the effect of the top-level attribute 'maxSize'; use 'outputChecks' instead +warning: 'structuredAttrs' disables the effect of the top-level attribute 'maxClosureSize'; use 'outputChecks' instead