[Qemu-devel] [PATCH v3 0/2] Let PKGVERSION include the "git describe" output

[Qemu-devel] [PATCH v3 0/2] Let PKGVERSION include the "git describe" output

[Fam Zheng]

v3: Use the "2.6.50 (v2.6.0-603-g684a494)" format. [Laszlo]
    printf '-dirty' -> printf -- '-dirty'.

v2: Address Eric's comments:
    "echo -n" -> "printf".
    "-unclean" -> "-dirty".

Makefile happened to be in my working set because of the docker test work, so I
went ahead to try this nice feature last evening before going to bed. My
apologies if we have duplicated work.

Please review, especially please comment on the redundant version string:

$ qemu-img --version
qemu-img version 2.6.50-v2.6.0-603-g684a494, Copyright (c) 2004-2008 Fabrice Bellard


Fam Zheng (2):
  Makefile: Add a "FORCE" target
  Makefile: Derive "PKGVERSION" from "git describe" by default

 Makefile              | 24 ++++++++++++++++++++++--
 linux-user/main.c     |  1 +
 qemu-img.c            |  1 +
 qmp.c                 |  1 +
 scripts/create_config |  4 ----
 vl.c                  |  1 +
 6 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.8.2

[Qemu-devel] [PATCH v3 1/2] Makefile: Add a "FORCE" target

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 3a3c5dc..1e2d6f9 100644
--- a/Makefile
+++ b/Makefile
@@ -81,7 +81,7 @@ Makefile: ;
 configure: ;
 
 .PHONY: all clean cscope distclean dvi html info install install-doc \
-	pdf recurse-all speed test dist msi
+	pdf recurse-all speed test dist msi FORCE
 
 $(call set-vpath, $(SRC_PATH))
 
-- 
2.8.2

[Qemu-devel] [PATCH v3 2/2] Makefile: Derive "PKGVERSION" from "git describe" by default

[Fam Zheng]

Currently, if not specified in "./configure", QEMU_PKGVERSION will be
empty. Write a rule in Makefile to generate a value from "git describe"
combined with a possible git tree cleanness suffix, and write into a new
header.

    $ cat qemu-version.h
    #define QEMU_PKGVERSION "-v2.6.0-557-gd6550e9-dirty"

Include the header in .c files where the macro is referenced. It's not
necessary to include it in all files, otherwise each time the content of
the file changes, all sources have to be recompiled.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile              | 22 +++++++++++++++++++++-
 linux-user/main.c     |  1 +
 qemu-img.c            |  1 +
 qmp.c                 |  1 +
 scripts/create_config |  4 ----
 vl.c                  |  1 +
 6 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 1e2d6f9..5124f54 100644
--- a/Makefile
+++ b/Makefile
@@ -49,7 +49,7 @@ ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fa
 endif
 endif
 
-GENERATED_HEADERS = config-host.h qemu-options.def
+GENERATED_HEADERS = qemu-version.h config-host.h qemu-options.def
 GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
 GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
 GENERATED_HEADERS += qmp-introspect.h
@@ -166,6 +166,26 @@ endif
 
 all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all modules
 
+qemu-version.h: FORCE
+	$(call quiet-command, \
+		(cd $(SRC_PATH); \
+		printf '#define QEMU_PKGVERSION '; \
+		if test -n "$(PKGVERSION)"; then \
+			printf '"$(PKGVERSION)"\n'; \
+		else \
+			printf '" ('; \
+			if ! git status &>/dev/null; then \
+				printf "no-git"; \
+			else \
+				git describe 2>/dev/null | tr -d '\n'; \
+				if ! git diff-index --quiet HEAD &>/dev/null; then \
+					printf -- '-dirty'; \
+				fi \
+			fi; \
+			printf ')"\n'; \
+		fi) > $@.tmp)
+	$(call quiet-command, cmp --quiet $@ $@.tmp || mv $@.tmp $@)
+
 config-host.h: config-host.h-timestamp
 config-host.h-timestamp: config-host.mak
 qemu-options.def: $(SRC_PATH)/qemu-options.hx
diff --git a/linux-user/main.c b/linux-user/main.c
index b2bc6ab..8a11d02 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -17,6 +17,7 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
+#include "qemu-version.h"
 #include <sys/mman.h>
 #include <sys/syscall.h>
 #include <sys/resource.h>
diff --git a/qemu-img.c b/qemu-img.c
index 4b56ad3..32e307c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu-version.h"
 #include "qapi/error.h"
 #include "qapi-visit.h"
 #include "qapi/qmp-output-visitor.h"
