Skip to content

Commit

Permalink
replaceDependencies: do not build unused replacements
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alois31 authored and Yureka committed Sep 24, 2024
1 parent 3b78fd1 commit 8bbea7f
Showing 1 changed file with 18 additions and 17 deletions.
35 changes: 18 additions & 17 deletions pkgs/build-support/replace-dependencies.nix
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ let
''
).outPath;

targetDerivations = [ drv ] ++ map ({ newDependency, ... }: newDependency) replacements;
realisation =
drv:
if isStorePath drv then
Expand All @@ -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;
Expand All @@ -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".
Expand Down

0 comments on commit 8bbea7f

Please sign in to comment.