ci/eval: Allow disallowed attrs to depend on other disallowed attrs

[infinisil]

This is a follow up to #533376 and will be necessary for #531180

Motivation

With a dependency chain like

a -> b -> c

it was not possible to e.g. add a meta.problems.removal to b, because that would cause b to be entirely unavailable as a dependency.

This PR slightly lifts that restriction by allowing both a and b to be marked with such a problem without causing a CI failure.

This is useful for:

• Having a problem on a package that needs to be removed but still has dependents (which then also need a problem)
• Having a problem on a package that is aliased under another attribute, which counts as a dependency

Things done

• Tested that packages with internally disallowed meta.problems kinds are still allowed by CI for packages without dependents:
  • Add problems.removal.message = "Removed" to CuboCore.coregarage
  • Run nix-build ci -A eval.singleSystem --arg quickTest true, it should succeed
• Tested that such packages are still evaluated:
  • (continued from steps above)
  • Break coregarage's evaluation by changing qt6.qtbase to qt6.qbase
  • Run nix-build ci -A eval.singleSystem --arg quickTest true, it should fail with an error indicating the problem:

    error: attribute 'qbase' missing
       at /nix/store/jninim9a6czcn64dd41plax34g017sd2-source/pkgs/applications/misc/cubocore-packages/coregarage/default.nix:32:5:
           31|   buildInputs = [
           32|     qt6.qbase
             |     ^
           33|     libarchive
       Did you mean one of qtbase or qmake?
    

• Tested that such packages still can't be used as dependencies of allowed packages:
  • (continued from steps above)
  • Fix evaluation of coregarage again
  • Add this to all-packages.nix:

    AAAtestPackage = pkgs.buildEnv {
      name = "testPackage";
      paths = [ CuboCore.coregarage ];
      meta.description = "Test package";
    };

  • Run nix-build ci -A eval.singleSystem --arg quickTest true, it should fail with an error indicating that coregarage is used internally:

    error: evaluation aborted with the following error message: 'CuboCore.coregarage is disallowed from being used within Nixpkgs, because it has certain meta.problems whose kinds are disallowed: {
      removal = {
        kind = "removal";
        message = "Removal";
      };
    }'
    

• Tested that disallowed packages can now depend on disallowed packages:
  • (continued from steps above)
  • Add the following to AAAtestPackage:

    meta.problems.removal.message = "Depends on CuboCore.coregarage";
    

  • Run nix-build ci -A eval.singleSystem --arg quickTest true, it should succeed (this failed before)

[MattSturgeon]

I like the idea that packages that depend on problematic packages should themselves be marked with a corresponding problem.

I wonder if it's better to have a dedicated "transitive problem" problem class, so that we don't need slightly contrived "a is broken because its dependency b is also broken" messages to be manually maintained?

[MattSturgeon]

Beyond the scope of this PR: writeShellScript -> writeShellApplication would run shellcheck in the checkPhase, catching issues like unquoted variables. I'll consider opening a separate PR.

[MattSturgeon]

Just based off the diff, discussion, and description, this SGTM. Thanks!

I've tried not to give too much "wording suggestion"-style feedback, but I did comment on a couple minor spots that tripped me up a little. (Nothing blocking)

I still think we should consider how depending-on-a-problem should be represented in meta and how it should be displayed to end-users:

I wonder if it's better to have a dedicated "transitive problem" problem class, so that we don't need slightly contrived "a is broken because its dependency b is also broken" messages to be manually maintained?

but that doesn't block the core impl or core concept.

[MattSturgeon]

It's fine as is, but this flows better in my brain:

Suggested change

	# The first eval evaluates only attributes that are not disallowed for internal Nixpkgs use, ensuring that they don't depend on disallowed attributes
	# The first eval evaluates only attributes that are allowed for internal Nixpkgs use, ensuring that they don't depend on disallowed attributes

[MattSturgeon]

Something about my brain reads "prep for second eval" and assumes the "first eval" has already happened. I spent longer than I should admit scrolling up looking for an earlier usage of chunkedEval 😁

Thinking about it, it's kinda arbitrary which eval is first and which is second. Maybe we can give them more semantic names, like "allowed eval" and "disallowed eval" or "normal eval" and "problems eval" or something?

We already kinda do this in the debug echos:

echo "Evaluating the internally allowed attributes"
echo "Evaluating the internally disallowed attributes"

I think it'd also help if we had a better name for "the file passed to chunkedEval; we currently call it preEvalFile because historically it has come from the preEval derivation, but disallowedAttributesPreEvalFile is subtly breaking that axiom. I don't have a better suggestion though 🤷🏻

[MattSturgeon]

Not sure whether you planned to address the minor quibbles above, but I don't think they're worth delaying this PR longer. Let's merge.

[nixpkgs-ci]

Successfully created backport PR for release-26.05:

• [Backport release-26.05] ci/eval: Allow disallowed attrs to depend on other disallowed attrs #537612