stdenv: support opt-in __structuredAttrs

Co-authored-by: Robin Gloster <mail@glob.in>

stdenv: print message if structuredAttrs is enabled

stdenv: add _append

reduces the chance of a user doing it wrong

fix nix develop issue

output hooks don't work yet in nix develop though

making $outputs be the same on non-structuredAttrs and structuredAttrs
is too much trouble.

lets instead make a function that gets the output names

reading environment file '/nix/store/2x7m69a2sm2kh0r6v0q5s9z1dh41m4xf-xz-5.2.5-env-bin'
nix: src/nix/develop.cc:299: std::string Common::makeRcScript(nix::ref<nix::Store>, const BuildEnvironment&, const Path&): Assertion `outputs != buildEnvironment.vars.end()' failed.

use a function to get all output names instead of using $outputs

copy env functionality from https://github.com/NixOS/nixpkgs/pull/76732/commits

3 files changed, 198 insertions, 59 deletions

diff --git a/pkgs/stdenv/generic/default-builder.sh b/pkgs/stdenv/generic/default-builder.sh
index 273fc55c755..8c6fec7873b 100644
--- a/pkgs/stdenv/generic/default-builder.sh
+++ b/pkgs/stdenv/generic/default-builder.sh
@@ -1,2 +1,6 @@
+if [ -f .attrs.sh ]; then
+    . .attrs.sh
+fi
+
 source $stdenv/setup
 genericBuild
diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix
index 510537aac9f..09a71da93b6 100644
--- a/pkgs/stdenv/generic/make-derivation.nix
+++ b/pkgs/stdenv/generic/make-derivation.nix
@@ -154,6 +154,12 @@ let
   (! attrs ? outputHash) # Fixed-output drvs can't be content addressed too
   && config.contentAddressedByDefault
 
+# Experimental.  For simple packages mostly just works,
+# but for anything complex, be prepared to debug if enabling.
+, __structuredAttrs ? false
+
+, env ? { }
+
 , ... } @ attrs:
 
 let
@@ -259,13 +265,16 @@ else let
     lib.unique (lib.concatMap (input: input.__propagatedImpureHostDeps or [])
       (lib.concatLists propagatedDependencies));
 
+  envIsExportable = lib.isAttrs env && !lib.isDerivation env;
+
   derivationArg =
     (removeAttrs attrs
-      ["meta" "passthru" "pos"
+      (["meta" "passthru" "pos"
        "checkInputs" "installCheckInputs"
        "__darwinAllowLocalNetworking"
        "__impureHostDeps" "__propagatedImpureHostDeps"
-       "sandboxProfile" "propagatedSandboxProfile"])
+       "sandboxProfile" "propagatedSandboxProfile"]
+       ++ lib.optionals envIsExportable [ "env" ]))
     // (lib.optionalAttrs (attrs ? name || (attrs ? pname && attrs ? version)) {
       name =
         let
@@ -289,7 +298,7 @@ else let
           then attrs.name + hostSuffix
           else "${attrs.pname}${staticMarker}${hostSuffix}-${attrs.version}"
         );
-    }) // {
+    }) // lib.optionalAttrs (envIsExportable && __structuredAttrs) { env = checkedEnv; } // {
       builder = attrs.realBuilder or stdenv.shell;
       args = attrs.args or ["-e" (attrs.builder or ./default-builder.sh)];
       inherit stdenv;
@@ -304,8 +313,7 @@ else let
 
       userHook = config.stdenv.userHook or null;
       __ignoreNulls = true;
-
-      inherit strictDeps;
+      inherit __structuredAttrs strictDeps;
 
       depsBuildBuild              = lib.elemAt (lib.elemAt dependencies 0) 0;
       nativeBuildInputs           = lib.elemAt (lib.elemAt dependencies 0) 1;
@@ -473,6 +481,17 @@ else let
                    else true);
     };
 
+  checkedEnv =
+    let
+      overlappingNames = lib.intersectLists (lib.attrNames env) (lib.attrNames derivationArg);
+    in
+    assert lib.assertMsg (overlappingNames == [ ])
+      "The ‘env’ attribute set cannot contain any attributes passed to derivation. The following attributes are overlapping: ${lib.concatStringsSep ", " overlappingNames}";
+    lib.mapAttrs
+      (n: v: assert lib.assertMsg (lib.isString v || lib.isBool v || lib.isInt v)
+        "The ‘env’ attribute set can only contain string, boolean or integer attributes. The ‘${n}’ attribute is of type ${builtins.typeOf v}."; v)
+      env;
+
 in
 
 lib.extendDerivation
@@ -509,7 +528,7 @@ lib.extendDerivation
    # should be made available to Nix expressions using the
    # derivation (e.g., in assertions).
    passthru)
-  (derivation derivationArg);
+  (derivation (derivationArg // lib.optionalAttrs envIsExportable checkedEnv));
 
 in
   fnOrAttrs:
diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh
index c6cdb6c3df7..de1dad5a169 100644
--- a/pkgs/stdenv/generic/setup.sh
+++ b/pkgs/stdenv/generic/setup.sh
@@ -15,8 +15,29 @@ if (( "${NIX_DEBUG:-0}" >= 6 )); then
     set -x
 fi
 
-: ${outputs:=out}
+if [ -f .attrs.sh ]; then
+    __structuredAttrs=1
+    echo "structuredAttrs is enabled"
+else
+    __structuredAttrs=
+fi
+
+if [ -n "$__structuredAttrs" ]; then
+    for outputName in "${!outputs[@]}"; do
+        # ex: out=/nix/store/...
+        export "$outputName=${outputs[$outputName]}"
+    done
+else
+    : ${outputs:=out}
+fi
 
+getAllOutputNames() {
+    if [ -n "$__structuredAttrs" ]; then
+        echo "${!outputs[*]}"
+    else
+        echo "$outputs"
+    fi
+}
 
 ######################################################################
 # Hook handling.
@@ -175,6 +196,63 @@ addToSearchPath() {
     addToSearchPathWithCustomDelimiter ":" "$@"
 }
 
+# Prepend elements to variable "$1", which may come from an attr.
+#
+# This is useful in generic setup code, which must (for now) support
+# both derivations with and without __structuredAttrs true, so the
+# variable may be an array or a space-separated string.
+#
+# Expressions for individual packages should simply switch to array
+# syntax when they switch to setting __structuredAttrs = true.
+prependToVar() {
+    local -n nameref="$1"; shift
+    if [ -n "$__structuredAttrs" ]; then
+        nameref=( "$@" ${nameref+"${nameref[@]}"} )
+    else
+        nameref="$* ${nameref-}"
+    fi
+}
+
+# Same as above
+appendToVar() {
+    local -n nameref="$1"; shift
+    if [ -n "$__structuredAttrs" ]; then
+        nameref=( ${nameref+"${nameref[@]}"} "$@" )
+    else
+        nameref="${nameref-} $*"
+    fi
+}
+
+# Accumulate into `flagsArray` the flags from the named variables.
+#
+# If __structuredAttrs, the variables are all treated as arrays
+# and simply concatenated onto `flagsArray`.
+#
+# If not __structuredAttrs, then:
+#   * Each variable is treated as a string, and split on whitespace;
+#   * except variables whose names end in "Array", which are treated
+#     as arrays.
+_accumFlagsArray() {
+    local name
+    if [ -n "$__structuredAttrs" ]; then
+        for name in "$@"; do
+            local -n nameref="$name"
+            flagsArray+=( ${nameref+"${nameref[@]}"} )
+        done
+    else
+        for name in "$@"; do
+            local -n nameref="$name"
+            case "$name" in
+                *Array)
+                    flagsArray+=( ${nameref+"${nameref[@]}"} ) ;;
+                *)
+                    flagsArray+=( ${nameref-} ) ;;
+            esac
+        done
+    fi
+
+}
+
 # Add $1/lib* into rpaths.
 # The function is used in multiple-outputs.sh hook,
 # so it is defined here but tried after the hook.
@@ -255,6 +333,11 @@ printWords() {
 ######################################################################
 # Initialisation.
 
+# export all vars that should be in the ENV
+for envVar in "${!env[@]}"; do
+    declare -x "${envVar}=${env[${envVar}]}"
+done
+
 
 # Set a fallback default value for SOURCE_DATE_EPOCH, used by some build tools
 # to provide a deterministic substitute for the "current" time. Note that
@@ -469,6 +552,10 @@ findInputs() {
     done
 }
 
+# The way we handle deps* and *Inputs works with structured attrs
+# either enabled or disabled.  For this it's convenient that the items
+# in each list must be store paths, and therefore space-free.
+
 # Make sure all are at least defined as empty
 : ${depsBuildBuild=} ${depsBuildBuildPropagated=}
 : ${nativeBuildInputs=} ${propagatedNativeBuildInputs=} ${defaultNativeBuildInputs=}
@@ -477,29 +564,29 @@ findInputs() {
 : ${buildInputs=} ${propagatedBuildInputs=} ${defaultBuildInputs=}
 : ${depsTargetTarget=} ${depsTargetTargetPropagated=}
 
-for pkg in $depsBuildBuild $depsBuildBuildPropagated; do
+for pkg in ${depsBuildBuild[@]} ${depsBuildBuildPropagated[@]}; do
     findInputs "$pkg" -1 -1
 done
-for pkg in $nativeBuildInputs $propagatedNativeBuildInputs; do
+for pkg in ${nativeBuildInputs[@]} ${propagatedNativeBuildInputs[@]}; do
     findInputs "$pkg" -1  0
 done
-for pkg in $depsBuildTarget $depsBuildTargetPropagated; do
+for pkg in ${depsBuildTarget[@]} ${depsBuildTargetPropagated[@]}; do
     findInputs "$pkg" -1  1
 done
-for pkg in $depsHostHost $depsHostHostPropagated; do
+for pkg in ${depsHostHost[@]} ${depsHostHostPropagated[@]}; do
     findInputs "$pkg"  0  0
 done
-for pkg in $buildInputs $propagatedBuildInputs ; do
+for pkg in ${buildInputs[@]} ${propagatedBuildInputs[@]} ; do
     findInputs "$pkg"  0  1
 done
-for pkg in $depsTargetTarget $depsTargetTargetPropagated; do
+for pkg in ${depsTargetTarget[@]} ${depsTargetTargetPropagated[@]}; do
     findInputs "$pkg"  1  1
 done
 # Default inputs must be processed last
-for pkg in $defaultNativeBuildInputs; do
+for pkg in ${defaultNativeBuildInputs[@]}; do
     findInputs "$pkg" -1  0
 done
-for pkg in $defaultBuildInputs; do
+for pkg in ${defaultBuildInputs[@]}; do
     findInputs "$pkg"  0  1
 done
 
@@ -909,6 +996,13 @@ unpackPhase() {
         srcs="$src"
     fi
 
+    local -a srcsArray
+    if [ -n "$__structuredAttrs" ]; then
+        srcsArray=( "${srcs[@]}" )
+    else
+        srcsArray=( $srcs )
+    fi
+
     # To determine the source directory created by unpacking the
     # source archives, we record the contents of the current
     # directory, then look below which directory got added.  Yeah,
@@ -921,7 +1015,7 @@ unpackPhase() {
     done
 
     # Unpack all source archives.
-    for i in $srcs; do
+    for i in "${srcsArray[@]}"; do
         unpackFile "$i"
     done
 
@@ -971,7 +1065,14 @@ unpackPhase() {
 patchPhase() {
     runHook prePatch
 
-    for i in ${patches:-}; do
+    local -a patchesArray
+    if [ -n "$__structuredAttrs" ]; then
+        patchesArray=( ${patches:+"${patches[@]}"} )
+    else
+        patchesArray=( ${patches:-} )
+    fi
+
+    for i in "${patchesArray[@]}"; do
         header "applying patch $i" 3
         local uncompress=cat
         case "$i" in
@@ -988,9 +1089,17 @@ patchPhase() {
                 uncompress="lzma -d"
                 ;;
         esac
+
+        local -a flagsArray
+        if [ -n "$__structuredAttrs" ]; then
+            flagsArray=( "${patchFlags[@]:--p1}" )
+        else
+            # shellcheck disable=SC2086
+            flagsArray=( ${patchFlags:--p1} )
+        fi
         # "2>&1" is a hack to make patch fail if the decompressor fails (nonexistent patch, etc.)
         # shellcheck disable=SC2086
-        $uncompress < "$i" 2>&1 | patch ${patchFlags:--p1}
+        $uncompress < "$i" 2>&1 | patch "${flagsArray[@]}"
     done
 
     runHook postPatch
@@ -1018,7 +1127,6 @@ configurePhase() {
 
     # set to empty if unset
     : ${configureScript=}
-    : ${configureFlags=}
 
     if [[ -z "$configureScript" && -x ./configure ]]; then
         configureScript=./configure
@@ -1049,31 +1157,29 @@ configurePhase() {
     fi
 
     if [[ -z "${dontAddPrefix:-}" && -n "$prefix" ]]; then
-        configureFlags="${prefixKey:---prefix=}$prefix $configureFlags"
+        prependToVar configureFlags "${prefixKey:---prefix=}$prefix"
     fi
 
     if [[ -f "$configureScript" ]]; then
         # Add --disable-dependency-tracking to speed up some builds.
         if [ -z "${dontAddDisableDepTrack:-}" ]; then
             if grep -q dependency-tracking "$configureScript"; then
-                configureFlags="--disable-dependency-tracking $configureFlags"
+                prependToVar configureFlags --disable-dependency-tracking
             fi
         fi
 
         # By default, disable static builds.
         if [ -z "${dontDisableStatic:-}" ]; then
             if grep -q enable-static "$configureScript"; then
-                configureFlags="--disable-static $configureFlags"
+                prependToVar configureFlags --disable-static
             fi
         fi
     fi
 
     if [ -n "$configureScript" ]; then
-        # Old bash empty array hack
-        # shellcheck disable=SC2086
-        local flagsArray=(
-            $configureFlags "${configureFlagsArray[@]}"
-        )
+        local -a flagsArray
+        _accumFlagsArray configureFlags configureFlagsArray
+
         echoCmd 'configure flags' "${flagsArray[@]}"
         # shellcheck disable=SC2086
         $configureScript "${flagsArray[@]}"
@@ -1089,22 +1195,17 @@ configurePhase() {
 buildPhase() {
     runHook preBuild
 
-    # set to empty if unset
-    : ${makeFlags=}
-
-    if [[ -z "$makeFlags" && -z "${makefile:-}" && ! ( -e Makefile || -e makefile || -e GNUmakefile ) ]]; then
+    if [[ -z "${makeFlags-}" && -z "${makefile:-}" && ! ( -e Makefile || -e makefile || -e GNUmakefile ) ]]; then
         echo "no Makefile, doing nothing"
     else
         foundMakefile=1
 
-        # Old bash empty array hack
         # shellcheck disable=SC2086
         local flagsArray=(
             ${enableParallelBuilding:+-j${NIX_BUILD_CORES}}
             SHELL=$SHELL
-            $makeFlags "${makeFlagsArray[@]}"
-            $buildFlags "${buildFlagsArray[@]}"
         )
+        _accumFlagsArray makeFlags makeFlagsArray buildFlags buildFlagsArray
 
         echoCmd 'build flags' "${flagsArray[@]}"
         make ${makefile:+-f $makefile} "${flagsArray[@]}"
@@ -1141,11 +1242,17 @@ checkPhase() {
         local flagsArray=(
             ${enableParallelChecking:+-j${NIX_BUILD_CORES}}
             SHELL=$SHELL
-            $makeFlags "${makeFlagsArray[@]}"
-            ${checkFlags:-VERBOSE=y} "${checkFlagsArray[@]}"
-            ${checkTarget}
         )
 
+        _accumFlagsArray makeFlags makeFlagsArray
+        if [ -n "$__structuredAttrs" ]; then
+            flagsArray+=( "${checkFlags[@]:-VERBOSE=y}" )
+        else
+            flagsArray+=( ${checkFlags:-VERBOSE=y} )
+        fi
+        _accumFlagsArray checkFlagsArray
+        flagsArray+=( ${checkTarget} )
+
         echoCmd 'check flags' "${flagsArray[@]}"
         make ${makefile:+-f $makefile} "${flagsArray[@]}"
 
@@ -1163,14 +1270,16 @@ installPhase() {
         mkdir -p "$prefix"
     fi
 
-    # Old bash empty array hack
     # shellcheck disable=SC2086
     local flagsArray=(
         SHELL=$SHELL
-        $makeFlags "${makeFlagsArray[@]}"
-        $installFlags "${installFlagsArray[@]}"
-        ${installTargets:-install}
     )
+    _accumFlagsArray makeFlags makeFlagsArray installFlags installFlagsArray
+    if [ -n "$__structuredAttrs" ]; then
+        flagsArray+=( "${installTargets[@]:-install}" )
+    else
+        flagsArray+=( ${installTargets:-install} )
+    fi
 
     echoCmd 'install flags' "${flagsArray[@]}"
     make ${makefile:+-f $makefile} "${flagsArray[@]}"
@@ -1186,7 +1295,7 @@ installPhase() {
 fixupPhase() {
     # Make sure everything is writable so "strip" et al. work.
     local output
-    for output in $outputs; do
+    for output in $(getAllOutputNames); do
         if [ -e "${!output}" ]; then chmod -R u+w "${!output}"; fi
     done
 
@@ -1194,7 +1303,7 @@ fixupPhase() {
 
     # Apply fixup to each output.
     local output
-    for output in $outputs; do
+    for output in $(getAllOutputNames); do
         prefix="${!output}" runHook fixupOutput
     done
 
@@ -1239,7 +1348,10 @@ fixupPhase() {
     if [ -n "${setupHooks:-}" ]; then
         mkdir -p "${!outputDev}/nix-support"
         local hook
-        for hook in $setupHooks; do
+        # have to use ${setupHooks[@]} without quotes because it needs to support setupHooks being a array or a whitespace separated string
+        # # values of setupHooks won't have spaces so it won't cause problems
+        # shellcheck disable=2068
+        for hook in ${setupHooks[@]}; do
             local content
             consumeEntire content < "$hook"
             substituteAllStream content "file '$hook'" >> "${!outputDev}/nix-support/setup-hook"
@@ -1275,11 +1387,12 @@ installCheckPhase() {
         local flagsArray=(
             ${enableParallelChecking:+-j${NIX_BUILD_CORES}}
             SHELL=$SHELL
-            $makeFlags "${makeFlagsArray[@]}"
-            $installCheckFlags "${installCheckFlagsArray[@]}"
-            ${installCheckTarget:-installcheck}
         )
 
+        _accumFlagsArray makeFlags makeFlagsArray \
+          installCheckFlags installCheckFlagsArray
+        flagsArray+=( ${installCheckTarget:-installcheck} )
+
         echoCmd 'installcheck flags' "${flagsArray[@]}"
         make ${makefile:+-f $makefile} "${flagsArray[@]}"
         unset flagsArray
@@ -1292,11 +1405,9 @@ installCheckPhase() {
 distPhase() {
     runHook preDist
 
-    # Old bash empty array hack
-    # shellcheck disable=SC2086
-    local flagsArray=(
-        $distFlags "${distFlagsArray[@]}" ${distTarget:-dist}
-    )
+    local flagsArray=()
+    _accumFlagsArray distFlags distFlagsArray
+    flagsArray+=( ${distTarget:-dist} )
 
     echo 'dist flags: %q' "${flagsArray[@]}"
     make ${makefile:+-f $makefile} "${flagsArray[@]}"
@@ -1307,7 +1418,7 @@ distPhase() {
         # Note: don't quote $tarballs, since we explicitly permit
         # wildcards in there.
         # shellcheck disable=SC2086
-        cp -pvd ${tarballs:-*.tar.gz} "$out/tarballs"
+        cp -pvd ${tarballs[*]:-*.tar.gz} "$out/tarballs"
     fi
 
     runHook postDist
@@ -1357,14 +1468,18 @@ genericBuild() {
         return
     fi
 
-    if [ -z "${phases:-}" ]; then
-        phases="${prePhases:-} unpackPhase patchPhase ${preConfigurePhases:-} \
-            configurePhase ${preBuildPhases:-} buildPhase checkPhase \
-            ${preInstallPhases:-} installPhase ${preFixupPhases:-} fixupPhase installCheckPhase \
-            ${preDistPhases:-} distPhase ${postPhases:-}";
+    if [ -z "${phases[*]:-}" ]; then
+        phases="${prePhases[*]:-} unpackPhase patchPhase ${preConfigurePhases[*]:-} \
+            configurePhase ${preBuildPhases[*]:-} buildPhase checkPhase \
+            ${preInstallPhases[*]:-} installPhase ${preFixupPhases[*]:-} fixupPhase installCheckPhase \
+            ${preDistPhases[*]:-} distPhase ${postPhases[*]:-}";
     fi
 
-    for curPhase in $phases; do
+    # The use of ${phases[*]} gives the correct behavior both with and
+    # without structured attrs.  This relies on the fact that each
+    # phase name is space-free, which it must be because it's the name
+    # of either a shell variable or a shell function.
+    for curPhase in ${phases[*]}; do
         if [[ "$curPhase" = unpackPhase && -n "${dontUnpack:-}" ]]; then continue; fi
         if [[ "$curPhase" = patchPhase && -n "${dontPatch:-}" ]]; then continue; fi
         if [[ "$curPhase" = configurePhase && -n "${dontConfigure:-}" ]]; then continue; fi
@@ -1414,6 +1529,7 @@ runHook userHook
 
 dumpVars
 
+
 # Restore the original options for nix-shell
 [[ $__nixpkgs_setup_set_original == *e* ]] || set +e
 [[ $__nixpkgs_setup_set_original == *u* ]] || set +u