ci/eval: fail on asserts when generating attrpaths #426629

wolfgangwalther · 2025-07-19T10:43:17Z

Work towards #397184.

TLDR of that issue: In the attrpaths generation step for Eval, we catch and ignore evaluation errors here:

nixpkgs/pkgs/top-level/release-attrpaths-superset.nix

Lines 99 to 107 in ab88976

    
           tried = builtins.tryEval seq; 
        
           result = 
        
             if tried.success then 
        
               tried.value 
        
             else if enableWarnings && path != [ "AAAAAASomeThingsFailToEvaluate" ] then 
        
               lib.warn "tryEval failed at: ${lib.concatStringsSep "." path}" [ ] 
        
             else 
        
               [ ];

We do this because a lot of asserts/throws are used to give feedback about invalid combinations of things for the current system, throw errors for non-accepted licenses etc. But that also means we silently ignore asserts/throws where they point at a real problem, for example when mkDerivation triggers an assert.

We can show these eval failures by turning on enableWarnings for release-attrpaths-superset.nix. This will give us:

48 attributes with warning for x86_64-linux
110 attributes with warning for aarch64-linux
6040 attributes with warning for x86_64-darwin
6041 attributes with warning for aarch64-darwin
(5898 of these darwin warnings are about the linux kernels)

This PR fixes only the warnings for x86_64-linux, and then disables the tryEval logic only on that platform. Arguably this gives us a lot already, because most attributes are evaluated on x86_64-linux, too, and most real eval failures will surface this way. Eventually, it would be good to get rid of all the warnings, though... ✅

We need to do the following things first:

Some throws about removals are just made aliases, which they should have been anyway. various: move throws to aliases #426639
Use meta.broken for some lua packages, instead of rolling their own disabled + throw. lua{54,Jit}Packages.lua-pam: mark as broken #426640
Fix an actual eval failure for flutterPackages.beta which tries to call lib.last on an empty list. flutterPackages.beta: fix eval #426641
Move certain license related check "behind" eval of meta. For these, the generic "unfree" error will now be thrown first, and CI will already filter them out because of that. This has a nice side-effect: These packages should also show up in the package search now, where they didn't before, because they'd fail evaluation. various: move license check "behind" meta.unfree eval #426642
Move some top-level attributes to different versions of the underlying package set (ocaml, elixir), so that they build again. {ocamlformat_0_26_2,elixir_1_{14_15_16}}: fix build #426643
Move some "incompatibility asserts" behind a check on config.allowAliases as mentioned in Eval check not catching certain eval errors #397184 (comment). various: avoid throwing for invalid platforms combinations in CI #426645

In here we also do:

Avoid generating attrpaths for "variants", here pkgsx86_64Darwin, which doesn't eval on Linux.

Things done

Built ci -A eval.attrpathsSuperset on platform:
- x86_64-linux
- aarch64-linux
- x86_64-darwin
- aarch64-darwin
Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and others READMEs.

Add a 👍 reaction to pull requests you find important.

philiptaron · 2025-07-19T20:54:32Z

Amazing work. I merged the pieces that I think are completely uncontroversial.

philiptaron

I don't love AAAAAASomeThingsFailToEvaluate but it's fine.

wolfgangwalther · 2025-07-24T10:30:14Z

Rebased to trigger CI once more and confirm that no other breakage has been introduced on master.

Given that there have been no objections other than keeping the AAAAAASomeThingsFailToEvaluate, I intend to merge this after CI passes.

The related issue still needs more work as pointed out above, but this will ensure that this first part doesn't regress.

wolfgangwalther · 2025-07-24T11:03:38Z

Not merging because of a regression in #427219 (comment).

Edit: Will wait for the channel to be unblocked in case there are any other eval errors hiding that require a revert of the linux commit.

This is already not used in-tree, so should be a variant.

This removes pkgsx86_64Darwin from the top-level attributes when generating attrpaths for evaluation on Linux. Needed for attrpaths generation to succeed without silently catching eval errors. Variants are still available during actual Eval for references from other packages, just not during attrpath generation. To remove it from Eval as well, issues around pkgsLLVM will have to be fixed first.

