structured attrs: improve support / usage of NIX_ATTRS_{SH,JSON}_FILE

In NixOS#4770 I implemented proper `nix-shell(1)` support for derivations
using `__structuredAttrs = true;`. Back then we decided to introduce two
new environment variables, `NIX_ATTRS_SH_FILE` for `.attrs.sh` and
`NIX_ATTRS_JSON_FILE` for `.attrs.json`. This was to avoid having to
copy these files to `$NIX_BUILD_TOP` in a `nix-shell(1)` session which
effectively meant copying these files to the project dir without
cleaning up afterwords[1].

On last NixCon I resumed hacking on `__structuredAttrs = true;` by
default for `nixpkgs` with a few other folks and getting back to it,
I identified a few problems with the current state and how it's
used in `nixpkgs`:

* A lot of builders in `nixpkgs` don't care about the env vars and
  assume that `.attrs.sh` and `.attrs.json` are in `$NIX_BUILD_TOP`.
  The sole reason why this works is that `nix-shell(1)` sources
  the contents of `.attrs.sh` and then sources `$stdenv/setup` if it
  exists. This may not be pretty, but it works.

  However, the same assumption cannot be made for `.attrs.json` which
  can only be used in a `nix-shell(1)` session when using
  `$NIX_ATTRS_JSON_FILE`.

  I considered changing Nix to be compatible with what nixpkgs
  effectively does, but then we'd have to either move $NIX_BUILD_TOP for
  shell sessions to a temporary location (and thus breaking a lot of
  assumptions) or we'd reintroduce all the problems we solved back then
  by using these two env vars.

  In other words, this is pretty inconsistent with how the buildscripts
  behave in an actual build vs in a debug shell and I think that relying
  solely on the environment variabels is the best way to solve that.

  Also, I think this is partly because I didn't document these variables back
  then (mea culpa), so I decided to drop all mentions of
  `.attrs.{json,sh}` in the  manual and only refer to `$NIX_ATTRS_SH_FILE`
  and `$NIX_ATTRS_JSON_FILE`. The same applies to all our integration tests.
  Theoretically we could deprecated using `"$NIX_BUILD_TOP"/.attrs.sh` in
  the future now.

* `nix develop` and `nix print-dev-env` don't support this environment
  variable at all even though they're supposed to be part of the replacement
  for `nix-shell` - for the drv debugging part to be precise.

  This isn't a big deal for the vast majority of derivations, i.e.
  derivations relying on nixpkgs' `stdenv` wiring things together
  properly. This is because `nix develop` effectively "clones" the
  derivation and replaces the builder with a script that dumps all of
  the environment, shell variables, functions etc, so the state of
  structured attrs being "sourced" is transmitted into the dev shell and
  most of the time you don't need to worry about `.attrs.sh` not
  existing because the shell is correctly configured and the

      if [ -e .attrs.sh ]; then source .attrs.sh; fi

  is simply omitted.

  However, this will break when having a derivation that reads e.g. from
  `.attrs.json` like

      with import <nixpkgs> {};
      runCommand "foo" { __structuredAttrs = true; foo.bar = 23; } ''
        cat $NIX_ATTRS_JSON_FILE # doesn't work because it points to /build/.attrs.json
      ''

  To work around this I employed a similar approach as it exists for
  `nix-shell`: the `NIX_ATTRS_{JSON,SH}_FILE` vars are replaced with
  temporary locations.

  The contents of `.attrs.sh` and `.attrs.json` are now written into the
  JSON by `get-env.sh`, the builder that `nix develop` injects into the
  derivation it's debugging. So finally the exact file contents are
  present and exported by `nix develop`.

  I also made `.attrs.json` a JSON string in the JSON printed by
  `get-env.sh` on purpose because then it's not necessary to serialize
  the object structure again. `nix develop` only needs the JSON
  as string because it's only written into the temporary file.

  I'm not entirely sure if it makes sense to also use a temporary
  location for `nix print-dev-env` (rather than just skipping the
  rewrite in there), but this would probably break certain cases where
  it's relied upon `$NIX_ATTRS_SH_FILE` to exist (prime example are the
  `nix print-dev-env` test-cases I wrote in this patch using
  `tests/shell.nix`, these would fail because the env var exists, but it
  cannot read from it).

