ci/eval: don't evaluate packages marked as broken

[wolfgangwalther]

I took this out of #406825, which we realized needs more work. Still, the change to evaluation of broken packages is good to fix cases like #355847 (comment).

Since the first two commits of that PR kind of belong together for me, I squashed them. I also adjusted the commit message slightly, so that it doesn't refer to the "next commit" anymore, which would be the release-checks. Instead the reasoning is now more along the lines of fixing the above mentioned issue without causing regressions for unfree packages.

Things done

• Let's look at the eval results / "rebuilds" this produces.
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[wolfgangwalther]

Eval / Summary — Package: added 0, removed 3726, changed 0, Rebuild: linux 0, darwin 0

Did a couple of random checks: Packages marked as "removed" here are indeed broken (on at least on eval system). I cross-checked an unfree package, which did not appear in the "removed" list.

So this seems to do exactly what we want: If somebody marks a broken package as "not broken anymore", then this will appear as an "added" package - and thus cause rebuilds.

[FliegendeWurst]

We really can't expect packages that are marked as broken to evaluate, and especially not on unsupported platforms.

But isn't that what CI currently checks?

[wolfgangwalther]

We really can't expect packages that are marked as broken to evaluate, and especially not on unsupported platforms.

But isn't that what CI currently checks?

I'm not exactly sure what you're asking, but let me expand a bit on the topic:

We have two steps: Collecting attrpaths, and then evaluating those attrpaths.

Previously, before some recent changes, we would collect attrpaths for x86_64-linux only and then run eval on the 4 systems for them. Because packages were marked broken on some systems, but not on others, we had to collect broken packages during the attrpaths collection step - they could evaluate fine on one platform.

Currently, before this PR, we collect attrpaths separately for each system - and then evaluate those for that system. At this stage, we have not adjusted the broken logic, yet - so we are collecting broken packages, but they will 100% fail evaluation during the eval step (which we then happily ignore).

With this new change, broken packages are not even collected in the attrpath step, thus leading to this added/removed logic.

Does this answer it?

[FliegendeWurst]

Yes, then I think this change makes sense. (I was just wondering if there were some errors we would have caught previously, that could now be introduced.)

[wolfgangwalther]

I was just wondering if there were some errors we would have caught previously, that could now be introduced.

Yes, there are some, theoretically: That's when you try to build a broken package with NIXPKGS_ALLOW_BROKEN=1. There could be an eval error for that case, which I think we caught before this PR, but don't do so anymore.

This is a trade-off, and I think doing it the way this PR does it is better: Having an after-eval-broken-eval failure for a package, won't cause trouble with hydra or anything. But skipping rebuilds with nixpkgs-review for packages which have their broken attribute removed is really annoying.

[wolfgangwalther]

Hearing no objections, I'd say we'll try it this way. If this turns out to be wrong we can always adjust later.

[nixpkgs-ci]

Successfully created backport PR for release-25.05:

• [Backport release-25.05] ci/eval: don't evaluate packages marked as broken #411732