stdenv: fix env pass through with structured attrs

[tie]

Description of changes

Note

I need this for #334662, so ideally I’d like to target staging even if there are no mass rebuilds.

When using structured attributes, env attributes must not be derivation arguments to allow reusing drvAttrs as an argument for mkDerivation (e.g. see srcOnly).

Fixes the following example:

let
  drv = stdenv.mkDerivation {
    name = "foo";
    __structuredAttrs = true;
    env.FOO = "foo";
  };
in
stdenv.mkDerivation drv.drvAttrs

In practice, it fixes srcOnly for derivations with structured attributes that set env.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[emilazy]

I’m too tired to understand the subtleties here right now, but FWIW you don’t need to target staging for that reason. master is automatically merged into staging on a regular basis, so there’s no much lag between a change landing in master and a staging PR being able to make use of it.

[roberth]

stdenv.mkDerivation drv.drvAttrs

drvAttrs is a package attribute that is added by derivation and although unfortunately it isn't documented, its purpose is to be a valid argument for derivation.
When we're talking about normal packages (ie no messing around with .outputs and its corresponding attrs; no multi-derivation packages as in NixOS/nix#6507), I am pretty sure that by now many user expressions expect drvAttrs to contain the exact parameters that make up the .drv file.
In other words, drvAttrs does exactly what it should.

So what to do instead?
I'm not entirely sure what the best approach is, but I wouldn't mind a pkg.mkDerivationAttrs attribute, or even better pkg.internals.mkDerivationAttrs, because dumping all implementation details into the main attrset is bad for discoverability and expectation management. I'm sure we'll have more a lot more stuff to put into internals.

Another option that's less "invasive" perhaps, is to add a function like stdenv.unDerivation, removing the attributes that are already in env, with the goal that stdenv.mkDerivation args == stdenv.mkDerivation (stdenv.unDerivation (stdenv.mkDerivation args)) for all possible args or some large set of sensible args.

---

For context, I should note that I intend to use drvAttrs in #330822 which to be fair isn't merged, but it is a crucial ingredient for it. drvAttrs reflects the low level "derivation" interface between Nix and the world, where breaking changes are effectively forbidden, whereas Nixpkgs can actually make breaking changes to mkDerivation without much trouble.
Emulating the derivation environment setup part of Nix is only ok because the derivation interface is very stable (and not just ok but good because nix-shell is effectively unfixable as part of Nix).

It's not the only reason to preserve the meaning of drvAttrs, but it shows the value of it.

[tie]

@roberth, thanks for detailed answer! Unfortunately, I don’t have enough time to design and implement the proposed alternatives. Besides, and I’m not really a fan of how mkDerivation currently exposes all attributes right now. I think your PR #217243 was a move in the right direction.

For #334662, I’ll just use applyPatches instead of srcOnly to avoid dealing with drvAttrs altogether since srcOnly also does not filter outputChecks, {,dis}allowed{Requisites,References}, etc. I’m not sure if it’s worth keeping srcOnly in its current state given that it’s not really useful because of these edge cases. It’s not used much in Nixpkgs, so perhaps we can deprecated and eventually remove it?