diff --git a/qmp.c b/qmp.c
index 3165f87..7df6543 100644
--- a/qmp.c
+++ b/qmp.c
@@ -14,6 +14,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu-version.h"
 #include "qemu/cutils.h"
 #include "monitor/monitor.h"
 #include "sysemu/sysemu.h"
diff --git a/scripts/create_config b/scripts/create_config
index b2d2ebb..d47057b 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -9,10 +9,6 @@ case $line in
     version=${line#*=}
     echo "#define QEMU_VERSION \"$version\""
     ;;
- PKGVERSION=*) # configuration
-    pkgversion=${line#*=}
-    echo "#define QEMU_PKGVERSION \"$pkgversion\""
-    ;;
  qemu_*dir=*) # qemu-specific directory configuration
     name=${line%=*}
     value=${line#*=}
diff --git a/vl.c b/vl.c
index 18d1423..97cad71 100644
--- a/vl.c
+++ b/vl.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu-version.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
 
-- 
2.8.2

Re: [Qemu-devel] [PATCH v3 1/2] Makefile: Add a "FORCE" target

[Daniel P. Berrange]

On Wed, Jun 01, 2016 at 05:44:20PM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 3a3c5dc..1e2d6f9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -81,7 +81,7 @@ Makefile: ;
>  configure: ;
>  
>  .PHONY: all clean cscope distclean dvi html info install install-doc \
> -	pdf recurse-all speed test dist msi
> +	pdf recurse-all speed test dist msi FORCE
>  
>  $(call set-vpath, $(SRC_PATH))

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v3 2/2] Makefile: Derive "PKGVERSION" from "git describe" by default

[Daniel P. Berrange]

On Wed, Jun 01, 2016 at 05:44:21PM +0800, Fam Zheng wrote:
> Currently, if not specified in "./configure", QEMU_PKGVERSION will be
> empty. Write a rule in Makefile to generate a value from "git describe"
> combined with a possible git tree cleanness suffix, and write into a new
> header.
> 
>     $ cat qemu-version.h
>     #define QEMU_PKGVERSION "-v2.6.0-557-gd6550e9-dirty"
> 
> Include the header in .c files where the macro is referenced. It's not
> necessary to include it in all files, otherwise each time the content of
> the file changes, all sources have to be recompiled.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile              | 22 +++++++++++++++++++++-
>  linux-user/main.c     |  1 +
>  qemu-img.c            |  1 +
>  qmp.c                 |  1 +
>  scripts/create_config |  4 ----
>  vl.c                  |  1 +
>  6 files changed, 25 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v3 2/2] Makefile: Derive "PKGVERSION" from "git describe" by default

[Gerd Hoffmann]

  Hi,

> +				git describe 2>/dev/null | tr -d '\n'; \
> +				if ! git diff-index --quiet HEAD &>/dev/null; then \
> +					printf -- '-dirty'; \
> +				fi \

/me suggests "git describe --dirty --match 'v*'"

Saves the extra effort to check for a dirty tree manually.

Also greatly reduces the chance non-release tags are matched, so I don't
get results like "pull-vga-20160523-1-236-g9fd5eb7".

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v3 2/2] Makefile: Derive "PKGVERSION" from "git describe" by default

[Laszlo Ersek]

On 06/01/16 12:40, Gerd Hoffmann wrote:
>   Hi,
> 
>> +				git describe 2>/dev/null | tr -d '\n'; \
>> +				if ! git diff-index --quiet HEAD &>/dev/null; then \
>> +					printf -- '-dirty'; \
>> +				fi \
> 
> /me suggests "git describe --dirty --match 'v*'"
> 
> Saves the extra effort to check for a dirty tree manually.

We couldn't convince ourselves that support for "--dirty" is ubiquitous;
please see the sub-thread rooted at
<http://thread.gmane.org/gmane.comp.emulators.qemu/414824/focus=414828>.

> Also greatly reduces the chance non-release tags are matched, so I don't
> get results like "pull-vga-20160523-1-236-g9fd5eb7".

Since what version is "--match" supported? ;)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v3 2/2] Makefile: Derive "PKGVERSION" from "git describe" by default

[Paolo Bonzini]

On 01/06/2016 13:13, Laszlo Ersek wrote:
> On 06/01/16 12:40, Gerd Hoffmann wrote:
>>   Hi,
>>
>>> +				git describe 2>/dev/null | tr -d '\n'; \
>>> +				if ! git diff-index --quiet HEAD &>/dev/null; then \
>>> +					printf -- '-dirty'; \
>>> +				fi \
>>
>> /me suggests "git describe --dirty --match 'v*'"
>>
>> Saves the extra effort to check for a dirty tree manually.
> 
> We couldn't convince ourselves that support for "--dirty" is ubiquitous;
> please see the sub-thread rooted at
> <http://thread.gmane.org/gmane.comp.emulators.qemu/414824/focus=414828>.
> 
>> Also greatly reduces the chance non-release tags are matched, so I don't
>> get results like "pull-vga-20160523-1-236-g9fd5eb7".
> 
> Since what version is "--match" supported? ;)

