[PATCH 1/5] make: Check tools/qemu-xen[-traditional] for qemu before downloading

[PATCH 1/5] make: Check tools/qemu-xen[-traditional] for qemu before downloading

[George Dunlap]

Currently xen, qemu-xen, and qemu-xen-traditional are kept in separate
repositories, but when we release them as a tarball, qemu-xen and
qemu-xen-traditional are in-lined into the tools/ directory.

In order to make this "just work", at the moment developer doing the
release manually modifies Config.mk as part of the relase process so
that CONFIG_QEMU and QEMU_UPSTREAM_URL point into the tools/ directory
instead.

Modify Config.mk to automatically check there before trying a remote
repository.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

---

Since we're essentially manually doing this for each release anyway, I
propose this c/s be backported to all currently-maintained branches.

v2:
 - Make *_INTREE variables to avoid duplication
 - Make assignment more compact for CONFIG_QEMU

CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
 Config.mk |   19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/Config.mk b/Config.mk
index ab95b1c..a9b3468 100644
--- a/Config.mk
+++ b/Config.mk
@@ -224,6 +224,21 @@ else
 QEMU_REMOTE ?= git://xenbits.xen.org/qemu-xen-unstable.git
 endif
 
+# Where to look for inlined subtrees (for example, from a tarball)
+QEMU_UPSTREAM_INTREE ?= $(XEN_ROOT)/tools/qemu-xen
+QEMU_TRADITIONAL_INTREE ?= $(XEN_ROOT)/tools/qemu-xen-traditional
+
+
+# Specify which qemu-dm to use. This may be `ioemu' to use the old
+# Mercurial in-tree version, or a local directory, or a git URL.
+# CONFIG_QEMU ?= `pwd`/$(XEN_ROOT)/../qemu-xen.git
+CONFIG_QEMU ?= $(or $(wildcard $(QEMU_TRADITIONAL_INTREE)),\
+                    $(QEMU_REMOTE))
+
+ifneq (,$(wildcard $(QEMU_UPSTREAM_INTREE)))
+QEMU_UPSTREAM_URL ?= $(QEMU_UPSTREAM_INTREE)
+endif
+
 ifeq ($(GIT_HTTP),y)
 OVMF_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/ovmf.git
 QEMU_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/qemu-upstream-unstable.git
@@ -241,10 +256,6 @@ SEABIOS_UPSTREAM_TAG ?= rel-1.7.5
 
 ETHERBOOT_NICS ?= rtl8139 8086100e
 
-# Specify which qemu-dm to use. This may be `ioemu' to use the old
-# Mercurial in-tree version, or a local directory, or a git URL.
-# CONFIG_QEMU ?= `pwd`/$(XEN_ROOT)/../qemu-xen.git
-CONFIG_QEMU ?= $(QEMU_REMOTE)
 
 QEMU_TAG ?= d0395cc49b2ec6d1723c01f1daf2394b9264ca29
 # Tue Apr 8 16:50:06 2014 +0000
-- 
1.7.9.5

[PATCH 2/5] make: Normalize config options for external trees

[George Dunlap]

We have four different external trees, and four different naming
conventions for specifying the URL, where they live, and what revision
to check out.

Normalize config options on the following bases:
 - QEMU_UPSTREAM
 - QEMU_TRADITIONAL
 - SEABIOS_UPSTREAM
 - OVMF_UPSTREAM

The following suffixes for all trees:
 - _URL : A remote repository to clone from
 - _REVISION : The revision to check out

And the following suffixes for the qemu trees:
 - _INTREE : The location of an inlined tree (for tarball releases)
 - _LOC : Where to actually look (either a directory or a URL)

The following parameters are still supported for backwards
compatibility:
 - CONFIG_QEMU          (=> QEMU_TRADITIONAL_LOC)
 - QEMU_REMOTE          (=> QEMU_TRADITIONAL_URL)
 - QEMU_TAG             (=> QEMU_TRADITIONAL_REVISION)
 - SEABIOS_UPSTREAM_TAG (=> SEABIOS_UPSTREAM_REVISION)

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---

We should relnote these and say that they are deprecated, and may be
removed in a future release.

CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
---
 Config.mk               |   43 +++++++++++++++++++++++++++----------------
 tools/Makefile          |   18 +++++++++---------
 tools/firmware/Makefile |    6 +++---
 3 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/Config.mk b/Config.mk
index a9b3468..5370e23 100644
--- a/Config.mk
+++ b/Config.mk
@@ -218,49 +218,60 @@ XEN_EXTFILES_URL ?= http://xenbits.xen.org/xen-extfiles
 # the internet.  The original download URL is preserved as a comment
 # near the place in the Xen Makefiles where the file is used.
 
-ifeq ($(GIT_HTTP),y)
-QEMU_REMOTE ?= http://xenbits.xen.org/git-http/qemu-xen-unstable.git
-else
-QEMU_REMOTE ?= git://xenbits.xen.org/qemu-xen-unstable.git
-endif
-
 # Where to look for inlined subtrees (for example, from a tarball)
 QEMU_UPSTREAM_INTREE ?= $(XEN_ROOT)/tools/qemu-xen
 QEMU_TRADITIONAL_INTREE ?= $(XEN_ROOT)/tools/qemu-xen-traditional
 
 
-# Specify which qemu-dm to use. This may be `ioemu' to use the old
-# Mercurial in-tree version, or a local directory, or a git URL.
-# CONFIG_QEMU ?= `pwd`/$(XEN_ROOT)/../qemu-xen.git
-CONFIG_QEMU ?= $(or $(wildcard $(QEMU_TRADITIONAL_INTREE)),\
-                    $(QEMU_REMOTE))
-
-ifneq (,$(wildcard $(QEMU_UPSTREAM_INTREE)))
-QEMU_UPSTREAM_URL ?= $(QEMU_UPSTREAM_INTREE)
+# Handle legacy options
+ifneq (,$(SEABIOS_UPSTREAM_TAG))
+SEABIOS_UPSTREAM_REVISION ?= $(SEABIOS_UPSTREAM_TAG)
+endif
+ifneq (,$(QEMU_REMOTE))
+QEMU_TRADITIONAL_URL ?= $(QEMU_REMOTE)
+endif
+ifneq (,$(CONFIG_QEMU))
+QEMU_TRADITIONAL_LOC ?= $(CONFIG_QEMU)
+endif
+ifneq (,$(QEMU_TAG))
+QEMU_TRADITIONAL_REVISION ?= $(QEMU_TAG)
 endif
 
 ifeq ($(GIT_HTTP),y)
 OVMF_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/ovmf.git
 QEMU_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/qemu-upstream-unstable.git
+QEMU_TRADITIONAL_URL ?= http://xenbits.xen.org/git-http/qemu-xen-unstable.git
 SEABIOS_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/seabios.git
 else
 OVMF_UPSTREAM_URL ?= git://xenbits.xen.org/ovmf.git
 QEMU_UPSTREAM_URL ?= git://xenbits.xen.org/qemu-upstream-unstable.git
+QEMU_TRADITIONAL_URL ?= git://xenbits.xen.org/qemu-xen-unstable.git
 SEABIOS_UPSTREAM_URL ?= git://xenbits.xen.org/seabios.git
 endif
 OVMF_UPSTREAM_REVISION ?= 447d264115c476142f884af0be287622cd244423
 QEMU_UPSTREAM_REVISION ?= master
