From 8d76effc178747f0c8f456fe619c1b014736a6af Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Tue, 11 Jul 2017 16:04:14 -0400
Subject: [PATCH 1/6] stdenv-setup: Make the package accumulators associative
 arrays instead of strings

This is generally cleaner: less eval, less worrying about separators,
and probably also faster. I got the idea from that python wrapper
script.
---
 .../haskell-modules/generic-builder.nix       |  2 +-
 pkgs/development/interpreters/python/wrap.sh  |  2 +-
 pkgs/servers/x11/xorg/builder.sh              |  4 +--
 pkgs/stdenv/generic/setup.sh                  | 27 +++++++++----------
 4 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/pkgs/development/haskell-modules/generic-builder.nix b/pkgs/development/haskell-modules/generic-builder.nix
index e097fd5af33e..a8da63493a43 100644
--- a/pkgs/development/haskell-modules/generic-builder.nix
+++ b/pkgs/development/haskell-modules/generic-builder.nix
@@ -211,7 +211,7 @@ stdenv.mkDerivation ({
     configureFlags="${concatStringsSep " " defaultConfigureFlags} $configureFlags"
 
     # nativePkgs defined in stdenv/setup.hs
-    for p in $nativePkgs; do
+    for p in "''${!nativePkgs[@]}"; do
       if [ -d "$p/lib/${ghc.name}/package.conf.d" ]; then
         cp -f "$p/lib/${ghc.name}/package.conf.d/"*.conf $packageConfDir/
         continue
diff --git a/pkgs/development/interpreters/python/wrap.sh b/pkgs/development/interpreters/python/wrap.sh
index 1c74e612b559..4b9b2024981a 100644
--- a/pkgs/development/interpreters/python/wrap.sh
+++ b/pkgs/development/interpreters/python/wrap.sh
@@ -86,7 +86,7 @@ wrapPythonProgramsIn() {
 _addToPythonPath() {
     local dir="$1"
     # Stop if we've already visited here.
-    if [ -n "${pythonPathsSeen[$dir]}" ]; then return; fi
+    [ -n "${pythonPathsSeen[$dir]}" ] || return 0
     pythonPathsSeen[$dir]=1
     # addToSearchPath is defined in stdenv/generic/setup.sh. It will have
     # the effect of calling `export program_X=$dir/...:$program_X`.
diff --git a/pkgs/servers/x11/xorg/builder.sh b/pkgs/servers/x11/xorg/builder.sh
index 055886374df4..3a8cf6fa6c8e 100644
--- a/pkgs/servers/x11/xorg/builder.sh
+++ b/pkgs/servers/x11/xorg/builder.sh
@@ -18,14 +18,14 @@ postInstall() {
 
     for r in $requires; do
         if test -n "$crossConfig"; then
-            for p in $crossPkgs; do
+            for p in "${!crossPkgs[@]}"; do
                 if test -e $p/lib/pkgconfig/$r.pc; then
                     echo "  found requisite $r in $p"
                     propagatedBuildInputs="$propagatedBuildInputs $p"
                 fi
             done
         else
-            for p in $nativePkgs; do
+            for p in "${!nativePkgs[@]}"; do
                 if test -e $p/lib/pkgconfig/$r.pc; then
                     echo "  found requisite $r in $p"
                     propagatedNativeBuildInputs="$propagatedNativeBuildInputs $p"
diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh
index de94565ed6d7..e90c7e03473b 100644
--- a/pkgs/stdenv/generic/setup.sh
+++ b/pkgs/stdenv/generic/setup.sh
@@ -275,17 +275,14 @@ runHook addInputsHook
 
 # Recursively find all build inputs.
 findInputs() {
-    local pkg="$1"
+    local pkg=$1
     local var=$2
+    local -n varDeref=$var
     local propagatedBuildInputsFile=$3
 
-    case ${!var} in
-        *\ $pkg\ *)
-            return 0
-            ;;
-    esac
-
-    eval $var="'${!var} $pkg '"
+    # Stop if we've already added this one
+    [[ -z "${varDeref["$pkg"]}" ]] || return 0
+    varDeref["$pkg"]=1
 
     if ! [ -e "$pkg" ]; then
         echo "build input $pkg does not exist" >&2
@@ -296,8 +293,8 @@ findInputs() {
         source "$pkg"
     fi
 
-    if [ -d $1/bin ]; then
-        addToSearchPath _PATH $1/bin
+    if [ -d "$pkg/bin" ]; then
+        addToSearchPath _PATH "$pkg/bin"
     fi
 
     if [ -f "$pkg/nix-support/setup-hook" ]; then
@@ -317,19 +314,19 @@ findInputs() {
 if [ -z "$crossConfig" ]; then
     # Not cross-compiling - both buildInputs (and variants like propagatedBuildInputs)
     # are handled identically to nativeBuildInputs
-    nativePkgs=""
+    declare -gA nativePkgs
     for i in $nativeBuildInputs $buildInputs \
              $defaultNativeBuildInputs $defaultBuildInputs \
              $propagatedNativeBuildInputs $propagatedBuildInputs; do
         findInputs $i nativePkgs propagated-native-build-inputs
     done
 else
-    crossPkgs=""
+    declare -gA crossPkgs
     for i in $buildInputs $defaultBuildInputs $propagatedBuildInputs; do
         findInputs $i crossPkgs propagated-build-inputs
     done
 
-    nativePkgs=""
+    declare -gA nativePkgs
     for i in $nativeBuildInputs $defaultNativeBuildInputs $propagatedNativeBuildInputs; do
         findInputs $i nativePkgs propagated-native-build-inputs
     done
@@ -345,7 +342,7 @@ _addToNativeEnv() {
     runHook envHook "$pkg"
 }
 
-for i in $nativePkgs; do
+for i in "${!nativePkgs[@]}"; do
     _addToNativeEnv $i
 done
 
@@ -356,7 +353,7 @@ _addToCrossEnv() {
     runHook crossEnvHook "$pkg"
 }
 
-for i in $crossPkgs; do
+for i in "${!crossPkgs[@]}"; do
     _addToCrossEnv $i
 done
 

From 5d4efb2c816d2143f29cad8153faad1686557b2a Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Tue, 11 Jul 2017 20:23:06 -0400
Subject: [PATCH 2/6] stdenv-setup: Misc improvements as directed by ShellCheck

I took some liberties with the flags-echoing code to make it more
concise and correct. Also, a few warnings in findInputs and friends I
skipped because I am going to rewrite those anyways.

Thanks @grahamc for telling me about this great linter!
---
 pkgs/stdenv/generic/setup.sh | 140 +++++++++++++++++++++++------------
 1 file changed, 93 insertions(+), 47 deletions(-)

diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh
index e90c7e03473b..2e424c1f5813 100644
--- a/pkgs/stdenv/generic/setup.sh
+++ b/pkgs/stdenv/generic/setup.sh
@@ -52,9 +52,9 @@ runOneHook() {
 _callImplicitHook() {
     local def="$1"
     local hookName="$2"
-    case "$(type -t $hookName)" in
-        (function|alias|builtin) $hookName;;
-        (file) source $hookName;;
+    case "$(type -t "$hookName")" in
+        (function|alias|builtin) "$hookName";;
+        (file) source "$hookName";;
         (keyword) :;;
         (*) if [ -z "${!hookName}" ]; then return "$def"; else eval "${!hookName}"; fi;;
     esac
@@ -66,7 +66,7 @@ _callImplicitHook() {
 _eval() {
     local code="$1"
     shift
-    if [ "$(type -t $code)" = function ]; then
+    if [ "$(type -t "$code")" = function ]; then
         eval "$code \"\$@\""
     else
         eval "$code"
@@ -80,12 +80,14 @@ _eval() {
 nestingLevel=0
 
 startNest() {
-    nestingLevel=$(($nestingLevel + 1))
+    # Assert natural as sanity check.
+    let nestingLevel+=1 "nestingLevel>=0"
     echo -en "\033[$1p"
 }
 
 stopNest() {
-    nestingLevel=$(($nestingLevel - 1))
+    # Assert natural as sanity check.
+    let nestingLevel-=1 "nestingLevel>=0"
     echo -en "\033[q"
 }
 
@@ -120,7 +122,7 @@ exitHandler() {
         # - system time for the shell
         # - user time for all child processes
         # - system time for all child processes
-        echo "build time elapsed: " ${times[*]}
+        echo "build time elapsed: " "${times[@]}"
     fi
 
     if [ $exitCode != 0 ]; then
@@ -197,7 +199,7 @@ isELF() {
     local fd
     local magic
     exec {fd}< "$fn"
-    read -n 4 -u $fd magic
+    read -r -n 4 -u $fd magic
     exec {fd}<&-
     if [[ "$magic" =~ ELF ]]; then return 0; else return 1; fi
 }
@@ -210,7 +212,7 @@ isScript() {
     local magic
     if ! [ -x /bin/sh ]; then return 0; fi
     exec {fd}< "$fn"
-    read -n 2 -u $fd magic
+    read -r -n 2 -u $fd magic
     exec {fd}<&-
     if [[ "$magic" =~ \#! ]]; then return 0; else return 1; fi
 }
@@ -439,14 +441,14 @@ substitute() {
         p="${params[$n]}"
 
         if [ "$p" = --replace ]; then
-            pattern="${params[$((n + 1))]}"
-            replacement="${params[$((n + 2))]}"
-            n=$((n + 2))
+            pattern="${params[$n + 1]}"
+            replacement="${params[$n + 2]}"
+            let n+=2
         fi
 
         if [ "$p" = --subst-var ]; then
-            varName="${params[$((n + 1))]}"
-            n=$((n + 1))
+            varName="${params[$n + 1]}"
+            let n+=1
             # check if the used nix attribute name is a valid bash name
             if ! [[ "$varName" =~ ^[a-zA-Z_][a-zA-Z0-9_]*$ ]]; then
                 echo "WARNING: substitution variables should be valid bash names,"
@@ -459,9 +461,9 @@ substitute() {
         fi
 
         if [ "$p" = --subst-var-by ]; then
-            pattern="@${params[$((n + 1))]}@"
-            replacement="${params[$((n + 2))]}"
-            n=$((n + 2))
+            pattern="@${params[$n + 1]}@"
+            replacement="${params[$n + 2]}"
+            let n+=2
         fi
 
         content="${content//"$pattern"/$replacement}"
@@ -525,7 +527,9 @@ dumpVars() {
 # Utility function: echo the base name of the given path, with the
 # prefix `HASH-' removed, if present.
 stripHash() {
-    local strippedName="$(basename "$1")";
+    local strippedName
+    # On separate line for `set -e`
+    strippedName="$(basename "$1")"
     if echo "$strippedName" | grep -q '^[a-z0-9]\{32\}-'; then
         echo "$strippedName" | cut -c34-
     else
@@ -582,6 +586,7 @@ unpackPhase() {
 
     if [ -z "$srcs" ]; then
         if [ -z "$src" ]; then
+            # shellcheck disable=SC2016
             echo 'variable $src or $srcs should point to the source'
             exit 1
         fi
@@ -601,7 +606,7 @@ unpackPhase() {
 
     # Unpack all source archives.
     for i in $srcs; do
-        unpackFile $i
+        unpackFile "$i"
     done
 
     # Find the source directory.
@@ -665,6 +670,7 @@ patchPhase() {
                 ;;
         esac
         # "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}
         stopNest
     done
@@ -681,18 +687,19 @@ fixLibtool() {
 configurePhase() {
     runHook preConfigure
 
-    if [ -z "$configureScript" -a -x ./configure ]; then
+    if [[ -z "$configureScript" && -x ./configure ]]; then
         configureScript=./configure
     fi
 
     if [ -z "$dontFixLibtool" ]; then
-        find . -iname "ltmain.sh" | while read i; do
+        local i
+        find . -iname "ltmain.sh" -print0 | while IFS='' read -r -d '' i; do
             echo "fixing libtool script $i"
-            fixLibtool $i
+            fixLibtool "$i"
         done
     fi
 
-    if [ -z "$dontAddPrefix" -a -n "$prefix" ]; then
+    if [[ -z "$dontAddPrefix" && -n "$prefix" ]]; then
         configureFlags="${prefixKey:---prefix=}$prefix $configureFlags"
     fi
 
@@ -711,8 +718,14 @@ configurePhase() {
     fi
 
     if [ -n "$configureScript" ]; then
-        echo "configure flags: $configureFlags ${configureFlagsArray[@]}"
-        $configureScript $configureFlags "${configureFlagsArray[@]}"
+        # shellcheck disable=SC2086
+        local flagsArray=($configureFlags "${configureFlagsArray[@]}")
+        printf 'configure flags:'
+        printf ' %q' "${flagsArray[@]}"
+        echo
+        # shellcheck disable=SC2086
+        $configureScript "${flagsArray[@]}"
+        unset flagsArray
     else
         echo "no configure script, doing nothing"
     fi
@@ -724,17 +737,23 @@ configurePhase() {
 buildPhase() {
     runHook preBuild
 
-    if [ -z "$makeFlags" ] && ! [ -n "$makefile" -o -e "Makefile" -o -e "makefile" -o -e "GNUmakefile" ]; then
+    if [ -z "$makeFlags" ] && ! [[ -n "$makefile" || -e "Makefile" || -e "makefile" || -e "GNUmakefile" ]]; then
         echo "no Makefile, doing nothing"
     else
         # See https://github.com/NixOS/nixpkgs/pull/1354#issuecomment-31260409
         makeFlags="SHELL=$SHELL $makeFlags"
 
-        echo "make flags: $makeFlags ${makeFlagsArray[@]} $buildFlags ${buildFlagsArray[@]}"
-        make ${makefile:+-f $makefile} \
+        # shellcheck disable=SC2086
+        local flagsArray=( \
             ${enableParallelBuilding:+-j${NIX_BUILD_CORES} -l${NIX_BUILD_CORES}} \
             $makeFlags "${makeFlagsArray[@]}" \
-            $buildFlags "${buildFlagsArray[@]}"
+            $buildFlags "${buildFlagsArray[@]}")
+
+        printf 'build flags:'
+        printf ' %q' "${flagsArray[@]}"
+        echo
+        make ${makefile:+-f $makefile} "${flagsArray[@]}"
+        unset flagsArray
     fi
 
     runHook postBuild
@@ -744,11 +763,17 @@ buildPhase() {
 checkPhase() {
     runHook preCheck
 
-    echo "check flags: $makeFlags ${makeFlagsArray[@]} $checkFlags ${checkFlagsArray[@]}"
-    make ${makefile:+-f $makefile} \
+    # shellcheck disable=SC2086
+    local flagsArray=( \
         ${enableParallelBuilding:+-j${NIX_BUILD_CORES} -l${NIX_BUILD_CORES}} \
         $makeFlags "${makeFlagsArray[@]}" \
-        ${checkFlags:-VERBOSE=y} "${checkFlagsArray[@]}" ${checkTarget:-check}
+        ${checkFlags:-VERBOSE=y} "${checkFlagsArray[@]}" ${checkTarget:-check})
+
+    printf 'check flags:'
+    printf ' %q' "${flagsArray[@]}"
+    echo
+    make ${makefile:+-f $makefile} "${flagsArray[@]}"
+    unset flagsArray
 
     runHook postCheck
 }
@@ -762,10 +787,17 @@ installPhase() {
     fi
 
     installTargets=${installTargets:-install}
-    echo "install flags: $installTargets $makeFlags ${makeFlagsArray[@]} $installFlags ${installFlagsArray[@]}"
-    make ${makefile:+-f $makefile} $installTargets \
+
+    # shellcheck disable=SC2086
+    local flagsArray=( $installTargets \
         $makeFlags "${makeFlagsArray[@]}" \
-        $installFlags "${installFlagsArray[@]}"
+        $installFlags "${installFlagsArray[@]}")
+
+    printf 'install flags:'
+    printf ' %q' "${flagsArray[@]}"
+    echo
+    make ${makefile:+-f $makefile} "${flagsArray[@]}"
+    unset flagsArray
 
     runHook postInstall
 }
@@ -799,16 +831,19 @@ fixupPhase() {
         fi
         if [ -n "$propagated" ]; then
             mkdir -p "${!outputDev}/nix-support"
+            # shellcheck disable=SC2086
             printLines $propagated > "${!outputDev}/nix-support/propagated-native-build-inputs"
         fi
     else
         if [ -n "$propagatedBuildInputs" ]; then
             mkdir -p "${!outputDev}/nix-support"
+            # shellcheck disable=SC2086
             printLines $propagatedBuildInputs > "${!outputDev}/nix-support/propagated-build-inputs"
         fi
 
         if [ -n "$propagatedNativeBuildInputs" ]; then
             mkdir -p "${!outputDev}/nix-support"
+            # shellcheck disable=SC2086
             printLines $propagatedNativeBuildInputs > "${!outputDev}/nix-support/propagated-native-build-inputs"
         fi
     fi
@@ -822,6 +857,7 @@ fixupPhase() {
 
     if [ -n "$propagatedUserEnvPkgs" ]; then
         mkdir -p "${!outputBin}/nix-support"
+        # shellcheck disable=SC2086
         printLines $propagatedUserEnvPkgs > "${!outputBin}/nix-support/propagated-user-env-packages"
     fi
 
@@ -832,11 +868,17 @@ fixupPhase() {
 installCheckPhase() {
     runHook preInstallCheck
 
-    echo "installcheck flags: $makeFlags ${makeFlagsArray[@]} $installCheckFlags ${installCheckFlagsArray[@]}"
-    make ${makefile:+-f $makefile} \
+    # shellcheck disable=SC2086
+    local flagsArray=( \
         ${enableParallelBuilding:+-j${NIX_BUILD_CORES} -l${NIX_BUILD_CORES}} \
         $makeFlags "${makeFlagsArray[@]}" \
-        $installCheckFlags "${installCheckFlagsArray[@]}" ${installCheckTarget:-installcheck}
+        $installCheckFlags "${installCheckFlagsArray[@]}" ${installCheckTarget:-installcheck})
+
+    printf 'installcheck flags:'
+    printf ' %q' "${flagsArray[@]}"
+    echo
+    make ${makefile:+-f $makefile} "${flagsArray[@]}"
+    unset flagsArray
 
     runHook postInstallCheck
 }
@@ -845,14 +887,18 @@ installCheckPhase() {
 distPhase() {
     runHook preDist
 
-    echo "dist flags: $distFlags ${distFlagsArray[@]}"
-    make ${makefile:+-f $makefile} $distFlags "${distFlagsArray[@]}" ${distTarget:-dist}
+    # shellcheck disable=SC2086
+    local flagsArray=($distFlags "${distFlagsArray[@]}" ${distTarget:-dist})
+
+    echo 'dist flags: %q' "${flagsArray[@]}"
+    make ${makefile:+-f $makefile} "${flagsArray[@]}"
 
     if [ "$dontCopyDist" != 1 ]; then
         mkdir -p "$out/tarballs"
 
         # Note: don't quote $tarballs, since we explicitly permit
         # wildcards in there.
+        # shellcheck disable=SC2086
         cp -pvd ${tarballs:-*.tar.gz} $out/tarballs
     fi
 
@@ -894,14 +940,14 @@ genericBuild() {
     fi
 
     for curPhase in $phases; do
-        if [ "$curPhase" = buildPhase -a -n "$dontBuild" ]; then continue; fi
-        if [ "$curPhase" = checkPhase -a -z "$doCheck" ]; then continue; fi
-        if [ "$curPhase" = installPhase -a -n "$dontInstall" ]; then continue; fi
-        if [ "$curPhase" = fixupPhase -a -n "$dontFixup" ]; then continue; fi
-        if [ "$curPhase" = installCheckPhase -a -z "$doInstallCheck" ]; then continue; fi
-        if [ "$curPhase" = distPhase -a -z "$doDist" ]; then continue; fi
+        if [[ "$curPhase" = buildPhase && -n "$dontBuild" ]]; then continue; fi
+        if [[ "$curPhase" = checkPhase && -z "$doCheck" ]]; then continue; fi
+        if [[ "$curPhase" = installPhase && -n "$dontInstall" ]]; then continue; fi
+        if [[ "$curPhase" = fixupPhase && -n "$dontFixup" ]]; then continue; fi
+        if [[ "$curPhase" = installCheckPhase && -z "$doInstallCheck" ]]; then continue; fi
+        if [[ "$curPhase" = distPhase && -z "$doDist" ]]; then continue; fi
 
-        if [ -n "$tracePhases" ]; then
+        if [[ -n "$tracePhases" ]]; then
             echo
             echo "@ phase-started $out $curPhase"
         fi

From 5d693c84d2e492080e982bf7429d6e923229d721 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 12 Jul 2017 11:56:43 -0400
Subject: [PATCH 3/6] stdenv-setup: Clean up 'substitute()' for style and error
 handling

It now blows up on null byte in file (rather than silently truncating),
and invalid arguments (rather than silently skipping).
---
 pkgs/stdenv/generic/setup.sh | 75 +++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 35 deletions(-)

diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh
index 2e424c1f5813..0d9a647a5c30 100644
--- a/pkgs/stdenv/generic/setup.sh
+++ b/pkgs/stdenv/generic/setup.sh
@@ -421,50 +421,55 @@ fi
 
 
 substitute() {
-    local input="$1"
-    local output="$2"
+    local input=$1
+    local output=$2
+    shift 2
 
     if [ ! -f "$input" ]; then
-      echo "substitute(): file '$input' does not exist"
+      echo "${FUNCNAME[0]}(): ERROR: file '$input' does not exist" >&2
       return 1
     fi
 
-    local -a params=("$@")
+    local content
+    # read returns non-0 on EOF, so we want read to fail
+    if IFS='' read -r -N 0 content < "$input"; then
+        echo "${FUNCNAME[0]}(): ERROR: File \"$input\" has null bytes, won't process" >&2
+        return 1
+    fi
 
-    local n p pattern replacement varName content
+    while (( "$#" )); do
+        case "$1" in
+            --replace)
+                pattern=$2
+                replacement=$3
+                shift 3
+                ;;
 
-    # a slightly hacky way to keep newline at the end
-    content="$(cat "$input"; printf "%s" X)"
-    content="${content%X}"
+            --subst-var)
+                local varName=$2
+                shift 2
+                # check if the used nix attribute name is a valid bash name
+                if ! [[ "$varName" =~ ^[a-zA-Z_][a-zA-Z0-9_]*$ ]]; then
+                    echo "${FUNCNAME[0]}(): WARNING: substitution variables should be valid bash names," >&2
+                    echo "  \"$varName\" isn't and therefore was skipped; it might be caused" >&2
+                    echo "  by multi-line phases in variables - see #14907 for details." >&2
+                    continue
+                fi
+                pattern=@$varName@
+                replacement=${!varName}
+                ;;
 
-    for ((n = 2; n < ${#params[*]}; n += 1)); do
-        p="${params[$n]}"
+            --subst-var-by)
+                pattern=@$2@
+                replacement=$3
+                shift 3
+                ;;
 
-        if [ "$p" = --replace ]; then
-            pattern="${params[$n + 1]}"
-            replacement="${params[$n + 2]}"
-            let n+=2
-        fi
-
-        if [ "$p" = --subst-var ]; then
-            varName="${params[$n + 1]}"
-            let n+=1
-            # check if the used nix attribute name is a valid bash name
-            if ! [[ "$varName" =~ ^[a-zA-Z_][a-zA-Z0-9_]*$ ]]; then
-                echo "WARNING: substitution variables should be valid bash names,"
-                echo "  \"$varName\" isn't and therefore was skipped; it might be caused"
-                echo "  by multi-line phases in variables - see #14907 for details."
-                continue
-            fi
-            pattern="@$varName@"
-            replacement="${!varName}"
-        fi
-
-        if [ "$p" = --subst-var-by ]; then
-            pattern="@${params[$n + 1]}@"
-            replacement="${params[$n + 2]}"
-            let n+=2
-        fi
+            *)
+                echo "${FUNCNAME[0]}(): ERROR: Invalid command line argument: $1" >&2
+                return 1
+                ;;
+        esac
 
         content="${content//"$pattern"/$replacement}"
     done

From 273a4c1c78d9e0b3aa852f197ce9ecc0e63ce42b Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Thu, 13 Jul 2017 14:56:40 -0400
Subject: [PATCH 4/6] stdenv-setup: Combine [[ .. ]] && [[ .. ]] into one [[ ..
 && .. ]]

Also remove useless quotes on same line
---
 pkgs/stdenv/generic/setup.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh
index 0d9a647a5c30..de83e4fb18f0 100644
--- a/pkgs/stdenv/generic/setup.sh
+++ b/pkgs/stdenv/generic/setup.sh
@@ -742,7 +742,7 @@ configurePhase() {
 buildPhase() {
     runHook preBuild
 
-    if [ -z "$makeFlags" ] && ! [[ -n "$makefile" || -e "Makefile" || -e "makefile" || -e "GNUmakefile" ]]; then
+    if [[ -z $makeFlags && ! ( -n $makefile || -e Makefile || -e makefile || -e GNUmakefile[[ ) ]]; then
         echo "no Makefile, doing nothing"
     else
         # See https://github.com/NixOS/nixpkgs/pull/1354#issuecomment-31260409

From 2743078f664ae07c4bed06a96182c6a86bd7fa32 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Thu, 13 Jul 2017 14:59:53 -0400
Subject: [PATCH 5/6] stdenv-setup: Remove useless quotes

foo=$1 surprisingly doesn't need quotes in Bash. Word splits are only
syntactic in string variable (not array var!) assignments.
---
 pkgs/stdenv/generic/setup.sh | 70 ++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh
index de83e4fb18f0..f95b2e58837a 100644
--- a/pkgs/stdenv/generic/setup.sh
+++ b/pkgs/stdenv/generic/setup.sh
@@ -13,10 +13,10 @@ set -o pipefail
 # code). The hooks for <hookName> are the shell function or variable
 # <hookName>, and the values of the shell array ‘<hookName>Hooks’.
 runHook() {
-    local hookName="$1"
+    local hookName=$1
     shift
     local var="$hookName"
-    if [[ "$hookName" =~ Hook$ ]]; then var+=s; else var+=Hooks; fi
+    if [[ $hookName =~ Hook$ ]]; then var+=s; else var+=Hooks; fi
     local -n var
     local hook
     for hook in "_callImplicitHook 0 $hookName" "${var[@]}"; do
@@ -29,10 +29,10 @@ runHook() {
 # Run all hooks with the specified name, until one succeeds (returns a
 # zero exit code). If none succeed, return a non-zero exit code.
 runOneHook() {
-    local hookName="$1"
+    local hookName=$1
     shift
     local var="$hookName"
-    if [[ "$hookName" =~ Hook$ ]]; then var+=s; else var+=Hooks; fi
+    if [[ $hookName =~ Hook$ ]]; then var+=s; else var+=Hooks; fi
     local -n var
     local hook
     for hook in "_callImplicitHook 1 $hookName" "${var[@]}"; do
@@ -50,8 +50,8 @@ runOneHook() {
 # environment variables) and from shell scripts (as functions). If you
 # want to allow multiple hooks, use runHook instead.
 _callImplicitHook() {
-    local def="$1"
-    local hookName="$2"
+    local def=$1
+    local hookName=$2
     case "$(type -t "$hookName")" in
         (function|alias|builtin) "$hookName";;
         (file) source "$hookName";;
@@ -64,7 +64,7 @@ _callImplicitHook() {
 # A function wrapper around ‘eval’ that ensures that ‘return’ inside
 # hooks exits the hook, not the caller.
 _eval() {
-    local code="$1"
+    local code=$1
     shift
     if [ "$(type -t "$code")" = function ]; then
         eval "$code \"\$@\""
@@ -195,26 +195,26 @@ _addRpathPrefix() {
 
 # Return success if the specified file is an ELF object.
 isELF() {
-    local fn="$1"
+    local fn=$1
     local fd
     local magic
     exec {fd}< "$fn"
     read -r -n 4 -u $fd magic
     exec {fd}<&-
-    if [[ "$magic" =~ ELF ]]; then return 0; else return 1; fi
+    if [[ $magic =~ ELF ]]; then return 0; else return 1; fi
 }
 
 # Return success if the specified file is a script (i.e. starts with
 # "#!").
 isScript() {
-    local fn="$1"
+    local fn=$1
     local fd
     local magic
     if ! [ -x /bin/sh ]; then return 0; fi
     exec {fd}< "$fn"
     read -r -n 2 -u $fd magic
     exec {fd}<&-
-    if [[ "$magic" =~ \#! ]]; then return 0; else return 1; fi
+    if [[ $magic =~ \#! ]]; then return 0; else return 1; fi
 }
 
 # printf unfortunately will print a trailing newline regardless
@@ -255,8 +255,8 @@ fi
 
 # Check that the pre-hook initialised SHELL.
 if [ -z "$SHELL" ]; then echo "SHELL not set"; exit 1; fi
-BASH="$SHELL"
-export CONFIG_SHELL="$SHELL"
+BASH=$SHELL
+export CONFIG_SHELL=$SHELL
 
 
 # Dummy implementation of the paxmark function. On Linux, this is
@@ -283,7 +283,7 @@ findInputs() {
     local propagatedBuildInputsFile=$3
 
     # Stop if we've already added this one
-    [[ -z "${varDeref["$pkg"]}" ]] || return 0
+    [[ -z ${varDeref[$pkg]} ]] || return 0
     varDeref["$pkg"]=1
 
     if ! [ -e "$pkg" ]; then
@@ -373,7 +373,7 @@ export TZ=UTC
 # for instance if we just want to perform a test build/install to a
 # temporary location and write a build report to $out.
 if [ -z "$prefix" ]; then
-    prefix="$out";
+    prefix=$out;
 fi
 
 if [ "$useTempPrefix" = 1 ]; then
@@ -449,7 +449,7 @@ substitute() {
                 local varName=$2
                 shift 2
                 # check if the used nix attribute name is a valid bash name
-                if ! [[ "$varName" =~ ^[a-zA-Z_][a-zA-Z0-9_]*$ ]]; then
+                if ! [[ $varName =~ ^[a-zA-Z_][a-zA-Z0-9_]*$ ]]; then
                     echo "${FUNCNAME[0]}(): WARNING: substitution variables should be valid bash names," >&2
                     echo "  \"$varName\" isn't and therefore was skipped; it might be caused" >&2
                     echo "  by multi-line phases in variables - see #14907 for details." >&2
@@ -480,7 +480,7 @@ substitute() {
 
 
 substituteInPlace() {
-    local fileName="$1"
+    local fileName=$1
     shift
     substitute "$fileName" "$fileName" "$@"
 }
@@ -490,8 +490,8 @@ substituteInPlace() {
 # character or underscore. Note: other names that aren't bash-valid
 # will cause an error during `substitute --subst-var`.
 substituteAll() {
-    local input="$1"
-    local output="$2"
+    local input=$1
+    local output=$2
     local -a args=()
 
     # Select all environment variables that start with a lowercase character.
@@ -507,7 +507,7 @@ substituteAll() {
 
 
 substituteAllInPlace() {
-    local fileName="$1"
+    local fileName=$1
     shift
     substituteAll "$fileName" "$fileName" "$@"
 }
@@ -534,7 +534,7 @@ dumpVars() {
 stripHash() {
     local strippedName
     # On separate line for `set -e`
-    strippedName="$(basename "$1")"
+    strippedName=$(basename "$1")
     if echo "$strippedName" | grep -q '^[a-z0-9]\{32\}-'; then
         echo "$strippedName" | cut -c34-
     else
@@ -545,7 +545,7 @@ stripHash() {
 
 unpackCmdHooks+=(_defaultUnpack)
 _defaultUnpack() {
-    local fn="$1"
+    local fn=$1
 
     if [ -d "$fn" ]; then
 
@@ -576,7 +576,7 @@ _defaultUnpack() {
 
 
 unpackFile() {
-    curSrc="$1"
+    curSrc=$1
     header "unpacking source archive $curSrc" 3
     if ! runOneHook unpackCmd "$curSrc"; then
         echo "do not know how to unpack source archive $curSrc"
@@ -595,7 +595,7 @@ unpackPhase() {
             echo 'variable $src or $srcs should point to the source'
             exit 1
         fi
-        srcs="$src"
+        srcs=$src
     fi
 
     # To determine the source directory created by unpacking the
@@ -629,7 +629,7 @@ unpackPhase() {
                             echo "unpacker produced multiple directories"
                             exit 1
                         fi
-                        sourceRoot="$i"
+                        sourceRoot=$i
                         ;;
                 esac
             fi
@@ -692,7 +692,7 @@ fixLibtool() {
 configurePhase() {
     runHook preConfigure
 
-    if [[ -z "$configureScript" && -x ./configure ]]; then
+    if [[ -z $configureScript && -x ./configure ]]; then
         configureScript=./configure
     fi
 
@@ -704,7 +704,7 @@ configurePhase() {
         done
     fi
 
-    if [[ -z "$dontAddPrefix" && -n "$prefix" ]]; then
+    if [[ -z $dontAddPrefix && -n $prefix ]]; then
         configureFlags="${prefixKey:---prefix=}$prefix $configureFlags"
     fi
 
@@ -912,7 +912,7 @@ distPhase() {
 
 
 showPhaseHeader() {
-    local phase="$1"
+    local phase=$1
     case $phase in
         unpackPhase) header "unpacking sources";;
         patchPhase) header "patching sources";;
@@ -945,14 +945,14 @@ genericBuild() {
     fi
 
     for curPhase in $phases; do
-        if [[ "$curPhase" = buildPhase && -n "$dontBuild" ]]; then continue; fi
-        if [[ "$curPhase" = checkPhase && -z "$doCheck" ]]; then continue; fi
-        if [[ "$curPhase" = installPhase && -n "$dontInstall" ]]; then continue; fi
-        if [[ "$curPhase" = fixupPhase && -n "$dontFixup" ]]; then continue; fi
-        if [[ "$curPhase" = installCheckPhase && -z "$doInstallCheck" ]]; then continue; fi
-        if [[ "$curPhase" = distPhase && -z "$doDist" ]]; then continue; fi
+        if [[ $curPhase = buildPhase && -n $dontBuild ]]; then continue; fi
+        if [[ $curPhase = checkPhase && -z $doCheck ]]; then continue; fi
+        if [[ $curPhase = installPhase && -n $dontInstall ]]; then continue; fi
+        if [[ $curPhase = fixupPhase && -n $dontFixup ]]; then continue; fi
+        if [[ $curPhase = installCheckPhase && -z $doInstallCheck ]]; then continue; fi
+        if [[ $curPhase = distPhase && -z $doDist ]]; then continue; fi
 
-        if [[ -n "$tracePhases" ]]; then
+        if [[ -n $tracePhases ]]; then
             echo
             echo "@ phase-started $out $curPhase"
         fi

From 30a14204149c8fb43001c4f2188e8e655a9a389a Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Thu, 13 Jul 2017 16:31:39 -0400
Subject: [PATCH 6/6] stdenv-setup: Pull out and explain 3-part printing of
 commands

@Dezgeg made the good point that the reasons for doing this were not at
all intuitive.
---
 pkgs/stdenv/generic/setup.sh | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh
index f95b2e58837a..35fe8fd69504 100644
--- a/pkgs/stdenv/generic/setup.sh
+++ b/pkgs/stdenv/generic/setup.sh
@@ -104,6 +104,17 @@ closeNest() {
     done
 }
 
+# Prints a command such that all word splits are unambiguous. We need
+# to split the command in three parts because the middle format string
+# will be, and must be, repeated for each argument. The first argument
+# goes before the ':' and is just for convenience.
+echoCmd() {
+    printf "%s:" "$1"
+    shift
+    printf ' %q' "$@"
+    echo
+}
+
 
 ######################################################################
 # Error handling.
@@ -725,9 +736,7 @@ configurePhase() {
     if [ -n "$configureScript" ]; then
         # shellcheck disable=SC2086
         local flagsArray=($configureFlags "${configureFlagsArray[@]}")
-        printf 'configure flags:'
-        printf ' %q' "${flagsArray[@]}"
-        echo
+        echoCmd 'configure flags' "${flagsArray[@]}"
         # shellcheck disable=SC2086
         $configureScript "${flagsArray[@]}"
         unset flagsArray
@@ -754,9 +763,7 @@ buildPhase() {
             $makeFlags "${makeFlagsArray[@]}" \
             $buildFlags "${buildFlagsArray[@]}")
 
-        printf 'build flags:'
-        printf ' %q' "${flagsArray[@]}"
-        echo
+        echoCmd 'build flags' "${flagsArray[@]}"
         make ${makefile:+-f $makefile} "${flagsArray[@]}"
         unset flagsArray
     fi
@@ -774,9 +781,7 @@ checkPhase() {
         $makeFlags "${makeFlagsArray[@]}" \
         ${checkFlags:-VERBOSE=y} "${checkFlagsArray[@]}" ${checkTarget:-check})
 
-    printf 'check flags:'
-    printf ' %q' "${flagsArray[@]}"
-    echo
+    echoCmd 'check flags' "${flagsArray[@]}"
     make ${makefile:+-f $makefile} "${flagsArray[@]}"
     unset flagsArray
 
@@ -798,9 +803,7 @@ installPhase() {
         $makeFlags "${makeFlagsArray[@]}" \
         $installFlags "${installFlagsArray[@]}")
 
-    printf 'install flags:'
-    printf ' %q' "${flagsArray[@]}"
-    echo
+    echoCmd 'install flags' "${flagsArray[@]}"
     make ${makefile:+-f $makefile} "${flagsArray[@]}"
     unset flagsArray
 
@@ -879,9 +882,7 @@ installCheckPhase() {
         $makeFlags "${makeFlagsArray[@]}" \
         $installCheckFlags "${installCheckFlagsArray[@]}" ${installCheckTarget:-installcheck})
 
-    printf 'installcheck flags:'
-    printf ' %q' "${flagsArray[@]}"
-    echo
+    echoCmd 'installcheck flags' "${flagsArray[@]}"
     make ${makefile:+-f $makefile} "${flagsArray[@]}"
     unset flagsArray