Skip to content

ci/eval/compare: don't treat renames as rebuilds #431448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wolfgangwalther merged 1 commit into from
Aug 6, 2025
Merged

ci/eval/compare: don't treat renames as rebuilds #431448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions ci/eval/compare/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,11 @@ let
;

# Attrs
# - keys: "added", "changed" and "removed"
# - keys: "added", "changed", "removed" and "rebuilds"
# - values: lists of `packagePlatformPath`s
diffAttrs = builtins.fromJSON (builtins.readFile "${combinedDir}/combined-diff.json");

rebuilds = diffAttrs.added ++ diffAttrs.changed;
rebuildsPackagePlatformAttrs = convertToPackagePlatformAttrs rebuilds;
rebuildsPackagePlatformAttrs = convertToPackagePlatformAttrs diffAttrs.rebuilds;

changed-paths =
let
Expand All @@ -90,7 +89,7 @@ let
in
writeText "changed-paths.json" (
builtins.toJSON {
attrdiff = lib.mapAttrs (_: extractPackageNames) diffAttrs;
attrdiff = lib.mapAttrs (_: extractPackageNames) { inherit (diffAttrs) added changed removed; };
inherit
rebuildsByPlatform
rebuildsByKernel
Expand Down
3 changes: 2 additions & 1 deletion ci/eval/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ let
reduce .[] as $item ({}; {
added: (.added + $item.added),
changed: (.changed + $item.changed),
removed: (.removed + $item.removed)
removed: (.removed + $item.removed),
rebuilds: (.rebuilds + $item.rebuilds)
})
' > $out/combined-diff.json

Expand Down
19 changes: 18 additions & 1 deletion ci/eval/diff.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,20 @@ let
added: [ <keys only in the second object> ],
removed: [ <keys only in the first object> ],
changed: [ <keys with different values between the two objects> ],
rebuilds: [ <keys in the second object with values not present at all in first object> ],
}
*/
diff =
old: new:
let
filterKeys = cond: attrs: lib.attrNames (lib.filterAttrs cond attrs);
oldOutputs = lib.pipe old [
Copy link
Contributor

@philiptaron philiptaron Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Contributor Author

@wolfgangwalther wolfgangwalther Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filterKeys was before the arguments before. Does any of this make a difference if this diff function is only ever called exactly once?

Copy link
Contributor

@philiptaron philiptaron Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since filterKeys is a function itself, I suspect it doesn't matter.

(lib.mapAttrsToList (_: lib.attrValues))
lib.concatLists
(lib.flip lib.genAttrs (_: true))
Comment on lines +29 to +31
Copy link
Contributor

@philiptaron philiptaron Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

];
in
old: new: {
{
added = filterKeys (n: _: !(old ? ${n})) new;
removed = filterKeys (n: _: !(new ? ${n})) old;
changed = filterKeys (
Expand All @@ -35,6 +42,16 @@ let
# Filter out attributes that are the same as the new value
&& (v != (new.${n}))
) old;
# A "rebuild" is every attrpath ...
rebuilds = filterKeys (
_: pkg:
# ... that has at least one output ...
lib.any (
output:
# ... which has not been built in "old" already.
!(oldOutputs ? ${output})
) (lib.attrValues pkg)
) new;
};

getAttrs =
Expand Down
Loading