-SEABIOS_UPSTREAM_TAG ?= rel-1.7.5
+SEABIOS_UPSTREAM_REVISION ?= rel-1.7.5
 # Thu May 22 16:59:16 2014 -0400
 # python3 fixes for vgabios and csm builds.
 
 ETHERBOOT_NICS ?= rtl8139 8086100e
 
 
-QEMU_TAG ?= d0395cc49b2ec6d1723c01f1daf2394b9264ca29
+QEMU_TRADITIONAL_REVISION ?= d0395cc49b2ec6d1723c01f1daf2394b9264ca29
 # Tue Apr 8 16:50:06 2014 +0000
 # qemu-xen-trad: free all the pirqs for msi/msix when driver unloads
 
+# Specify which qemu-dm to use. This may be `ioemu' to use the old
+# Mercurial in-tree version, or a local directory, or a git URL.
+# QEMU_UPSTREAM_LOC ?= `pwd`/$(XEN_ROOT)/../qemu-xen.git
+
+# Defaults for subtree locations
+QEMU_TRADITIONAL_LOC ?= $(or $(wildcard $(QEMU_TRADITIONAL_INTREE)),\
+                        $(QEMU_TRADITIONAL_URL))
+
+QEMU_UPSTREAM_LOC ?= $(or $(wildcard $(QEMU_UPSTREAM_INTREE)),\
+                        $(QEMU_UPSTREAM_URL))
+
 # Short answer -- do not enable this unless you know what you are
 # doing and are prepared for some pain.
 
diff --git a/tools/Makefile b/tools/Makefile
index e558e6e..0c4e49c 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -111,35 +111,35 @@ IOEMU_CONFIGURE_CROSS ?= --cpu=$(XEN_TARGET_ARCH) \
 			 --interp-prefix=$(CROSS_SYS_ROOT)
 endif
 
-QEMU_ROOT := $(shell if [ -d "$(CONFIG_QEMU)" ]; then echo "$(CONFIG_QEMU)"; else echo .; fi)
+QEMU_ROOT := $(shell if [ -d "$(QEMU_TRADITIONAL_LOC)" ]; then echo "$(QEMU_TRADITIONAL_LOC)"; else echo .; fi)
 ifneq ($(QEMU_ROOT),.)
 export QEMU_ROOT
 endif
 
 qemu-xen-traditional-dir-find:
 	set -ex; \
-	if test -d $(CONFIG_QEMU); then \
+	if test -d $(QEMU_TRADITIONAL_LOC); then \
 		mkdir -p qemu-xen-traditional-dir; \
 	else \
 		export GIT=$(GIT); \
-		$(XEN_ROOT)/scripts/git-checkout.sh $(CONFIG_QEMU) $(QEMU_TAG) qemu-xen-traditional-dir; \
+		$(XEN_ROOT)/scripts/git-checkout.sh $(QEMU_TRADITIONAL_LOC) $(QEMU_TRADITIONAL_REVISION) qemu-xen-traditional-dir; \
 	fi
 
 qemu-xen-dir-find:
-	if test -d $(QEMU_UPSTREAM_URL) ; then \
+	if test -d $(QEMU_UPSTREAM_LOC) ; then \
 		mkdir -p qemu-xen-dir; \
 	else \
 		export GIT=$(GIT); \
-		$(XEN_ROOT)/scripts/git-checkout.sh $(QEMU_UPSTREAM_URL) $(QEMU_UPSTREAM_REVISION) qemu-xen-dir ; \
+		$(XEN_ROOT)/scripts/git-checkout.sh $(QEMU_UPSTREAM_LOC) $(QEMU_UPSTREAM_REVISION) qemu-xen-dir ; \
 	fi
 
 .PHONY: qemu-xen-traditional-dir-force-update
 qemu-xen-traditional-dir-force-update:
 	set -ex; \
-	if [ "$(QEMU_TAG)" ]; then \
+	if [ "$(QEMU_TRADITIONAL_REVISION)" ]; then \
 		cd qemu-xen-traditional-dir-remote; \
 		$(GIT) fetch origin; \
-		$(GIT) reset --hard $(QEMU_TAG); \
+		$(GIT) reset --hard $(QEMU_TRADITIONAL_REVISION); \
 	fi
 
 subdir-all-qemu-xen-traditional-dir: qemu-xen-traditional-dir-find
@@ -179,8 +179,8 @@ QEMU_XEN_ENABLE_DEBUG :=
 endif
 
 subdir-all-qemu-xen-dir: qemu-xen-dir-find
-	if test -d $(QEMU_UPSTREAM_URL) ; then \
-		source=$(QEMU_UPSTREAM_URL); \
+	if test -d $(QEMU_UPSTREAM_LOC) ; then \
+		source=$(QEMU_UPSTREAM_LOC); \
 	else \
 		source=.; \
 	fi; \
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index 2f1d10d..2c2ed34 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -24,7 +24,7 @@ ovmf-dir:
 	cp ovmf-makefile ovmf-dir/Makefile;
 
 seabios-dir:
-	GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh $(SEABIOS_UPSTREAM_URL) $(SEABIOS_UPSTREAM_TAG) seabios-dir
+	GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh $(SEABIOS_UPSTREAM_URL) $(SEABIOS_UPSTREAM_REVISION) seabios-dir
 	$(MAKE) -C seabios-dir defconfig
 
 .PHONY: all
@@ -78,10 +78,10 @@ subdir-clean-ovmf:
 .PHONY: seabios-dir-force-update
 seabios-dir-force-update:
 	set -ex; \
-	if [ "$(SEABIOS_UPSTREAM_TAG)" ]; then \
+	if [ "$(SEABIOS_UPSTREAM_REVISION)" ]; then \
 		cd seabios-dir-remote; \
 		$(GIT) fetch origin; \
-		$(GIT) reset --hard $(SEABIOS_UPSTREAM_TAG); \
+		$(GIT) reset --hard $(SEABIOS_UPSTREAM_REVISION); \
 	fi
 
 subdir-clean-seabios-dir:
-- 
1.7.9.5

Re: [PATCH 2/5] make: Normalize config options for external trees

[Ian Campbell]

On Mon, 2014-07-14 at 17:15 +0100, George Dunlap wrote:
> We have four different external trees, and four different naming
> conventions for specifying the URL, where they live, and what revision
> to check out.
> 
> Normalize config options on the following bases:
>  - QEMU_UPSTREAM
>  - QEMU_TRADITIONAL
>  - SEABIOS_UPSTREAM
>  - OVMF_UPSTREAM
> 
> The following suffixes for all trees:
>  - _URL : A remote repository to clone from
>  - _REVISION : The revision to check out
> 
> And the following suffixes for the qemu trees:
>  - _INTREE : The location of an inlined tree (for tarball releases)
>  - _LOC : Where to actually look (either a directory or a URL)
> 
> The following parameters are still supported for backwards
> compatibility:
>  - CONFIG_QEMU          (=> QEMU_TRADITIONAL_LOC)
>  - QEMU_REMOTE          (=> QEMU_TRADITIONAL_URL)
>  - QEMU_TAG             (=> QEMU_TRADITIONAL_REVISION)
>  - SEABIOS_UPSTREAM_TAG (=> SEABIOS_UPSTREAM_REVISION)
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> 
> We should relnote these and say that they are deprecated, and may be
> removed in a future release.

Note that you (or someone) would need to make osstest work with/without
this in advance of any removal.

> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> ---
>  Config.mk               |   43 +++++++++++++++++++++++++++----------------
>  tools/Makefile          |   18 +++++++++---------
>  tools/firmware/Makefile |    6 +++---
>  3 files changed, 39 insertions(+), 28 deletions(-)
> 
> diff --git a/Config.mk b/Config.mk
> index a9b3468..5370e23 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -218,49 +218,60 @@ XEN_EXTFILES_URL ?= http://xenbits.xen.org/xen-extfiles
>  # the internet.  The original download URL is preserved as a comment
>  # near the place in the Xen Makefiles where the file is used.
>  
> -ifeq ($(GIT_HTTP),y)
> -QEMU_REMOTE ?= http://xenbits.xen.org/git-http/qemu-xen-unstable.git
> -else
> -QEMU_REMOTE ?= git://xenbits.xen.org/qemu-xen-unstable.git
> -endif
> -
>  # Where to look for inlined subtrees (for example, from a tarball)
>  QEMU_UPSTREAM_INTREE ?= $(XEN_ROOT)/tools/qemu-xen
>  QEMU_TRADITIONAL_INTREE ?= $(XEN_ROOT)/tools/qemu-xen-traditional
>  
> 
> -# Specify which qemu-dm to use. This may be `ioemu' to use the old
> -# Mercurial in-tree version, or a local directory, or a git URL.
> -# CONFIG_QEMU ?= `pwd`/$(XEN_ROOT)/../qemu-xen.git
> -CONFIG_QEMU ?= $(or $(wildcard $(QEMU_TRADITIONAL_INTREE)),\
> -                    $(QEMU_REMOTE))
> -
> -ifneq (,$(wildcard $(QEMU_UPSTREAM_INTREE)))
> -QEMU_UPSTREAM_URL ?= $(QEMU_UPSTREAM_INTREE)
> +# Handle legacy options
> +ifneq (,$(SEABIOS_UPSTREAM_TAG))
> +SEABIOS_UPSTREAM_REVISION ?= $(SEABIOS_UPSTREAM_TAG)
> +endif
> +ifneq (,$(QEMU_REMOTE))
> +QEMU_TRADITIONAL_URL ?= $(QEMU_REMOTE)
> +endif
> +ifneq (,$(CONFIG_QEMU))
> +QEMU_TRADITIONAL_LOC ?= $(CONFIG_QEMU)
> +endif
> +ifneq (,$(QEMU_TAG))
> +QEMU_TRADITIONAL_REVISION ?= $(QEMU_TAG)
>  endif
>  
>  ifeq ($(GIT_HTTP),y)
>  OVMF_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/ovmf.git
>  QEMU_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/qemu-upstream-unstable.git
> +QEMU_TRADITIONAL_URL ?= http://xenbits.xen.org/git-http/qemu-xen-unstable.git
>  SEABIOS_UPSTREAM_URL ?= http://xenbits.xen.org/git-http/seabios.git
>  else
>  OVMF_UPSTREAM_URL ?= git://xenbits.xen.org/ovmf.git
>  QEMU_UPSTREAM_URL ?= git://xenbits.xen.org/qemu-upstream-unstable.git
> +QEMU_TRADITIONAL_URL ?= git://xenbits.xen.org/qemu-xen-unstable.git
>  SEABIOS_UPSTREAM_URL ?= git://xenbits.xen.org/seabios.git
>  endif
>  OVMF_UPSTREAM_REVISION ?= 447d264115c476142f884af0be287622cd244423
>  QEMU_UPSTREAM_REVISION ?= master
> -SEABIOS_UPSTREAM_TAG ?= rel-1.7.5
> +SEABIOS_UPSTREAM_REVISION ?= rel-1.7.5
>  # Thu May 22 16:59:16 2014 -0400
>  # python3 fixes for vgabios and csm builds.
>  
>  ETHERBOOT_NICS ?= rtl8139 8086100e
>  
> 
> -QEMU_TAG ?= d0395cc49b2ec6d1723c01f1daf2394b9264ca29
> +QEMU_TRADITIONAL_REVISION ?= d0395cc49b2ec6d1723c01f1daf2394b9264ca29
>  # Tue Apr 8 16:50:06 2014 +0000
>  # qemu-xen-trad: free all the pirqs for msi/msix when driver unloads
>  
> +# Specify which qemu-dm to use. This may be `ioemu' to use the old
> +# Mercurial in-tree version, or a local directory, or a git URL.
> +# QEMU_UPSTREAM_LOC ?= `pwd`/$(XEN_ROOT)/../qemu-xen.git
> +
> +# Defaults for subtree locations
> +QEMU_TRADITIONAL_LOC ?= $(or $(wildcard $(QEMU_TRADITIONAL_INTREE)),\
> +                        $(QEMU_TRADITIONAL_URL))
> +
> +QEMU_UPSTREAM_LOC ?= $(or $(wildcard $(QEMU_UPSTREAM_INTREE)),\
> +                        $(QEMU_UPSTREAM_URL))
> +
>  # Short answer -- do not enable this unless you know what you are
>  # doing and are prepared for some pain.
>  
> diff --git a/tools/Makefile b/tools/Makefile
> index e558e6e..0c4e49c 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -111,35 +111,35 @@ IOEMU_CONFIGURE_CROSS ?= --cpu=$(XEN_TARGET_ARCH) \
>  			 --interp-prefix=$(CROSS_SYS_ROOT)
>  endif
>  
> -QEMU_ROOT := $(shell if [ -d "$(CONFIG_QEMU)" ]; then echo "$(CONFIG_QEMU)"; else echo .; fi)
> +QEMU_ROOT := $(shell if [ -d "$(QEMU_TRADITIONAL_LOC)" ]; then echo "$(QEMU_TRADITIONAL_LOC)"; else echo .; fi)
>  ifneq ($(QEMU_ROOT),.)
>  export QEMU_ROOT
>  endif
>  
>  qemu-xen-traditional-dir-find:
>  	set -ex; \
> -	if test -d $(CONFIG_QEMU); then \
> +	if test -d $(QEMU_TRADITIONAL_LOC); then \
>  		mkdir -p qemu-xen-traditional-dir; \
>  	else \
>  		export GIT=$(GIT); \
> -		$(XEN_ROOT)/scripts/git-checkout.sh $(CONFIG_QEMU) $(QEMU_TAG) qemu-xen-traditional-dir; \
> +		$(XEN_ROOT)/scripts/git-checkout.sh $(QEMU_TRADITIONAL_LOC) $(QEMU_TRADITIONAL_REVISION) qemu-xen-traditional-dir; \
>  	fi
>  
>  qemu-xen-dir-find:
> -	if test -d $(QEMU_UPSTREAM_URL) ; then \
> +	if test -d $(QEMU_UPSTREAM_LOC) ; then \
>  		mkdir -p qemu-xen-dir; \
>  	else \
>  		export GIT=$(GIT); \
> -		$(XEN_ROOT)/scripts/git-checkout.sh $(QEMU_UPSTREAM_URL) $(QEMU_UPSTREAM_REVISION) qemu-xen-dir ; \
> +		$(XEN_ROOT)/scripts/git-checkout.sh $(QEMU_UPSTREAM_LOC) $(QEMU_UPSTREAM_REVISION) qemu-xen-dir ; \
>  	fi
>  
>  .PHONY: qemu-xen-traditional-dir-force-update
>  qemu-xen-traditional-dir-force-update:
>  	set -ex; \
> -	if [ "$(QEMU_TAG)" ]; then \
> +	if [ "$(QEMU_TRADITIONAL_REVISION)" ]; then \
>  		cd qemu-xen-traditional-dir-remote; \
>  		$(GIT) fetch origin; \
> -		$(GIT) reset --hard $(QEMU_TAG); \
> +		$(GIT) reset --hard $(QEMU_TRADITIONAL_REVISION); \
>  	fi
>  
>  subdir-all-qemu-xen-traditional-dir: qemu-xen-traditional-dir-find
> @@ -179,8 +179,8 @@ QEMU_XEN_ENABLE_DEBUG :=
>  endif
>  
>  subdir-all-qemu-xen-dir: qemu-xen-dir-find
> -	if test -d $(QEMU_UPSTREAM_URL) ; then \
> -		source=$(QEMU_UPSTREAM_URL); \
> +	if test -d $(QEMU_UPSTREAM_LOC) ; then \
> +		source=$(QEMU_UPSTREAM_LOC); \
>  	else \
>  		source=.; \
>  	fi; \
> diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
> index 2f1d10d..2c2ed34 100644
> --- a/tools/firmware/Makefile
> +++ b/tools/firmware/Makefile
> @@ -24,7 +24,7 @@ ovmf-dir:
>  	cp ovmf-makefile ovmf-dir/Makefile;
>  
>  seabios-dir:
> -	GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh $(SEABIOS_UPSTREAM_URL) $(SEABIOS_UPSTREAM_TAG) seabios-dir
> +	GIT=$(GIT) $(XEN_ROOT)/scripts/git-checkout.sh $(SEABIOS_UPSTREAM_URL) $(SEABIOS_UPSTREAM_REVISION) seabios-dir
>  	$(MAKE) -C seabios-dir defconfig
>  
>  .PHONY: all
> @@ -78,10 +78,10 @@ subdir-clean-ovmf:
>  .PHONY: seabios-dir-force-update
>  seabios-dir-force-update:
>  	set -ex; \
> -	if [ "$(SEABIOS_UPSTREAM_TAG)" ]; then \
> +	if [ "$(SEABIOS_UPSTREAM_REVISION)" ]; then \
>  		cd seabios-dir-remote; \
>  		$(GIT) fetch origin; \
> -		$(GIT) reset --hard $(SEABIOS_UPSTREAM_TAG); \
> +		$(GIT) reset --hard $(SEABIOS_UPSTREAM_REVISION); \
>  	fi
>  
>  subdir-clean-seabios-dir:

