configure: fix POSIX compatibility issue

[Dmitrij D. Czarkoff]

When running configure script from 2.5.0-rc4 on OpenBSD-current (amd64), I get the following error:

  ./configure[4756]: ${nettle:+($nettle_version)}": bad substitution
  *** Error 1 in . (/usr/ports/infrastructure/mk/bsd.port.mk:2747 '/usr/ports/pobj/qemu-2.5.0rc4/.configure_done')
  *** Error 1 in /usr/ports/openbsd-wip/emulators/qemu (/usr/ports/infrastructure/mk/bsd.port.mk:2491 'configure')

Indeed, construct "${nettle:+($nettle_version)}" does not conform to POSIX Shell Command Language. The attached patch fixes the issue.

[Dmitrij D. Czarkoff (czarkoff-gmail)]

Sorry, wrong patch.

[Stefan Hajnoczi (stefanha)]

On Sun, Dec 13, 2015 at 06:39:22PM -0000, Dmitrij D. Czarkoff wrote:
> Sorry, wrong patch.
>
> ** Patch added: "0001-configure-fix-POSIX-compatibility-issue.patch"
> https://bugs.launchpad.net/qemu/+bug/1525682/+attachment/4534158/+files/0001-configure-fix-POSIX-compatibility-issue.patch
>
> ** Patch removed: "0001-configure-fix-POSIX-compatibility-issue.patch"
> https://bugs.launchpad.net/qemu/+bug/1525682/+attachment/4534156/+files/0001-configure-fix-POSIX-compatibility-issue.patch

Please send patches to <email address hidden>. Guidelines on submitting
patches are here:
http://qemu-project.org/Contribute/SubmitAPatch

Thanks!

[Peter Maydell (pmaydell)]

In particular, the Signed-off-by: line is critically important -- we cannot apply a patch without one.

git blame says this + syntax was originally introduced in commit becaeb726 in July (though at that point the variable name was slightly different: ${gnutls_nettle+($nettle_version)} ). That means we were using this construct in v2.4.0, so this is not a regression for 2.5.0.

I'm also a bit confused by your patch and your original bug report. The error message you quote is complaining about a line with ":+" syntax, but upstream configure is not using ":+" syntax here. Indeed your patch is changing it from + to :+.

   -echo "nettle $nettle ${nettle+($nettle_version)}"
   +echo "nettle $nettle ${nettle:+($nettle_version)}"

It's not clear to me why this would help, because
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
(section "Parameter Expansion") which documents the syntax describes both ":+" and "+", so whatever the shell is complaining about it presumably isn't the + vs :+ distinction.

Which shell is this?

[Dmitrij D. Czarkoff (czarkoff-gmail)]

OK, so I misidentified the issue and screwed up my bug report.

The shell is pdksh on OpenBSD, and the real issue is with parentheses:

  $ a=1
  $ b=2
  $ echo "${a+($b)}"
  ksh: ${a+($b)}": bad substitution
  $ echo "${a+\($b\)}"
  (2)

[Peter Maydell (pmaydell)]

Unfortunately in bash and dash backslash-escaping the brackets results in the backslashes being printed verbatim:
$ (a=1 b=2 ; echo "${a+\($b\)}")
\(2\)

Can you try this syntax with extra quote characters? (It works in bash/dash):
(a=1 b=2 ; echo "${a+"($b)"}")
(2)

[Dmitrij D. Czarkoff (czarkoff-gmail)]

It works.

[Peter Maydell (pmaydell)]

Thanks. I'll send out a patch.

[Peter Maydell (pmaydell)]

Actually it turns out we really shouldn't be using the ${} syntax anyway, because if nettle is not present we end up printing
"nettle: no ()"
because $nettle is set to "no", not null or unset. So we should just write this out like:
if test "$nettle" = "yes"; then
    echo "nettle $nettle ($nettle_version)"
else
    echo "nettle $nettle"
fi

[Dmitrij D. Czarkoff (czarkoff-gmail)]

FWIW this way it is also consistent with other check results reporting, eg. spice.

[Peter Maydell (pmaydell)]

The patch to fix this is at: http://patchwork.ozlabs.org/patch/556537/

Unfortunately it has just missed the cutoff to get into 2.5.0 (since it has been present since 2.4.0 and there is a workaround of running "/path/to/bash configure"). We'll put it into the next 2.5.x stable release, though.

[Eric Blake (eblake)]

[adding autoconf, which likes to document shell bugs]

On 12/14/2015 04:34 AM, Dmitrij D. Czarkoff wrote:
> OK, so I misidentified the issue and screwed up my bug report.
>
> The shell is pdksh on OpenBSD, and the real issue is with parentheses:
>
> $ a=1
> $ b=2
> $ echo "${a+($b)}"
> ksh: ${a+($b)}": bad substitution

That's a bug in pdksh; see the POSIX interpretation:

http://austingroupbugs.net/view.php?id=221#c399

    For parameter expansions other than the four varieties that provide
    for substring processing, within the string of characters from an
    enclosed "${" to the matching '}', the double-quotes within which
    the expansion occurs shall preserve the literal value of all
    characters, with the exception of the characters double-quote,
    backquote, <dollar-sign>, and <backslash>.

The fact that you are using "" outside the ${} means that all characters
between + and } should be used literally (the same as if you had done
'echo "($b)"'). According to POSIX, it should not be a syntax error, so
you should report this to the pdksh shell developers.

--
Eric Blake eblake redhat com [phone]
Libvirt virtualization library http://libvirt.org

[Thorsten Glaser (mirabilos)]

Note that mksh is virtually a superset of OpenBSD ksh and accepts this construct, for a quick fix.

[Thomas Huth (th-huth)]

The patch has been included here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=18f49881cf8359e89396aac
... which should be part of QEMU 2.6.0, so let's mark this bug report as fixed.