Re: [Qemu-devel] [PATCH] configure: Add missing space when using --with-pkgversion

[Eric Blake <eblake@redhat.com>]

On 02/14/2018 11:31 AM, Thomas Huth wrote:
> When running configure with --with-pkgversion=foo there is no
> space anymore between the version number and the parentheses:
> 
> $ m68k-softmmu/qemu-system-m68k -version
> QEMU emulator version 2.11.50(foo)
> 
> Fix it by moving the space from the configure script to the Makefile.
> 
> Fixes: 67a1de0d195a6185c39b436159c9ffc7720bf979
> Buglink: https://bugs.launchpad.net/qemu/+bug/1673373
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   Makefile  | 2 +-
>   configure | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 4ec7a3c..41adbc9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -369,7 +369,7 @@ qemu-version.h: FORCE
>   		(cd $(SRC_PATH); \
>   		printf '#define QEMU_PKGVERSION '; \
>   		if test -n "$(PKGVERSION)"; then \
> -			printf '"$(PKGVERSION)"\n'; \
> +			printf '" ($(PKGVERSION))"\n'; \

I would argue that putting a space here is awkward; wouldn't it instead 
be easier to have all CLIENTS of QEMU_PKGVERSION in the source code 
assume that the macro does NOT have a leading space, and to supply a 
space themselves?

That is, change THESE locations:

bsd-user/main.c:    printf("qemu-" TARGET_NAME " version " QEMU_VERSION 
QEMU_PKGVERSION
linux-user/main.c:    printf("qemu-" TARGET_NAME " version " 
QEMU_VERSION QEMU_PKGVERSION
qemu-img.c:#define QEMU_IMG_VERSION "qemu-img version " QEMU_VERSION 
QEMU_PKGVERSION \
qemu-io.c:            printf("%s version " QEMU_VERSION QEMU_PKGVERSION "\n"
qemu-nbd.c:"%s " QEMU_VERSION QEMU_PKGVERSION "\n"
qga/main.c:"QEMU Guest Agent " QEMU_VERSION QEMU_PKGVERSION "\n"
scsi/qemu-pr-helper.c:"%s " QEMU_VERSION QEMU_PKGVERSION "\n"
ui/cocoa.m:    @"QEMU emulator version %s%s", QEMU_VERSION, 
QEMU_PKGVERSION];
vl.c:    printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION "\n"

to instead supply the missing space, and have configure/Makefile always 
generate without a leading space.

> +++ b/configure
> @@ -1162,7 +1162,7 @@ for opt do
>     ;;
>     --disable-blobs) blobs="no"
>     ;;
> -  --with-pkgversion=*) pkgversion=" ($optarg)"
> +  --with-pkgversion=*) pkgversion="$optarg"

Hmm - here you're changing who supplies the ().  But that argues that 
maybe the callsites should supply " (" and ")" themselves.

Here's how coreutils does it, by using gnulib's version-etc module:

src/local.mk:     $(AM_V_at)printf 'char const *Version = 
"$(PACKAGE_VERSION)";\n' >> $@t

src/system.h:    version_etc (stdout, Program_name, PACKAGE_NAME, 
Version, Authors,

version_etc_arn (FILE *stream,
                  const char *command_name, const char *package,
                  const char *version,
                  const char * const * authors, size_t n_authors)
{
   if (command_name)
     fprintf (stream, "%s (%s) %s\n", command_name, package, version);

which means that the Makefile magic outputs JUST the text that goes 
inside the (), and the callsite that outputs version information is what 
supplies the " (" and ")".

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