Re: [PATCH 2/5] make: Normalize config options for external trees

[Ian Jackson]

George Dunlap writes ("[PATCH 2/5] make: Normalize config options for external trees"):
> We have four different external trees, and four different naming
> conventions for specifying the URL, where they live, and what revision
> to check out.
...
> +# Specify which qemu-dm to use. This may be `ioemu' to use the old
> +# Mercurial in-tree version, or a local directory, or a git URL.
> +# QEMU_UPSTREAM_LOC ?= `pwd`/$(XEN_ROOT)/../qemu-xen.git

This comment about `the old Mercurial in-tree version' is years and
years out of date.  I appreciate that this is only showing up as a `+'
because it's being moved, so:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

Re: [PATCH 2/5] make: Normalize config options for external trees

[Olaf Hering]

On Mon, Jul 14, George Dunlap wrote:

>  - CONFIG_QEMU          (=> QEMU_TRADITIONAL_LOC)
>  - QEMU_REMOTE          (=> QEMU_TRADITIONAL_URL)
>  - QEMU_TAG             (=> QEMU_TRADITIONAL_REVISION)
>  - SEABIOS_UPSTREAM_TAG (=> SEABIOS_UPSTREAM_REVISION)

This broke my script, which I fixed just now.

And I think the patch missed ipxe:
tools/firmware/etherboot/Makefile:IPXE_GIT_TAG := 9a93db3f0947484e30e753bbd61a10b17336e20e


Olaf

Re: [PATCH 2/5] make: Normalize config options for external trees

[George Dunlap]

On 07/21/2014 11:22 AM, Olaf Hering wrote:
> On Mon, Jul 14, George Dunlap wrote:
>
>>   - CONFIG_QEMU          (=> QEMU_TRADITIONAL_LOC)
>>   - QEMU_REMOTE          (=> QEMU_TRADITIONAL_URL)
>>   - QEMU_TAG             (=> QEMU_TRADITIONAL_REVISION)
>>   - SEABIOS_UPSTREAM_TAG (=> SEABIOS_UPSTREAM_REVISION)
> This broke my script, which I fixed just now.

How did it break your script?  The intent was that it would be 
backwards-compatible enough not to break anyone's scripts...

> And I think the patch missed ipxe:
> tools/firmware/etherboot/Makefile:IPXE_GIT_TAG := 9a93db3f0947484e30e753bbd61a10b17336e20e

Hmm, so this should probably be moved into Config.mk.

  -George

Re: [PATCH 2/5] make: Normalize config options for external trees

[Olaf Hering]

On Mon, Jul 21, George Dunlap wrote:

> On 07/21/2014 11:22 AM, Olaf Hering wrote:
> >On Mon, Jul 14, George Dunlap wrote:
> >
> >>  - CONFIG_QEMU          (=> QEMU_TRADITIONAL_LOC)
> >>  - QEMU_REMOTE          (=> QEMU_TRADITIONAL_URL)
> >>  - QEMU_TAG             (=> QEMU_TRADITIONAL_REVISION)
> >>  - SEABIOS_UPSTREAM_TAG (=> SEABIOS_UPSTREAM_REVISION)
> >This broke my script, which I fixed just now.
> 
> How did it break your script?  The intent was that it would be
> backwards-compatible enough not to break anyone's scripts...

To export the tree(s) I have to parse the given revisions. I changed my
script like that (skip first QEMU_TRADITIONAL_REVISION):

--- xen-unstable-export.sh (revision 1090)
+++ xen-unstable-export.sh (revision 1091)
@@ -116,9 +116,9 @@
 then
        exit 1
 fi
-qemu_xen_traditional_tag=`sed -n '/^QEMU_TAG/{s@^.* @@;p;q}' Config.mk`
+qemu_xen_traditional_tag=` sed -n '/^QEMU_TRADITIONAL_REVISION/{/QEMU_TAG/d;s@^.* @@;p;q}' Config.mk `
 qemu_xen_upstream_tag=`sed -n '/^QEMU_UPSTREAM_REVISION/{s@^.* @@;p;q}' Config.mk`


I was thinking of a parsable 'make showrevisions' target, but I think thats
overdoing things.

Olaf

Re: [PATCH 2/5] make: Normalize config options for external trees

[George Dunlap]

On 07/21/2014 11:44 AM, Olaf Hering wrote:
> On Mon, Jul 21, George Dunlap wrote:
>
>> On 07/21/2014 11:22 AM, Olaf Hering wrote:
>>> On Mon, Jul 14, George Dunlap wrote:
>>>
>>>>   - CONFIG_QEMU          (=> QEMU_TRADITIONAL_LOC)
>>>>   - QEMU_REMOTE          (=> QEMU_TRADITIONAL_URL)
>>>>   - QEMU_TAG             (=> QEMU_TRADITIONAL_REVISION)
>>>>   - SEABIOS_UPSTREAM_TAG (=> SEABIOS_UPSTREAM_REVISION)
>>> This broke my script, which I fixed just now.
>> How did it break your script?  The intent was that it would be
>> backwards-compatible enough not to break anyone's scripts...
> To export the tree(s) I have to parse the given revisions. I changed my
> script like that (skip first QEMU_TRADITIONAL_REVISION):
>
> --- xen-unstable-export.sh (revision 1090)
> +++ xen-unstable-export.sh (revision 1091)
> @@ -116,9 +116,9 @@
>   then
>          exit 1
>   fi
> -qemu_xen_traditional_tag=`sed -n '/^QEMU_TAG/{s@^.* @@;p;q}' Config.mk`
> +qemu_xen_traditional_tag=` sed -n '/^QEMU_TRADITIONAL_REVISION/{/QEMU_TAG/d;s@^.* @@;p;q}' Config.mk `
>   qemu_xen_upstream_tag=`sed -n '/^QEMU_UPSTREAM_REVISION/{s@^.* @@;p;q}' Config.mk`
>
>
> I was thinking of a parsable 'make showrevisions' target, but I think thats
> overdoing things.

