From 540071dd77f8267fbcf2ae9721706d0b97448b5f Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 11 May 2025 22:50:32 +0200 Subject: [PATCH] cli: disallow dynamic derivations with impure derivations gone we move on to dynamic derivations. this too is not done in a single commit because dynamic derivations are invasive, modifying semantics of all references to derivation output paths and all derivation dependency calculations. removing dynamic derivations cleanly is made significantly harder by the multiple did-you-mean-sum types, aka "wrappers for std::variant", holding all derivation outpath information. Change-Id: Ice7a7700c7b54c6a6061d4beb322b4175923d27a --- doc/manual/rl-next/remove-xp-derivations.md | 15 ++-- lix/libutil/config.cc | 3 +- tests/functional/dyn-drv/build-built-drv.sh | 21 ----- tests/functional/dyn-drv/common.sh | 8 -- tests/functional/dyn-drv/config.nix.in | 1 - tests/functional/dyn-drv/dep-built-drv.sh | 11 --- tests/functional/dyn-drv/eval-outputOf.sh | 80 ------------------ tests/functional/dyn-drv/meson.build | 6 -- .../dyn-drv/old-daemon-error-hack.nix | 20 ----- .../dyn-drv/old-daemon-error-hack.sh | 11 --- .../functional/dyn-drv/text-hashed-output.nix | 33 -------- .../functional/dyn-drv/text-hashed-output.sh | 26 ------ tests/functional/meson.build | 7 -- tests/unit/libexpr/value/context.cc | 4 +- .../derivation/bad-old-version-dyn-deps.drv | 1 - .../libstore/derivation/dynDerivationDeps.drv | 1 - .../derivation/dynDerivationDeps.json | 38 --------- tests/unit/libstore/derivation.cc | 84 ------------------- tests/unit/libstore/derived-path.cc | 4 +- tests/unit/libstore/downstream-placeholder.cc | 4 +- 20 files changed, 19 insertions(+), 359 deletions(-) delete mode 100755 tests/functional/dyn-drv/build-built-drv.sh delete mode 100644 tests/functional/dyn-drv/common.sh delete mode 120000 tests/functional/dyn-drv/config.nix.in delete mode 100755 tests/functional/dyn-drv/dep-built-drv.sh delete mode 100755 tests/functional/dyn-drv/eval-outputOf.sh delete mode 100644 tests/functional/dyn-drv/meson.build delete mode 100644 tests/functional/dyn-drv/old-daemon-error-hack.nix delete mode 100644 tests/functional/dyn-drv/old-daemon-error-hack.sh delete mode 100644 tests/functional/dyn-drv/text-hashed-output.nix delete mode 100755 tests/functional/dyn-drv/text-hashed-output.sh delete mode 100644 tests/unit/libstore/data/libstore/derivation/bad-old-version-dyn-deps.drv delete mode 100644 tests/unit/libstore/data/libstore/derivation/dynDerivationDeps.drv delete mode 100644 tests/unit/libstore/data/libstore/derivation/dynDerivationDeps.json diff --git a/doc/manual/rl-next/remove-xp-derivations.md b/doc/manual/rl-next/remove-xp-derivations.md index 4d6b6ce2c..0c7332da2 100644 --- a/doc/manual/rl-next/remove-xp-derivations.md +++ b/doc/manual/rl-next/remove-xp-derivations.md @@ -1,5 +1,5 @@ --- -synopsis: Remove impure derivations +synopsis: Remove impure derivations and dynamic derivations issues: [fj#815] cls: [3210] significance: significant @@ -7,8 +7,11 @@ category: "Breaking Changes" credits: [horrors] --- -The `impure-derivations` experimental feature has been removed. New impure -derivations cannot be created from this point forward, and existing impure store -derivations canot be read or built any more. Derivation outputs created by -building an impure derivation are still valid until garbage collected; existing -impure store derivations can only be garbage collected. +The `impure-derivations` and `dynamic-derivations` experimental feature have +been removed. + +New impure or dynamic derivations cannot be created from this point forward, and +any such pre-existing store derivations canot be read or built any more. +Derivation outputs created by building such a derivation are still valid +until garbage collected; existing store derivations can only be garbage +collected. diff --git a/lix/libutil/config.cc b/lix/libutil/config.cc index f32700042..d284cda5a 100644 --- a/lix/libutil/config.cc +++ b/lix/libutil/config.cc @@ -361,8 +361,7 @@ template<> ExperimentalFeatures BaseSetting::parse(const s static std::once_flag warned; warnDeprecated294(warned, s); } else if (*thisXpFeature == Xp::DynamicDerivations) { - static std::once_flag warned; - warnDeprecated294(warned, s); + throw Error("dynamic derivations are no longer supported"); } res = res | thisXpFeature.value(); } else diff --git a/tests/functional/dyn-drv/build-built-drv.sh b/tests/functional/dyn-drv/build-built-drv.sh deleted file mode 100755 index 647be9457..000000000 --- a/tests/functional/dyn-drv/build-built-drv.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/usr/bin/env bash - -source common.sh - -# In the corresponding nix file, we have two derivations: the first, named `hello`, -# is a normal recursive derivation, while the second, named dependent, has the -# new outputHashMode "text". Note that in "dependent", we don't refer to the -# build output of `hello`, but only to the path of the drv file. For this reason, -# we only need to: -# -# - instantiate `hello` -# - build `producingDrv` -# - check that the path of the output coincides with that of the original derivation - -out1=$(nix build -f ./text-hashed-output.nix hello --no-link) - -clearStore - -drvDep=$(nix-instantiate ./text-hashed-output.nix -A producingDrv) - -expectStderr 1 nix build "${drvDep}^out^out" --no-link | grepQuiet "Building dynamic derivations in one shot is not yet implemented" diff --git a/tests/functional/dyn-drv/common.sh b/tests/functional/dyn-drv/common.sh deleted file mode 100644 index c786f6925..000000000 --- a/tests/functional/dyn-drv/common.sh +++ /dev/null @@ -1,8 +0,0 @@ -source ../common.sh - -# Need backend to support text-hashing too -requireDaemonNewerThan "2.16.0pre20230419" - -enableFeatures "ca-derivations dynamic-derivations" - -restartDaemon diff --git a/tests/functional/dyn-drv/config.nix.in b/tests/functional/dyn-drv/config.nix.in deleted file mode 120000 index af24ddb30..000000000 --- a/tests/functional/dyn-drv/config.nix.in +++ /dev/null @@ -1 +0,0 @@ -../config.nix.in \ No newline at end of file diff --git a/tests/functional/dyn-drv/dep-built-drv.sh b/tests/functional/dyn-drv/dep-built-drv.sh deleted file mode 100755 index 4f6e9b080..000000000 --- a/tests/functional/dyn-drv/dep-built-drv.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/usr/bin/env bash - -source common.sh - -out1=$(nix-build ./text-hashed-output.nix -A hello --no-out-link) - -clearStore - -expectStderr 1 nix-build ./text-hashed-output.nix -A wrapper --no-out-link | grepQuiet "Building dynamic derivations in one shot is not yet implemented" - -# diff -r $out1 $out2 diff --git a/tests/functional/dyn-drv/eval-outputOf.sh b/tests/functional/dyn-drv/eval-outputOf.sh deleted file mode 100755 index 3681bd098..000000000 --- a/tests/functional/dyn-drv/eval-outputOf.sh +++ /dev/null @@ -1,80 +0,0 @@ -#!/usr/bin/env bash - -source ./common.sh - -# Without the dynamic-derivations XP feature, we don't have the builtin. -nix --experimental-features 'nix-command' eval --impure --expr \ - 'assert ! (builtins ? outputOf); ""' - -# Test that a string is required. -# -# We currently require a string to be passed, rather than a derivation -# object that could be coerced to a string. We might liberalise this in -# the future so it does work, but there are some design questions to -# resolve first. Adding a test so we don't liberalise it by accident. -expectStderr 1 nix --experimental-features 'nix-command dynamic-derivations' eval --impure --expr \ - 'builtins.outputOf (import ../dependencies.nix {}) "out"' \ - | grepQuiet "expected a string but found a set" - -# Test that "DrvDeep" string contexts are not supported at this time -# -# Like the above, this is a restriction we could relax later. -expectStderr 1 nix --experimental-features 'nix-command dynamic-derivations' eval --impure --expr \ - 'builtins.outputOf (import ../dependencies.nix {}).drvPath "out"' \ - | grepQuiet "has a context which refers to a complete source and binary closure. This is not supported at this time" - -# Test using `builtins.outputOf` with static derivations -testStaticHello () { - nix eval --impure --expr \ - 'with (import ./text-hashed-output.nix); let - a = hello.outPath; - b = builtins.outputOf (builtins.unsafeDiscardOutputDependency hello.drvPath) "out"; - in builtins.trace a - (builtins.trace b - (assert a == b; null))' -} - -# Test with a regular old input-addresed derivation -# -# `builtins.outputOf` works without ca-derivations and doesn't create a -# placeholder but just returns the output path. -testStaticHello - -# Test with content addressed derivation. -NIX_TESTS_CA_BY_DEFAULT=1 testStaticHello - -# Test with derivation-producing derivation -# -# This is hardly different from the preceding cases, except that we're -# only taking 1 outputOf out of 2 possible outputOfs. Note that -# `.outPath` could be defined as `outputOf drvPath`, which is what we're -# testing here. The other `outputOf` that we're not testing here is the -# use of _dynamic_ derivations. -nix eval --impure --expr \ - 'with (import ./text-hashed-output.nix); let - a = producingDrv.outPath; - b = builtins.outputOf (builtins.builtins.unsafeDiscardOutputDependency producingDrv.drvPath) "out"; - in builtins.trace a - (builtins.trace b - (assert a == b; null))' - -# Test with unbuilt output of derivation-producing derivation. -# -# This function similar to `testStaticHello` used above, but instead of -# checking the property on a constant derivation, we check it on a -# derivation that's from another derivation's output (outPath). -testDynamicHello () { - nix eval --impure --expr \ - 'with (import ./text-hashed-output.nix); let - a = builtins.outputOf producingDrv.outPath "out"; - b = builtins.outputOf (builtins.outputOf (builtins.unsafeDiscardOutputDependency producingDrv.drvPath) "out") "out"; - in builtins.trace a - (builtins.trace b - (assert a == b; null))' -} - -# inner dynamic derivation is input-addressed -testDynamicHello - -# inner dynamic derivation is content-addressed -NIX_TESTS_CA_BY_DEFAULT=1 testDynamicHello diff --git a/tests/functional/dyn-drv/meson.build b/tests/functional/dyn-drv/meson.build deleted file mode 100644 index f3f4a3b2b..000000000 --- a/tests/functional/dyn-drv/meson.build +++ /dev/null @@ -1,6 +0,0 @@ -# test_confdata set from tests/functional/meson.build -configure_file( - input : 'config.nix.in', - output : 'config.nix', - configuration : test_confdata, -) diff --git a/tests/functional/dyn-drv/old-daemon-error-hack.nix b/tests/functional/dyn-drv/old-daemon-error-hack.nix deleted file mode 100644 index c9d4a62d4..000000000 --- a/tests/functional/dyn-drv/old-daemon-error-hack.nix +++ /dev/null @@ -1,20 +0,0 @@ -with import ./config.nix; - -# A simple content-addressed derivation. -# The derivation can be arbitrarily modified by passing a different `seed`, -# but the output will always be the same -rec { - stub = mkDerivation { - name = "stub"; - buildCommand = '' - echo stub > $out - ''; - }; - wrapper = mkDerivation { - name = "has-dynamic-drv-dep"; - buildCommand = '' - exit 1 # we're not building this derivation - ${builtins.outputOf stub.outPath "out"} - ''; - }; -} diff --git a/tests/functional/dyn-drv/old-daemon-error-hack.sh b/tests/functional/dyn-drv/old-daemon-error-hack.sh deleted file mode 100644 index 43b049973..000000000 --- a/tests/functional/dyn-drv/old-daemon-error-hack.sh +++ /dev/null @@ -1,11 +0,0 @@ -# Purposely bypassing our usual common for this subgroup -source ../common.sh - -# Need backend to support text-hashing too -isDaemonNewer "2.18.0pre20230906" && skipTest "Daemon is too new" - -enableFeatures "ca-derivations dynamic-derivations" - -restartDaemon - -expectStderr 1 nix-instantiate --read-write-mode ./old-daemon-error-hack.nix | grepQuiet "the daemon is too old to understand dependencies on dynamic derivations" diff --git a/tests/functional/dyn-drv/text-hashed-output.nix b/tests/functional/dyn-drv/text-hashed-output.nix deleted file mode 100644 index 99203b518..000000000 --- a/tests/functional/dyn-drv/text-hashed-output.nix +++ /dev/null @@ -1,33 +0,0 @@ -with import ./config.nix; - -# A simple content-addressed derivation. -# The derivation can be arbitrarily modified by passing a different `seed`, -# but the output will always be the same -rec { - hello = mkDerivation { - name = "hello"; - buildCommand = '' - set -x - echo "Building a CA derivation" - mkdir -p $out - echo "Hello World" > $out/hello - ''; - }; - producingDrv = mkDerivation { - name = "hello.drv"; - buildCommand = '' - echo "Copying the derivation" - cp ${builtins.unsafeDiscardOutputDependency hello.drvPath} $out - ''; - __contentAddressed = true; - outputHashMode = "text"; - outputHashAlgo = "sha256"; - }; - wrapper = mkDerivation { - name = "use-dynamic-drv-in-non-dynamic-drv"; - buildCommand = '' - echo "Copying the output of the dynamic derivation" - cp -r ${builtins.outputOf producingDrv.outPath "out"} $out - ''; - }; -} diff --git a/tests/functional/dyn-drv/text-hashed-output.sh b/tests/functional/dyn-drv/text-hashed-output.sh deleted file mode 100755 index f3e5aa93b..000000000 --- a/tests/functional/dyn-drv/text-hashed-output.sh +++ /dev/null @@ -1,26 +0,0 @@ -#!/usr/bin/env bash - -source common.sh - -# In the corresponding nix file, we have two derivations: the first, named root, -# is a normal recursive derivation, while the second, named dependent, has the -# new outputHashMode "text". Note that in "dependent", we don't refer to the -# build output of root, but only to the path of the drv file. For this reason, -# we only need to: -# -# - instantiate the root derivation -# - build the dependent derivation -# - check that the path of the output coincides with that of the original derivation - -drv=$(nix-instantiate ./text-hashed-output.nix -A hello) -nix show-derivation "$drv" - -drvProducingDrv=$(nix-instantiate ./text-hashed-output.nix -A producingDrv) -nix show-derivation "$drvProducingDrv" - -out1=$(nix-build ./text-hashed-output.nix -A producingDrv --no-out-link) - -nix path-info $drv --derivation --json | jq -nix path-info $out1 --derivation --json | jq - -test $out1 == $drv diff --git a/tests/functional/meson.build b/tests/functional/meson.build index 621a4c134..d989713d0 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -25,8 +25,6 @@ config_nix_in = configure_file( # Just configures `ca/config.nix.in`. Done as a subdir() for the same reason as above. subdir('ca') -# Just configures `dyn-drv/config.nix.in`. Same as above. -subdir('dyn-drv') # Just configures `nix-shell/config.nix.in`. Same as above. subdir('nix-shell') @@ -54,11 +52,6 @@ functional_tests_scripts = [ 'ca/signatures.sh', 'ca/substitute.sh', 'ca/why-depends.sh', - 'dyn-drv/text-hashed-output.sh', - 'dyn-drv/build-built-drv.sh', - 'dyn-drv/eval-outputOf.sh', - 'dyn-drv/dep-built-drv.sh', - 'dyn-drv/old-daemon-error-hack.sh', 'flakes/flakes.sh', 'flakes/develop.sh', 'flakes/develop-r8854.sh', diff --git a/tests/unit/libexpr/value/context.cc b/tests/unit/libexpr/value/context.cc index 7a079972c..8ea4cd1d4 100644 --- a/tests/unit/libexpr/value/context.cc +++ b/tests/unit/libexpr/value/context.cc @@ -94,7 +94,9 @@ TEST(NixStringContextElemTest, built_built) { * to worry about race conditions if the tests run concurrently. */ ExperimentalFeatureSettings mockXpSettings; - mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations"); + mockXpSettings.experimentalFeatures.override( + ExperimentalFeatures{} | Xp::DynamicDerivations | Xp::CaDerivations + ); std::string_view built = "!foo!bar!g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv"; auto elem = NixStringContextElem::parse(built, mockXpSettings); diff --git a/tests/unit/libstore/data/libstore/derivation/bad-old-version-dyn-deps.drv b/tests/unit/libstore/data/libstore/derivation/bad-old-version-dyn-deps.drv deleted file mode 100644 index 3cd1ded02..000000000 --- a/tests/unit/libstore/data/libstore/derivation/bad-old-version-dyn-deps.drv +++ /dev/null @@ -1 +0,0 @@ -Derive([],[("/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep2.drv",(["cat","dog"],[("cat",["kitten"]),("goose",["gosling"])]))],["/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1"],"wasm-sel4","foo",["bar","baz"],[("BIG_BAD","WOLF")]) \ No newline at end of file diff --git a/tests/unit/libstore/data/libstore/derivation/dynDerivationDeps.drv b/tests/unit/libstore/data/libstore/derivation/dynDerivationDeps.drv deleted file mode 100644 index cfffe48ec..000000000 --- a/tests/unit/libstore/data/libstore/derivation/dynDerivationDeps.drv +++ /dev/null @@ -1 +0,0 @@ -DrvWithVersion("xp-dyn-drv",[],[("/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep2.drv",(["cat","dog"],[("cat",["kitten"]),("goose",["gosling"])]))],["/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1"],"wasm-sel4","foo",["bar","baz"],[("BIG_BAD","WOLF")]) \ No newline at end of file diff --git a/tests/unit/libstore/data/libstore/derivation/dynDerivationDeps.json b/tests/unit/libstore/data/libstore/derivation/dynDerivationDeps.json deleted file mode 100644 index 9dbeb1f15..000000000 --- a/tests/unit/libstore/data/libstore/derivation/dynDerivationDeps.json +++ /dev/null @@ -1,38 +0,0 @@ -{ - "args": [ - "bar", - "baz" - ], - "builder": "foo", - "env": { - "BIG_BAD": "WOLF" - }, - "inputDrvs": { - "/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep2.drv": { - "dynamicOutputs": { - "cat": { - "dynamicOutputs": {}, - "outputs": [ - "kitten" - ] - }, - "goose": { - "dynamicOutputs": {}, - "outputs": [ - "gosling" - ] - } - }, - "outputs": [ - "cat", - "dog" - ] - } - }, - "inputSrcs": [ - "/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1" - ], - "name": "dyn-dep-derivation", - "outputs": {}, - "system": "wasm-sel4" -} diff --git a/tests/unit/libstore/derivation.cc b/tests/unit/libstore/derivation.cc index 77a27f5e9..2d7af3cd5 100644 --- a/tests/unit/libstore/derivation.cc +++ b/tests/unit/libstore/derivation.cc @@ -33,14 +33,6 @@ class CaDerivationTest : public DerivationTest } }; -class DynDerivationTest : public DerivationTest -{ - void SetUp() override - { - mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations"); - } -}; - TEST_F(DerivationTest, BadATerm_version) { ASSERT_THROW( parseDerivation( @@ -51,16 +43,6 @@ TEST_F(DerivationTest, BadATerm_version) { FormatError); } -TEST_F(DynDerivationTest, BadATerm_oldVersionDynDeps) { - ASSERT_THROW( - parseDerivation( - *store, - readFile(goldenMaster("bad-old-version-dyn-deps.drv")), - "dyn-dep-derivation", - mockXpSettings), - FormatError); -} - #define TEST_JSON(FIXTURE, NAME, VAL, DRV_NAME, OUTPUT_NAME) \ TEST_F(FIXTURE, DerivationOutput_ ## NAME ## _from_json) { \ if (testAccept()) \ @@ -127,14 +109,6 @@ TEST_JSON(DerivationTest, caFixedNAR, }), "drv-name", "output-name") -TEST_JSON(DynDerivationTest, caFixedText, - (DerivationOutput::CAFixed { - .ca = { - .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), - }, - }), - "drv-name", "output-name") - TEST_JSON(CaDerivationTest, caFloating, (DerivationOutput::CAFloating { .method = FileIngestionMethod::Recursive, @@ -263,64 +237,6 @@ TEST_ATERM(DerivationTest, simple, makeSimpleDrv(*store), "simple-derivation") -Derivation makeDynDepDerivation(const Store & store) { - Derivation drv; - drv.name = "dyn-dep-derivation"; - drv.inputSrcs = { - store.parseStorePath("/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1"), - }; - drv.inputDrvs = { - .map = { - { - store.parseStorePath("/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep2.drv"), - DerivedPathMap::ChildNode { - .value = { - "cat", - "dog", - }, - .childMap = { - { - "cat", - DerivedPathMap::ChildNode { - .value = { - "kitten", - }, - }, - }, - { - "goose", - DerivedPathMap::ChildNode { - .value = { - "gosling", - }, - }, - }, - }, - }, - }, - }, - }; - drv.platform = "wasm-sel4"; - drv.builder = "foo"; - drv.args = { - "bar", - "baz", - }; - drv.env = { - { - "BIG_BAD", - "WOLF", - }, - }; - return drv; -} - -TEST_JSON(DynDerivationTest, dynDerivationDeps, makeDynDepDerivation(*store)) - -TEST_ATERM(DynDerivationTest, dynDerivationDeps, - makeDynDepDerivation(*store), - "dyn-dep-derivation") - #undef TEST_JSON #undef TEST_ATERM diff --git a/tests/unit/libstore/derived-path.cc b/tests/unit/libstore/derived-path.cc index b351def58..9f296a557 100644 --- a/tests/unit/libstore/derived-path.cc +++ b/tests/unit/libstore/derived-path.cc @@ -54,7 +54,9 @@ TEST_F(DerivedPathTest, built_built) { * to worry about race conditions if the tests run concurrently. */ ExperimentalFeatureSettings mockXpSettings; - mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations"); + mockXpSettings.experimentalFeatures.override( + ExperimentalFeatures{} | Xp::DynamicDerivations | Xp::CaDerivations + ); std::string_view built = "/nix/store/g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv^foo^bar,baz"; auto elem = DerivedPath::parse(*store, built, mockXpSettings); diff --git a/tests/unit/libstore/downstream-placeholder.cc b/tests/unit/libstore/downstream-placeholder.cc index f8b26730b..8de615dd7 100644 --- a/tests/unit/libstore/downstream-placeholder.cc +++ b/tests/unit/libstore/downstream-placeholder.cc @@ -25,7 +25,9 @@ TEST(DownstreamPlaceholder, unknownDerivation) { * Same reason as above */ ExperimentalFeatureSettings mockXpSettings; - mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations"); + mockXpSettings.experimentalFeatures.override( + ExperimentalFeatures{} | Xp::DynamicDerivations | Xp::CaDerivations + ); ASSERT_EQ( DownstreamPlaceholder::unknownDerivation(