git's own version history says 1.5.5.

Another small point is that some people put the whole home directory in
git, so I would test for $(SRC_PATH)/.git instead of using "git
status".  And no-git is unnecessary if the git part is included in
parentheses.  This gives:

diff --git a/Makefile b/Makefile
index a4d7da0..412c2b8 100644
--- a/Makefile
+++ b/Makefile
@@ -173,16 +173,16 @@ qemu-version.h: FORCE
 		if test -n "$(PKGVERSION)"; then \
 			printf '"$(PKGVERSION)"\n'; \
 		else \
-			printf '" ('; \
-			if ! git status &>/dev/null; then \
-				printf "no-git"; \
-			else \
-				git describe 2>/dev/null | tr -d '\n'; \
+			if test -d .git; then \
+			        printf '" ('; \
+				git describe --match 'v*' 2>/dev/null | tr -d '\n'; \
 				if ! git diff-index --quiet HEAD &>/dev/null; then \
 					printf -- '-dirty'; \
-				fi \
+				fi; \
+			        printf ')"\n'; \
+			else \
+				printf '""\n'; \
 			fi; \
-			printf ')"\n'; \
 		fi) > $@.tmp)
 	$(call quiet-command, cmp --quiet $@ $@.tmp || mv $@.tmp $@)
 


Looks good?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH v3 2/2] Makefile: Derive "PKGVERSION" from "git describe" by default

[Laszlo Ersek]

On 06/01/16 15:55, Paolo Bonzini wrote:
> 
> 
> On 01/06/2016 13:13, Laszlo Ersek wrote:
>> On 06/01/16 12:40, Gerd Hoffmann wrote:
>>>   Hi,
>>>
>>>> +				git describe 2>/dev/null | tr -d '\n'; \
>>>> +				if ! git diff-index --quiet HEAD &>/dev/null; then \
>>>> +					printf -- '-dirty'; \
>>>> +				fi \
>>>
>>> /me suggests "git describe --dirty --match 'v*'"
>>>
>>> Saves the extra effort to check for a dirty tree manually.
>>
>> We couldn't convince ourselves that support for "--dirty" is ubiquitous;
>> please see the sub-thread rooted at
>> <http://thread.gmane.org/gmane.comp.emulators.qemu/414824/focus=414828>.
>>
>>> Also greatly reduces the chance non-release tags are matched, so I don't
>>> get results like "pull-vga-20160523-1-236-g9fd5eb7".
>>
>> Since what version is "--match" supported? ;)
> 
> git's own version history says 1.5.5.
> 
> Another small point is that some people put the whole home directory in
> git, so I would test for $(SRC_PATH)/.git instead of using "git
> status".  And no-git is unnecessary if the git part is included in
> parentheses.  This gives:
> 
> diff --git a/Makefile b/Makefile
> index a4d7da0..412c2b8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -173,16 +173,16 @@ qemu-version.h: FORCE
>  		if test -n "$(PKGVERSION)"; then \
>  			printf '"$(PKGVERSION)"\n'; \
>  		else \
> -			printf '" ('; \
> -			if ! git status &>/dev/null; then \
> -				printf "no-git"; \
> -			else \
> -				git describe 2>/dev/null | tr -d '\n'; \
> +			if test -d .git; then \
> +			        printf '" ('; \
> +				git describe --match 'v*' 2>/dev/null | tr -d '\n'; \
>  				if ! git diff-index --quiet HEAD &>/dev/null; then \
>  					printf -- '-dirty'; \
> -				fi \
> +				fi; \
> +			        printf ')"\n'; \
> +			else \
> +				printf '""\n'; \
>  			fi; \
> -			printf ')"\n'; \
>  		fi) > $@.tmp)
>  	$(call quiet-command, cmp --quiet $@ $@.tmp || mv $@.tmp $@)
>  
> 
> 
> Looks good?

It does to me, yes.

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH v3 2/2] Makefile: Derive "PKGVERSION" from "git describe" by default

[Fam Zheng]