Oh, right -- yeah, I was focusing more on *specifying* the value, rather 
than *reporting* the value.  I don't see how I could work around someone 
reading the config code itself, at least off the top of my head.

  -George

Re: [PATCH 2/5] make: Normalize config options for external trees

[Olaf Hering]

On Mon, Jul 21, George Dunlap wrote:

> >tools/firmware/etherboot/Makefile:IPXE_GIT_TAG := 9a93db3f0947484e30e753bbd61a10b17336e20e
> 
> Hmm, so this should probably be moved into Config.mk.

Just an idea:
Maybe that (and I think also seabios) could deserve more attention.
Right now a 'git clean -dfx' leaves the qemu trees around, but things
like ipxe will be downloaded again. Maybe ipxe should be cloned just
like qemu* so it survives a 'wipe everything'?


Olaf

[PATCH 3/5] make: Make *-dir-force-update depend on *-dir-find

[George Dunlap]

Make the targets depend on the "-find" targets, so that if the
subtrees are cloned if they haven't been cloned already.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
---
 tools/Makefile          |    4 ++--
 tools/firmware/Makefile |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/Makefile b/tools/Makefile
index 0c4e49c..b6476c9 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -134,7 +134,7 @@ qemu-xen-dir-find:
 	fi
 
 .PHONY: qemu-xen-traditional-dir-force-update
-qemu-xen-traditional-dir-force-update:
+qemu-xen-traditional-dir-force-update: qemu-xen-traditional-dir-find
 	set -ex; \
 	if [ "$(QEMU_TRADITIONAL_REVISION)" ]; then \
 		cd qemu-xen-traditional-dir-remote; \
@@ -164,7 +164,7 @@ subdir-clean-qemu-xen-traditional-dir:
 	fi
 
 .PHONY: qemu-xen-dir-force-update
-qemu-xen-dir-force-update:
+qemu-xen-dir-force-update: qemu-xen-dir-find
 	set -ex; \
 	if [ "$(QEMU_UPSTREAM_REVISION)" ]; then \
 		cd qemu-xen-dir-remote; \
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index 2c2ed34..660bacb 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -62,7 +62,7 @@ subdir-distclean-seabios-dir: .phony
 	rm -rf seabios-dir seabios-dir-remote
 
 .PHONY: ovmf-dir-force-update
-ovmf-dir-force-update:
+ovmf-dir-force-update: ovmf-dir
 	set -ex; \
 	if [ "$(OVMF_UPSTREAM_REVISION)" ]; then \
 		cd ovmf-dir-remote; \
@@ -76,7 +76,7 @@ subdir-clean-ovmf:
 	fi
 
 .PHONY: seabios-dir-force-update
-seabios-dir-force-update:
+seabios-dir-force-update: seabios-dir
 	set -ex; \
 	if [ "$(SEABIOS_UPSTREAM_REVISION)" ]; then \
 		cd seabios-dir-remote; \
-- 
1.7.9.5

Re: [PATCH 3/5] make: Make *-dir-force-update depend on *-dir-find

[Ian Jackson]

George Dunlap writes ("[PATCH 3/5] make: Make *-dir-force-update depend on *-dir-find"):
> Make the targets depend on the "-find" targets, so that if the
> subtrees are cloned if they haven't been cloned already.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH 4/5] make: Add subtree-update target

[George Dunlap]

subtree-update will update all subtrees according to the current TAG specified
in Config.mk.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
---
 Makefile                |    4 ++++
 tools/Makefile          |    9 +++++++++
 tools/firmware/Makefile |    8 ++++++++
 3 files changed, 21 insertions(+)

diff --git a/Makefile b/Makefile
index 41dabbf..b804bcf 100644
--- a/Makefile
+++ b/Makefile
@@ -135,6 +135,10 @@ debball: dist
 rpmball: dist
 	bash ./tools/misc/mkrpm $(XEN_ROOT) $$($(MAKE) -C xen xenversion --no-print-directory)
 
+.PHONY: subtree-update
+subtree-update:
+	$(MAKE) -C tools subtree-update
+
 .PHONY: clean
 clean::
 	$(MAKE) -C xen clean
diff --git a/tools/Makefile b/tools/Makefile
index b6476c9..8725c59 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -240,3 +240,12 @@ subdir-all-debugger/kdd: .phony
 
 subdir-distclean-firmware: .phony
 	$(MAKE) -C firmware distclean
+
+subtree-update:
+ifeq ($(CONFIG_QEMU_XEN),y)
+	$(MAKE) qemu-xen-dir-force-update
+endif
+ifeq ($(CONFIG_QEMU_TRAD),y)
+	$(MAKE) qemu-xen-traditional-dir-force-update
+endif
+	$(MAKE) -C firmware subtree-update
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index 660bacb..377a8cf 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -88,3 +88,11 @@ subdir-clean-seabios-dir:
 	set -e; if test -d seabios-dir/.; then \
 		$(MAKE) -C seabios-dir clean; \
 	fi
+
+subtree-update:
+ifeq ($(CONFIG_SEABIOS),y)
+	$(MAKE) seabios-dir-force-update
+endif
+ifeq ($(CONFIG_OVMF),y)
+	$(MAKE) ovmf-dir-force-update
+endif
-- 
1.7.9.5

Re: [PATCH 4/5] make: Add subtree-update target

[Ian Campbell]

On Mon, 2014-07-14 at 17:15 +0100, George Dunlap wrote:
> subtree-update will update all subtrees according to the current TAG specified
> in Config.mk.

I've often wanted this, thanks!

Due to the previous it'll also clone/etc all the trees for you if they
don't exist, won't it? People may well find that quite useful too.

> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> ---
>  Makefile                |    4 ++++
>  tools/Makefile          |    9 +++++++++
>  tools/firmware/Makefile |    8 ++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 41dabbf..b804bcf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -135,6 +135,10 @@ debball: dist
>  rpmball: dist
>  	bash ./tools/misc/mkrpm $(XEN_ROOT) $$($(MAKE) -C xen xenversion --no-print-directory)
>  
> +.PHONY: subtree-update
> +subtree-update:
> +	$(MAKE) -C tools subtree-update
> +
>  .PHONY: clean
>  clean::
>  	$(MAKE) -C xen clean
> diff --git a/tools/Makefile b/tools/Makefile
> index b6476c9..8725c59 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -240,3 +240,12 @@ subdir-all-debugger/kdd: .phony
>  
>  subdir-distclean-firmware: .phony
>  	$(MAKE) -C firmware distclean
> +
> +subtree-update:
> +ifeq ($(CONFIG_QEMU_XEN),y)
> +	$(MAKE) qemu-xen-dir-force-update
> +endif
> +ifeq ($(CONFIG_QEMU_TRAD),y)
> +	$(MAKE) qemu-xen-traditional-dir-force-update
> +endif
> +	$(MAKE) -C firmware subtree-update
> diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
> index 660bacb..377a8cf 100644
> --- a/tools/firmware/Makefile
> +++ b/tools/firmware/Makefile
> @@ -88,3 +88,11 @@ subdir-clean-seabios-dir:
>  	set -e; if test -d seabios-dir/.; then \
>  		$(MAKE) -C seabios-dir clean; \
>  	fi
> +
> +subtree-update:
> +ifeq ($(CONFIG_SEABIOS),y)
> +	$(MAKE) seabios-dir-force-update
> +endif
> +ifeq ($(CONFIG_OVMF),y)
> +	$(MAKE) ovmf-dir-force-update
> +endif

