treewide: load structured attributes in all bash builders consistently

[wolfgangwalther]

It's hard to put the sourcing of ./.attrs.sh into all builder consistently - mistakes will happen. Thus, load structured attrs once in make-derivation and then source the remaining builder on top.

This should fix quite a few builders with structured attributes in principle. Most importantly it helps substitute / substituteAll, which are required for bootstrap on some platforms.

Part of the overall effort to enable structuredAttrs by default eventually, tracking: #205690

As the author of #257919, you might be interested in reviewing this, @Ma27?

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/)
• 25.05 Release Notes (or backporting 24.11 and 25.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.

[wolfgangwalther]

All the files which already load the attrs file before sourcing stdenv are straight-forward.

A lot of files can easily be verified to be passed to mkDerivation's builder argument. Those are the cases that are straight up broken with structuredAttrs and easily fixed by loading the attrs file globally.

There are some remaining cases which are more special and need more attention. I left comments on each of them. Maybe I need to split some of these into a separate clean-up commit.

[wolfgangwalther]

I split up the commits better, all the straight-forward cases are in the first commit now, the other cases in the remaining respectively.

[wolfgangwalther]

Found a few more cases of source $stdenv/setup outside pkgs/. Added fixes for the simple ones, but still need to look into the last two commits I added.

FTR: I can't make any sense of this at all, yet, will probably leave it out of this PR:

nixpkgs/pkgs/development/web/nodejs/node-npm-build-npm-package-logic.patch

Line 53 in d225878

+ const patchShebangs = () => spawn('bash', ['-c', 'source $stdenv/setup; patchShebangs node_modules'], { cwd: dir })

Putting in on draft until I looked at the last two commits again.

[Ma27]

The Nix deprecation story is messy enough as it is

I don't think this is strictly related to the Nix deprecation story: minver is supposed to be bumped conservatively to give even older instances a chance to upgrade.

The 2.3 thing just became a very hot topic since 2.4+ are somewhat controversial (tl;dr flakes and a very large diff) which is why I'm a little hesitant to write it off (also, it's still maintained).

I fully agree with what @wolfgangwalther said here, we should stop supporting Nix versions to get rid of workarounds like that (I mean, there are a bunch of places that don't check for both cases, so technically 2.3 is broken there currently) and I think this is the quicker way to achieve that. Also, I think Tvix is targeting Nix 2.3 as baseline?

Anyways, at the end of the day, somebody has to make that call. I think we even had a Nixpkgs architecture team in the past ;-)

We could also consider somehow making the distinction between "version needed for eval" and "version needed for building"

I get the motivation, but wouldn't it be awkward if your Nix is sufficient to evaluate, but not to build nixpkgs?

Anyways, @wolfgangwalther the current state looks reasonable. I'd go through the changes soonish one last time and merge then.

[Ma27]

Meh this now needs a rebase @wolfgangwalther. Feel free to merge after that.

[emilazy]

Late to the party here, but the Nix minimum version bump that makes sense to me is 2.18, as the “new 2.3” and a reasonable baseline for Nix semantics, given Lix’s explicit choice of it as a base and the year that Nixpkgs was stuck on it. That cuts off 23.05, but the upside is that 23.11 doesn’t just contain 2.18 but also defaults to it, so the story is pretty simple: “upgrade to 23.11 before upgrade to 25.05”.

I don’t think that anyone really cares about the versions between 2.4 and 2.18, and the latter has had an above‐average level of maintenance put into it given its use as the default version across multiple Nixpkgs releases. I know Tvix is aiming for 2.3 compatibility, but I believe that relates to post‐2.3 eval behaviour regressions that Nixpkgs assuming any newer version might run into anyway, and I imagine there is room for adjustment there given the importance of Nixpkgs compatibility. (But of course we should ping representatives of all implementations, and anyone else who might have a stake in the choice, on the PR.)

[Ma27]

I'm not really opposed to your suggestion, I just tried to be as conservative as possible to get it through. But I'm not sure I'll follow through now that things got a little more complicated.

[emilazy]

Sorry if I contributed to any of the complication :) I do think some bump is a reasonable idea and I’m not too fussy about what exact version.

[Ma27]

Sorry if I contributed to any of the complication :)

I think I used the wrong wording here, all good!
With this patch we have a single place that needs to cover the .attrs.sh vs. NIX_ATTRS_SH_FILE case (that I'm responsible for since I messed up the very first patch for that and it wasn't noticed for quite a while - hence there's a bunch of Nix minors being broken in that regard). For me, the current solution is good enough to not invest energy in discussing a big minver bump that I'd expect given past experiences of efforts trying to drop 2.3 (my personal expectation is that most opposition will come from 2.3 being dropped).

That being said, I do think your approach is reasonable and I'd be in favor of such a change.

[emilazy]

Gotcha. Admittedly the (IMO premature) removal of 2.18 from Nixpkgs might make dropping 2.3 more controversial still. (Though probably most people who would move to that would just use Lix instead these days, I suppose?)

[philiptaron]

Bisect says that this broke nix-build -A tests.stdenv.outputs-no-out. It fails with the same error as #214822 but is not that issue, I think, since @emilazy fixed it in August.

The only output is:

/nix/store/shkw4qm9qcw5sc5n1k5jznc83ny02r39-default-builder.sh: line 1: pop_var_context: head of shell_variables not a function context

[wolfgangwalther]

I get this output:

original builder: /nix/store/shkw4qm9qcw5sc5n1k5jznc83ny02r39-default-builder.sh: line 1: genericBuild: command not found
testBuildFailure: Original builder produced exit code: 127
building '/nix/store/9a9gp7babzs0n664pwwjdda88x0jb1dg-outputs-no-out-assert.drv'...
/nix/store/shkw4qm9qcw5sc5n1k5jznc83ny02r39-default-builder.sh: line 1: pop_var_context: head of shell_variables not a function context

[wolfgangwalther]

I also note that tests.testers.testBuildFailure fails, which might be related.

[wolfgangwalther]

So the problem is:

• in this PR we changed pkgs/stdenv/generic/default-builder.sh
• we told pkgs/stdenv/generic/make-derivation.nix about it, which is the main user
• we never considered others might be using default-builder.sh directly as well
• the only other user is testBuildFailure

Once I fix that, both tests.testers.testBuildFailure and tests.stdenv.outputs-no-out pass again.

Will open a PR.