From 36a60bca72a33c666d3c2119ba6b0871884c6627 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sun, 24 Sep 2023 20:32:50 +0200 Subject: [PATCH 1/8] replaceDependencies: evolve from replaceDependency Rewrite replaceDependency so that it can apply multiple replacements in one go. This includes correctly handling the case where one of the replacements itself needs to have another replacement applied as well. This rewritten function is now aptly called replaceDependencies. For compatibility, replaceDependency is retained as a simple wrapper over replaceDependencies. It will cause a rebuild because the unpatched dependency is now referenced by derivation instead of by storePath, but the functionality is equivalent. Fixes: https://github.com/NixOS/nixpkgs/issues/199162 --- pkgs/build-support/replace-dependencies.nix | 186 ++++++++++++++++++++ pkgs/build-support/replace-dependency.nix | 94 ---------- pkgs/top-level/all-packages.nix | 11 +- 3 files changed, 196 insertions(+), 95 deletions(-) create mode 100644 pkgs/build-support/replace-dependencies.nix delete mode 100644 pkgs/build-support/replace-dependency.nix diff --git a/pkgs/build-support/replace-dependencies.nix b/pkgs/build-support/replace-dependencies.nix new file mode 100644 index 0000000000000..2af6d19c268db --- /dev/null +++ b/pkgs/build-support/replace-dependencies.nix @@ -0,0 +1,186 @@ +{ + runCommandLocal, + nix, + lib, +}: + +# Replace some dependencies in the requisites tree of drv, propagating the change all the way up the tree, even within other replacements, without a full rebuild. +# This can be useful, for example, to patch a security hole in libc and still use your system safely without rebuilding the world. +# This should be a short term solution, as soon as a rebuild can be done the properly rebuilt derivation should be used. +# Each old dependency and the corresponding new dependency MUST have the same-length name, and ideally should have close-to-identical directory layout. +# +# Example: safeFirefox = replaceDependencies { +# drv = firefox; +# replacements = [ +# { +# oldDependency = glibc; +# newDependency = glibc.overrideAttrs (oldAttrs: { +# patches = oldAttrs.patches ++ [ ./fix-glibc-hole.patch ]; +# }); +# } +# { +# oldDependency = libwebp; +# newDependency = libwebp.overrideAttrs (oldAttrs: { +# patches = oldAttrs.patches ++ [ ./fix-libwebp-hole.patch ]; +# }); +# } +# ]; +# }; +# This will first rebuild glibc and libwebp with your security patches. +# Then it copies over firefox (and all of its dependencies) without rebuilding further. +# In particular, the glibc dependency of libwebp will be replaced by the patched version as well. +# +# In rare cases, it is possible for the replacement process to cause breakage (for example due to checksum mismatch). +# The cutoffPackages argument can be used to exempt the problematic packages from the replacement process. +{ + drv, + replacements, + cutoffPackages ? [ ], + verbose ? true, +}: + +let + inherit (builtins) unsafeDiscardStringContext appendContext; + inherit (lib) + trace + substring + stringLength + concatStringsSep + mapAttrsToList + listToAttrs + attrValues + mapAttrs + filter + hasAttr + all + ; + inherit (lib.attrsets) mergeAttrsList; + + toContextlessString = x: unsafeDiscardStringContext (toString x); + warn = if verbose then trace else (x: y: y); + + referencesOf = + drv: + import + (runCommandLocal "references.nix" + { + exportReferencesGraph = [ + "graph" + drv + ]; + } + '' + (echo { + while read path + do + echo " \"$path\" = [" + read count + read count + while [ "0" != "$count" ] + do + read ref_path + if [ "$ref_path" != "$path" ] + then + echo " \"$ref_path\"" + fi + count=$(($count - 1)) + done + echo " ];" + done < graph + echo }) > $out + '' + ).outPath; + rewriteHashes = + drv: rewrites: + if rewrites == { } then + drv + else + let + drvName = substring 33 (stringLength (baseNameOf drv)) (baseNameOf drv); + in + runCommandLocal drvName { nixStore = "${nix}/bin/nix-store"; } '' + $nixStore --dump ${drv} | sed 's|${baseNameOf drv}|'$(basename $out)'|g' | sed -e ${ + concatStringsSep " -e " ( + mapAttrsToList (name: value: "'s|${baseNameOf name}|${baseNameOf value}|g'") rewrites + ) + } | $nixStore --restore $out + ''; + + knownDerivations = [ drv ] ++ map ({ newDependency, ... }: newDependency) replacements; + referencesMemo = listToAttrs ( + map (drv: { + name = toContextlessString drv; + value = referencesOf drv; + }) knownDerivations + ); + relevantReferences = mergeAttrsList (attrValues referencesMemo); + # Make sure a derivation is returned even when no replacements are actually applied. + # Yes, even in the stupid edge case where the root derivation itself is replaced. + storePathOrKnownDerivationMemo = + mapAttrs ( + drv: _references: + # builtins.storePath does not work in pure evaluation mode, even though it is not impure. + # This reimplementation in Nix works as long as the path is already allowed in the evaluation state. + # This is always the case here, because all paths come from the closure of the original derivation. + appendContext drv { ${drv}.path = true; } + ) relevantReferences + // listToAttrs ( + map (drv: { + name = toContextlessString drv; + value = drv; + }) knownDerivations + ); + + relevantReplacements = filter ( + { oldDependency, newDependency }: + if toString oldDependency == toString newDependency then + warn "replaceDependencies: attempting to replace dependency ${oldDependency} of ${drv} with itself" + # Attempting to replace a dependency by itself is completely useless, and would only lead to infinite recursion. + # Hence it must not be attempted to apply this replacement in any case. + false + else if !hasAttr (toContextlessString oldDependency) referencesMemo.${toContextlessString drv} then + warn "replaceDependencies: ${drv} does not depend on ${oldDependency}" + # Handle the corner case where one of the other replacements introduces the dependency. + # It would be more correct to not show the warning in this case, but the added complexity is probably not worth it. + true + else + true + ) replacements; + + rewriteMemo = + # Mind the order of how the three attrsets are merged here. + # The order of precedence needs to be "explicitly specified replacements" > "rewrite exclusion (cutoffPackages)" > "rewrite". + # So the attrset merge order is the opposite. + mapAttrs ( + drv: references: + let + rewrittenReferences = filter (dep: dep != drv && toString rewriteMemo.${dep} != dep) references; + rewrites = listToAttrs ( + map (reference: { + name = reference; + value = rewriteMemo.${reference}; + }) rewrittenReferences + ); + in + rewriteHashes storePathOrKnownDerivationMemo.${drv} rewrites + ) relevantReferences + // listToAttrs ( + map (drv: { + name = toContextlessString drv; + value = storePathOrKnownDerivationMemo.${toContextlessString drv}; + }) cutoffPackages + ) + // listToAttrs ( + map ( + { oldDependency, newDependency }: + { + name = toContextlessString oldDependency; + value = rewriteMemo.${toContextlessString newDependency}; + } + ) relevantReplacements + ); +in +assert all ( + { oldDependency, newDependency }: stringLength oldDependency == stringLength newDependency +) replacements; +rewriteMemo.${toContextlessString drv} diff --git a/pkgs/build-support/replace-dependency.nix b/pkgs/build-support/replace-dependency.nix deleted file mode 100644 index 7912d21bfd692..0000000000000 --- a/pkgs/build-support/replace-dependency.nix +++ /dev/null @@ -1,94 +0,0 @@ -{ runCommandLocal, nix, lib }: - -# Replace a single dependency in the requisites tree of drv, propagating -# the change all the way up the tree, without a full rebuild. This can be -# useful, for example, to patch a security hole in libc and still use your -# system safely without rebuilding the world. This should be a short term -# solution, as soon as a rebuild can be done the properly rebuild derivation -# should be used. The old dependency and new dependency MUST have the same-length -# name, and ideally should have close-to-identical directory layout. -# -# Example: safeFirefox = replaceDependency { -# drv = firefox; -# oldDependency = glibc; -# newDependency = overrideDerivation glibc (attrs: { -# patches = attrs.patches ++ [ ./fix-glibc-hole.patch ]; -# }); -# }; -# This will rebuild glibc with your security patch, then copy over firefox -# (and all of its dependencies) without rebuilding further. -{ drv, oldDependency, newDependency, verbose ? true }: - -let - inherit (lib) - any - attrNames - concatStringsSep - elem - filter - filterAttrs - listToAttrs - mapAttrsToList - stringLength - substring - ; - - warn = if verbose then builtins.trace else (x: y: y); - references = import (runCommandLocal "references.nix" { exportReferencesGraph = [ "graph" drv ]; } '' - (echo { - while read path - do - echo " \"$path\" = [" - read count - read count - while [ "0" != "$count" ] - do - read ref_path - if [ "$ref_path" != "$path" ] - then - echo " (builtins.storePath (/. + \"$ref_path\"))" - fi - count=$(($count - 1)) - done - echo " ];" - done < graph - echo }) > $out - '').outPath; - - discard = builtins.unsafeDiscardStringContext; - - oldStorepath = builtins.storePath (discard (toString oldDependency)); - - referencesOf = drv: references.${discard (toString drv)}; - - dependsOnOldMemo = listToAttrs (map - (drv: { name = discard (toString drv); - value = elem oldStorepath (referencesOf drv) || - any dependsOnOld (referencesOf drv); - }) (attrNames references)); - - dependsOnOld = drv: dependsOnOldMemo.${discard (toString drv)}; - - drvName = drv: - discard (substring 33 (stringLength (builtins.baseNameOf drv)) (builtins.baseNameOf drv)); - - rewriteHashes = drv: hashes: runCommandLocal (drvName drv) { nixStore = "${nix.out}/bin/nix-store"; } '' - $nixStore --dump ${drv} | sed 's|${baseNameOf drv}|'$(basename $out)'|g' | sed -e ${ - concatStringsSep " -e " (mapAttrsToList (name: value: - "'s|${baseNameOf name}|${baseNameOf value}|g'" - ) hashes) - } | $nixStore --restore $out - ''; - - rewrittenDeps = listToAttrs [ {name = discard (toString oldDependency); value = newDependency;} ]; - - rewriteMemo = listToAttrs (map - (drv: { name = discard (toString drv); - value = rewriteHashes (builtins.storePath drv) - (filterAttrs (n: v: elem (builtins.storePath (discard (toString n))) (referencesOf drv)) rewriteMemo); - }) - (filter dependsOnOld (attrNames references))) // rewrittenDeps; - - drvHash = discard (toString drv); -in assert (stringLength (drvName (toString oldDependency)) == stringLength (drvName (toString newDependency))); -rewriteMemo.${drvHash} or (warn "replace-dependency.nix: Derivation ${drvHash} does not depend on ${discard (toString oldDependency)}" drv) diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix index 587c69d8ed058..646c4273f4ed7 100644 --- a/pkgs/top-level/all-packages.nix +++ b/pkgs/top-level/all-packages.nix @@ -1306,7 +1306,16 @@ with pkgs; substituteAllFiles = callPackage ../build-support/substitute-files/substitute-all-files.nix { }; - replaceDependency = callPackage ../build-support/replace-dependency.nix { }; + replaceDependencies = callPackage ../build-support/replace-dependencies.nix { }; + + replaceDependency = { drv, oldDependency, newDependency, verbose ? true }: replaceDependencies { + inherit drv verbose; + replacements = [{ + inherit oldDependency newDependency; + }]; + # When newDependency depends on drv, instead of causing infinite recursion, keep it as is. + cutoffPackages = [ newDependency ]; + }; replaceVars = callPackage ../build-support/replace-vars { }; From 1dc5fd5306d9b9ff6ecdf85e25763150822d647d Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sun, 24 Sep 2023 20:52:50 +0200 Subject: [PATCH 2/8] nixos/top-level: improve replaceRuntimeDependencies Instead of iterating over all replacements and applying them one by one, use the newly introduced replaceDependencies function to apply them all at once for replaceRuntimeDependencies. The advantages are twofold in case there are multiple replacements: * Performance is significantly improved, because there is only one pass over the closure to be made. * Correctness is improved, because replaceDependencies also replaces dependencies of the replacements themselves if applicable. Fixes: https://github.com/NixOS/nixpkgs/issues/4336 --- nixos/modules/system/activation/top-level.nix | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/nixos/modules/system/activation/top-level.nix b/nixos/modules/system/activation/top-level.nix index ed0ece19f2fa2..95c4782ebd9e9 100644 --- a/nixos/modules/system/activation/top-level.nix +++ b/nixos/modules/system/activation/top-level.nix @@ -68,9 +68,17 @@ let else showWarnings config.warnings baseSystem; # Replace runtime dependencies - system = foldr ({ oldDependency, newDependency }: drv: - pkgs.replaceDependency { inherit oldDependency newDependency drv; } - ) baseSystemAssertWarn config.system.replaceRuntimeDependencies; + system = let replacements = config.system.replaceRuntimeDependencies; in + if replacements == [] then + # Avoid IFD if possible, by sidestepping replaceDependencies if no replacements are specified. + baseSystemAssertWarn + else + (pkgs.replaceDependencies.override { + nix = config.nix.package; + }) { + drv = baseSystemAssertWarn; + inherit replacements; + }; systemWithBuildDeps = system.overrideAttrs (o: { systemBuildClosure = pkgs.closureInfo { rootPaths = [ system.drvPath ]; }; From 2ab2aedc8b2e5bad7e15def269ab681208a210e3 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sat, 14 Oct 2023 18:22:08 +0200 Subject: [PATCH 3/8] nixos/top-level: wire up cutoffPackages for replaceDependencies Move replaceRuntimeDependencies to the replaceDependencies namespace, where the structure is more consistent with the replaceDependencies function. This makes space for wiring up cutoffPackages as an option too. By default, the system's initrd is excluded. The replacement process does not work properly anyway due to the structure of the initrd (the files being copied into it, and it being compressed). In the worst case (which has been observed to actually occur in practice), a store path makes it into the incompressible parts of the archive, checksums are broken, and the system won't boot. --- nixos/modules/system/activation/top-level.nix | 71 ++++++++++++------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/nixos/modules/system/activation/top-level.nix b/nixos/modules/system/activation/top-level.nix index 95c4782ebd9e9..1fdf4c3883da9 100644 --- a/nixos/modules/system/activation/top-level.nix +++ b/nixos/modules/system/activation/top-level.nix @@ -68,7 +68,7 @@ let else showWarnings config.warnings baseSystem; # Replace runtime dependencies - system = let replacements = config.system.replaceRuntimeDependencies; in + system = let inherit (config.system.replaceDependencies) replacements cutoffPackages; in if replacements == [] then # Avoid IFD if possible, by sidestepping replaceDependencies if no replacements are specified. baseSystemAssertWarn @@ -77,7 +77,7 @@ let nix = config.nix.package; }) { drv = baseSystemAssertWarn; - inherit replacements; + inherit replacements cutoffPackages; }; systemWithBuildDeps = system.overrideAttrs (o: { @@ -95,6 +95,7 @@ in (mkRemovedOptionModule [ "nesting" "clone" ] "Use `specialisation.«name» = { inheritParentConfig = true; configuration = { ... }; }` instead.") (mkRemovedOptionModule [ "nesting" "children" ] "Use `specialisation.«name».configuration = { ... }` instead.") (mkRenamedOptionModule [ "system" "forbiddenDependenciesRegex" ] [ "system" "forbiddenDependenciesRegexes" ]) + (mkRenamedOptionModule [ "system" "replaceRuntimeDependencies" ] [ "system" "replaceDependencies" "replacements" ]) ]; options = { @@ -213,31 +214,47 @@ in ''; }; - system.replaceRuntimeDependencies = mkOption { - default = []; - example = lib.literalExpression "[ ({ original = pkgs.openssl; replacement = pkgs.callPackage /path/to/openssl { }; }) ]"; - type = types.listOf (types.submodule ( - { ... }: { - options.original = mkOption { - type = types.package; - description = "The original package to override."; - }; - - options.replacement = mkOption { - type = types.package; - description = "The replacement package."; - }; - }) - ); - apply = map ({ original, replacement, ... }: { - oldDependency = original; - newDependency = replacement; - }); - description = '' - List of packages to override without doing a full rebuild. - The original derivation and replacement derivation must have the same - name length, and ideally should have close-to-identical directory layout. - ''; + system.replaceDependencies = { + replacements = mkOption { + default = []; + example = lib.literalExpression "[ ({ oldDependency = pkgs.openssl; newDependency = pkgs.callPackage /path/to/openssl { }; }) ]"; + type = types.listOf (types.submodule ( + { ... }: { + imports = [ + (mkRenamedOptionModule [ "original" ] [ "oldDependency" ]) + (mkRenamedOptionModule [ "replacement" ] [ "newDependency" ]) + ]; + + options.oldDependency = mkOption { + type = types.package; + description = "The original package to override."; + }; + + options.newDependency = mkOption { + type = types.package; + description = "The replacement package."; + }; + }) + ); + apply = map ({ oldDependency, newDependency, ... }: { + inherit oldDependency newDependency; + }); + description = '' + List of packages to override without doing a full rebuild. + The original derivation and replacement derivation must have the same + name length, and ideally should have close-to-identical directory layout. + ''; + }; + + cutoffPackages = mkOption { + default = [ config.system.build.initialRamdisk ]; + defaultText = literalExpression "[ config.system.build.initialRamdisk ]"; + type = types.listOf types.package; + description = '' + Packages to which no replacements should be applied. + The initrd is matched by default, because its structure renders the replacement process ineffective and prone to breakage. + ''; + }; }; system.name = mkOption { From 9846cc679a34bae8b77de5ba6ed2426bcc411c24 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sat, 16 Dec 2023 19:02:17 +0100 Subject: [PATCH 4/8] replaceDependencies: add tests The tests cannot be directly built by Hydra, because replaceDependencies relies on IFD. Instead, they are put inside a NixOS test where they are built on the guest. --- nixos/tests/all-tests.nix | 1 + nixos/tests/replace-dependencies/default.nix | 18 +++ nixos/tests/replace-dependencies/guest.nix | 132 +++++++++++++++++++ 3 files changed, 151 insertions(+) create mode 100644 nixos/tests/replace-dependencies/default.nix create mode 100644 nixos/tests/replace-dependencies/guest.nix diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix index bcbc439168104..d47ba85cf042e 100644 --- a/nixos/tests/all-tests.nix +++ b/nixos/tests/all-tests.nix @@ -854,6 +854,7 @@ in { redlib = handleTest ./redlib.nix {}; redmine = handleTest ./redmine.nix {}; renovate = handleTest ./renovate.nix {}; + replace-dependencies = handleTest ./replace-dependencies {}; restartByActivationScript = handleTest ./restart-by-activation-script.nix {}; restic-rest-server = handleTest ./restic-rest-server.nix {}; restic = handleTest ./restic.nix {}; diff --git a/nixos/tests/replace-dependencies/default.nix b/nixos/tests/replace-dependencies/default.nix new file mode 100644 index 0000000000000..822b0fbecade7 --- /dev/null +++ b/nixos/tests/replace-dependencies/default.nix @@ -0,0 +1,18 @@ +import ../make-test-python.nix ( + { pkgs, ... }: + { + name = "replace-dependencies"; + meta.maintainers = with pkgs.lib.maintainers; [ alois31 ]; + + nodes.machine = + { ... }: + { + system.extraDependencies = [ pkgs.stdenvNoCC ]; + }; + + testScript = '' + start_all() + machine.succeed("nix-build --option substitute false ${pkgs.path}/nixos/tests/replace-dependencies/guest.nix") + ''; + } +) diff --git a/nixos/tests/replace-dependencies/guest.nix b/nixos/tests/replace-dependencies/guest.nix new file mode 100644 index 0000000000000..32376e0a56368 --- /dev/null +++ b/nixos/tests/replace-dependencies/guest.nix @@ -0,0 +1,132 @@ +# This needs to be run in a NixOS test, because Hydra cannot do IFD. +let + pkgs = import ../../.. { }; + inherit (pkgs) + runCommand + writeShellScriptBin + replaceDependency + replaceDependencies + ; + inherit (pkgs.lib) escapeShellArg; + mkCheckOutput = + name: test: output: + runCommand name { } '' + actualOutput="$(${escapeShellArg "${test}/bin/test"})" + if [ "$(${escapeShellArg "${test}/bin/test"})" != ${escapeShellArg output} ]; then + echo >&2 "mismatched output: expected \""${escapeShellArg output}"\", got \"$actualOutput\"" + exit 1 + fi + touch "$out" + ''; + oldDependency = writeShellScriptBin "dependency" '' + echo "got old dependency" + ''; + newDependency = writeShellScriptBin "dependency" '' + echo "got new dependency" + ''; + basic = writeShellScriptBin "test" '' + ${oldDependency}/bin/dependency + ''; + transitive = writeShellScriptBin "test" '' + ${basic}/bin/test + ''; + weirdDependency = writeShellScriptBin "dependency" '' + echo "got weird dependency" + ${basic}/bin/test + ''; + oldDependency1 = writeShellScriptBin "dependency1" '' + echo "got old dependency 1" + ''; + newDependency1 = writeShellScriptBin "dependency1" '' + echo "got new dependency 1" + ''; + oldDependency2 = writeShellScriptBin "dependency2" '' + ${oldDependency1}/bin/dependency1 + echo "got old dependency 2" + ''; + newDependency2 = writeShellScriptBin "dependency2" '' + ${oldDependency1}/bin/dependency1 + echo "got new dependency 2" + ''; + deep = writeShellScriptBin "test" '' + ${oldDependency2}/bin/dependency2 + ''; +in +{ + replacedependency-basic = mkCheckOutput "replacedependency-basic" (replaceDependency { + drv = basic; + inherit oldDependency newDependency; + }) "got new dependency"; + + replacedependency-transitive = mkCheckOutput "replacedependency-transitive" (replaceDependency { + drv = transitive; + inherit oldDependency newDependency; + }) "got new dependency"; + + replacedependency-weird = + mkCheckOutput "replacedependency-weird" + (replaceDependency { + drv = basic; + inherit oldDependency; + newDependency = weirdDependency; + }) + '' + got weird dependency + got old dependency''; + + replacedependencies-precedence = mkCheckOutput "replacedependencies-precedence" (replaceDependencies + { + drv = basic; + replacements = [ { inherit oldDependency newDependency; } ]; + cutoffPackages = [ oldDependency ]; + } + ) "got new dependency"; + + replacedependencies-self = mkCheckOutput "replacedependencies-self" (replaceDependencies { + drv = basic; + replacements = [ + { + inherit oldDependency; + newDependency = oldDependency; + } + ]; + }) "got old dependency"; + + replacedependencies-deep-order1 = + mkCheckOutput "replacedependencies-deep-order1" + (replaceDependencies { + drv = deep; + replacements = [ + { + oldDependency = oldDependency1; + newDependency = newDependency1; + } + { + oldDependency = oldDependency2; + newDependency = newDependency2; + } + ]; + }) + '' + got new dependency 1 + got new dependency 2''; + + replacedependencies-deep-order2 = + mkCheckOutput "replacedependencies-deep-order2" + (replaceDependencies { + drv = deep; + replacements = [ + { + oldDependency = oldDependency2; + newDependency = newDependency2; + } + { + oldDependency = oldDependency1; + newDependency = newDependency1; + } + ]; + }) + '' + got new dependency 1 + got new dependency 2''; +} From ef100396b24c406db0f23833550bb6aedbaa2724 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Fri, 12 Jan 2024 10:14:38 +0100 Subject: [PATCH 5/8] replaceDirectDependencies: split off from replaceDependencies This allows both swapping out and reusing the rewrite machinery. --- nixos/modules/system/activation/top-level.nix | 4 ++- pkgs/build-support/replace-dependencies.nix | 31 ++++++------------- .../replace-direct-dependencies.nix | 25 +++++++++++++++ pkgs/top-level/all-packages.nix | 2 ++ 4 files changed, 40 insertions(+), 22 deletions(-) create mode 100644 pkgs/build-support/replace-direct-dependencies.nix diff --git a/nixos/modules/system/activation/top-level.nix b/nixos/modules/system/activation/top-level.nix index 1fdf4c3883da9..1b0a62c2e8e7d 100644 --- a/nixos/modules/system/activation/top-level.nix +++ b/nixos/modules/system/activation/top-level.nix @@ -74,7 +74,9 @@ let baseSystemAssertWarn else (pkgs.replaceDependencies.override { - nix = config.nix.package; + replaceDirectDependencies = pkgs.replaceDirectDependencies.override { + nix = config.nix.package; + }; }) { drv = baseSystemAssertWarn; inherit replacements cutoffPackages; diff --git a/pkgs/build-support/replace-dependencies.nix b/pkgs/build-support/replace-dependencies.nix index 2af6d19c268db..af0a2c05d6239 100644 --- a/pkgs/build-support/replace-dependencies.nix +++ b/pkgs/build-support/replace-dependencies.nix @@ -1,7 +1,7 @@ { - runCommandLocal, - nix, lib, + runCommandLocal, + replaceDirectDependencies, }: # Replace some dependencies in the requisites tree of drv, propagating the change all the way up the tree, even within other replacements, without a full rebuild. @@ -43,15 +43,13 @@ let inherit (builtins) unsafeDiscardStringContext appendContext; inherit (lib) trace - substring stringLength - concatStringsSep - mapAttrsToList listToAttrs attrValues mapAttrs filter hasAttr + mapAttrsToList all ; inherit (lib.attrsets) mergeAttrsList; @@ -90,21 +88,6 @@ let echo }) > $out '' ).outPath; - rewriteHashes = - drv: rewrites: - if rewrites == { } then - drv - else - let - drvName = substring 33 (stringLength (baseNameOf drv)) (baseNameOf drv); - in - runCommandLocal drvName { nixStore = "${nix}/bin/nix-store"; } '' - $nixStore --dump ${drv} | sed 's|${baseNameOf drv}|'$(basename $out)'|g' | sed -e ${ - concatStringsSep " -e " ( - mapAttrsToList (name: value: "'s|${baseNameOf name}|${baseNameOf value}|g'") rewrites - ) - } | $nixStore --restore $out - ''; knownDerivations = [ drv ] ++ map ({ newDependency, ... }: newDependency) replacements; referencesMemo = listToAttrs ( @@ -162,7 +145,13 @@ let }) rewrittenReferences ); in - rewriteHashes storePathOrKnownDerivationMemo.${drv} rewrites + replaceDirectDependencies { + drv = storePathOrKnownDerivationMemo.${drv}; + replacements = mapAttrsToList (name: value: { + oldDependency = name; + newDependency = value; + }) rewrites; + } ) relevantReferences // listToAttrs ( map (drv: { diff --git a/pkgs/build-support/replace-direct-dependencies.nix b/pkgs/build-support/replace-direct-dependencies.nix new file mode 100644 index 0000000000000..b579a097c442d --- /dev/null +++ b/pkgs/build-support/replace-direct-dependencies.nix @@ -0,0 +1,25 @@ +{ + lib, + runCommandLocal, + nix, +}: + +# Replace some direct dependencies of drv, not recursing into the dependency tree. +# You likely want to use replaceDependencies instead, unless you plan to implement your own recursion mechanism. +{ drv, replacements ? [ ] }: +let inherit (lib) all stringLength substring concatStringsSep; +in assert all ({ oldDependency, newDependency }: + stringLength oldDependency == stringLength newDependency) replacements; +if replacements == [ ] then + drv +else + let drvName = substring 33 (stringLength (baseNameOf drv)) (baseNameOf drv); + in runCommandLocal drvName { nixStore = "${nix}/bin/nix-store"; } '' + $nixStore --dump ${drv} | sed 's|${ + baseNameOf drv + }|'$(basename $out)'|g' | sed -e ${ + concatStringsSep " -e " (map ({ oldDependency, newDependency }: + "'s|${baseNameOf oldDependency}|${baseNameOf newDependency}|g'") + replacements) + } | $nixStore --restore $out + '' diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix index 646c4273f4ed7..714d4623d25aa 100644 --- a/pkgs/top-level/all-packages.nix +++ b/pkgs/top-level/all-packages.nix @@ -1319,6 +1319,8 @@ with pkgs; replaceVars = callPackage ../build-support/replace-vars { }; + replaceDirectDependencies = callPackage ../build-support/replace-direct-dependencies.nix { }; + nukeReferences = callPackage ../build-support/nuke-references { inherit (darwin) signingUtils; }; From 3e2ff7f425107b2c07048818aa407144966457c0 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sat, 27 Jan 2024 11:59:26 +0100 Subject: [PATCH 6/8] replaceDependencies: add support for ca-derivations Unlike regular input-addressed or fixed-output derivations, floating and deferred derivations do not have their store path available at evaluation time, so their outPath is a placeholder. The following changes are needed for replaceDependencies to continue working: * Detect the placeholder and retrieve the store path using another IFD hack when collecting the rewrite plan. * Try to obtain the derivation name needed for replaceDirectDependencies from the derivation arguments if a placeholder is detected. * Move the length mismatch detection to build time, since the placeholder has a fixed length which is unrelated to the store path. --- nixos/tests/replace-dependencies/default.nix | 1 + nixos/tests/replace-dependencies/guest.nix | 17 +++++ pkgs/build-support/replace-dependencies.nix | 54 +++++++++----- .../replace-direct-dependencies.nix | 73 +++++++++++++++---- 4 files changed, 114 insertions(+), 31 deletions(-) diff --git a/nixos/tests/replace-dependencies/default.nix b/nixos/tests/replace-dependencies/default.nix index 822b0fbecade7..ce0013a868c7a 100644 --- a/nixos/tests/replace-dependencies/default.nix +++ b/nixos/tests/replace-dependencies/default.nix @@ -7,6 +7,7 @@ import ../make-test-python.nix ( nodes.machine = { ... }: { + nix.settings.experimental-features = [ "ca-derivations" ]; system.extraDependencies = [ pkgs.stdenvNoCC ]; }; diff --git a/nixos/tests/replace-dependencies/guest.nix b/nixos/tests/replace-dependencies/guest.nix index 32376e0a56368..f022f17285995 100644 --- a/nixos/tests/replace-dependencies/guest.nix +++ b/nixos/tests/replace-dependencies/guest.nix @@ -21,12 +21,17 @@ let oldDependency = writeShellScriptBin "dependency" '' echo "got old dependency" ''; + oldDependency-ca = oldDependency.overrideAttrs { __contentAddressed = true; }; newDependency = writeShellScriptBin "dependency" '' echo "got new dependency" ''; + newDependency-ca = newDependency.overrideAttrs { __contentAddressed = true; }; basic = writeShellScriptBin "test" '' ${oldDependency}/bin/dependency ''; + basic-ca = writeShellScriptBin "test" '' + ${oldDependency-ca}/bin/dependency + ''; transitive = writeShellScriptBin "test" '' ${basic}/bin/test ''; @@ -58,6 +63,18 @@ in inherit oldDependency newDependency; }) "got new dependency"; + replacedependency-basic-old-ca = mkCheckOutput "replacedependency-basic" (replaceDependency { + drv = basic-ca; + oldDependency = oldDependency-ca; + inherit newDependency; + }) "got new dependency"; + + replacedependency-basic-new-ca = mkCheckOutput "replacedependency-basic" (replaceDependency { + drv = basic; + inherit oldDependency; + newDependency = newDependency-ca; + }) "got new dependency"; + replacedependency-transitive = mkCheckOutput "replacedependency-transitive" (replaceDependency { drv = transitive; inherit oldDependency newDependency; diff --git a/pkgs/build-support/replace-dependencies.nix b/pkgs/build-support/replace-dependencies.nix index af0a2c05d6239..c276b3e09e49c 100644 --- a/pkgs/build-support/replace-dependencies.nix +++ b/pkgs/build-support/replace-dependencies.nix @@ -43,14 +43,14 @@ let inherit (builtins) unsafeDiscardStringContext appendContext; inherit (lib) trace - stringLength listToAttrs + isStorePath + readFile attrValues mapAttrs filter hasAttr mapAttrsToList - all ; inherit (lib.attrsets) mergeAttrsList; @@ -89,17 +89,38 @@ let '' ).outPath; - knownDerivations = [ drv ] ++ map ({ newDependency, ... }: newDependency) replacements; + targetDerivations = [ drv ] ++ map ({ newDependency, ... }: newDependency) replacements; + realisation = + drv: + if isStorePath drv then + # Input-addressed and fixed-output derivations have their realisation as outPath. + toContextlessString drv + else + # Floating and deferred derivations have a placeholder outPath. + # The realisation can only be obtained by performing an actual build. + unsafeDiscardStringContext ( + readFile ( + runCommandLocal "realisation" + { + env = { + inherit drv; + }; + } + '' + echo -n "$drv" > $out + '' + ) + ); referencesMemo = listToAttrs ( map (drv: { - name = toContextlessString drv; + name = realisation drv; value = referencesOf drv; - }) knownDerivations + }) targetDerivations ); relevantReferences = mergeAttrsList (attrValues referencesMemo); # Make sure a derivation is returned even when no replacements are actually applied. # Yes, even in the stupid edge case where the root derivation itself is replaced. - storePathOrKnownDerivationMemo = + storePathOrKnownTargetDerivationMemo = mapAttrs ( drv: _references: # builtins.storePath does not work in pure evaluation mode, even though it is not impure. @@ -109,9 +130,9 @@ let ) relevantReferences // listToAttrs ( map (drv: { - name = toContextlessString drv; + name = realisation drv; value = drv; - }) knownDerivations + }) targetDerivations ); relevantReplacements = filter ( @@ -121,7 +142,7 @@ let # Attempting to replace a dependency by itself is completely useless, and would only lead to infinite recursion. # Hence it must not be attempted to apply this replacement in any case. false - else if !hasAttr (toContextlessString oldDependency) referencesMemo.${toContextlessString drv} then + else if !hasAttr (realisation oldDependency) referencesMemo.${realisation drv} then warn "replaceDependencies: ${drv} does not depend on ${oldDependency}" # Handle the corner case where one of the other replacements introduces the dependency. # It would be more correct to not show the warning in this case, but the added complexity is probably not worth it. @@ -146,7 +167,7 @@ let ); in replaceDirectDependencies { - drv = storePathOrKnownDerivationMemo.${drv}; + drv = storePathOrKnownTargetDerivationMemo.${drv}; replacements = mapAttrsToList (name: value: { oldDependency = name; newDependency = value; @@ -155,21 +176,18 @@ let ) relevantReferences // listToAttrs ( map (drv: { - name = toContextlessString drv; - value = storePathOrKnownDerivationMemo.${toContextlessString drv}; + name = realisation drv; + value = storePathOrKnownTargetDerivationMemo.${realisation drv}; }) cutoffPackages ) // listToAttrs ( map ( { oldDependency, newDependency }: { - name = toContextlessString oldDependency; - value = rewriteMemo.${toContextlessString newDependency}; + name = realisation oldDependency; + value = rewriteMemo.${realisation newDependency}; } ) relevantReplacements ); in -assert all ( - { oldDependency, newDependency }: stringLength oldDependency == stringLength newDependency -) replacements; -rewriteMemo.${toContextlessString drv} +rewriteMemo.${realisation drv} diff --git a/pkgs/build-support/replace-direct-dependencies.nix b/pkgs/build-support/replace-direct-dependencies.nix index b579a097c442d..57036ebd74d14 100644 --- a/pkgs/build-support/replace-direct-dependencies.nix +++ b/pkgs/build-support/replace-direct-dependencies.nix @@ -6,20 +6,67 @@ # Replace some direct dependencies of drv, not recursing into the dependency tree. # You likely want to use replaceDependencies instead, unless you plan to implement your own recursion mechanism. -{ drv, replacements ? [ ] }: -let inherit (lib) all stringLength substring concatStringsSep; -in assert all ({ oldDependency, newDependency }: - stringLength oldDependency == stringLength newDependency) replacements; +{ + drv, + replacements ? [ ], +}: +let + inherit (lib) + isStorePath + substring + stringLength + optionalString + escapeShellArgs + concatMap + ; +in if replacements == [ ] then drv else - let drvName = substring 33 (stringLength (baseNameOf drv)) (baseNameOf drv); - in runCommandLocal drvName { nixStore = "${nix}/bin/nix-store"; } '' - $nixStore --dump ${drv} | sed 's|${ - baseNameOf drv - }|'$(basename $out)'|g' | sed -e ${ - concatStringsSep " -e " (map ({ oldDependency, newDependency }: - "'s|${baseNameOf oldDependency}|${baseNameOf newDependency}|g'") - replacements) - } | $nixStore --restore $out + let + drvName = + if isStorePath drv then + # Reconstruct the name from the actual store path if available. + substring 33 (stringLength (baseNameOf drv)) (baseNameOf drv) + else if drv ? drvAttrs.name then + # Try to get the name from the derivation arguments otherwise (for floating or deferred derivations). + drv.drvAttrs.name + + ( + let + outputName = drv.outputName or "out"; + in + optionalString (outputName != "out") "-${outputName}" + ) + else + throw "cannot reconstruct the derivation name from ${drv}"; + in + runCommandLocal drvName { nativeBuildInputs = [ nix.out ]; } '' + createRewriteScript() { + while [ $# -ne 0 ]; do + oldBasename="$(basename "$1")" + newBasename="$(basename "$2")" + shift 2 + if [ ''${#oldBasename} -ne ''${#newBasename} ]; then + echo "cannot rewrite $oldBasename to $newBasename: length does not match" >&2 + exit 1 + fi + echo "s|$oldBasename|$newBasename|g" >> rewrite.sed + done + } + createRewriteScript ${ + escapeShellArgs ( + [ + drv + (placeholder "out") + ] + ++ concatMap ( + { oldDependency, newDependency }: + [ + oldDependency + newDependency + ] + ) replacements + ) + } + nix-store --dump ${drv} | sed -f rewrite.sed | nix-store --restore $out '' From 3b78fd146a606dbe5d5f8a340c587edb7af249ab Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Fri, 7 Jun 2024 18:35:36 +0200 Subject: [PATCH 7/8] replaceDependencies: show warnings as such --- pkgs/build-support/replace-dependencies.nix | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkgs/build-support/replace-dependencies.nix b/pkgs/build-support/replace-dependencies.nix index c276b3e09e49c..780ede9d019ad 100644 --- a/pkgs/build-support/replace-dependencies.nix +++ b/pkgs/build-support/replace-dependencies.nix @@ -42,7 +42,6 @@ let inherit (builtins) unsafeDiscardStringContext appendContext; inherit (lib) - trace listToAttrs isStorePath readFile @@ -55,7 +54,7 @@ let inherit (lib.attrsets) mergeAttrsList; toContextlessString = x: unsafeDiscardStringContext (toString x); - warn = if verbose then trace else (x: y: y); + warn = if verbose then lib.warn else (x: y: y); referencesOf = drv: From 8bbea7f8360803275238deead9fc1a0ce830106d Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Fri, 7 Jun 2024 18:52:45 +0200 Subject: [PATCH 8/8] replaceDependencies: do not build unused replacements To prevent excessive build times when replacement lists are shared between partially overlapping closures, skip the build of unused replacements. Unfortunately, this also means that the replacement won't be applied any more if another replacement that is applied introduces its source. But this is a corner case, and we already show a warning, so make it clearer that handling this situation (should it ever arise) is the responsibility of the user. --- pkgs/build-support/replace-dependencies.nix | 35 +++++++++++---------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/pkgs/build-support/replace-dependencies.nix b/pkgs/build-support/replace-dependencies.nix index 780ede9d019ad..fe325b175fe7b 100644 --- a/pkgs/build-support/replace-dependencies.nix +++ b/pkgs/build-support/replace-dependencies.nix @@ -88,7 +88,6 @@ let '' ).outPath; - targetDerivations = [ drv ] ++ map ({ newDependency, ... }: newDependency) replacements; realisation = drv: if isStorePath drv then @@ -110,6 +109,24 @@ let '' ) ); + rootReferences = referencesOf drv; + relevantReplacements = filter ( + { oldDependency, newDependency }: + if toString oldDependency == toString newDependency then + warn "replaceDependencies: attempting to replace dependency ${oldDependency} of ${drv} with itself" + # Attempting to replace a dependency by itself is completely useless, and would only lead to infinite recursion. + # Hence it must not be attempted to apply this replacement in any case. + false + else if !hasAttr (realisation oldDependency) rootReferences then + warn "replaceDependencies: ${drv} does not depend on ${oldDependency}, so it will not be replaced" + # Strictly speaking, another replacement could introduce the dependency. + # However, handling this corner case would add significant complexity. + # So we just leave it to the user to apply the replacement at the correct place, but show a warning to let them know. + false + else + true + ) replacements; + targetDerivations = [ drv ] ++ map ({ newDependency, ... }: newDependency) relevantReplacements; referencesMemo = listToAttrs ( map (drv: { name = realisation drv; @@ -134,22 +151,6 @@ let }) targetDerivations ); - relevantReplacements = filter ( - { oldDependency, newDependency }: - if toString oldDependency == toString newDependency then - warn "replaceDependencies: attempting to replace dependency ${oldDependency} of ${drv} with itself" - # Attempting to replace a dependency by itself is completely useless, and would only lead to infinite recursion. - # Hence it must not be attempted to apply this replacement in any case. - false - else if !hasAttr (realisation oldDependency) referencesMemo.${realisation drv} then - warn "replaceDependencies: ${drv} does not depend on ${oldDependency}" - # Handle the corner case where one of the other replacements introduces the dependency. - # It would be more correct to not show the warning in this case, but the added complexity is probably not worth it. - true - else - true - ) replacements; - rewriteMemo = # Mind the order of how the three attrsets are merged here. # The order of precedence needs to be "explicitly specified replacements" > "rewrite exclusion (cutoffPackages)" > "rewrite".