From 965289e5e07243f1cde3212d8bcaf726d36c5c46 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Fri, 7 Jun 2024 18:52:45 +0200 Subject: [PATCH] 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 780ede9d019a..fe325b175fe7 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".