Re: [PATCH 4/5] make: Add subtree-update target

[Ian Jackson]

George Dunlap writes ("[PATCH 4/5] make: Add subtree-update target"):
> subtree-update will update all subtrees according to the current TAG
> specified in Config.mk.

Maybe it should have the word "force" in the new target name, since it
runs a bunch of specific force targets ?

Ian.

[PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[George Dunlap]

At the moment, making a release tarball is an annoyingly manual
process that involves running "git archive" into a temporary directory.

Script this process up and make a target, so that the release manager
can simply type "make src-tarball-release" and have everything show up
nice and neat in dist/xen-$version.tar.gz.  "make src-tarball" will
make a version number based on git describe, which will typically have
the most recent tag, number of commits since that tag, and the git
commit id of the current HEAD.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

---

v2:
* Put quotes around assignments with variables in them
* Avoid pushd/popd
* Get rid of unnecessary error checks (since set -e will take care of it)
* Make a function to do the git archive
* Add "src-tarball-release" to do a clean version (e.g., 4.4.1)
* "src-tarball" will name it based on "git describe" (includes a short commit hash).
* Add mkdir -p $xen_root/dist/ to the script so that it works on a freshly cloned tree

CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Anthony Perard <anthony.perard@citrix.com>
---
 Makefile             |   27 ++++++++++++++++++++-------
 tools/misc/mktarball |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 7 deletions(-)
 create mode 100755 tools/misc/mktarball

diff --git a/Makefile b/Makefile
index b804bcf..28fecab 100644
--- a/Makefile
+++ b/Makefile
@@ -139,6 +139,23 @@ rpmball: dist
 subtree-update:
 	$(MAKE) -C tools subtree-update
 
+# Make a source tarball, including qemu sub-trees.
+#
+# src-tarball will use "git describe" for the version number.  This
+# will have the most recent tag, number of commits since that tag, and
+# git commit id of the head.  This is suitable for a "snapshot"
+# tarball of an unreleased tree.
+#
+# src-tarball-release will use "make xenversion" as the version
+# number.  This is suitable for release tarballs.
+.PHONY: src-tarball-release
+src-tarball-release: subtree-update
+	bash ./tools/misc/mktarball $(XEN_ROOT) $$($(MAKE) -C xen xenversion --no-print-directory)
+
+.PHONY: src-tarball
+src-tarball: subtree-update
+	bash ./tools/misc/mktarball $(XEN_ROOT) $$(git describe)
+
 .PHONY: clean
 clean::
 	$(MAKE) -C xen clean
@@ -167,13 +184,6 @@ endif
 .PHONY: mrproper
 mrproper: distclean
 
-# Prepare for source tarball
-.PHONY: src-tarball
-src-tarball: distclean
-	$(MAKE) -C xen .banner
-	rm -rf xen/tools/figlet .[a-z]*
-	$(MAKE) -C xen distclean
-
 .PHONY: help
 help:
 	@echo 'Installation targets:'
@@ -206,6 +216,9 @@ help:
 	@echo '  install-tboot    - download, build, and install the tboot module'
 	@echo '  clean-tboot      - clean the tboot module if it exists'
 	@echo
+	@echo 'Tarball targets:'
+	@echo '  src-tarball      - make a source tarball with xen and qemu suitable for a release'
+	@echo
 	@echo 'Environment:'
 	@echo '  [ this documentation is sadly not complete ]'
 
diff --git a/tools/misc/mktarball b/tools/misc/mktarball
new file mode 100755
index 0000000..06102eb
--- /dev/null
+++ b/tools/misc/mktarball
@@ -0,0 +1,42 @@
+#!/bin/bash
+#
+# mktarball: Make a release tarball (including xen, qemu, and qemu-traditional)
+#
+# Takes 2 arguments, the path to the dist directory and the version
+
+set -e
+
+function finish {
+    [[ -n "$tdir" ]] && rm -rf $tdir
+}
+trap finish EXIT
+
+function git_archive_into {
+    mkdir "$2"
+
+    git --git-dir="$1"/.git \
+	archive --format=tar HEAD | \
+	tar Cxf "$2" - 
+}
+
+if [[ -z "$1" || -z "$2" ]] ; then
+  echo "usage: $0 path-to-XEN_ROOT xen-version"
+  exit 1
+fi
+
+xen_root="$1"
+desc="$2"
+
+mkdir -p $xen_root/dist/
+
+tdir="$(mktemp -d $xen_root/dist/xen.XXXXXXXX)"
+
+git_archive_into $xen_root $tdir/xen-$desc
+
+git_archive_into $xen_root/tools/qemu-xen-dir-remote $tdir/xen-$desc/tools/qemu-xen
+
+git_archive_into $xen_root/tools/qemu-xen-traditional-dir-remote $tdir/xen-$desc/tools/qemu-xen-traditional
+
+GZIP=-9v tar cz -f $xen_root/dist/xen-$desc.tar.gz -C $tdir xen-$desc
+
+echo "Source tarball in $xen_root/dist/xen-$desc.tar.gz"
\ No newline at end of file
-- 
1.7.9.5

Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[Ian Jackson]

George Dunlap writes ("[PATCH 5/5] make: Make "src-tarball" target actually make a source tarball"):
> At the moment, making a release tarball is an annoyingly manual
> process that involves running "git archive" into a temporary directory.
...
> +src-tarball: subtree-update
> +	bash ./tools/misc/mktarball $(XEN_ROOT) $$(git describe)

Does this absolutely have to depend on subtree-update ?  Perhaps we
need a new "make the subtrees exist" target ?

> diff --git a/tools/misc/mktarball b/tools/misc/mktarball
...
> +function finish {
> +    [[ -n "$tdir" ]] && rm -rf $tdir
> +}

The use of the POSIX syntax
   finish() {
seems much more prevalent in-tree, than the use of `function'.

> +trap finish EXIT

The EXIT trap handler should probably `set +e'.

> +function git_archive_into {
> +    mkdir "$2"
> +
> +    git --git-dir="$1"/.git \
> +	archive --format=tar HEAD | \
> +	tar Cxf "$2" - 
> +}
> +
> +if [[ -z "$1" || -z "$2" ]] ; then
> +  echo "usage: $0 path-to-XEN_ROOT xen-version"
> +  exit 1
> +fi
> +
> +xen_root="$1"
> +desc="$2"
> +
> +mkdir -p $xen_root/dist/
> +
> +tdir="$(mktemp -d $xen_root/dist/xen.XXXXXXXX)"

Why not use a fixed filename ?  This script isn't safe for concurrent
invocation anyway, because the output filename is (mostly) fixed.

If you do that you can do away with the trap handler entirely.  You
should probably make git_archive_into delete the destination
directory.

> +GZIP=-9v tar cz -f $xen_root/dist/xen-$desc.tar.gz -C $tdir xen-$desc
> +
> +echo "Source tarball in $xen_root/dist/xen-$desc.tar.gz"
> \ No newline at end of file

No newline at end of file.

Thanks,
Ian.

Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[Ian Campbell]

On Wed, 2014-07-16 at 16:06 +0100, Ian Jackson wrote:
> George Dunlap writes ("[PATCH 5/5] make: Make "src-tarball" target actually make a source tarball"):
> > At the moment, making a release tarball is an annoyingly manual
> > process that involves running "git archive" into a temporary directory.
> ...
> > +src-tarball: subtree-update
> > +	bash ./tools/misc/mktarball $(XEN_ROOT) $$(git describe)
> 
> Does this absolutely have to depend on subtree-update ?  Perhaps we
> need a new "make the subtrees exist" target ?

Given that he makes the force targets depend on the dir targets in one
of the earlier patches I think subtree-update might already be that
target...

Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[George Dunlap]

On 07/16/2014 05:56 PM, Ian Campbell wrote:
> On Wed, 2014-07-16 at 16:06 +0100, Ian Jackson wrote:
>> George Dunlap writes ("[PATCH 5/5] make: Make "src-tarball" target actually make a source tarball"):
>>> At the moment, making a release tarball is an annoyingly manual
>>> process that involves running "git archive" into a temporary directory.
>> ...
>>> +src-tarball: subtree-update
>>> +	bash ./tools/misc/mktarball $(XEN_ROOT) $$(git describe)
>>
>> Does this absolutely have to depend on subtree-update ?  Perhaps we
>> need a new "make the subtrees exist" target ?
>
> Given that he makes the force targets depend on the dir targets in one
> of the earlier patches I think subtree-update might already be that
> target...

I think he might have been thinking that it would be nice to be able to 
make a tarball without doing the force update -- i.e., if you had 
checked out a particular version of one of the trees, just use that; but 
if you hadn't checked anything out, go fetch the tree.

I could do that. :-)  It means SOP would have to be two steps "make 
subdir-update ; make src-tarball-release".  Personally I think it's a 
bit of a trap waiting to happen, but I can see the logic behind it.

  -George

Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[Ian Campbell]

On Wed, 2014-07-16 at 18:45 +0100, George Dunlap wrote:
> On 07/16/2014 05:56 PM, Ian Campbell wrote:
> > On Wed, 2014-07-16 at 16:06 +0100, Ian Jackson wrote:
> >> George Dunlap writes ("[PATCH 5/5] make: Make "src-tarball" target actually make a source tarball"):
> >>> At the moment, making a release tarball is an annoyingly manual
> >>> process that involves running "git archive" into a temporary directory.
> >> ...
> >>> +src-tarball: subtree-update
> >>> +	bash ./tools/misc/mktarball $(XEN_ROOT) $$(git describe)
> >>
> >> Does this absolutely have to depend on subtree-update ?  Perhaps we
> >> need a new "make the subtrees exist" target ?
> >
> > Given that he makes the force targets depend on the dir targets in one
> > of the earlier patches I think subtree-update might already be that
> > target...
> 
> I think he might have been thinking that it would be nice to be able to 
> make a tarball without doing the force update -- i.e., if you had 
> checked out a particular version of one of the trees, just use that; but 
> if you hadn't checked anything out, go fetch the tree.

> 
> I could do that. :-)  It means SOP would have to be two steps "make 
> subdir-update ; make src-tarball-release".  Personally I think it's a 
> bit of a trap waiting to happen, but I can see the logic behind it.