[1] NixOS#4770 (comment)

doc/manual/src/language/advanced-attributes.md

@@ -271,18 +271,21 @@ Derivations can declare some infrequently used optional attributes.
 
   - [`__structuredAttrs`]{#adv-attr-structuredAttrs}\
     If the special attribute `__structuredAttrs` is set to `true`, the other derivation
-    attributes are serialised in JSON format and made available to the
-    builder via the file `.attrs.json` in the builder’s temporary
-    directory. This obviates the need for [`passAsFile`](#adv-attr-passAsFile) since JSON files
-    have no size restrictions, unlike process environments.
+    attributes are serialised into a file in JSON format. The environment variable
+    `NIX_ATTRS_JSON_FILE` points to the exact location of that file both in a build
+    and a [`nix-shell`](../command-ref/nix-shell.md). This obviates the need for
+    [`passAsFile`](#adv-attr-passAsFile) since JSON files have no size restrictions,
+    unlike process environments.
 
     It also makes it possible to tweak derivation settings in a structured way; see
     [`outputChecks`](#adv-attr-outputChecks) for example.
 
     As a convenience to Bash builders,
-    Nix writes a script named `.attrs.sh` to the builder’s directory
-    that initialises shell variables corresponding to all attributes
-    that are representable in Bash. This includes non-nested
+    Nix writes a script that initialises shell variables
+    corresponding to all attributes that are representable in Bash. The
+    environment variable `NIX_ATTRS_SH_FILE` points to the exact
+    location of the script, both in a build and a
+    [`nix-shell`](../command-ref/nix-shell.md). This includes non-nested
     (associative) arrays. For example, the attribute `hardening.format = true`
     ends up as the Bash associative array element `${hardening[format]}`.
 

src/nix/develop.cc

@@ -8,9 +8,12 @@
 #include "derivations.hh"
 #include "progress-bar.hh"
 #include "run.hh"
+#include "util.hh"
 
+#include <iterator>
 #include <memory>
 #include <nlohmann/json.hpp>
+#include <algorithm>
 
 using namespace nix;
 
@@ -51,6 +54,7 @@ struct BuildEnvironment
 
     std::map<std::string, Value> vars;
     std::map<std::string, std::string> bashFunctions;
+    std::optional<std::pair<std::string, std::string>> structuredAttrs;
 
     static BuildEnvironment fromJSON(std::string_view in)
     {
@@ -74,6 +78,10 @@ struct BuildEnvironment
             res.bashFunctions.insert({name, def});
         }
 
+        if (json.contains("structuredAttrs")) {
+            res.structuredAttrs = {json["structuredAttrs"][".attrs.json"], json["structuredAttrs"][".attrs.sh"]};
+        }
+
         return res;
     }
 
@@ -102,13 +110,37 @@ struct BuildEnvironment
 
         res["bashFunctions"] = bashFunctions;
 
+        if (providesStructuredAttrs()) {
+            auto contents = nlohmann::json::object();
+            contents[".attrs.sh"] = getAttrsSH();
+            contents[".attrs.json"] = getAttrsJSON();
+            res["structuredAttrs"] = std::move(contents);
+        }
+
         auto json = res.dump();
 
         assert(BuildEnvironment::fromJSON(json) == *this);
 
         return json;
     }
 
+    bool providesStructuredAttrs() const
+    {
+        return structuredAttrs.has_value();
+    }
+
+    std::string getAttrsJSON() const
+    {
+        assert(providesStructuredAttrs());
+        return structuredAttrs->first;
+    }
+
+    std::string getAttrsSH() const
+    {
+        assert(providesStructuredAttrs());
+        return structuredAttrs->second;
+    }
+
     void toBash(std::ostream & out, const std::set<std::string> & ignoreVars) const
     {
         for (auto & [name, value] : vars) {
@@ -291,6 +323,7 @@ struct Common : InstallableCommand, MixProfile
     std::string makeRcScript(
         ref<Store> store,
         const BuildEnvironment & buildEnvironment,
+        const Path & tmpDir,
         const Path & outputsDir = absPath(".") + "/outputs")
     {
         // A list of colon-separated environment variables that should be
@@ -353,9 +386,42 @@ struct Common : InstallableCommand, MixProfile
             }
         }
 
+        if (buildEnvironment.providesStructuredAttrs()) {
+            fixupStructuredAttrs("sh", buildEnvironment.getAttrsSH(), rewrites, buildEnvironment, tmpDir);
+            fixupStructuredAttrs("json", buildEnvironment.getAttrsJSON(), rewrites, buildEnvironment, tmpDir);
+        }
+
         return rewriteStrings(script, rewrites);
     }
 
+    void fixupStructuredAttrs(
+        const std::string & ext,
+        const std::string & content,
+        StringMap & rewrites,
+        const BuildEnvironment & buildEnvironment,
+        const Path & tmpDir)
+    {
+        auto targetFilePath = tmpDir + "/.attrs." + ext;
+        writeFile(targetFilePath, content);
+
+        auto fileInBuilderEnv = buildEnvironment.vars.find(extToEnvVar(ext));
+        assert(fileInBuilderEnv != buildEnvironment.vars.end());
+        rewrites.insert({BuildEnvironment::getString(fileInBuilderEnv->second), targetFilePath});
+    }
+
+    std::string extToEnvVar(const std::string & ext) const
+    {
+        std::string out;
+        std::transform(
+            ext.begin(),
+            ext.end(),
+            std::back_inserter(out),
+            [](unsigned char c){ return std::toupper(c); }
+        );
+
+        return "NIX_ATTRS_" + out + "_FILE";
+    }
+
     Strings getDefaultFlakeAttrPaths() override
     {
         Strings paths{
@@ -487,7 +553,9 @@ struct CmdDevelop : Common, MixEnvironment
 
         auto [rcFileFd, rcFilePath] = createTempFile("nix-shell");
 
-        auto script = makeRcScript(store, buildEnvironment);
+        AutoDelete tmpDir(createTempDir("", "nix-develop"), true);
+
+        auto script = makeRcScript(store, buildEnvironment, (Path) tmpDir);
 
         if (verbosity >= lvlDebug)
             script += "set -x\n";
@@ -615,10 +683,12 @@ struct CmdPrintDevEnv : Common, MixJSON
 
         stopProgressBar();
 
-        logger->writeToStdout(
-            json
-            ? buildEnvironment.toJSON()
-            : makeRcScript(store, buildEnvironment));
+        if (json) {
+            logger->writeToStdout(buildEnvironment.toJSON());
+        } else {
+            AutoDelete tmpDir(createTempDir("", "nix-dev-env"), true);
+            logger->writeToStdout(makeRcScript(store, buildEnvironment, tmpDir));
+        }
     }
 };
 

src/nix/get-env.sh

@@ -1,5 +1,5 @@
 set -e
-if [ -e .attrs.sh ]; then source .attrs.sh; fi
+if [ -e "$NIX_ATTRS_SH_FILE" ]; then source "$NIX_ATTRS_SH_FILE"; fi
 
 export IN_NIX_SHELL=impure
 export dontAddDisableDepTrack=1
@@ -101,7 +101,21 @@ __dumpEnv() {
 
         printf "}"
     done < <(printf "%s\n" "$__vars")
-    printf '\n  }\n}'
+    printf '\n  }'
+
+    if [ -e "$NIX_ATTRS_SH_FILE" ]; then
+        printf ',\n  "structuredAttrs": {\n    '
+        __escapeString ".attrs.sh"
+        printf ': '
+        __escapeString "$(<"$NIX_ATTRS_SH_FILE")"
+        printf ',\n    '
+        __escapeString ".attrs.json"
+        printf ': '
+        __escapeString "$(<"$NIX_ATTRS_JSON_FILE")"
+        printf '\n  }'
+    fi
+
+    printf '\n}'
 }
 
 __escapeString() {
@@ -117,7 +131,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
+if [ -e "$NIX_ATTRS_SH_FILE" ]; then
     __olist="${!outputs[@]}"
 else
     __olist=$outputs

tests/build-hook-ca-fixed.nix

@@ -8,7 +8,10 @@ let
     derivation ({
       inherit system;
       builder = busybox;
-      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")];
+      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" ''
+        if [ -e "$NIX_ATTRS_SH_FILE" ]; then source $NIX_ATTRS_SH_FILE; fi;
+        eval "$buildCommand"
+      '')];
       outputHashMode = "recursive";
       outputHashAlgo = "sha256";
     } // removeAttrs args ["builder" "meta" "passthru"])

tests/build-hook.nix

@@ -14,7 +14,10 @@ let
     derivation ({
       inherit system;
       builder = busybox;
-      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")];
+      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" ''
+        if [ -e "$NIX_ATTRS_SH_FILE" ]; then source $NIX_ATTRS_SH_FILE; fi;
+        eval "$buildCommand"
+      '')];
     } // removeAttrs args ["builder" "meta" "passthru"]
     // caArgs)
     // { meta = args.meta or {}; passthru = args.passthru or {}; };

tests/config.nix.in

@@ -20,7 +20,10 @@ rec {
     derivation ({
       inherit system;
       builder = shell;
-      args = ["-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")];
+      args = ["-e" args.builder or (builtins.toFile "builder-${args.name}.sh" ''
+        if [ -e "$NIX_ATTRS_SH_FILE" ]; then source $NIX_ATTRS_SH_FILE; fi;
+        eval "$buildCommand"
+      '')];
       PATH = path;
     } // caArgs // removeAttrs args ["builder" "meta"])
     // { meta = args.meta or {}; };

tests/failing.nix

@@ -6,7 +6,10 @@ let
     derivation ({
       inherit system;
       builder = busybox;
-      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")];
+      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" ''
+        if [ -e "$NIX_ATTRS_SH_FILE" ]; then source $NIX_ATTRS_SH_FILE; fi;
+        eval "$buildCommand"
+      '')];
     } // removeAttrs args ["builder" "meta"])
     // { meta = args.meta or {}; };
 in

tests/hermetic.nix

@@ -14,7 +14,10 @@ let
     derivation ({
       inherit system;
       builder = busybox;
-      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")];
+      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" ''
+        if [ -e "$NIX_ATTRS_SH_FILE" ]; then source $NIX_ATTRS_SH_FILE; fi;
+        eval "$buildCommand"
+      '')];
     } // removeAttrs args ["builder" "meta" "passthru"]
     // caArgs)
     // { meta = args.meta or {}; passthru = args.passthru or {}; };

