Skip to content

Commit

Permalink
setup.sh: fatal: This word should yield a string, but it contains an …
Browse files Browse the repository at this point in the history
…array

remove implicit array comparison in case
  • Loading branch information
happysalada committed Jul 6, 2021
1 parent 1a71a5f commit bf99a81
Showing 1 changed file with 7 additions and 10 deletions.
17 changes: 7 additions & 10 deletions pkgs/stdenv/generic/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -364,16 +364,13 @@ findInputs() {
local var="${!varRef}"
unset -v varVar varRef

# TODO(@Ericson2314): Restore using associative array once Darwin
# nix-shell doesn't use impure bash. This should replace the O(n)
# case with an O(1) hash map lookup, assuming bash is implemented
# well :D.
local varSlice="${var}[*]"
# ${..-} to hack around old bash empty array problem
case "${!varSlice-}" in
*" $pkg "*) return 0 ;;
esac
unset -v varSlice
# var is a reference to an array and can sometimes be undefined
# so checking the array with "${!var}[@]" does not work
# check if $pkgs is in the var ref array
# TODO(@Ericson2314): Restore using associative array
if [[ "${var}[*]" = *" $pkg "* ]]; then

This comment has been minimized.

Copy link
@aszlig

aszlig Jul 10, 2021

Member

The issue here is that you're comparing foo[*] here instead of the actual content. What the original code did in pseudocode was:

if [[ "${${var}[*]}" = *" $pkg "* ]]; then

I tried to come up with a less ugly way then we had before, but didn't succeed.

@Ericson2314: Maybe you got a better idea? :-)

This comment has been minimized.

Copy link
@happysalada

happysalada Jul 11, 2021

Author Contributor

How about a regular for loop for expansion here.
My toy test implentation looks like

array=(one two three)
elementRef=two
ref=array
for el in "${!ref}[@]"; do
    if [[ "$el" = "$elementRef" ]]; then
        echo "success"
        return 0
    fi
done

I'm thinking that the for loop would not run slower than array expansion in the condition.
I'm testing

This comment has been minimized.

Copy link
@aszlig

aszlig Jul 11, 2021

Member
for el in "${!ref}[@]"; do

This has the same issue as mentioned in my comment above, namely that it will just loop over one[@] (note: verbatim, not array-expanded) rather than the contents of one (which isn't defined in your case).

This comment has been minimized.

Copy link
@happysalada

happysalada Jul 11, 2021

Author Contributor

There is something I am missing here.
If you try to run the small self contained example above it works. Do you think it is the wrong example for our use case?

return 0
fi

eval "$var"'+=("$pkg")'

Expand Down

0 comments on commit bf99a81

Please sign in to comment.