stdenv: support opt-in __structuredAttrs

[Artturin]

Description of changes

supersedes #85042
Read #72074
Built hello with structuredAttrs

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• 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/)
• 22.11 Release Notes (or backporting 22.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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[Mindavi]

I think this is really good to get in, but if an RFC is needed (work has been done on nix to make this work, so I'm really not sure why we'd do that but then require a RFC here) we should do that first.

Some documentation on how to migrate would be nice (just an url would be a good start already).

Will do some tests soon and then I'm okay with getting this in. Would be good to have some idea of how we'll handle migration though.

[Mindavi]

Should a file be converted to __structuredAttrs as example?

[Artturin]

there's a issue with nix develop due to us converting the structured outputs to old outputs

https://github.com/NixOS/nix/blob/1892355766af57868f74a9efd75aa279b29a04f6/src/nix/get-env.sh#L114-L121

$ $NIXGITS/nix/outputs/out/bin/nix develop --extra-experimental-features 'flakes nix-command' ".#stdenv.__bootPackages.stdenv.__bootPackages.stdenv.__bootPackages.stdenv.__bootPackages.xz"
error: builder for '/nix/store/8lx3d0d59vss0hczpd08y882688hirid-xz-5.2.5-env.drv' failed with exit code 1;
       last 2 log lines:
       > structuredAttrs is enabled
       > /nix/store/ml6njbj66c2r6h6wvn7id2kvx9gnrnxc-get-env.sh: line 129: /nix/store/ml6njbj66c2r6h6wvn7id2kvx9gnrnxc-get-env.sh: Permission denied
       For full logs, run 'nix log /nix/store/8lx3d0d59vss0hczpd08y882688hirid-xz-5.2.5-env.drv'.

if i apply this path(a simplified version of what i tried)

diff --git a/src/nix/get-env.sh b/src/nix/get-env.sh
index 42c806450..f554fd3fd 100644
--- a/src/nix/get-env.sh
+++ b/src/nix/get-env.sh
@@ -114,11 +114,7 @@ __escapeString() {
 # In case of `__structuredAttrs = true;` the list of outputs is an associative
 # array with a format like `outname => /nix/store/hash-drvname-outname`, so `__olist`
 # must contain the array's keys (hence `${!...[@]}`) in this case.
-if [ -e .attrs.sh ]; then
-    __olist="${!outputs[@]}"
-else
-    __olist=$outputs
-fi
+__olist=$outputs
 
 for __output in $__olist; do
     if [[ -z $__done ]]; then

then i get

[1/0/1 built, 0.0 MiB DL] building xz-5.2.5-env: structuredAttrs is enabled
nix: src/nix/develop.cc:299: std::string Common::makeRcScript(nix::ref<nix::Store>, const BuildEnvironment&, const Path&): Assertion `outputs != buildEnvironment.vars.end()' failed.
[1]    1523221 IOT instruction (core dumped)  $NIXGITS/nix/outputs/out/bin/nix develop --extra-experimental-features

[Artturin]

Doc suggestions applied
Rendered

[fricklerhandwerk]

Docs LGTM!

[roberth]

Without opting in, this builds at least

• nixosTests.simple
• nixosTests.acme
• hci
  • haskell
  • rust
  • go
• a whole bunch of python, llvm, texlive, nix, ..., through dependencies of the above

This makes me confident that this won't break staging.

[aanderse]

🚀

[Artturin]

TODO fill #205690 with the TODO comments i posted here

[trofi]

Bisect claims 238a605 stdenv: support opt-in __structuredAttrs broke ronn:

$ nix build -f. ronn
/nix/store/rnd3ck83nkh6civlh8kfmfxzl8aysdbh-stdenv-linux/setup: line 347: declare: `0=/nix/store/g88x9az6zamw9axgwadrfddqn22hanr2-ronn-gems': not a valid identifier

[ncfavier]

Plausible explanation: ronn passes a variable named env to the builder, which overwrites the structured env that setup.sh is supposed to operate on:

$ env=/nix/foo
$ echo "${!env[@]}"
0
$ for envVar in "${!env[@]}"; do declare -x "$envVar=${env[$envVar]}"; done
bash: declare: `0=/nix/foo': not a valid identifier

Proposed fix: use __env instead of env since this is an internal thing. (If Nix doesn't pass along __-prefixed variables, then use structured_env or whatever)

[ncfavier]

Or, probably better, just check that env is an associative array:

if [[ ${env@a} == *A* ]]; then ...; fi

(requires Bash 5+, can we assume that?)

[SuperSandro2000]

Proposed fix: use __env instead of env since this is an internal thing. (If Nix doesn't pass along __-prefixed variables, then use structured_env or whatever)

probably also solves similar errors in the future.

Or, probably better, just check that env is an associative array:

We should maybe do that in addition to the above just to be sure.

can we assume that?

stdenv always has it IIRC

[Artturin]

Proposed fix: use __env instead of env since this is an internal thing. (If Nix doesn't pass along __-prefixed variables, then use structured_env or whatever)

this isn't a internal variable or a structured attrs specific thing so im leaving the name as env

Or, probably better, just check that env is an associative array:

sounds good

can we assume that?

adding echo $BASH_VERSION to setup.sh gives

bootstrap-stage0-glibc-bootstrap> 4.4.23(1)-release

[ncfavier]

this isn't a internal variable or a structured attrs specific thing so im leaving the name as env

Fair.

Maybe we can simply condition that block on whether .attrs.sh is present (or whether __structuredAttrs is set)?

[ncfavier]

#205944

[DavHau]

lib.intersectLists has quadratic runtime behavior. Will that be a problem?

[ncfavier]

We should probably use builtins.attrNames (builtins.intersectAttrs env derivationArg) instead.

[DavHau]

Or maybe change the implementation of intersectLists to do that by default?

[ncfavier]

intersectLists has to work with any type of elements, not just strings (or strings-that-can-be-attribute-names, which I'm not sure whether that's a different type)

But we could have intersectListsOfStrings, I guess.

[ncfavier]

It looks like there's a function lib.attrsets.unionOfDisjoint. We could probably just use that, but that would make the error message worse.

[ncfavier]

#206525