[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Add missing space whe
From: |
Thomas Huth |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Add missing space when using --with-pkgversion |
Date: |
Thu, 15 Feb 2018 07:02:40 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
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 <address@hidden>
>> ---
>> 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