ci/eval/compare: don't treat renames as rebuilds#431448
Merged
wolfgangwalther merged 1 commit intoNixOS:masterfrom Aug 6, 2025
Merged
ci/eval/compare: don't treat renames as rebuilds#431448wolfgangwalther merged 1 commit intoNixOS:masterfrom
wolfgangwalther merged 1 commit intoNixOS:masterfrom
Conversation
wolfgangwalther
force-pushed
the
ci-renamed-rebuilds
branch
2 times, most recently
from
28783b9 to
07015f8
Compare
nixpkgs-ci
bot
added
10.rebuild-linux: 501+
wolfgangwalther
force-pushed
the
ci-renamed-rebuilds
branch
3 times, most recently
from
47bdb4a to
24efeab
Compare
13 tasks
nix-owners
bot
requested review from
MattSturgeon,
Mic92,
philiptaron and
zowoq
nixpkgs-ci
bot
added
10.rebuild-darwin: 0
MattSturgeon
reviewed
Aug 6, 2025
wolfgangwalther
force-pushed
the
ci-renamed-rebuilds
branch
from
24efeab to
7f22594
Compare
wolfgangwalther
commented
Aug 6, 2025
When a package's attrpath is renamed it is currently treated as a rebuild, even though the outpath already exists and is already cached. This also happens when adding new names for packagesets that already exist, for example when starting to eval `perlPackages` in CI, which is just the same as `perl540Packages` currently. It would also happen when `perlPackages` is switched from `perl540Packages` to `perl999Packages`. Assuming that `perl999Packages` had already been built before, this doesn't really cause any rebuilds.
wolfgangwalther
force-pushed
the
ci-renamed-rebuilds
branch
from
7f22594 to
c5c7cd5
Compare
nixpkgs-ci
bot
added
the
12.approvals: 1
philiptaron
approved these changes
Aug 6, 2025
Contributor
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
I suspect there's a load of performance work we could do here, but it likely doesn't matter.
| old: new: | ||
| let | ||
| filterKeys = cond: attrs: lib.attrNames (lib.filterAttrs cond attrs); | ||
| oldOutputs = lib.pipe old [ |
Contributor
There was a problem hiding this comment.
This can be computed without respect to the new and hence can be in a let block before the new.
let
filterKeys = ...;
in
old:
let
oldOutputs = ...;
in
new:
Contributor
Author
There was a problem hiding this comment.
filterKeys was before the arguments before. Does any of this make a difference if this diff function is only ever called exactly once?
Contributor
There was a problem hiding this comment.
Since filterKeys is a function itself, I suspect it doesn't matter.
Comment on lines
+29
to
+31
| (lib.mapAttrsToList (_: lib.attrValues)) | ||
| lib.concatLists | ||
| (lib.flip lib.genAttrs (_: true)) |
Contributor
There was a problem hiding this comment.
My mind tells me there's an opportunity to use builtins.intersectAttrs here to good effect, but I'm too lazy to do the work needed to productionize it.
It also tells me that there's something in builtins.hasAttr that might be of use below but I'm not sure what exactly.
nixpkgs-ci
bot
added
12.approvals: 2
Contributor
|
Successfully created backport PR for |
github-actions
bot
added
the
8.has: port to stable
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a package's attrpath is renamed it is currently treated as a rebuild, even though the outpath already exists and is already cached.
This also happens when adding new names for packagesets that already exist, for example when starting to eval
perlPackagesin CI, which is just the same asperl540Packagescurrently. It would also happen whenperlPackagesis switched fromperl540Packagestoperl999Packages. Assuming thatperl999Packageshad already been built before, this doesn't really cause any rebuilds.Things done
perlPackages = {}inrelease.nix, should result in no rebuilds (only works in my fork, because the comparison changes need to be on the target branch already)Add a 👍 reaction to pull requests you find important.