top-level/stage: abort lazily in internallyDisallowedAttrPathsOverlay

[MattSturgeon]

Followup to #533376 (comment), I noticed in #522364 that inheriting a problematic package's version was enough to trigger the abort.

There's a debate to be had about what kinda relationships should require re-declaring dependencies' problems. My view is that depending on derivation outputs requires declaring a problem, while inheriting metadata from a problem-package could easily be inlined if/when that package is removed, so should not require declaring a problem.

---

Instead of replacing internally disallowed attrs entirely with an abort, we now lazily abort on access while preserving a small allowlist of metadata attributes. EDIT: After discussion around the intended semantics of lib.warnOnInstantiate, I've changed the implementation to take a different approach.

Instead of replacing internally disallowed attrs with an immediate abort, we now abort lazily when a derivation (or one of its outputs) is instantiated. This preserves access to metadata and helper attributes while still preventing dependencies on the derivation's outputs.

Unlike lib.warnOnInstantiate, this only intercepts instantiation (i.e. evaluation of drvPath, outPath, shellPath, or an output's corresponding paths), rather than warning on most interactions with the derivation.

I also refactored the abort message to use lib.showAttrPath instead of concatStringsSep ".", which correctly formats attrpaths containing non-identifier segments.

Previous implementation

The original version of this PR preserved access to a small allowlist of attributes (version, meta, passthru, etc.) by wrapping derivations in a similar fashion to lib.warnOnInstantiate.

The current implementation instead preserves the entire derivation object and only aborts at instantiation time, which better matches the intended policy.

---

Tested in the repl:

nix-repl> pkgs = import ./. { config.attrPathsDisallowedForInternalUse = [ { attrPath = ["hello"]; reason = "hi!"; } ]; }

nix-repl> pkgs.mold-unwrapped.tests.wrapped
error:
       … while calling the 'derivationStrict' builtin
         at «nix-internal»/derivation-internal.nix:37:12:
           36|
           37|   strict = derivationStrict drvAttrs;
             |            ^
           38|

       … while evaluating derivation 'mold-wrapped-test'
         whose name attribute is located at /home/matt/nixpkgs/ci/pkgs/stdenv/generic/make-derivation.nix:651:11

       … while evaluating attribute 'buildCommand' of derivation 'mold-wrapped-test'
         at /home/matt/nixpkgs/ci/pkgs/build-support/trivial-builders/default.nix:81:17:
           80|         enableParallelBuilding = true;
           81|         inherit buildCommand name;
             |                 ^
           82|         passAsFile = [ "buildCommand" ] ++ (derivationArgs.passAsFile or [ ]);

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: evaluation aborted with the following error message: 'hello is disallowed from being used within Nixpkgs, because hi!'

nix-repl> pkgs.hello
error: attribute 'hello' missing
       at «string»:1:1:
            1| pkgs.hello
             | ^
       Did you mean one of jello, _3llo, bella, cell or delly?

nix-repl> pkgs.jello
«derivation /nix/store/0kvlydnamlcvxby6i2rmavl8xax37hbv-python3.13-jello-1.6.1.drv»

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.
• Follows the automation/AI policy.

[infinisil]

Makes sense, especially also because meta.problems only trigger on instantiate!

[MattSturgeon]

Following the discussion around lib.warnOnInstantiate, I've pushed an outputs-based approach and updated the PR description.

@infinisil and @llakala, it'd be great if you're able to take another look.

[MattSturgeon]

I'd like to land this if there are no objections, but I'm conscious that the implementation has changed since it was reviewed and performance could be better optimised (although performance only affects CI).

[MattSturgeon]

Friendly bump; would be great to get some eyeballs on the revised approach!

[MattSturgeon]

I'll merge this soon unless anyone objects 🙂

[llakala]

Sorry for missing this.

I'm fine to merge this. To be honest, I disagree with the design behind meta.problems only being allowed for leaf packages - @whispersofthedawn has convinced me that it should be allowed everywhere. But this is a fine incremental improvement.