It's a bit niche perhaps, so perhaps make it so you can do
	make src-tarball-release FORCE_CURRENT_SUBTREES=1
by conditionalising the dep?

Ian.

Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[George Dunlap]

On 07/16/2014 04:06 PM, Ian Jackson wrote:
> George Dunlap writes ("[PATCH 5/5] make: Make "src-tarball" target actually make a source tarball"):
>> At the moment, making a release tarball is an annoyingly manual
>> process that involves running "git archive" into a temporary directory.
> ...
>> +src-tarball: subtree-update
>> +	bash ./tools/misc/mktarball $(XEN_ROOT) $$(git describe)
> Does this absolutely have to depend on subtree-update ?  Perhaps we
> need a new "make the subtrees exist" target ?

I'll add a "subtree-find" command (perhaps renaming the other to 
"subtree-force-update"; then as per IanC's suggestion, I'll make this 
depend on subtree-force-update unless some sensible parameter is set.

>
>> diff --git a/tools/misc/mktarball b/tools/misc/mktarball
> ...
>> +function finish {
>> +    [[ -n "$tdir" ]] && rm -rf $tdir
>> +}
> The use of the POSIX syntax
>     finish() {
> seems much more prevalent in-tree, than the use of `function'.

Ack.

>
>> +trap finish EXIT
> The EXIT trap handler should probably `set +e'.

Ack.

>
>> +function git_archive_into {
>> +    mkdir "$2"
>> +
>> +    git --git-dir="$1"/.git \
>> +	archive --format=tar HEAD | \
>> +	tar Cxf "$2" -
>> +}
>> +
>> +if [[ -z "$1" || -z "$2" ]] ; then
>> +  echo "usage: $0 path-to-XEN_ROOT xen-version"
>> +  exit 1
>> +fi
>> +
>> +xen_root="$1"
>> +desc="$2"
>> +
>> +mkdir -p $xen_root/dist/
>> +
>> +tdir="$(mktemp -d $xen_root/dist/xen.XXXXXXXX)"
> Why not use a fixed filename ?  This script isn't safe for concurrent
> invocation anyway, because the output filename is (mostly) fixed.

I'm not so much worried about concurrent invocation per se, but about 
dealing with stuff possibly left over from a previous run.  If we use a 
fixed path, we have to always rm -rf the path before starting, which 
seems unnecessarily risky to me.  (Or we could not rm -rf the path, 
which is even worse from a consistency point of view.)  Creating a new 
temporary directory just seems like the cleanest, safest thing to do.  
It's not like it's that expensive.

(Re concurrent invocation: The output filename may collide, but at least 
you'll get a consistent snapshot of that one particular tree. What I'd 
be more worried about is racing with make subdir-force-update and 
getting inconsisent subtree archives.)

>
> If you do that you can do away with the trap handler entirely.  You
> should probably make git_archive_into delete the destination
> directory.

I'm not sure what the fixed filename has to do with cleanup -- I would 
want to remove the temporary directory on a failure regardless.

>
>> +GZIP=-9v tar cz -f $xen_root/dist/xen-$desc.tar.gz -C $tdir xen-$desc
>> +
>> +echo "Source tarball in $xen_root/dist/xen-$desc.tar.gz"
>> \ No newline at end of file
> No newline at end of file.

Ack.

  -George

Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[Ian Jackson]

George Dunlap writes ("Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball"):
> On 07/16/2014 04:06 PM, Ian Jackson wrote:
> > Why not use a fixed filename ?  This script isn't safe for concurrent
> > invocation anyway, because the output filename is (mostly) fixed.

Basically, I'm advocating that this script should be crash-only.[1]

> I'm not so much worried about concurrent invocation per se, but about 
> dealing with stuff possibly left over from a previous run.  If we use a 
> fixed path, we have to always rm -rf the path before starting, which 
> seems unnecessarily risky to me.  (Or we could not rm -rf the path, 
> which is even worse from a consistency point of view.)  Creating a new 
> temporary directory just seems like the cleanest, safest thing to do.  
> It's not like it's that expensive.

The temporary directory should have a name with "tmp" in it somewhere.
Running rm -rf on such a thing is not risky at all.

> > If you do that you can do away with the trap handler entirely.  You
> > should probably make git_archive_into delete the destination
> > directory.
> 
> I'm not sure what the fixed filename has to do with cleanup -- I would 
> want to remove the temporary directory on a failure regardless.

Why would you want to do that ?  Why not leave it for "make clean" or
the next run ?  Obviously it would be in .gitignore.

Build systems routinely leave temporary files and directories lying
around (on failure, or otherwise).  This is at the very least normal
and harmless.  Much of the time it is helpful because it can make it
easier to debug the build system.

> (Re concurrent invocation: The output filename may collide, but at least 
> you'll get a consistent snapshot of that one particular tree. What I'd 
> be more worried about is racing with make subdir-force-update and 
> getting inconsisent subtree archives.)

My point was simply that because the script cannot safely be run
concurrently with the same output filename, we can disregard worries
about concurrent execution as a reason for using a varying filename.
It is sufficient use a temporary filename derived from the output
filename.


[1] https://en.wikipedia.org/wiki/Crash-only_software
https://www.usenix.org/conference/hotos-ix/crash-only-software

This is a generally good software engineering principle which we
should conform to as much as possible.  The main argument in favour
goes like this:

In general it is not possible to reliably execute cleanup code at the
time of failure.  On a Unix system processes (or whole systems) may
just die.  For example due to being eaten by the OOM killer, or being
sent untrappable signals by administrators or users trying to recover
from failures (e.g. wedged network storage).  They may also lose
access to the filesystem in ways that mimic the effect of
instantaneous death.

That means that there must be mechanisms which can recover from
abnormal (dirty) termination.  Any missed cleanup must be doable - and
done - later.  So if the system is properly designed and implemented,
any missed cleanup will be sorted out by other abnormal state recovery
mechanisms (either as part of the next invocation, or in a
housekeeping task which runs regardless).

Given that such recovery mechanisms must exist and work, attempts to
do a "tidy crash" are unnecessary.  It is much simpler to simply omit
any code which attempts to do cleanup as a response to failure, and
leave recovery to the main single recovery path.

This is also much more reliable: exercising recovery becomes a routine
part of startup (or housekeeping) and is done after both routine
failures and successful executions.  It is therefore well tested.

Contrast "tidy crash" mechanisms which are complicated, difficult to
test, and rarely tested at all let alone fed the complete panoply of
different crash cases - and are then combined with hardly-ever-tested
(or even defective-by-design) recovery arrangements.


Thanks,
Ian.

Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[George Dunlap]

On Thu, Jul 17, 2014 at 5:46 PM, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> On 07/16/2014 04:06 PM, Ian Jackson wrote:
>>
>> George Dunlap writes ("[PATCH 5/5] make: Make "src-tarball" target
>> actually make a source tarball"):
>>>
>>> At the moment, making a release tarball is an annoyingly manual
>>> process that involves running "git archive" into a temporary directory.
>>
>> ...
>>>
>>> +src-tarball: subtree-update
>>> +       bash ./tools/misc/mktarball $(XEN_ROOT) $$(git describe)
>>
>> Does this absolutely have to depend on subtree-update ?  Perhaps we
>> need a new "make the subtrees exist" target ?
>
>
> I'll add a "subtree-find" command (perhaps renaming the other to
> "subtree-force-update"; then as per IanC's suggestion, I'll make this depend
> on subtree-force-update unless some sensible parameter is set.

Ian J,

I'm not sure if you remember much about this discussion from two
months ago; I've just realized that this patch series never quite made
it in, and am going back addressing things.

The original patch series will automatically call
"subtree-force-update", which will call force-update on all configured
subtrees (qemu-xen, qemu-trad, seabios, ovmf).

It sounded from your comment above like you'd like to be able to make
a version of src-tarball which will call "*-dir-find" to *instantiate*
the directory if it's not there, but will leave it alone if it is
there.

I started trying to implement a "subtree-find" command which would
call *-dir-find on all the subtrees; but it turns out to be a bit more
complicated because tools/firmware/Makefile doesn't have nice clean
"*-find-dir" targets for ovmf and seabios like qemu does.  Instead, it
has targets which will call git-checkout, and then do some kind of
configuration.  Making it do the configuration in the right place will
probably not be a simple change.

(In fact, technically ovmf-dir-force-update and
seabios-dir-force-update may be broken at the moment, because they
don't do the post-checkout update things.)

Which leaves me three options:
1. Do the work to add "clean" ovmf-find-dir and seabios-find-dir targets
2. Make "subtree-find" just call the current targets, which will stay
on the same
3. Not implement subtree-find at the moment, but just make src-tarball
depend only on subtree-force-update.

#2 I think is a bad idea, because it gives you the impression that
it's being nice and not messing with your config, when in fact it is.

Given that #1 will take some work, and how close we are to the code
freeze, and that it's just an "extra option" over the force-update
default, I propose going with #3.

Does that sound reasonable?

 -George

Re: [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball

[Ian Jackson]

George Dunlap writes ("Re: [Xen-devel] [PATCH 5/5] make: Make "src-tarball" target actually make a source tarball"):
> [discussion]

How annoying.

> Which leaves me three options:
> 1. Do the work to add "clean" ovmf-find-dir and seabios-find-dir targets
> 2. Make "subtree-find" just call the current targets, which will stay
> on the same
> 3. Not implement subtree-find at the moment, but just make src-tarball
> depend only on subtree-force-update.
> 
> #2 I think is a bad idea, because it gives you the impression that
> it's being nice and not messing with your config, when in fact it is.
> 
> Given that #1 will take some work, and how close we are to the code
> freeze, and that it's just an "extra option" over the force-update
> default, I propose going with #3.
> 
> Does that sound reasonable?

Yes.

Thanks,
Ian.

Re: [PATCH 1/5] make: Check tools/qemu-xen[-traditional] for qemu before downloading

[Ian Campbell]

On Mon, 2014-07-14 at 17:15 +0100, George Dunlap wrote:

This series looks good to me, but since it mostly seems to involve
automating release clart I'll leave it to Ian J.

Ian.

Re: [PATCH 1/5] make: Check tools/qemu-xen[-traditional] for qemu before downloading

[Ian Jackson]

George Dunlap writes ("[PATCH 1/5] make: Check tools/qemu-xen[-traditional] for qemu before downloading"):
> Currently xen, qemu-xen, and qemu-xen-traditional are kept in separate
> repositories, but when we release them as a tarball, qemu-xen and
> qemu-xen-traditional are in-lined into the tools/ directory.
> 
> In order to make this "just work", at the moment developer doing the
> release manually modifies Config.mk as part of the relase process so
> that CONFIG_QEMU and QEMU_UPSTREAM_URL point into the tools/ directory
> instead.
> 
> Modify Config.mk to automatically check there before trying a remote
> repository.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH 1/5] make: Check tools/qemu-xen[-traditional] for qemu before downloading

[Ian Campbell]

On Mon, 2014-07-14 at 17:15 +0100, George Dunlap wrote:

Ian seems to have acked #1..#3, so I've applied those.

Re: [PATCH 1/5] make: Check tools/qemu-xen[-traditional] for qemu before downloading

[Jan Beulich]

>>> On 14.07.14 at 18:15, <george.dunlap@eu.citrix.com> wrote:
> +CONFIG_QEMU ?= $(or $(wildcard $(QEMU_TRADITIONAL_INTREE)),\
> +                    $(QEMU_REMOTE))

Doesn't $(or ...) require make newer then 3.80 (as recently also
pointed out for the qemu-upstream build)?

Jan