From cdb2f2971c9a94a4fc15cf505015053ddff0ba1a Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 9 Jun 2024 14:05:47 +0200 Subject: [PATCH 01/14] stdenv: refactor appendToVar and prependToVar No need to call declare -p twice. The case statement is easier to read than the multi-if. --- pkgs/stdenv/generic/setup.sh | 40 ++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh index 40bf6554183c6..8d060493b5560 100644 --- a/pkgs/stdenv/generic/setup.sh +++ b/pkgs/stdenv/generic/setup.sh @@ -280,16 +280,16 @@ prependToVar() { fi # check if variable already exist and if it does then do extra checks - if declare -p "$1" 2> /dev/null | grep -q '^'; then - type="$(declare -p "$1")" - if [[ "$type" =~ "declare -A" ]]; then - echo "prependToVar(): ERROR: trying to use prependToVar on an associative array." >&2 - return 1 - elif [[ "$type" =~ "declare -a" ]]; then - useArray=true - else - useArray=false - fi + if type=$(declare -p "$1" 2> /dev/null); then + case "${type#* }" in + -A*) + echo "prependToVar(): ERROR: trying to use prependToVar on an associative array." >&2 + return 1 ;; + -a*) + useArray=true ;; + *) + useArray=false ;; + esac fi shift @@ -313,16 +313,16 @@ appendToVar() { fi # check if variable already exist and if it does then do extra checks - if declare -p "$1" 2> /dev/null | grep -q '^'; then - type="$(declare -p "$1")" - if [[ "$type" =~ "declare -A" ]]; then - echo "appendToVar(): ERROR: trying to use appendToVar on an associative array, use variable+=([\"X\"]=\"Y\") instead." >&2 - return 1 - elif [[ "$type" =~ "declare -a" ]]; then - useArray=true - else - useArray=false - fi + if type=$(declare -p "$1" 2> /dev/null); then + case "${type#* }" in + -A*) + echo "appendToVar(): ERROR: trying to use appendToVar on an associative array, use variable+=([\"X\"]=\"Y\") instead." >&2 + return 1 ;; + -a*) + useArray=true ;; + *) + useArray=false ;; + esac fi shift From 929db7bc228eac527c80658905e4afaf19765c3e Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Fri, 2 Aug 2024 21:52:55 +0200 Subject: [PATCH 02/14] tests.stdenv: fix spelling --- pkgs/test/stdenv/default.nix | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkgs/test/stdenv/default.nix b/pkgs/test/stdenv/default.nix index 00f1ce90ef3ad..72310b74061f1 100644 --- a/pkgs/test/stdenv/default.nix +++ b/pkgs/test/stdenv/default.nix @@ -74,19 +74,19 @@ let declare -p string declare -A associativeArray=(["X"]="Y") - [[ $(appendToVar associativeArray "fail" 2>&1) =~ "trying to use" ]] || (echo "prependToVar did not catch prepending associativeArray" && false) - [[ $(prependToVar associativeArray "fail" 2>&1) =~ "trying to use" ]] || (echo "prependToVar did not catch prepending associativeArray" && false) + [[ $(appendToVar associativeArray "fail" 2>&1) =~ "trying to use" ]] || (echo "appendToVar did not throw appending to associativeArray" && false) + [[ $(prependToVar associativeArray "fail" 2>&1) =~ "trying to use" ]] || (echo "prependToVar did not throw prepending associativeArray" && false) [[ $string == "world testing-string hello" ]] || (echo "'\$string' was not 'world testing-string hello'" && false) # test appending to a unset variable appendToVar nonExistant created hello - typeset -p nonExistant + declare -p nonExistant if [[ -n $__structuredAttrs ]]; then [[ "''${nonExistant[@]}" == "created hello" ]] else # there's a extra " " in front here and a extra " " in the end of prependToVar - # shouldn't matter because these functions will mostly be used for $*Flags and the Flag variable will in most cases already exit + # shouldn't matter because these functions will mostly be used for $*Flags and the Flag variable will in most cases already exist [[ "$nonExistant" == " created hello" ]] fi From bfd97a691f165c2a160d5c86a7dbfd89fcbdb33c Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 9 Jun 2024 14:06:28 +0200 Subject: [PATCH 03/14] stdenv: make _accumFlagsArray independent of structuredAttrs structuredAttrs was used here to make an assumption about the type of the named variables passed as arguments. This can be done better by looking at the actual types of those variables. This gives a bit more backwards compatibility as well: Once you turn to structuredAttrs, you should still be able to pass a bare string instead of a list and have it behave as a whitespace-separated string like before. --- pkgs/stdenv/generic/setup.sh | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh index 8d060493b5560..8255f3aaafb7b 100644 --- a/pkgs/stdenv/generic/setup.sh +++ b/pkgs/stdenv/generic/setup.sh @@ -336,34 +336,24 @@ appendToVar() { # 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. +# Arrays are simply concatenated, strings are split on whitespace. _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 name type + for name in "$@"; do + if type=$(declare -p "$name" 2> /dev/null); then local -n nameref="$name" - case "$name" in - *Array) - # shellcheck disable=SC2206 - flagsArray+=( ${nameref+"${nameref[@]}"} ) ;; + case "${type#* }" in + -A*) + echo "_accumFlagsArray(): ERROR: trying to use _accumFlagsArray on an associative array." >&2 + return 1 ;; + -a*) + flagsArray+=( "${nameref[@]}" ) ;; *) # shellcheck disable=SC2206 flagsArray+=( ${nameref-} ) ;; esac - done - fi - + fi + done } # Add $1/lib* into rpaths. From 6bdfef9d2de252bec22a5c4b5859ad578b60b004 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 9 Jun 2024 12:24:21 +0200 Subject: [PATCH 04/14] stdenv: generalize _accumFlagsArray to concatTo Passing "flagsArray" as the first argument allows using this function in a few more places. --- pkgs/by-name/lo/local-ai/package.nix | 2 +- pkgs/by-name/me/meson/setup-hook.sh | 2 +- pkgs/stdenv/generic/setup.sh | 40 +++++++------------ pkgs/test/stdenv/default.nix | 58 ++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 27 deletions(-) diff --git a/pkgs/by-name/lo/local-ai/package.nix b/pkgs/by-name/lo/local-ai/package.nix index 498cc96a454b9..56b2ba80bf4a9 100644 --- a/pkgs/by-name/lo/local-ai/package.nix +++ b/pkgs/by-name/lo/local-ai/package.nix @@ -488,7 +488,7 @@ let ''${enableParallelBuilding:+-j''${NIX_BUILD_CORES}} SHELL=$SHELL ) - _accumFlagsArray makeFlags makeFlagsArray buildFlags buildFlagsArray + concatTo flagsArray makeFlags makeFlagsArray buildFlags buildFlagsArray echoCmd 'build flags' "''${flagsArray[@]}" make build "''${flagsArray[@]}" unset flagsArray diff --git a/pkgs/by-name/me/meson/setup-hook.sh b/pkgs/by-name/me/meson/setup-hook.sh index 8266645452277..7ded366d40320 100644 --- a/pkgs/by-name/me/meson/setup-hook.sh +++ b/pkgs/by-name/me/meson/setup-hook.sh @@ -25,7 +25,7 @@ mesonConfigurePhase() { "--buildtype=${mesonBuildType:-plain}" ) - _accumFlagsArray mesonFlags mesonFlagsArray + concatTo flagsArray mesonFlags mesonFlagsArray echoCmd 'mesonConfigurePhase flags' "${flagsArray[@]}" diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh index 8255f3aaafb7b..810d64ba7a21e 100644 --- a/pkgs/stdenv/generic/setup.sh +++ b/pkgs/stdenv/generic/setup.sh @@ -334,23 +334,24 @@ appendToVar() { fi } -# Accumulate into `flagsArray` the flags from the named variables. +# Accumulate flags from the named variables $2+ into the indexed array $1. # # Arrays are simply concatenated, strings are split on whitespace. -_accumFlagsArray() { +concatTo() { + local -n targetref="$1"; shift local name type for name in "$@"; do if type=$(declare -p "$name" 2> /dev/null); then local -n nameref="$name" case "${type#* }" in -A*) - echo "_accumFlagsArray(): ERROR: trying to use _accumFlagsArray on an associative array." >&2 + echo "concatTo(): ERROR: trying to use concatTo on an associative array." >&2 return 1 ;; -a*) - flagsArray+=( "${nameref[@]}" ) ;; + targetref+=( "${nameref[@]}" ) ;; *) # shellcheck disable=SC2206 - flagsArray+=( ${nameref-} ) ;; + targetref+=( ${nameref-} ) ;; esac fi done @@ -1167,12 +1168,7 @@ unpackPhase() { fi local -a srcsArray - if [ -n "$__structuredAttrs" ]; then - srcsArray=( "${srcs[@]}" ) - else - # shellcheck disable=SC2206 - srcsArray=( $srcs ) - fi + concatTo srcsArray srcs # To determine the source directory created by unpacking the # source archives, we record the contents of the current @@ -1237,13 +1233,7 @@ patchPhase() { runHook prePatch local -a patchesArray - if [ -n "$__structuredAttrs" ]; then - # shellcheck disable=SC2206 - patchesArray=( ${patches:+"${patches[@]}"} ) - else - # shellcheck disable=SC2206 - patchesArray=( ${patches:-} ) - fi + concatTo patchesArray patches for i in "${patchesArray[@]}"; do echo "applying patch $i" @@ -1355,7 +1345,7 @@ configurePhase() { if [ -n "$configureScript" ]; then local -a flagsArray - _accumFlagsArray configureFlags configureFlagsArray + concatTo flagsArray configureFlags configureFlagsArray echoCmd 'configure flags' "${flagsArray[@]}" # shellcheck disable=SC2086 @@ -1382,7 +1372,7 @@ buildPhase() { ${enableParallelBuilding:+-j${NIX_BUILD_CORES}} SHELL="$SHELL" ) - _accumFlagsArray makeFlags makeFlagsArray buildFlags buildFlagsArray + concatTo flagsArray makeFlags makeFlagsArray buildFlags buildFlagsArray echoCmd 'build flags' "${flagsArray[@]}" make ${makefile:+-f $makefile} "${flagsArray[@]}" @@ -1421,14 +1411,14 @@ checkPhase() { SHELL="$SHELL" ) - _accumFlagsArray makeFlags makeFlagsArray + concatTo flagsArray makeFlags makeFlagsArray if [ -n "$__structuredAttrs" ]; then flagsArray+=( "${checkFlags[@]:-VERBOSE=y}" ) else # shellcheck disable=SC2206 flagsArray+=( ${checkFlags:-VERBOSE=y} ) fi - _accumFlagsArray checkFlagsArray + concatTo flagsArray checkFlagsArray # shellcheck disable=SC2206 flagsArray+=( ${checkTarget} ) @@ -1463,7 +1453,7 @@ installPhase() { ${enableParallelInstalling:+-j${NIX_BUILD_CORES}} SHELL="$SHELL" ) - _accumFlagsArray makeFlags makeFlagsArray installFlags installFlagsArray + concatTo flagsArray makeFlags makeFlagsArray installFlags installFlagsArray if [ -n "$__structuredAttrs" ]; then flagsArray+=( "${installTargets[@]:-install}" ) else @@ -1552,7 +1542,7 @@ installCheckPhase() { SHELL="$SHELL" ) - _accumFlagsArray makeFlags makeFlagsArray \ + concatTo flagsArray makeFlags makeFlagsArray \ installCheckFlags installCheckFlagsArray # shellcheck disable=SC2206 flagsArray+=( ${installCheckTarget:-installcheck} ) @@ -1570,7 +1560,7 @@ distPhase() { runHook preDist local flagsArray=() - _accumFlagsArray distFlags distFlagsArray + concatTo flagsArray distFlags distFlagsArray # shellcheck disable=SC2206 flagsArray+=( ${distTarget:-dist} ) diff --git a/pkgs/test/stdenv/default.nix b/pkgs/test/stdenv/default.nix index 72310b74061f1..996087ae32be0 100644 --- a/pkgs/test/stdenv/default.nix +++ b/pkgs/test/stdenv/default.nix @@ -96,6 +96,41 @@ let ''; } // extraAttrs); + testConcatTo = { name, stdenv', extraAttrs ? { } }: + stdenv'.mkDerivation + ({ + inherit name; + + string = "a b"; + list = ["c" "d"]; + + passAsFile = [ "buildCommand" ] ++ lib.optionals (extraAttrs ? extraTest) [ "extraTest" ]; + buildCommand = '' + declare -A associativeArray=(["X"]="Y") + [[ $(concatTo nowhere associativeArray 2>&1) =~ "trying to use" ]] || (echo "concatTo did not throw concatenating associativeArray" && false) + + declare -a flagsArray + concatTo flagsArray string list + declare -p flagsArray + [[ "''${flagsArray[0]}" == "a" ]] || (echo "'\$flagsArray[0]' was not 'a'" && false) + [[ "''${flagsArray[1]}" == "b" ]] || (echo "'\$flagsArray[1]' was not 'b'" && false) + [[ "''${flagsArray[2]}" == "c" ]] || (echo "'\$flagsArray[2]' was not 'c'" && false) + [[ "''${flagsArray[3]}" == "d" ]] || (echo "'\$flagsArray[3]' was not 'd'" && false) + + # test concatenating to unset variable + concatTo nonExistant string list + declare -p nonExistant + [[ "''${nonExistant[0]}" == "a" ]] || (echo "'\$nonExistant[0]' was not 'a'" && false) + [[ "''${nonExistant[1]}" == "b" ]] || (echo "'\$nonExistant[1]' was not 'b'" && false) + [[ "''${nonExistant[2]}" == "c" ]] || (echo "'\$nonExistant[2]' was not 'c'" && false) + [[ "''${nonExistant[3]}" == "d" ]] || (echo "'\$nonExistant[3]' was not 'd'" && false) + + eval "$extraTest" + + touch $out + ''; + } // extraAttrs); + in { @@ -196,6 +231,11 @@ in stdenv' = bootStdenv; }; + test-concat-to = testConcatTo { + name = "test-concat-to"; + stdenv' = bootStdenv; + }; + test-structured-env-attrset = testEnvAttrset { name = "test-structured-env-attrset"; stdenv' = bootStdenv; @@ -255,6 +295,24 @@ in }; }; + test-concat-to = testConcatTo { + name = "test-concat-to-structuredAttrsByDefault"; + stdenv' = bootStdenvStructuredAttrsByDefault; + extraAttrs = { + # test that whitespace is kept in the bash array for structuredAttrs + listWithSpaces = [ "c c" "d d" ]; + extraTest = '' + declare -a flagsWithSpaces + concatTo flagsWithSpaces string listWithSpaces + declare -p flagsWithSpaces + [[ "''${flagsWithSpaces[0]}" == "a" ]] || (echo "'\$flagsWithSpaces[0]' was not 'a'" && false) + [[ "''${flagsWithSpaces[1]}" == "b" ]] || (echo "'\$flagsWithSpaces[1]' was not 'b'" && false) + [[ "''${flagsWithSpaces[2]}" == "c c" ]] || (echo "'\$flagsWithSpaces[2]' was not 'c c'" && false) + [[ "''${flagsWithSpaces[3]}" == "d d" ]] || (echo "'\$flagsWithSpaces[3]' was not 'd d'" && false) + ''; + }; + }; + test-golden-example-structuredAttrs = let goldenSh = earlyPkgs.writeText "goldenSh" '' From 8cb51ec38e4579b62ee26d31bcdcdbc61f37f42d Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 9 Jun 2024 12:45:57 +0200 Subject: [PATCH 05/14] stdenv: refactor default flags without __structuredAttrs use Instead of checking for __structuredAttrs everywhere, it's easier to just set the default value via parameter expansion and then hand the array construction off to "concatTo". Once more setup-hooks will be made structuredAttrs-aware, this pattern will reduce the use of this implementation detail even more. --- pkgs/stdenv/generic/setup.sh | 40 ++++++++++-------------------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh index 810d64ba7a21e..ee894a260df90 100644 --- a/pkgs/stdenv/generic/setup.sh +++ b/pkgs/stdenv/generic/setup.sh @@ -1254,12 +1254,8 @@ patchPhase() { esac local -a flagsArray - if [ -n "$__structuredAttrs" ]; then - flagsArray=( "${patchFlags[@]:--p1}" ) - else - # shellcheck disable=SC2086,SC2206 - flagsArray=( ${patchFlags:--p1} ) - fi + : "${patchFlags:=-p1}" + concatTo flagsArray patchFlags # "2>&1" is a hack to make patch fail if the decompressor fails (nonexistent patch, etc.) # shellcheck disable=SC2086 $uncompress < "$i" 2>&1 | patch "${flagsArray[@]}" @@ -1411,16 +1407,8 @@ checkPhase() { SHELL="$SHELL" ) - concatTo flagsArray makeFlags makeFlagsArray - if [ -n "$__structuredAttrs" ]; then - flagsArray+=( "${checkFlags[@]:-VERBOSE=y}" ) - else - # shellcheck disable=SC2206 - flagsArray+=( ${checkFlags:-VERBOSE=y} ) - fi - concatTo flagsArray checkFlagsArray - # shellcheck disable=SC2206 - flagsArray+=( ${checkTarget} ) + : "${checkFlags:=VERBOSE=y}" + concatTo flagsArray makeFlags makeFlagsArray checkFlags checkFlagsArray checkTarget echoCmd 'check flags' "${flagsArray[@]}" make ${makefile:+-f $makefile} "${flagsArray[@]}" @@ -1453,13 +1441,9 @@ installPhase() { ${enableParallelInstalling:+-j${NIX_BUILD_CORES}} SHELL="$SHELL" ) - concatTo flagsArray makeFlags makeFlagsArray installFlags installFlagsArray - if [ -n "$__structuredAttrs" ]; then - flagsArray+=( "${installTargets[@]:-install}" ) - else - # shellcheck disable=SC2206 - flagsArray+=( ${installTargets:-install} ) - fi + + : "${installTargets:=install}" + concatTo flagsArray makeFlags makeFlagsArray installFlags installFlagsArray installTargets echoCmd 'install flags' "${flagsArray[@]}" make ${makefile:+-f $makefile} "${flagsArray[@]}" @@ -1542,10 +1526,9 @@ installCheckPhase() { SHELL="$SHELL" ) + : "${installCheckTarget:=installcheck}" concatTo flagsArray makeFlags makeFlagsArray \ - installCheckFlags installCheckFlagsArray - # shellcheck disable=SC2206 - flagsArray+=( ${installCheckTarget:-installcheck} ) + installCheckFlags installCheckFlagsArray installCheckTarget echoCmd 'installcheck flags' "${flagsArray[@]}" make ${makefile:+-f $makefile} "${flagsArray[@]}" @@ -1560,9 +1543,8 @@ distPhase() { runHook preDist local flagsArray=() - concatTo flagsArray distFlags distFlagsArray - # shellcheck disable=SC2206 - flagsArray+=( ${distTarget:-dist} ) + : "${distTarget:=dist}" + concatTo flagsArray distFlags distFlagsArray distTarget echo 'dist flags: %q' "${flagsArray[@]}" make ${makefile:+-f $makefile} "${flagsArray[@]}" From 471cbdd062bceda5a9761f65f0dddffbd72683db Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 12 Jun 2024 21:14:27 +0200 Subject: [PATCH 06/14] stdenv: add concatStringsSep helper This can be used to separate lists for example with commas, when creating argument strings. This works with both structuredAttrs disabled and enabled. --- pkgs/stdenv/generic/setup.sh | 30 ++++++++++++++++++++++++++++++ pkgs/test/stdenv/default.nix | 31 +++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/pkgs/stdenv/generic/setup.sh b/pkgs/stdenv/generic/setup.sh index ee894a260df90..7666360fb7bb9 100644 --- a/pkgs/stdenv/generic/setup.sh +++ b/pkgs/stdenv/generic/setup.sh @@ -357,6 +357,36 @@ concatTo() { done } +# Concatenate a list of strings ($2) with a separator ($1) between each element. +# The list can be an indexed array of strings or a single string. A single string +# is split on spaces and then concatenated with the separator. +# +# $ flags="lorem ipsum dolor sit amet" +# $ concatStringsSep ";" flags +# lorem;ipsum;dolor;sit;amet +# +# $ flags=("lorem ipsum" "dolor" "sit amet") +# $ concatStringsSep ";" flags +# lorem ipsum;dolor;sit amet +concatStringsSep() { + local sep="$1" + local name="$2" + local type oldifs + if type=$(declare -p "$name" 2> /dev/null); then + local -n nameref="$name" + case "${type#* }" in + -A*) + echo "concatStringsSep(): ERROR: trying to use concatStringsSep on an associative array." >&2 + return 1 ;; + -a*) + local IFS="$sep" + echo -n "${nameref[*]}" ;; + *) + echo -n "${nameref// /${sep}}" ;; + esac + 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. diff --git a/pkgs/test/stdenv/default.nix b/pkgs/test/stdenv/default.nix index 996087ae32be0..a6885d65cea88 100644 --- a/pkgs/test/stdenv/default.nix +++ b/pkgs/test/stdenv/default.nix @@ -131,6 +131,27 @@ let ''; } // extraAttrs); + testConcatStringsSep = { name, stdenv' }: + stdenv'.mkDerivation + { + inherit name; + + passAsFile = [ "buildCommand" ]; + buildCommand = '' + declare -A associativeArray=(["X"]="Y") + [[ $(concatStringsSep ";" associativeArray 2>&1) =~ "trying to use" ]] || (echo "concatStringsSep did not throw concatenating associativeArray" && false) + + string="lorem ipsum dolor sit amet" + stringWithSep="$(concatStringsSep ";" string)" + [[ "$stringWithSep" == "lorem;ipsum;dolor;sit;amet" ]] || (echo "'\$stringWithSep' was not 'lorem;ipsum;dolor;sit;amet'" && false) + + array=("lorem ipsum" "dolor" "sit amet") + arrayWithSep="$(concatStringsSep ";" array)" + [[ "$arrayWithSep" == "lorem ipsum;dolor;sit amet" ]] || (echo "'\$arrayWithSep' was not 'lorem ipsum;dolor;sit amet'" && false) + + touch $out + ''; + }; in { @@ -236,6 +257,11 @@ in stdenv' = bootStdenv; }; + test-concat-strings-sep = testConcatStringsSep { + name = "test-concat-strings-sep"; + stdenv' = bootStdenv; + }; + test-structured-env-attrset = testEnvAttrset { name = "test-structured-env-attrset"; stdenv' = bootStdenv; @@ -313,6 +339,11 @@ in }; }; + test-concat-strings-sep = testConcatStringsSep { + name = "test-concat-strings-sep-structuredAttrsByDefault"; + stdenv' = bootStdenvStructuredAttrsByDefault; + }; + test-golden-example-structuredAttrs = let goldenSh = earlyPkgs.writeText "goldenSh" '' From 55933d9bf6e62232eb48cbf08620f22cb7658b61 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 9 Jun 2024 18:42:23 +0200 Subject: [PATCH 07/14] meson: support structuredAttrs in setup hook Tested emilua with and without __structuredAttrs. --- pkgs/by-name/me/meson/setup-hook.sh | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkgs/by-name/me/meson/setup-hook.sh b/pkgs/by-name/me/meson/setup-hook.sh index 7ded366d40320..0757d06e889d3 100644 --- a/pkgs/by-name/me/meson/setup-hook.sh +++ b/pkgs/by-name/me/meson/setup-hook.sh @@ -50,7 +50,8 @@ mesonConfigurePhase() { mesonCheckPhase() { runHook preCheck - local flagsArray=($mesonCheckFlags "${mesonCheckFlagsArray[@]}") + local flagsArray=() + concatTo flagsArray mesonCheckFlags mesonCheckFlagsArray echoCmd 'mesonCheckPhase flags' "${flagsArray[@]}" meson test --no-rebuild --print-errorlogs "${flagsArray[@]}" @@ -64,12 +65,9 @@ mesonInstallPhase() { local flagsArray=() if [[ -n "$mesonInstallTags" ]]; then - flagsArray+=("--tags" "${mesonInstallTags// /,}") + flagsArray+=("--tags" "$(concatStringsSep "," mesonInstallTags)") fi - flagsArray+=( - $mesonInstallFlags - "${mesonInstallFlagsArray[@]}" - ) + concatTo flagsArray mesonInstallFlags mesonInstallFlagsArray echoCmd 'mesonInstallPhase flags' "${flagsArray[@]}" meson install --no-rebuild "${flagsArray[@]}" From 7c732de6e3b35497f14a70888fe48b492f0f3cac Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 9 Jun 2024 20:34:20 +0200 Subject: [PATCH 08/14] meson: remove unused crossMesonFlags from setup hook This was used in ##35666 to add the --cross-file argument, but #86080 moved that somewhere else - leaving crossMesonFlags unused. --- pkgs/by-name/me/meson/setup-hook.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/pkgs/by-name/me/meson/setup-hook.sh b/pkgs/by-name/me/meson/setup-hook.sh index 0757d06e889d3..55ea8b5c1d973 100644 --- a/pkgs/by-name/me/meson/setup-hook.sh +++ b/pkgs/by-name/me/meson/setup-hook.sh @@ -21,7 +21,6 @@ mesonConfigurePhase() { "--localedir=${!outputLib}/share/locale" "-Dauto_features=${mesonAutoFeatures:-enabled}" "-Dwrap_mode=${mesonWrapMode:-nodownload}" - ${crossMesonFlags} "--buildtype=${mesonBuildType:-plain}" ) From 7752cea66c18f1651a14b4a64074188b142d3a6c Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 9 Jun 2024 19:03:17 +0200 Subject: [PATCH 09/14] ninja: support structuredAttrs in setup hook Tested clasp-common-lisp with and without __structuredAttrs. --- pkgs/by-name/ni/ninja/setup-hook.sh | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkgs/by-name/ni/ninja/setup-hook.sh b/pkgs/by-name/ni/ninja/setup-hook.sh index 7fa5e4675f39b..6607082b43f3e 100644 --- a/pkgs/by-name/ni/ninja/setup-hook.sh +++ b/pkgs/by-name/ni/ninja/setup-hook.sh @@ -10,8 +10,8 @@ ninjaBuildPhase() { local flagsArray=( -j$buildCores - $ninjaFlags "${ninjaFlagsArray[@]}" ) + concatTo flagsArray ninjaFlags ninjaFlagsArray echoCmd 'build flags' "${flagsArray[@]}" TERM=dumb ninja "${flagsArray[@]}" @@ -39,9 +39,8 @@ ninjaCheckPhase() { local flagsArray=( -j$buildCores - $ninjaFlags "${ninjaFlagsArray[@]}" - $checkTarget ) + concatTo flagsArray ninjaFlags ninjaFlagsArray checkTarget echoCmd 'check flags' "${flagsArray[@]}" TERM=dumb ninja "${flagsArray[@]}" @@ -63,9 +62,9 @@ ninjaInstallPhase() { # shellcheck disable=SC2086 local flagsArray=( -j$buildCores - $ninjaFlags "${ninjaFlagsArray[@]}" - ${installTargets:-install} ) + : ${installTargets:=install} + concatTo flagsArray ninjaFlags ninjaFlagsArray installTargets echoCmd 'install flags' "${flagsArray[@]}" TERM=dumb ninja "${flagsArray[@]}" From 4b45acf52990ba892151928070d385657ebd51b9 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 9 Jun 2024 20:59:56 +0200 Subject: [PATCH 10/14] ninja: shellcheck setup hook --- pkgs/by-name/ni/ninja/setup-hook.sh | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkgs/by-name/ni/ninja/setup-hook.sh b/pkgs/by-name/ni/ninja/setup-hook.sh index 6607082b43f3e..4f3bc5b5acfa4 100644 --- a/pkgs/by-name/ni/ninja/setup-hook.sh +++ b/pkgs/by-name/ni/ninja/setup-hook.sh @@ -1,3 +1,5 @@ +# shellcheck shell=bash + ninjaBuildPhase() { runHook preBuild @@ -9,7 +11,7 @@ ninjaBuildPhase() { fi local flagsArray=( - -j$buildCores + "-j$buildCores" ) concatTo flagsArray ninjaFlags ninjaFlagsArray @@ -24,7 +26,7 @@ ninjaCheckPhase() { if [ -z "${checkTarget:-}" ]; then if ninja -t query test >/dev/null 2>&1; then - checkTarget=test + checkTarget="test" fi fi @@ -38,7 +40,7 @@ ninjaCheckPhase() { fi local flagsArray=( - -j$buildCores + "-j$buildCores" ) concatTo flagsArray ninjaFlags ninjaFlagsArray checkTarget @@ -61,9 +63,9 @@ ninjaInstallPhase() { # shellcheck disable=SC2086 local flagsArray=( - -j$buildCores + "-j$buildCores" ) - : ${installTargets:=install} + : "${installTargets:=install}" concatTo flagsArray ninjaFlags ninjaFlagsArray installTargets echoCmd 'install flags' "${flagsArray[@]}" @@ -72,14 +74,14 @@ ninjaInstallPhase() { runHook postInstall } -if [ -z "${dontUseNinjaBuild-}" -a -z "${buildPhase-}" ]; then +if [ -z "${dontUseNinjaBuild-}" ] && [ -z "${buildPhase-}" ]; then buildPhase=ninjaBuildPhase fi -if [ -z "${dontUseNinjaCheck-}" -a -z "${checkPhase-}" ]; then +if [ -z "${dontUseNinjaCheck-}" ] && [ -z "${checkPhase-}" ]; then checkPhase=ninjaCheckPhase fi -if [ -z "${dontUseNinjaInstall-}" -a -z "${installPhase-}" ]; then +if [ -z "${dontUseNinjaInstall-}" ] && [ -z "${installPhase-}" ]; then installPhase=ninjaInstallPhase fi From 34a2b7ae9f77ca1c39b871b1165c0aec44928227 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 9 Jun 2024 21:19:26 +0200 Subject: [PATCH 11/14] cmake: support structuredAttrs in setup hook Tested litehtml with and without __structuredAttrs. Resolves #289037 Supersedes #299622 (at least parts) --- pkgs/by-name/cm/cmake/setup-hook.sh | 59 +++++++++++++++-------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/pkgs/by-name/cm/cmake/setup-hook.sh b/pkgs/by-name/cm/cmake/setup-hook.sh index 9ca4a6abeebc2..7ceb24227857d 100755 --- a/pkgs/by-name/cm/cmake/setup-hook.sh +++ b/pkgs/by-name/cm/cmake/setup-hook.sh @@ -38,7 +38,7 @@ cmakeConfigurePhase() { fi if [ -z "${dontAddPrefix-}" ]; then - cmakeFlags="-DCMAKE_INSTALL_PREFIX=$prefix $cmakeFlags" + prependToVar cmakeFlags "-DCMAKE_INSTALL_PREFIX=$prefix" fi # We should set the proper `CMAKE_SYSTEM_NAME`. @@ -47,21 +47,21 @@ cmakeConfigurePhase() { # Unfortunately cmake seems to expect absolute paths for ar, ranlib, and # strip. Otherwise they are taken to be relative to the source root of the # package being built. - cmakeFlags="-DCMAKE_CXX_COMPILER=$CXX $cmakeFlags" - cmakeFlags="-DCMAKE_C_COMPILER=$CC $cmakeFlags" - cmakeFlags="-DCMAKE_AR=$(command -v $AR) $cmakeFlags" - cmakeFlags="-DCMAKE_RANLIB=$(command -v $RANLIB) $cmakeFlags" - cmakeFlags="-DCMAKE_STRIP=$(command -v $STRIP) $cmakeFlags" + prependToVar cmakeFlags "-DCMAKE_CXX_COMPILER=$CXX" + prependToVar cmakeFlags "-DCMAKE_C_COMPILER=$CC" + prependToVar cmakeFlags "-DCMAKE_AR=$(command -v $AR)" + prependToVar cmakeFlags "-DCMAKE_RANLIB=$(command -v $RANLIB)" + prependToVar cmakeFlags "-DCMAKE_STRIP=$(command -v $STRIP)" # on macOS we want to prefer Unix-style headers to Frameworks # because we usually do not package the framework - cmakeFlags="-DCMAKE_FIND_FRAMEWORK=LAST $cmakeFlags" + prependToVar cmakeFlags "-DCMAKE_FIND_FRAMEWORK=LAST" # we never want to use the global macOS SDK - cmakeFlags="-DCMAKE_OSX_SYSROOT= $cmakeFlags" + prependToVar cmakeFlags "-DCMAKE_OSX_SYSROOT=" # correctly detect our clang compiler - cmakeFlags="-DCMAKE_POLICY_DEFAULT_CMP0025=NEW $cmakeFlags" + prependToVar cmakeFlags "-DCMAKE_POLICY_DEFAULT_CMP0025=NEW" # This installs shared libraries with a fully-specified install # name. By default, cmake installs shared libraries with just the @@ -70,7 +70,7 @@ cmakeConfigurePhase() { # libraries are in a system path or in the same directory as the # executable. This flag makes the shared library accessible from its # nix/store directory. - cmakeFlags="-DCMAKE_INSTALL_NAME_DIR=${!outputLib}/lib $cmakeFlags" + prependToVar cmakeFlags "-DCMAKE_INSTALL_NAME_DIR=${!outputLib}/lib" # The docdir flag needs to include PROJECT_NAME as per GNU guidelines, # try to extract it from CMakeLists.txt. @@ -93,39 +93,42 @@ cmakeConfigurePhase() { # This ensures correct paths with multiple output derivations # It requires the project to use variables from GNUInstallDirs module # https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html - cmakeFlags="-DCMAKE_INSTALL_BINDIR=${!outputBin}/bin $cmakeFlags" - cmakeFlags="-DCMAKE_INSTALL_SBINDIR=${!outputBin}/sbin $cmakeFlags" - cmakeFlags="-DCMAKE_INSTALL_INCLUDEDIR=${!outputInclude}/include $cmakeFlags" - cmakeFlags="-DCMAKE_INSTALL_OLDINCLUDEDIR=${!outputInclude}/include $cmakeFlags" - cmakeFlags="-DCMAKE_INSTALL_MANDIR=${!outputMan}/share/man $cmakeFlags" - cmakeFlags="-DCMAKE_INSTALL_INFODIR=${!outputInfo}/share/info $cmakeFlags" - cmakeFlags="-DCMAKE_INSTALL_DOCDIR=${!outputDoc}/share/doc/${shareDocName} $cmakeFlags" - cmakeFlags="-DCMAKE_INSTALL_LIBDIR=${!outputLib}/lib $cmakeFlags" - cmakeFlags="-DCMAKE_INSTALL_LIBEXECDIR=${!outputLib}/libexec $cmakeFlags" - cmakeFlags="-DCMAKE_INSTALL_LOCALEDIR=${!outputLib}/share/locale $cmakeFlags" + prependToVar cmakeFlags "-DCMAKE_INSTALL_BINDIR=${!outputBin}/bin" + prependToVar cmakeFlags "-DCMAKE_INSTALL_SBINDIR=${!outputBin}/sbin" + prependToVar cmakeFlags "-DCMAKE_INSTALL_INCLUDEDIR=${!outputInclude}/include" + prependToVar cmakeFlags "-DCMAKE_INSTALL_OLDINCLUDEDIR=${!outputInclude}/include" + prependToVar cmakeFlags "-DCMAKE_INSTALL_MANDIR=${!outputMan}/share/man" + prependToVar cmakeFlags "-DCMAKE_INSTALL_INFODIR=${!outputInfo}/share/info" + prependToVar cmakeFlags "-DCMAKE_INSTALL_DOCDIR=${!outputDoc}/share/doc/${shareDocName}" + prependToVar cmakeFlags "-DCMAKE_INSTALL_LIBDIR=${!outputLib}/lib" + prependToVar cmakeFlags "-DCMAKE_INSTALL_LIBEXECDIR=${!outputLib}/libexec" + prependToVar cmakeFlags "-DCMAKE_INSTALL_LOCALEDIR=${!outputLib}/share/locale" # Don’t build tests when doCheck = false if [ -z "${doCheck-}" ]; then - cmakeFlags="-DBUILD_TESTING=OFF $cmakeFlags" + prependToVar cmakeFlags "-DBUILD_TESTING=OFF" fi # Always build Release, to ensure optimisation flags - cmakeFlags="-DCMAKE_BUILD_TYPE=${cmakeBuildType:-Release} $cmakeFlags" + prependToVar cmakeFlags "-DCMAKE_BUILD_TYPE=${cmakeBuildType:-Release}" # Disable user package registry to avoid potential side effects # and unecessary attempts to access non-existent home folder # https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html#disabling-the-package-registry - cmakeFlags="-DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON $cmakeFlags" - cmakeFlags="-DCMAKE_FIND_USE_PACKAGE_REGISTRY=OFF $cmakeFlags" - cmakeFlags="-DCMAKE_FIND_USE_SYSTEM_PACKAGE_REGISTRY=OFF $cmakeFlags" + prependToVar cmakeFlags "-DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON" + prependToVar cmakeFlags "-DCMAKE_FIND_USE_PACKAGE_REGISTRY=OFF" + prependToVar cmakeFlags "-DCMAKE_FIND_USE_SYSTEM_PACKAGE_REGISTRY=OFF" if [ "${buildPhase-}" = ninjaBuildPhase ]; then - cmakeFlags="-GNinja $cmakeFlags" + prependToVar cmakeFlags "-GNinja" fi - echo "cmake flags: $cmakeFlags ${cmakeFlagsArray[@]}" + local flagsArray=() + concatTo flagsArray cmakeFlags cmakeFlagsArray - cmake "$cmakeDir" $cmakeFlags "${cmakeFlagsArray[@]}" + echoCmd 'cmake flags' "${flagsArray[@]}" + + cmake "$cmakeDir" "${flagsArray[@]}" if ! [[ -v enableParallelBuilding ]]; then enableParallelBuilding=1 From d7c257035d5ffc0bf5d70fa85796aa238e4f4a73 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Tue, 11 Jun 2024 21:48:29 +0200 Subject: [PATCH 12/14] setup-hooks/autoreconf: support structuredAttrs Tested db with and without __structuredAttrs. --- pkgs/build-support/setup-hooks/autoreconf.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkgs/build-support/setup-hooks/autoreconf.sh b/pkgs/build-support/setup-hooks/autoreconf.sh index 6ce879ac092de..763ea649c1c43 100644 --- a/pkgs/build-support/setup-hooks/autoreconf.sh +++ b/pkgs/build-support/setup-hooks/autoreconf.sh @@ -2,6 +2,11 @@ preConfigurePhases="${preConfigurePhases:-} autoreconfPhase" autoreconfPhase() { runHook preAutoreconf - autoreconf ${autoreconfFlags:---install --force --verbose} + + local flagsArray=() + : "${autoreconfFlags:=--install --force --verbose}" + concatTo flagsArray autoreconfFlags + + autoreconf "${flagsArray[@]}" runHook postAutoreconf } From 64eaa6318185fe0c3e54b0e4e0f2d5b12ebe34ab Mon Sep 17 00:00:00 2001 From: Someone Serge Date: Sat, 10 Aug 2024 23:38:10 +0000 Subject: [PATCH 13/14] stdenv: concatStringsSep: quote ${sep} --- 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 7666360fb7bb9..5c9b2c1064db2 100644 --- a/pkgs/stdenv/generic/setup.sh +++ b/pkgs/stdenv/generic/setup.sh @@ -382,7 +382,7 @@ concatStringsSep() { local IFS="$sep" echo -n "${nameref[*]}" ;; *) - echo -n "${nameref// /${sep}}" ;; + echo -n "${nameref// /"${sep}"}" ;; esac fi } From 9876c2fe9e90b49f693da628e8b8b696ec5f63a4 Mon Sep 17 00:00:00 2001 From: Someone Serge Date: Sun, 11 Aug 2024 14:42:42 +0000 Subject: [PATCH 14/14] stdenv: concatStringsSep: test sep="&" The test fails without 64eaa6318185fe0c3e54b0e4e0f2d5b12ebe34ab ("stdenv: concatStringsSep: quote ${sep}") Co-authored-by: Ivan Trubach Co-authored-by: Wolfgang Walther --- pkgs/test/stdenv/default.nix | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkgs/test/stdenv/default.nix b/pkgs/test/stdenv/default.nix index a6885d65cea88..a5b571b5a9f43 100644 --- a/pkgs/test/stdenv/default.nix +++ b/pkgs/test/stdenv/default.nix @@ -136,18 +136,21 @@ let { inherit name; + # NOTE: Testing with "&" as separator is intentional, because unquoted + # "&" has a special meaning in the "${var//pattern/replacement}" syntax. + # Cf. https://github.com/NixOS/nixpkgs/pull/318614#discussion_r1706191919 passAsFile = [ "buildCommand" ]; buildCommand = '' declare -A associativeArray=(["X"]="Y") [[ $(concatStringsSep ";" associativeArray 2>&1) =~ "trying to use" ]] || (echo "concatStringsSep did not throw concatenating associativeArray" && false) string="lorem ipsum dolor sit amet" - stringWithSep="$(concatStringsSep ";" string)" - [[ "$stringWithSep" == "lorem;ipsum;dolor;sit;amet" ]] || (echo "'\$stringWithSep' was not 'lorem;ipsum;dolor;sit;amet'" && false) + stringWithSep="$(concatStringsSep "&" string)" + [[ "$stringWithSep" == "lorem&ipsum&dolor&sit&amet" ]] || (echo "'\$stringWithSep' was not 'lorem&ipsum&dolor&sit&amet'" && false) array=("lorem ipsum" "dolor" "sit amet") - arrayWithSep="$(concatStringsSep ";" array)" - [[ "$arrayWithSep" == "lorem ipsum;dolor;sit amet" ]] || (echo "'\$arrayWithSep' was not 'lorem ipsum;dolor;sit amet'" && false) + arrayWithSep="$(concatStringsSep "&" array)" + [[ "$arrayWithSep" == "lorem ipsum&dolor&sit amet" ]] || (echo "'\$arrayWithSep' was not 'lorem ipsum&dolor&sit amet'" && false) touch $out '';