This doesn't fail on *all* asserts, yet, because nix-env still ignores these in the main eval step. But it already gives some converage during the attrpath generation.

MattSturgeon · 2025-07-24T14:07:38Z

Not merging because of a regression in #427219 (comment).

Is this resolved now with #428023 merged?

Will wait for the channel to be unblocked in case there are any other eval errors hiding that require a revert of the linux commit.

Is there an unrelated channel blocker that's preventing us from knowing if this PR is good now?

wolfgangwalther · 2025-07-24T14:13:34Z

Not merging because of a regression in #427219 (comment).

Is this resolved now with #428023 merged?

Correct.

Will wait for the channel to be unblocked in case there are any other eval errors hiding that require a revert of the linux commit.

Is there an unrelated channel blocker that's preventing us from knowing if this PR is good now?

I don't know of any other channel blocker, no. But it could be that hydra spits out another Eval error, at least in theory. That's because the one above was not noticed because of #426629 (comment) - aka the outpath-step silently catching those errors.

TLDR:

Before this PR, all asserts were silently caught.
After this PR, we catch some asserts, but not all.
Leading up to this PR we caused one known regression, where a new eval error appeared because of us removing an assert.

This PR in itself is not a problem and could be merged. It's just the case of: What if we get another eval error from one of the earlier changes, that we can't easily fix - and thus we want to revert, for example, the "remove the assert in the linux kernel" commit? Once we revert that, we must revert this, too.

Thus, I'd rather wait a bit here to make sure that all the previous changes made it through hydra once.

MattSturgeon · 2025-07-24T14:18:39Z

Thus, I'd rather wait a bit here to make sure that all the previous changes made it through hydra once.

Ah, I see. That makes sense.

It is also possible that a new regression will show up on master after the changes that we're waiting to see go through hydra but before the point at which we eventually merge this PR. However that seems unlikely, so I agree it's better to wait.

wolfgangwalther · 2025-07-24T19:35:39Z

Release checks, which previously failed, are good now: https://hydra.nixos.org/job/nixpkgs/trunk/unstable#tabs-constituents

The only two pending jobs (manual, metrics) I see there have passed before, so if they fail it's unrelated to us.

nixpkgs-ci · 2025-07-24T19:36:39Z

Successfully created backport PR for release-25.05:

[Backport release-25.05] ci/eval: fail on asserts when generating attrpaths #428141

wolfgangwalther requested review from emilazy, infinisil, winterqt and a team July 19, 2025 10:43

wolfgangwalther force-pushed the ci-eval-asserts branch 2 times, most recently from da3ad86 to d388542 Compare July 19, 2025 10:59

This was referenced Jul 19, 2025

various: move throws to aliases #426639

Merged

lua{54,Jit}Packages.lua-pam: mark as broken #426640

Merged

flutterPackages.beta: fix eval #426641

Merged

nix-owners bot requested a review from traxys July 19, 2025 11:10

This was referenced Jul 19, 2025

various: move license check "behind" meta.unfree eval #426642

Merged

{ocamlformat_0_26_2,elixir_1_{14_15_16}}: fix build #426643

Merged

various: avoid throwing for invalid platforms combinations in CI #426645

Merged

philiptaron added the 1.severity: significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. label Jul 19, 2025

wolfgangwalther force-pushed the ci-eval-asserts branch from d388542 to 2b49c26 Compare July 20, 2025 10:15

This comment was marked as resolved.

Sign in to view

nix-owners bot requested review from Ericson2314, philiptaron, Mic92 and MattSturgeon July 20, 2025 10:16

wolfgangwalther mentioned this pull request Jul 23, 2025

tests.trivial-builders.writeShellApplication: fix assert #427781

Merged

1 task

wolfgangwalther force-pushed the ci-eval-asserts branch 2 times, most recently from cc4e9e8 to dde5e03 Compare July 23, 2025 16:38

wolfgangwalther mentioned this pull request Jul 23, 2025

gnomeExtensions: remove __attrsFailEvaluation #427795

Closed

1 task

philiptaron approved these changes Jul 23, 2025

