tvix: implement __structuredAttrs

#366
Opened by flokli at 2024-01-03T16·41+00

Similar to how tvix-glue's derivation_to_build_request now takes care of passAsFile, structured attrs modifies the build environment.

If __structuredAttrs is set to true, a .attrs.sh and .attrs.json file is created in the /build directory (and NIX_ATTRS_JSON_FILE and NIX_ATTRS_SH_FILE point to the paths).

See https://nixos.org/manual/nix/stable/language/advanced-attributes#adv-attr-structuredAttrs for docs.

https://github.com/NixOS/nixpkgs/pull/234883 also suggests Nix 2.3 did set ATTRS_JSON_FILE and ATTRS_SH_FILE(only) at some point.

My opinion is we should use the new env var in Tvix, backport the Nix fix to 2.3 too, so we can drop the workaround in nixpkgs/pkgs/stdenv/generic/setup.sh.

In terms of nixpkgs cleanups, we should also figure out why there's a ton of references to the .attrs.sh path itself, rather than reading NIX_ATTRS_SH_FILE. Maybe all this code can be dropped in Nixpkgs.

It's not entirely clear yet if and how __structuredAttrs affects the ATerm serialization / the contents of the Derivation struct. Maybe some of the JSON serialization code needs to live in builtins.derivationStrict instead - to be investigated.

There's also two fields getting new meanings in case __structuredAttrs is used - outputChecks and unsafeDiscardReferences

  1. flokli updated the body of this issue at 2024-01-03T16·43+00

  2. nixpkgs/pkgs/test/stdenv/default.nix has some tests, we should peek at them to see if the behaviour is correct.

    flokli at 2024-01-03T17·12+00

  3. cl/10604 implements the __structuredAttrs feature, at least as far as "populating the Derivation struct" is concerned.

    flokli at 2024-01-11T20·11+00

  4. There's still some open questions around string contexts inside the structured attributes themselves, a TODO for that is left in the code.

    flokli at 2024-01-12T22·10+00

  5. Apparently, builtins.toJson normally does propagate context, and we currently don't do that either - so we can't add it yet.

    So far we kept string contexts a somewhat private field of NixString, but we don't return a NixString for Value::into_json, but a serde_json::Value (which isn't necessarily a string at all).

    Adding contexts to all tvix_eval::Value would be annoying memory-wise.

    Manually dealing with context, forcing once, then traversing to collect all contexts on all NixString kinds, then another time to serialize to JSON would also suck.

    Probably a way out would be to have some opaque "SerdeValueWithContext", that can be easily "downcasted" to a context-free version.

    flokli at 2024-01-12T22·47+00