ci/eval/compare: ping maintainers of removed packages#438652
ci/eval/compare: ping maintainers of removed packages#438652wolfgangwalther merged 4 commits intoNixOS:masterfrom
Conversation
This change pings maintainers of actually removed packages, aka where the package's expression is deleted. This will not ping maintainers of packages that become invisible, because a (transitive) dependency of them is marked as insecure or broken.
This should not be necessary anymore, because packages that fail to evaluate should already be filtered out by the attrpath generation step in main eval.
It makes no sense to check newly added attrpaths for maintainers on the target branch - by definition these attrpaths won't exist, yet. We can avoid falling back to `null` for these etc.
Simplification after the last step.
nixpkgs-ci
bot
added
10.rebuild-darwin: 0
nix-owners
bot
requested review from
MattSturgeon,
Mic92,
philiptaron and
zowoq
MattSturgeon
left a comment
MattSturgeon
left a comment
There was a problem hiding this comment.
SGTM. Is it expected that we only tested prepare and not any of the pr.yml, push.yml, or merge-group.yml workflows?
|
Yes, the "Test" workflow only tests workflow files. That's because even if we ran it here, it would still use the nixpkgs/.github/workflows/eval.yml Lines 208 to 213 in 5965375 It will always be the target branch, because that's where the maintainer info needs to come from. Thus, this kind of change can't be tested in the PR alone, it must be tested in a fork. |
nixpkgs-ci
bot
added
the
12.approvals: 1
fabianhjr
left a comment
fabianhjr
left a comment
There was a problem hiding this comment.
Really like that it is both an devex improvement and a simplification of existing code
nixpkgs-ci
bot
added
12.approvals: 2
|
Successfully created backport PR for |
github-actions
bot
added
the
8.has: port to stable
|
Breaks in https://github.com/NixOS/nixpkgs/actions/runs/17377113706/job/49326131217, which is the staging-next PR. |
| validPackageAttributes = builtins.filter ( | ||
| pkg: | ||
| if (lib.attrsets.hasAttrByPath pkg.path pkgs) then | ||
| ( | ||
| let | ||
| value = lib.attrsets.attrByPath pkg.path null pkgs; | ||
| in | ||
| if (builtins.tryEval value).success then | ||
| if value != null then true else builtins.trace "${pkg.name} exists but is null" false | ||
| else | ||
| builtins.trace "Failed to access ${pkg.name} even though it exists" false | ||
| ) | ||
| else | ||
| builtins.trace "Failed to locate ${pkg.name}." false | ||
| ) enrichedAttrs; |
There was a problem hiding this comment.
This part did not only filter out packages that failed to evaluate, but also null values. Need to add that part back.
3 participants
This change pings maintainers of actually removed packages, aka where the package's expression is deleted.
This will not ping maintainers of packages that become invisible, because a (transitive) dependency of them is marked as insecure or broken.
Followed up by a few refactors that should work by now, some because of #426629.
Things done
Add a 👍 reaction to pull requests you find important.