[PATCH 0/5] configure: fix some non-portabilities

[PATCH 0/5] configure: fix some non-portabilities

[Peter Maydell]

This patchset fixes some non-portable code that has crept in recently:
notably, it fixes problems that are reported to cause configure
not to work correctly on OpenBSD and NetBSD, and a warning message
when using dash as /bin/sh on Linux. I threw in a less important
"drop some dead code" fix too, and a fix for the only other 'error'
category problem reported by shellcheck. (There are way too many
'warning' category reports to deal with all at once.)

If people who reported problems on NetBSD/OpenBSD could check that
this fixes them, that would be great.

thanks
-- PMM


Peter Maydell (5):
  configure: Add missing POSIX-required space
  configure: Add braces to clarify intent of $emu[[:space:]]
  configure: Don't use bash-specific string-replacement syntax
  configure: Drop dead code attempting to use -msmall-data on alpha
    hosts
  configure: Avoid '==' bashism

 configure | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH 1/5] configure: Add missing POSIX-required space

[Peter Maydell]

In commit 7d7dbf9dc15be6e1 we added a line to the configure script
which is not valid POSIX shell syntax, because it is missing a space
after a '!' character. shellcheck diagnoses this:

if !(GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
    ^-- SC1035: You are missing a required space after the !.

and the OpenBSD shell will not correctly handle this without the space.

Fixes: 7d7dbf9dc15be6e1 ("configure: replace --enable/disable-git-update with --with-git-submodules")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
David Gilbert noted the OpenBSD issue on IRC -- I have not tested
this fix there myself.
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 35e0b281985..dec6f030346 100755
--- a/configure
+++ b/configure
@@ -2425,7 +2425,7 @@ else
     cxx=
 fi
 
-if !(GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
+if ! (GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
     exit 1
 fi
 
-- 
2.25.1

Re: [PATCH 1/5] configure: Add missing POSIX-required space

[Thomas Huth]

On 20/07/2022 17.26, Peter Maydell wrote:
> In commit 7d7dbf9dc15be6e1 we added a line to the configure script
> which is not valid POSIX shell syntax, because it is missing a space
> after a '!' character. shellcheck diagnoses this:
> 
> if !(GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
>      ^-- SC1035: You are missing a required space after the !.
> 
> and the OpenBSD shell will not correctly handle this without the space.
> 
> Fixes: 7d7dbf9dc15be6e1 ("configure: replace --enable/disable-git-update with --with-git-submodules")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> David Gilbert noted the OpenBSD issue on IRC -- I have not tested
> this fix there myself.
> ---
>   configure | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 35e0b281985..dec6f030346 100755
> --- a/configure
> +++ b/configure
> @@ -2425,7 +2425,7 @@ else
>       cxx=
>   fi
>   
> -if !(GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
> +if ! (GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
>       exit 1
>   fi
>   

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 1/5] configure: Add missing POSIX-required space

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> In commit 7d7dbf9dc15be6e1 we added a line to the configure script
> which is not valid POSIX shell syntax, because it is missing a space
> after a '!' character. shellcheck diagnoses this:
> 
> if !(GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
>     ^-- SC1035: You are missing a required space after the !.
> 
> and the OpenBSD shell will not correctly handle this without the space.
> 
> Fixes: 7d7dbf9dc15be6e1 ("configure: replace --enable/disable-git-update with --with-git-submodules")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> David Gilbert noted the OpenBSD issue on IRC -- I have not tested
> this fix there myself.

Fixed it for me, so

Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 35e0b281985..dec6f030346 100755
> --- a/configure
> +++ b/configure
> @@ -2425,7 +2425,7 @@ else
>      cxx=
>  fi
>  
> -if !(GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
> +if ! (GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then
>      exit 1
>  fi
>  
> -- 
> 2.25.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[PATCH 2/5] configure: Add braces to clarify intent of $emu[[:space:]]

[Peter Maydell]

In shell script syntax, $var[something] is not special for variable
expansion: $emu is expanded.  However, as it can look as if it were
intended to be an array element access (the correct syntax for which
is ${var[something]}), shellcheck recommends using explicit braces
around ${var} to clarify the intended expansion.

This fixes the warning:

In ./configure line 2346:
        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
                                                              ^-- SC1087: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is our only 'error' level shellcheck warning, so it seems
worth suppressing even though it's not wrong as written.
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index dec6f030346..a56c3d921be 100755
--- a/configure
+++ b/configure
@@ -2343,7 +2343,7 @@ if test -n "$target_cc" &&
     # emulation. Linux and OpenBSD/amd64 use 'elf_i386'; FreeBSD uses the _fbsd
     # variant; OpenBSD/i386 uses the _obsd variant; and Windows uses i386pe.
     for emu in elf_i386 elf_i386_fbsd elf_i386_obsd i386pe; do
-        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
+        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*${emu}[[:space:]]*$"; then
             ld_i386_emulation="$emu"
             break
         fi
-- 
2.25.1

Re: [PATCH 2/5] configure: Add braces to clarify intent of $emu[[:space:]]

[Thomas Huth]

On 20/07/2022 17.26, Peter Maydell wrote:
> In shell script syntax, $var[something] is not special for variable
> expansion: $emu is expanded.  However, as it can look as if it were
> intended to be an array element access (the correct syntax for which
> is ${var[something]}), shellcheck recommends using explicit braces
> around ${var} to clarify the intended expansion.
> 
> This fixes the warning:
> 
> In ./configure line 2346:
>          if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
>                                                                ^-- SC1087: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is our only 'error' level shellcheck warning, so it seems
> worth suppressing even though it's not wrong as written.
> ---
>   configure | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index dec6f030346..a56c3d921be 100755
> --- a/configure
> +++ b/configure
> @@ -2343,7 +2343,7 @@ if test -n "$target_cc" &&
>       # emulation. Linux and OpenBSD/amd64 use 'elf_i386'; FreeBSD uses the _fbsd
>       # variant; OpenBSD/i386 uses the _obsd variant; and Windows uses i386pe.
>       for emu in elf_i386 elf_i386_fbsd elf_i386_obsd i386pe; do
> -        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
> +        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*${emu}[[:space:]]*$"; then
>               ld_i386_emulation="$emu"
>               break
>           fi

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 2/5] configure: Add braces to clarify intent of $emu[[:space:]]

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> In shell script syntax, $var[something] is not special for variable
> expansion: $emu is expanded.  However, as it can look as if it were

Do you mean "$var is expanded"?

> intended to be an array element access (the correct syntax for which
> is ${var[something]}), shellcheck recommends using explicit braces
> around ${var} to clarify the intended expansion.
>
> This fixes the warning:
>
> In ./configure line 2346:
>         if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
>                                                               ^-- SC1087: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is our only 'error' level shellcheck warning, so it seems
> worth suppressing even though it's not wrong as written.
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index dec6f030346..a56c3d921be 100755
> --- a/configure
> +++ b/configure
> @@ -2343,7 +2343,7 @@ if test -n "$target_cc" &&
>      # emulation. Linux and OpenBSD/amd64 use 'elf_i386'; FreeBSD uses the _fbsd
>      # variant; OpenBSD/i386 uses the _obsd variant; and Windows uses i386pe.
>      for emu in elf_i386 elf_i386_fbsd elf_i386_obsd i386pe; do
> -        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then
> +        if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*${emu}[[:space:]]*$"; then
>              ld_i386_emulation="$emu"
>              break
>          fi

Re: [PATCH 2/5] configure: Add braces to clarify intent of $emu[[:space:]]

[Peter Maydell]

On Thu, 21 Jul 2022 at 07:24, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > In shell script syntax, $var[something] is not special for variable
> > expansion: $emu is expanded.  However, as it can look as if it were
>
> Do you mean "$var is expanded"?

Yes. I changed my mind half way through writing the commit message
about whether to use as my example the specific variable name we
have in configure or a generic $var...

-- PMM

[PATCH 3/5] configure: Don't use bash-specific string-replacement syntax

[Peter Maydell]

The variable string-replacement syntax ${var/old/new} is a bashism
(though it is also supported by some other shells), and for instance
does not work with the NetBSD /bin/sh, which complains:
 ../src/configure: 687: Syntax error: Bad substitution

Replace it with a more portable sed-based approach, similar to
what we already do in quote_sh().

Note that shellcheck also diagnoses this:

In ./configure line 687:
    e=${e/'\'/'\\'}
      ^-----------^ SC2039: In POSIX sh, string replacement is undefined.
           ^-- SC1003: Want to escape a single quote? echo 'This is how it'\''s done'.
                ^-- SC1003: Want to escape a single quote? echo 'This is how it'\''s done'.


In ./configure line 688:
    e=${e/\"/'\"'}
      ^----------^ SC2039: In POSIX sh, string replacement is undefined.

Fixes: 8154f5e64b0cf ("meson: Prefix each element of firmware path")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index a56c3d921be..c05205b6085 100755
--- a/configure
+++ b/configure
@@ -684,9 +684,10 @@ meson_option_build_array() {
     IFS=:
   fi
   for e in $1; do
-    e=${e/'\'/'\\'}
-    e=${e/\"/'\"'}
-    printf '"""%s""",' "$e"
+    printf '"""'
+    # backslash escape any '\' and '"' characters
+    printf "%s" "$e" | sed -e 's/\([\"]\)/\\\1/g'
+    printf '""",'
   done)
   printf ']\n'
 }
-- 
2.25.1

Re: [PATCH 3/5] configure: Don't use bash-specific string-replacement syntax

[Thomas Huth]

On 20/07/2022 17.26, Peter Maydell wrote:
> The variable string-replacement syntax ${var/old/new} is a bashism
> (though it is also supported by some other shells), and for instance
> does not work with the NetBSD /bin/sh, which complains:
>   ../src/configure: 687: Syntax error: Bad substitution
> 
> Replace it with a more portable sed-based approach, similar to
> what we already do in quote_sh().
> 
> Note that shellcheck also diagnoses this:
> 
> In ./configure line 687:
>      e=${e/'\'/'\\'}
>        ^-----------^ SC2039: In POSIX sh, string replacement is undefined.
>             ^-- SC1003: Want to escape a single quote? echo 'This is how it'\''s done'.
>                  ^-- SC1003: Want to escape a single quote? echo 'This is how it'\''s done'.
> 
> 
> In ./configure line 688:
>      e=${e/\"/'\"'}
>        ^----------^ SC2039: In POSIX sh, string replacement is undefined.
> 
> Fixes: 8154f5e64b0cf ("meson: Prefix each element of firmware path")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   configure | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)

Thanks, this fixes "make vm-build-netbsd" for me!

Tested-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 3/5] configure: Don't use bash-specific string-replacement syntax

[Eric Blake]

On Wed, Jul 20, 2022 at 04:26:29PM +0100, Peter Maydell wrote:
> The variable string-replacement syntax ${var/old/new} is a bashism
> (though it is also supported by some other shells), and for instance
> does not work with the NetBSD /bin/sh, which complains:
>  ../src/configure: 687: Syntax error: Bad substitution
> 
> Replace it with a more portable sed-based approach, similar to
> what we already do in quote_sh().
> 

>    for e in $1; do
> -    e=${e/'\'/'\\'}
> -    e=${e/\"/'\"'}
> -    printf '"""%s""",' "$e"
> +    printf '"""'
> +    # backslash escape any '\' and '"' characters
> +    printf "%s" "$e" | sed -e 's/\([\"]\)/\\\1/g'

You've fixed the bashism, but at the expense of a non-POSIX use of
sed.  POSIX says the input to sed must be a text file (ending in a
newline; but $e does not), and as a result it always outputs a newline
(but you don't want a newline before the closing """).  GNU sed
happens to do what you want for input not ending in a newline, but I
don't remember off-hand whether BSD sed does, and I know that Solaris
sed does not.

If this passes on BSD, then I'm okay with it; but if we want to avoid
non-POSIX altogether, this should work (using the shell's $() to strip
the trailing newline we added to keep sed happy):

# backslash escape any '\' and '"' characters
printf '"""%s""",' "$(printf "%s\n" "$e" | sed -e '/s/\([\"]\)/\\\1/g')"

Your call.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 3/5] configure: Don't use bash-specific string-replacement syntax

[Peter Maydell]

On Wed, 20 Jul 2022 at 17:30, Eric Blake <eblake@redhat.com> wrote:
>
> On Wed, Jul 20, 2022 at 04:26:29PM +0100, Peter Maydell wrote:
> > The variable string-replacement syntax ${var/old/new} is a bashism
> > (though it is also supported by some other shells), and for instance
> > does not work with the NetBSD /bin/sh, which complains:
> >  ../src/configure: 687: Syntax error: Bad substitution
> >
> > Replace it with a more portable sed-based approach, similar to
> > what we already do in quote_sh().
> >
>
> >    for e in $1; do
> > -    e=${e/'\'/'\\'}
> > -    e=${e/\"/'\"'}
> > -    printf '"""%s""",' "$e"
> > +    printf '"""'
> > +    # backslash escape any '\' and '"' characters
> > +    printf "%s" "$e" | sed -e 's/\([\"]\)/\\\1/g'
>
> You've fixed the bashism, but at the expense of a non-POSIX use of
> sed.  POSIX says the input to sed must be a text file (ending in a
> newline; but $e does not), and as a result it always outputs a newline
> (but you don't want a newline before the closing """).  GNU sed
> happens to do what you want for input not ending in a newline, but I
> don't remember off-hand whether BSD sed does, and I know that Solaris
> sed does not.

I just copied the approach we already take in quote_sh:

quote_sh() {
    printf "%s" "$1" | sed "s,','\\\\'',g; s,.*,'&',"
}

Is that also relying on this non-portable sed use?

> If this passes on BSD, then I'm okay with it; but if we want to avoid
> non-POSIX altogether, this should work (using the shell's $() to strip
> the trailing newline we added to keep sed happy):
>
> # backslash escape any '\' and '"' characters
> printf '"""%s""",' "$(printf "%s\n" "$e" | sed -e '/s/\([\"]\)/\\\1/g')"

Mmm.

-- PMM

Re: [PATCH 3/5] configure: Don't use bash-specific string-replacement syntax

[Eric Blake]

On Wed, Jul 20, 2022 at 06:32:22PM +0100, Peter Maydell wrote:
> > > +    # backslash escape any '\' and '"' characters
> > > +    printf "%s" "$e" | sed -e 's/\([\"]\)/\\\1/g'
> >
> > You've fixed the bashism, but at the expense of a non-POSIX use of
> > sed.  POSIX says the input to sed must be a text file (ending in a
> > newline; but $e does not), and as a result it always outputs a newline
> > (but you don't want a newline before the closing """).  GNU sed
> > happens to do what you want for input not ending in a newline, but I
> > don't remember off-hand whether BSD sed does, and I know that Solaris
> > sed does not.
> 
> I just copied the approach we already take in quote_sh:
> 
> quote_sh() {
>     printf "%s" "$1" | sed "s,','\\\\'',g; s,.*,'&',"
> }
> 
> Is that also relying on this non-portable sed use?

Yep. And since no one has complained, BSD sed probably handles it the
way we want (input without a trailing line getting substituted and
re-output without a trailing newline).

If we cared, this one could also be fixed:

quote_sh() {
    printf "'%s'" "$(printf '%s\n' "$1" | sed "s,','\\\\'',g")"
}

Also note that we are depending on $1 never containing newlines, because
this change would alter the existing:

$ quote_sh "a
c"
'a'
'c'

into:

$ quote_sh "a
c"
'a
c'

which is probably more what you wanted.

> 
> > If this passes on BSD, then I'm okay with it; but if we want to avoid
> > non-POSIX altogether, this should work (using the shell's $() to strip
> > the trailing newline we added to keep sed happy):
> >
> > # backslash escape any '\' and '"' characters
> > printf '"""%s""",' "$(printf "%s\n" "$e" | sed -e '/s/\([\"]\)/\\\1/g')"
> 
> Mmm.

Given that no one has complained about quote_sh, insisting on POSIX
compliance (with its resulting increase in line noise and length) is
not winning any maintainability arguments ;)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[PATCH 4/5] configure: Drop dead code attempting to use -msmall-data on alpha hosts

[Peter Maydell]

In commit 823eb013452e93d we moved the setting of ARCH from configure
to meson.build, but we accidentally left behind one attempt to use
$ARCH in configure, which was trying to add -msmall-data to the
compiler flags on Alpha hosts.  Since ARCH is now never set, the test
always fails and we never add the flag.

There isn't actually any need to use this compiler flag on Alpha:
the original intent was that it would allow us to simplify our TCG
codegen on that platform, but we never actually made the TCG changes
that would rely on -msmall-data.

Drop the effectively-dead code from configure, as we don't need it.

This was spotted by shellcheck:

In ./configure line 2254:
case "$ARCH" in
      ^---^ SC2153: Possible misspelling: ARCH may not be assigned, but arch is.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/configure b/configure
index c05205b6085..d0e9a51462e 100755
--- a/configure
+++ b/configure
@@ -2251,13 +2251,6 @@ if test "$fortify_source" = "yes" ; then
   QEMU_CFLAGS="-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $QEMU_CFLAGS"
 fi
 
-case "$ARCH" in
-alpha)
-  # Ensure there's only a single GP
-  QEMU_CFLAGS="-msmall-data $QEMU_CFLAGS"
-;;
-esac
-
 if test "$have_asan" = "yes"; then
   QEMU_CFLAGS="-fsanitize=address $QEMU_CFLAGS"
   QEMU_LDFLAGS="-fsanitize=address $QEMU_LDFLAGS"
-- 
2.25.1

Re: [PATCH 4/5] configure: Drop dead code attempting to use -msmall-data on alpha hosts

[Thomas Huth]

On 20/07/2022 17.26, Peter Maydell wrote:
> In commit 823eb013452e93d we moved the setting of ARCH from configure
> to meson.build, but we accidentally left behind one attempt to use
> $ARCH in configure, which was trying to add -msmall-data to the
> compiler flags on Alpha hosts.  Since ARCH is now never set, the test
> always fails and we never add the flag.
> 
> There isn't actually any need to use this compiler flag on Alpha:
> the original intent was that it would allow us to simplify our TCG
> codegen on that platform, but we never actually made the TCG changes
> that would rely on -msmall-data.
> 
> Drop the effectively-dead code from configure, as we don't need it.
> 
> This was spotted by shellcheck:
> 
> In ./configure line 2254:
> case "$ARCH" in
>        ^---^ SC2153: Possible misspelling: ARCH may not be assigned, but arch is.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   configure | 7 -------
>   1 file changed, 7 deletions(-)
> 
> diff --git a/configure b/configure
> index c05205b6085..d0e9a51462e 100755
> --- a/configure
> +++ b/configure
> @@ -2251,13 +2251,6 @@ if test "$fortify_source" = "yes" ; then
>     QEMU_CFLAGS="-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $QEMU_CFLAGS"
>   fi
>   
> -case "$ARCH" in
> -alpha)
> -  # Ensure there's only a single GP
> -  QEMU_CFLAGS="-msmall-data $QEMU_CFLAGS"
> -;;
> -esac
> -
>   if test "$have_asan" = "yes"; then
>     QEMU_CFLAGS="-fsanitize=address $QEMU_CFLAGS"
>     QEMU_LDFLAGS="-fsanitize=address $QEMU_LDFLAGS"

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 5/5] configure: Avoid '==' bashism

[Peter Maydell]

The '==' operator to test is a bashism; the standard way to copmare
strings is '='. This causes dash to complain:

../../configure: 681: test: linux: unexpected operator

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index d0e9a51462e..2c19329d58c 100755
--- a/configure
+++ b/configure
@@ -678,7 +678,7 @@ werror=""
 
 meson_option_build_array() {
   printf '['
-  (if test "$targetos" == windows; then
+  (if test "$targetos" = windows; then
     IFS=\;
   else
     IFS=:
-- 
2.25.1

Re: [PATCH 5/5] configure: Avoid '==' bashism

[Thomas Huth]

On 20/07/2022 17.26, Peter Maydell wrote:
> The '==' operator to test is a bashism; the standard way to copmare
> strings is '='. This causes dash to complain:
> 
> ../../configure: 681: test: linux: unexpected operator
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   configure | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index d0e9a51462e..2c19329d58c 100755
> --- a/configure
> +++ b/configure
> @@ -678,7 +678,7 @@ werror=""
>   
>   meson_option_build_array() {
>     printf '['
> -  (if test "$targetos" == windows; then
> +  (if test "$targetos" = windows; then
>       IFS=\;
>     else
>       IFS=:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 0/5] configure: fix some non-portabilities

[Peter Maydell]

On Wed, 20 Jul 2022 at 16:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset fixes some non-portable code that has crept in recently:
> notably, it fixes problems that are reported to cause configure
> not to work correctly on OpenBSD and NetBSD, and a warning message
> when using dash as /bin/sh on Linux. I threw in a less important
> "drop some dead code" fix too, and a fix for the only other 'error'
> category problem reported by shellcheck. (There are way too many
> 'warning' category reports to deal with all at once.)
>
> If people who reported problems on NetBSD/OpenBSD could check that
> this fixes them, that would be great.

I'll take these via target-arm.next since I'm doing a pullreq anyway.

thanks
-- PMM