View reviewed changes

wolfgangwalther force-pushed the ci-eval-asserts branch from dde5e03 to 88f2a41 Compare July 24, 2025 10:28

wolfgangwalther force-pushed the ci-eval-asserts branch from 88f2a41 to 88329b1 Compare July 24, 2025 10:31

wolfgangwalther changed the title ~~ci/eval: fail on asserts~~ ci/eval: fail on asserts when generating attrpaths Jul 24, 2025

vcunat added 1.severity: channel blocker Blocks a channel and removed 1.severity: channel blocker Blocks a channel labels Jul 24, 2025

nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 24, 2025

wolfgangwalther added 3 commits July 24, 2025 15:31

pkgsx86_64Darwin: move behind allowVariants

612e76d

This is already not used in-tree, so should be a variant.

ci/eval: fail on asserts when generating attrpaths

30f19cc

This doesn't fail on *all* asserts, yet, because nix-env still ignores these in the main eval step. But it already gives some converage during the attrpath generation.

wolfgangwalther force-pushed the ci-eval-asserts branch from 88329b1 to 30f19cc Compare July 24, 2025 13:31

nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 24, 2025

philiptaron approved these changes Jul 24, 2025

View reviewed changes

wolfgangwalther merged commit 713e0c9 into NixOS:master Jul 24, 2025
24 of 27 checks passed

wolfgangwalther deleted the ci-eval-asserts branch July 24, 2025 19:36

github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label Jul 24, 2025

nixpkgs-ci bot mentioned this pull request Jul 24, 2025

[Backport release-25.05] ci/eval: fail on asserts when generating attrpaths #428141

Merged

1 task

jopejoe1 mentioned this pull request Jul 25, 2025

ocamlPackages: fix eval and let ci check that it still evals #406555

Open

13 tasks

wolfgangwalther mentioned this pull request Aug 30, 2025

ci/eval/compare: ping maintainers of removed packages #438652

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ci/eval: fail on asserts when generating attrpaths #426629

ci/eval: fail on asserts when generating attrpaths #426629

Uh oh!

wolfgangwalther commented Jul 19, 2025 •

edited

Loading

Uh oh!

philiptaron commented Jul 19, 2025

Uh oh!

This comment was marked as resolved.

philiptaron left a comment

Uh oh!

wolfgangwalther commented Jul 24, 2025

Uh oh!

wolfgangwalther commented Jul 24, 2025 •

edited

Loading

Uh oh!

MattSturgeon commented Jul 24, 2025

Uh oh!

wolfgangwalther commented Jul 24, 2025

Uh oh!

MattSturgeon commented Jul 24, 2025

Uh oh!

wolfgangwalther commented Jul 24, 2025

Uh oh!

Uh oh!

nixpkgs-ci bot commented Jul 24, 2025

Uh oh!

Uh oh!

	tried = builtins.tryEval seq;

	result =
	if tried.success then
	tried.value
	else if enableWarnings && path != [ "AAAAAASomeThingsFailToEvaluate" ] then
	lib.warn "tryEval failed at: ${lib.concatStringsSep "." path}" [ ]
	else
	[ ];

Uh oh!

ci/eval: fail on asserts when generating attrpaths #426629

ci/eval: fail on asserts when generating attrpaths #426629

Uh oh!

Conversation

wolfgangwalther commented Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

philiptaron commented Jul 19, 2025

Uh oh!

This comment was marked as resolved.

philiptaron left a comment

Choose a reason for hiding this comment

Uh oh!

wolfgangwalther commented Jul 24, 2025

Uh oh!

wolfgangwalther commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MattSturgeon commented Jul 24, 2025

Uh oh!

wolfgangwalther commented Jul 24, 2025

Uh oh!

MattSturgeon commented Jul 24, 2025

Uh oh!

wolfgangwalther commented Jul 24, 2025

Uh oh!

Uh oh!

nixpkgs-ci bot commented Jul 24, 2025

Uh oh!

Uh oh!

wolfgangwalther commented Jul 19, 2025 •

edited

Loading

wolfgangwalther commented Jul 24, 2025 •

edited

Loading