On 14.02.2018 21:23, Eric Blake wrote: > 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 <th...@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.
Yeah, that's likely the saner way to do this. The question is: What about the query-version QMP command? Should it report parentheses or not? I think I'd look nicer if it reports "package": "foo" instead of "package": "(foo)" - but we maybe could break some users who expect parentheses there (no matter whether there is a preceding space or not)? Thomas