From cd5dc76d8340c38487d1b48a3ba090683fa35493 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 9 Feb 2024 04:30:09 +0100 Subject: [PATCH] substitute: Deprecate `replacements`, introduce `replacementsList` Also: - Add tests - Treewide update - Improve docs --- .../modules/system/boot/systemd/coredump.nix | 2 +- pkgs/build-support/substitute/substitute.nix | 56 +++++++++-- pkgs/build-support/substitute/substitute.sh | 8 +- .../science/math/libamplsolver/default.nix | 2 +- .../tools/misc/binutils/2.38/default.nix | 2 +- .../tools/misc/binutils/default.nix | 2 +- .../summoning-pixel-dungeon.nix | 2 +- pkgs/test/default.nix | 2 + pkgs/test/substitute/default.nix | 96 +++++++++++++++++++ 9 files changed, 160 insertions(+), 12 deletions(-) create mode 100644 pkgs/test/substitute/default.nix diff --git a/nixos/modules/system/boot/systemd/coredump.nix b/nixos/modules/system/boot/systemd/coredump.nix index 03ef00e5683c..271d8f86d0e6 100644 --- a/nixos/modules/system/boot/systemd/coredump.nix +++ b/nixos/modules/system/boot/systemd/coredump.nix @@ -52,7 +52,7 @@ in { # See: https://github.com/NixOS/nixpkgs/issues/213408 pkgs.substitute { src = "${systemd}/example/sysctl.d/50-coredump.conf"; - replacements = [ + substitutions = [ "--replace" "${systemd}" "${pkgs.symlinkJoin { name = "systemd"; paths = [ systemd ]; }}" diff --git a/pkgs/build-support/substitute/substitute.nix b/pkgs/build-support/substitute/substitute.nix index 7f0332334585..37233a306840 100644 --- a/pkgs/build-support/substitute/substitute.nix +++ b/pkgs/build-support/substitute/substitute.nix @@ -1,14 +1,58 @@ -{ stdenvNoCC }: +{ lib, stdenvNoCC }: +/* +This is a wrapper around `substitute` in the stdenv. +Attribute arguments: +- `name` (optional): The name of the resulting derivation +- `src`: The path to the file to substitute +- `substitutions`: The list of substitution arguments to pass + See https://nixos.org/manual/nixpkgs/stable/#fun-substitute +- `replacements`: Deprecated version of `substitutions` + that doesn't support spaces in arguments. + +Example: + +```nix +{ substitute }: +substitute { + src = ./greeting.txt; + substitutions = [ + "--replace" + "world" + "paul" + ]; +} +``` + +See ../../test/substitute for tests +*/ args: -# This is a wrapper around `substitute` in the stdenv. -# The `replacements` attribute should be a list of list of arguments -# to `substitute`, such as `[ "--replace" "sourcetext" "replacementtext" ]` -stdenvNoCC.mkDerivation ({ +let name = if args ? name then args.name else baseNameOf (toString args.src); + deprecationReplacement = lib.pipe args.replacements [ + lib.toList + (map (lib.splitString " ")) + lib.concatLists + (lib.concatMapStringsSep " " lib.strings.escapeNixString) + ]; + optionalDeprecationWarning = + # substitutions is only available starting 24.05. + # TODO: Remove support for replacements sometime after the next release + lib.warnIf (args ? replacements && lib.isInOldestRelease 2405) '' + pkgs.substitute: For "${name}", `replacements` is used, which is deprecated since it doesn't support arguments with spaces. Use `substitutions` instead: + substitutions = [ ${deprecationReplacement} ];''; +in +optionalDeprecationWarning +stdenvNoCC.mkDerivation ({ + inherit name; builder = ./substitute.sh; inherit (args) src; preferLocalBuild = true; allowSubstitutes = false; -} // args // { replacements = args.replacements; }) +} // args // lib.optionalAttrs (args ? substitutions) { + substitutions = + assert lib.assertMsg (lib.isList args.substitutions) '' + pkgs.substitute: For "${name}", `substitutions` is passed, which is expected to be a list, but it's a ${builtins.typeOf args.substitutions} instead.''; + lib.escapeShellArgs args.substitutions; +}) diff --git a/pkgs/build-support/substitute/substitute.sh b/pkgs/build-support/substitute/substitute.sh index dbac275a80ed..d50a82446639 100644 --- a/pkgs/build-support/substitute/substitute.sh +++ b/pkgs/build-support/substitute/substitute.sh @@ -8,7 +8,13 @@ if test -n "$dir"; then mkdir -p $out/$dir fi -substitute $src $target $replacements +substitutionsList=($replacements) + +if [[ -v substitutions ]]; then + eval "substitutionsList+=($substitutions)" +fi + +substitute $src $target "${substitutionsList[@]}" if test -n "$isExecutable"; then chmod +x $target diff --git a/pkgs/development/libraries/science/math/libamplsolver/default.nix b/pkgs/development/libraries/science/math/libamplsolver/default.nix index c0bc89b492ea..a40091bac8b5 100644 --- a/pkgs/development/libraries/science/math/libamplsolver/default.nix +++ b/pkgs/development/libraries/science/math/libamplsolver/default.nix @@ -12,7 +12,7 @@ stdenv.mkDerivation rec { patches = [ (substitute { src = ./libamplsolver-sharedlib.patch; - replacements = [ "--replace" "@sharedlibext@" "${stdenv.hostPlatform.extensions.sharedLibrary}" ]; + substitutions = [ "--replace" "@sharedlibext@" "${stdenv.hostPlatform.extensions.sharedLibrary}" ]; }) ]; diff --git a/pkgs/development/tools/misc/binutils/2.38/default.nix b/pkgs/development/tools/misc/binutils/2.38/default.nix index fd9a3277532e..474a565f351f 100644 --- a/pkgs/development/tools/misc/binutils/2.38/default.nix +++ b/pkgs/development/tools/misc/binutils/2.38/default.nix @@ -91,7 +91,7 @@ stdenv.mkDerivation { # this patch is from debian: # https://sources.debian.org/data/main/b/binutils/2.38-3/debian/patches/mips64-default-n64.diff (if stdenv.targetPlatform.isMusl - then substitute { src = ./mips64-default-n64.patch; replacements = [ "--replace" "gnuabi64" "muslabi64" ]; } + then substitute { src = ./mips64-default-n64.patch; substitutions = [ "--replace" "gnuabi64" "muslabi64" ]; } else ./mips64-default-n64.patch) # On PowerPC, when generating assembly code, GCC generates a `.machine` # custom instruction which instructs the assembler to generate code for this diff --git a/pkgs/development/tools/misc/binutils/default.nix b/pkgs/development/tools/misc/binutils/default.nix index 2183d45b6f0d..3a5a07dadabb 100644 --- a/pkgs/development/tools/misc/binutils/default.nix +++ b/pkgs/development/tools/misc/binutils/default.nix @@ -105,7 +105,7 @@ stdenv.mkDerivation (finalAttrs: { # this patch is from debian: # https://sources.debian.org/data/main/b/binutils/2.38-3/debian/patches/mips64-default-n64.diff (if stdenv.targetPlatform.isMusl - then substitute { src = ./mips64-default-n64.patch; replacements = [ "--replace" "gnuabi64" "muslabi64" ]; } + then substitute { src = ./mips64-default-n64.patch; substitutions = [ "--replace" "gnuabi64" "muslabi64" ]; } else ./mips64-default-n64.patch) # This patch fixes a bug in 2.40 on MinGW, which breaks DXVK when cross-building from Darwin. # See https://sourceware.org/bugzilla/show_bug.cgi?id=30079 diff --git a/pkgs/games/shattered-pixel-dungeon/summoning-pixel-dungeon.nix b/pkgs/games/shattered-pixel-dungeon/summoning-pixel-dungeon.nix index 488110684c09..5e7ed1b817e0 100644 --- a/pkgs/games/shattered-pixel-dungeon/summoning-pixel-dungeon.nix +++ b/pkgs/games/shattered-pixel-dungeon/summoning-pixel-dungeon.nix @@ -18,7 +18,7 @@ callPackage ./generic.nix rec { patches = [(substitute { src = ./disable-git-version.patch; - replacements = [ "--subst-var-by" "version" version ]; + substitutions = [ "--subst-var-by" "version" version ]; })]; depsHash = "sha256-0P/BcjNnbDN25DguRcCyzPuUG7bouxEx1ySodIbSwvg="; diff --git a/pkgs/test/default.nix b/pkgs/test/default.nix index f66b05b9f6f1..9868bbc6033b 100644 --- a/pkgs/test/default.nix +++ b/pkgs/test/default.nix @@ -177,4 +177,6 @@ with pkgs; auto-patchelf-hook = callPackage ./auto-patchelf-hook { }; systemd = callPackage ./systemd { }; + + substitute = recurseIntoAttrs (callPackage ./substitute { }); } diff --git a/pkgs/test/substitute/default.nix b/pkgs/test/substitute/default.nix new file mode 100644 index 000000000000..3ff346b14658 --- /dev/null +++ b/pkgs/test/substitute/default.nix @@ -0,0 +1,96 @@ +{ substitute, testers, runCommand }: +let + # Ofborg doesn't allow any traces on stderr, + # so mock `lib` to not trace warnings, + # because substitute gives a deprecation warning + substituteSilent = substitute.override (prevArgs: { + lib = prevArgs.lib.extend (finalLib: prevLib: { + trivial = prevLib.trivial // { + warn = msg: value: value; + }; + }); + }); +in { + + substitutions = testers.testEqualContents { + assertion = "substitutions-spaces"; + actual = substitute { + src = builtins.toFile "source" '' + Hello world! + ''; + substitutions = [ + "--replace-fail" + "Hello world!" + "Yo peter!" + ]; + }; + expected = builtins.toFile "expected" '' + Yo peter! + ''; + }; + + legacySingleReplace = testers.testEqualContents { + assertion = "substitute-single-replace"; + actual = substituteSilent { + src = builtins.toFile "source" '' + Hello world! + ''; + replacements = [ + "--replace-fail" "world" "paul" + ]; + }; + expected = builtins.toFile "expected" '' + Hello paul! + ''; + }; + + legacyString = testers.testEqualContents { + assertion = "substitute-string"; + actual = substituteSilent { + src = builtins.toFile "source" '' + Hello world! + ''; + # Not great that this works at all, but is supported + replacements = "--replace-fail world string"; + }; + expected = builtins.toFile "expected" '' + Hello string! + ''; + }; + + legacySingleArg = testers.testEqualContents { + assertion = "substitute-single-arg"; + actual = substituteSilent { + src = builtins.toFile "source" '' + Hello world! + ''; + # Not great that this works at all, but is supported + replacements = [ + "--replace-fail world list" + ]; + }; + expected = builtins.toFile "expected" '' + Hello list! + ''; + }; + + legacyVar = testers.testEqualContents { + assertion = "substitute-var"; + actual = substituteSilent { + src = builtins.toFile "source" '' + @greeting@ @name@! + ''; + # Not great that this works at all, but is supported + replacements = [ + "--subst-var name" + "--subst-var-by greeting Yo" + ]; + name = "peter"; + }; + expected = builtins.toFile "expected" '' + Yo peter! + ''; + }; + + +}