On Wed, 06/01 15:55, Paolo Bonzini wrote:
> 
> 
> On 01/06/2016 13:13, Laszlo Ersek wrote:
> > On 06/01/16 12:40, Gerd Hoffmann wrote:
> >>   Hi,
> >>
> >>> +				git describe 2>/dev/null | tr -d '\n'; \
> >>> +				if ! git diff-index --quiet HEAD &>/dev/null; then \
> >>> +					printf -- '-dirty'; \
> >>> +				fi \
> >>
> >> /me suggests "git describe --dirty --match 'v*'"
> >>
> >> Saves the extra effort to check for a dirty tree manually.
> > 
> > We couldn't convince ourselves that support for "--dirty" is ubiquitous;
> > please see the sub-thread rooted at
> > <http://thread.gmane.org/gmane.comp.emulators.qemu/414824/focus=414828>.
> > 
> >> Also greatly reduces the chance non-release tags are matched, so I don't
> >> get results like "pull-vga-20160523-1-236-g9fd5eb7".
> > 
> > Since what version is "--match" supported? ;)
> 
> git's own version history says 1.5.5.
> 
> Another small point is that some people put the whole home directory in
> git, so I would test for $(SRC_PATH)/.git instead of using "git
> status".  And no-git is unnecessary if the git part is included in
> parentheses.  This gives:
> 
> diff --git a/Makefile b/Makefile
> index a4d7da0..412c2b8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -173,16 +173,16 @@ qemu-version.h: FORCE
>  		if test -n "$(PKGVERSION)"; then \
>  			printf '"$(PKGVERSION)"\n'; \
>  		else \
> -			printf '" ('; \
> -			if ! git status &>/dev/null; then \
> -				printf "no-git"; \
> -			else \
> -				git describe 2>/dev/null | tr -d '\n'; \
> +			if test -d .git; then \
> +			        printf '" ('; \
> +				git describe --match 'v*' 2>/dev/null | tr -d '\n'; \
>  				if ! git diff-index --quiet HEAD &>/dev/null; then \
>  					printf -- '-dirty'; \
> -				fi \
> +				fi; \
> +			        printf ')"\n'; \
> +			else \
> +				printf '""\n'; \
>  			fi; \
> -			printf ')"\n'; \
>  		fi) > $@.tmp)
>  	$(call quiet-command, cmp --quiet $@ $@.tmp || mv $@.tmp $@)
>  
> 
> 
> Looks good?

Looks good except I'd use tabs everywhere. Can you fix when applying? :)

Fam

Re: [Qemu-devel] [PATCH v3 0/2] Let PKGVERSION include the "git describe" output

[Changlong Xie]

For this patch, i think you forget to add qemu-version.h and/or 
qemu-version.h.tmp in .gitignore

changlox ~/w/qemu/qemu% gs
On branch master
Your branch is up-to-date with 'origin/master'.

Untracked files:
   (use "git add <file>..." to include in what will be committed)

         qemu-version.h
         qemu-version.h.tmp

nothing added to commit but untracked files present (use "git add" to track)


On 06/01/2016 05:44 PM, Fam Zheng wrote:
> v3: Use the "2.6.50 (v2.6.0-603-g684a494)" format. [Laszlo]
>      printf '-dirty' -> printf -- '-dirty'.
>
> v2: Address Eric's comments:
>      "echo -n" -> "printf".
>      "-unclean" -> "-dirty".
>
> Makefile happened to be in my working set because of the docker test work, so I
> went ahead to try this nice feature last evening before going to bed. My
> apologies if we have duplicated work.
>
> Please review, especially please comment on the redundant version string:
>
> $ qemu-img --version
> qemu-img version 2.6.50-v2.6.0-603-g684a494, Copyright (c) 2004-2008 Fabrice Bellard
>
>
> Fam Zheng (2):
>    Makefile: Add a "FORCE" target
>    Makefile: Derive "PKGVERSION" from "git describe" by default
>
>   Makefile              | 24 ++++++++++++++++++++++--
>   linux-user/main.c     |  1 +
>   qemu-img.c            |  1 +
>   qmp.c                 |  1 +
>   scripts/create_config |  4 ----
>   vl.c                  |  1 +
>   6 files changed, 26 insertions(+), 6 deletions(-)
>

Re: [Qemu-devel] [PATCH v3 0/2] Let PKGVERSION include the "git describe" output

[Fam Zheng]

On Mon, 06/20 12:50, Changlong Xie wrote:
> For this patch, i think you forget to add qemu-version.h and/or
> qemu-version.h.tmp in .gitignore
> 
> changlox ~/w/qemu/qemu% gs
> On branch master
> Your branch is up-to-date with 'origin/master'.
> 
> Untracked files:
>   (use "git add <file>..." to include in what will be committed)
> 
>         qemu-version.h
>         qemu-version.h.tmp
> 
> nothing added to commit but untracked files present (use "git add" to track)

Yes, sorry for the inconvenience. There is a patch on list by Eric Blake that
should fix this:

https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg02375.html

Fam