From 6599be1a9f76f0fba2a6905ea85235aeb7b7eae9 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 11 May 2025 01:51:08 +0200 Subject: [PATCH] cli: disallow impure derivations we don't remove the entire feature in one go to make review easier. impure derivations are rather unintrusive on their own, at least if we compare them to dynamic or ca derivations in general, so we will be done with this soon. as it stands impure derivations cannot work without ca derivations, and those we *really* want to leave behind. Change-Id: I4f01d8d758b2c85dcd6c3078304b5ee1b52f65b0 --- doc/manual/rl-next/remove-xp-derivations.md | 14 ++++ lix/libexpr/fetchurl.nix | 1 + lix/libutil/config.cc | 3 +- tests/functional/impure-derivations.nix | 16 ----- tests/functional/impure-derivations.sh | 65 ------------------- tests/functional/meson.build | 1 - tests/unit/libexpr/error_traces.cc | 10 --- .../libstore/derivation/output-impure.json | 4 -- tests/unit/libstore/derivation.cc | 15 ----- 9 files changed, 16 insertions(+), 113 deletions(-) create mode 100644 doc/manual/rl-next/remove-xp-derivations.md delete mode 100644 tests/functional/impure-derivations.sh delete mode 100644 tests/unit/libstore/data/libstore/derivation/output-impure.json diff --git a/doc/manual/rl-next/remove-xp-derivations.md b/doc/manual/rl-next/remove-xp-derivations.md new file mode 100644 index 000000000..4d6b6ce2c --- /dev/null +++ b/doc/manual/rl-next/remove-xp-derivations.md @@ -0,0 +1,14 @@ +--- +synopsis: Remove impure derivations +issues: [fj#815] +cls: [3210] +significance: significant +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. diff --git a/lix/libexpr/fetchurl.nix b/lix/libexpr/fetchurl.nix index 50007c738..670114058 100644 --- a/lix/libexpr/fetchurl.nix +++ b/lix/libexpr/fetchurl.nix @@ -34,6 +34,7 @@ executable ? false, unpack ? false, name ? baseNameOf (toString url), + # still translates to __impure to trigger derivationStrict error checks. impure ? false, }: diff --git a/lix/libutil/config.cc b/lix/libutil/config.cc index 713b1b964..8e5912081 100644 --- a/lix/libutil/config.cc +++ b/lix/libutil/config.cc @@ -364,8 +364,7 @@ template<> ExperimentalFeatures BaseSetting::parse(const s static std::once_flag warned; warnDeprecated294(warned, s); } else if (*thisXpFeature == Xp::ImpureDerivations) { - static std::once_flag warned; - warnDeprecated294(warned, s); + throw Error("impure derivations are no longer supported"); } res = res | thisXpFeature.value(); } else diff --git a/tests/functional/impure-derivations.nix b/tests/functional/impure-derivations.nix index 98547e6c1..4514cbe50 100644 --- a/tests/functional/impure-derivations.nix +++ b/tests/functional/impure-derivations.nix @@ -2,22 +2,6 @@ with import ./config.nix; rec { - impure = mkDerivation { - name = "impure"; - outputs = [ "out" "stuff" ]; - buildCommand = - '' - echo impure - x=$(< $TEST_ROOT/counter) - mkdir $out $stuff - echo $x > $out/n - ln -s $out/n $stuff/bla - printf $((x + 1)) > $TEST_ROOT/counter - ''; - __impure = true; - impureEnvVars = [ "TEST_ROOT" ]; - }; - impureOnImpure = mkDerivation { name = "impure-on-impure"; buildCommand = diff --git a/tests/functional/impure-derivations.sh b/tests/functional/impure-derivations.sh deleted file mode 100644 index 38be4234e..000000000 --- a/tests/functional/impure-derivations.sh +++ /dev/null @@ -1,65 +0,0 @@ -source common.sh - -requireDaemonNewerThan "2.8pre20220311" - -enableFeatures "ca-derivations impure-derivations" -restartDaemon - -clearStore - -# Basic test of impure derivations: building one a second time should not use the previous result. -printf 0 > $TEST_ROOT/counter - -# `nix derivation add` with impure derivations work -drvPath=$(nix-instantiate ./impure-derivations.nix -A impure) -nix derivation show $drvPath | jq .[] > $TEST_HOME/impure-drv.json -drvPath2=$(nix derivation add < $TEST_HOME/impure-drv.json) -[[ "$drvPath" = "$drvPath2" ]] - -# But only with the experimental feature! -expectStderr 1 nix derivation add < $TEST_HOME/impure-drv.json --experimental-features nix-command | grepQuiet "experimental Lix feature 'impure-derivations' is disabled" - -nix build --dry-run --json --file ./impure-derivations.nix impure.all -json=$(nix build -L --no-link --json --file ./impure-derivations.nix impure.all) -path1=$(echo $json | jq -r .[].outputs.out) -path1_stuff=$(echo $json | jq -r .[].outputs.stuff) -[[ $(< $path1/n) = 0 ]] -[[ $(< $path1_stuff/bla) = 0 ]] - -[[ $(nix path-info --json $path1 | jq .[].ca) =~ fixed:r:sha256: ]] - -path2=$(nix build -L --no-link --json --file ./impure-derivations.nix impure | jq -r .[].outputs.out) -[[ $(< $path2/n) = 1 ]] - -# Test impure derivations that depend on impure derivations. -path3=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure | jq -r .[].outputs.out) -[[ $(< $path3/n) = X2 ]] - -path4=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure | jq -r .[].outputs.out) -[[ $(< $path4/n) = X3 ]] - -# Test that (self-)references work. -[[ $(< $path4/symlink/bla) = 3 ]] -[[ $(< $path4/self/n) = X3 ]] - -# Input-addressed derivations cannot depend on impure derivations directly. -(! nix build -L --no-link --json --file ./impure-derivations.nix inputAddressed 2>&1) | grep 'depends on impure derivation' - -drvPath=$(nix eval --json --file ./impure-derivations.nix impure.drvPath | jq -r .) -[[ $(nix derivation show $drvPath | jq ".[\"$drvPath\"].outputs.out.impure") = true ]] -[[ $(nix derivation show $drvPath | jq ".[\"$drvPath\"].outputs.stuff.impure") = true ]] - -# Fixed-output derivations *can* depend on impure derivations. -path5=$(nix build -L --no-link --json --file ./impure-derivations.nix contentAddressed | jq -r .[].outputs.out) -[[ $(< $path5) = X ]] -[[ $(< $TEST_ROOT/counter) = 5 ]] - -# And they should not be rebuilt. -path5=$(nix build -L --no-link --json --file ./impure-derivations.nix contentAddressed | jq -r .[].outputs.out) -[[ $(< $path5) = X ]] -[[ $(< $TEST_ROOT/counter) = 5 ]] - -# Input-addressed derivations can depend on fixed-output derivations that depend on impure derivations. -path6=$(nix build -L --no-link --json --file ./impure-derivations.nix inputAddressedAfterCA | jq -r .[].outputs.out) -[[ $(< $path6) = X ]] -[[ $(< $TEST_ROOT/counter) = 5 ]] diff --git a/tests/functional/meson.build b/tests/functional/meson.build index 2c3f843d3..621a4c134 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -186,7 +186,6 @@ functional_tests_scripts = [ 'fetchClosure.sh', 'completions.sh', 'flakes/show.sh', - 'impure-derivations.sh', 'path-from-hash-part.sh', 'toString-path.sh', 'read-only-store.sh', diff --git a/tests/unit/libexpr/error_traces.cc b/tests/unit/libexpr/error_traces.cc index 84c1b8733..f3163437c 100644 --- a/tests/unit/libexpr/error_traces.cc +++ b/tests/unit/libexpr/error_traces.cc @@ -1290,16 +1290,6 @@ namespace nix { HintFmt("expected a Boolean but found %s: %s", "a string", "\"true\""), HintFmt("while evaluating the attribute '__contentAddressed' of derivation 'foo'")); - ASSERT_TRACE2("derivationStrict { name = \"foo\"; builder = 1; system = 1; outputs = \"out\"; __impure = \"true\"; }", - TypeError, - HintFmt("expected a Boolean but found %s: %s", "a string", "\"true\""), - HintFmt("while evaluating the attribute '__impure' of derivation 'foo'")); - - ASSERT_TRACE2("derivationStrict { name = \"foo\"; builder = 1; system = 1; outputs = \"out\"; __impure = \"true\"; }", - TypeError, - HintFmt("expected a Boolean but found %s: %s", "a string", "\"true\""), - HintFmt("while evaluating the attribute '__impure' of derivation 'foo'")); - ASSERT_TRACE2("derivationStrict { name = \"foo\"; builder = 1; system = 1; outputs = \"out\"; args = \"foo\"; }", TypeError, HintFmt("expected a list but found %s: %s", "a string", "\"foo\""), diff --git a/tests/unit/libstore/data/libstore/derivation/output-impure.json b/tests/unit/libstore/data/libstore/derivation/output-impure.json deleted file mode 100644 index 62b61cdca..000000000 --- a/tests/unit/libstore/data/libstore/derivation/output-impure.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "hashAlgo": "r:sha256", - "impure": true -} diff --git a/tests/unit/libstore/derivation.cc b/tests/unit/libstore/derivation.cc index edcabef44..77a27f5e9 100644 --- a/tests/unit/libstore/derivation.cc +++ b/tests/unit/libstore/derivation.cc @@ -41,14 +41,6 @@ class DynDerivationTest : public DerivationTest } }; -class ImpureDerivationTest : public DerivationTest -{ - void SetUp() override - { - mockXpSettings.set("experimental-features", "impure-derivations"); - } -}; - TEST_F(DerivationTest, BadATerm_version) { ASSERT_THROW( parseDerivation( @@ -154,13 +146,6 @@ TEST_JSON(DerivationTest, deferred, DerivationOutput::Deferred { }, "drv-name", "output-name") -TEST_JSON(ImpureDerivationTest, impure, - (DerivationOutput::Impure { - .method = FileIngestionMethod::Recursive, - .hashType = HashType::SHA256, - }), - "drv-name", "output-name") - #undef TEST_JSON #define TEST_JSON(FIXTURE, NAME, VAL) \