tests/structured-attrs.sh

@@ -15,9 +15,21 @@ nix-build structured-attrs.nix -A all -o $TEST_ROOT/result
 
 export NIX_BUILD_SHELL=$SHELL
 env NIX_PATH=nixpkgs=shell.nix nix-shell structured-attrs-shell.nix \
-    --run 'test -e .attrs.json; test "3" = "$(jq ".my.list|length" < $NIX_ATTRS_JSON_FILE)"'
+    --run 'test "3" = "$(jq ".my.list|length" < $NIX_ATTRS_JSON_FILE)"'
+
+nix develop -f structured-attrs-shell.nix -c bash -c 'test "3" = "$(jq ".my.list|length" < $NIX_ATTRS_JSON_FILE)"'
 
 # `nix develop` is a slightly special way of dealing with environment vars, it parses
 # these from a shell-file exported from a derivation. This is to test especially `outputs`
 # (which is an associative array in thsi case) being fine.
 nix develop -f structured-attrs-shell.nix -c bash -c 'test -n "$out"'
+
+nix print-dev-env -f structured-attrs-shell.nix | grep -q 'NIX_ATTRS_JSON_FILE='
+nix print-dev-env -f structured-attrs-shell.nix | grep -q 'NIX_ATTRS_SH_FILE='
+nix print-dev-env -f shell.nix shellDrv | grep -vq 'NIX_ATTRS_SH_FILE'
+
+jsonOut="$(nix print-dev-env -f structured-attrs-shell.nix --json)"
+
+test "$(<<<"$jsonOut" jq '.structuredAttrs|keys|.[]' -r)" = "$(printf ".attrs.json\n.attrs.sh")"
+
+test "$(<<<"$jsonOut" jq '.variables.out.value' -r)" = "$(<<<"$jsonOut" jq '.structuredAttrs.".attrs.json"' -r | jq -r '.outputs.out')"