diff mbox series

[XEN,v2] build: add --full to version.sh to guess $(XEN_FULLVERSION)

Message ID 20210909143306.466836-1-anthony.perard@citrix.com (mailing list archive)
State New, archived
Headers show
Series [XEN,v2] build: add --full to version.sh to guess $(XEN_FULLVERSION) | expand

Commit Message

Anthony PERARD Sept. 9, 2021, 2:33 p.m. UTC 
Running $(MAKE) like that in a $(shell ) while parsing the Makefile
doesn't work reliably. In some case, make will complain with
"jobserver unavailable: using -j1.  Add '+' to parent make rule.".
Also, it isn't possible to distinguish between the output produced by
the target "xenversion" and `make`'s own output.

Instead of running make, this patch "improve" `version.sh` to try to
guess the output of `make xenversion`.

In order to have version.sh works in more scenario, it will use
XEN_EXTRAVERSION and XEN_VENDORVERSION from the environment when
present. As for the cases were those two variables are overridden by a
make command line arguments, we export them when invoking version.sh
via a new $(XEN_FULLVERSION) macro.

That should hopefully get us to having ./version.sh returning the same
value that `make xenversion` would.

This fix GitLab CI's build job "debian-unstable-gcc-arm64".

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
v2:
- use $(SHELL) to execute ./version.sh
- get XEN_EXTRAVERSION from the environment if exist
- use XEN_VENDORVERSION when exist in the environment
- introduce a new macro $(XEN_FULLVERSION) whose jobs is to export
  $(XEN_EXTRAVERSION) and $(XEN_VENDORVERSION) in cases where those
  are overridden by command line argument of make.
  (we can't just use make's "export" because it doesn't work when
  parsing the makefile which is when $(shell ) get's executed for
  POLICY_FILENAME, so it wouldn't work when running `make -C
  tools/flask/policy XEN_VENDORVERSION=-os`.)
---
 tools/Rules.mk                     |  5 +++++
 tools/flask/policy/Makefile.common |  2 +-
 version.sh                         | 18 +++++++++++++++++-
 3 files changed, 23 insertions(+), 2 deletions(-)

Comments

Anthony PERARD Sept. 27, 2021, 12:16 p.m. UTC | #1 
Anyone to review this?

Thanks.

On Thu, Sep 09, 2021 at 03:33:06PM +0100, Anthony PERARD wrote:
> Running $(MAKE) like that in a $(shell ) while parsing the Makefile
> doesn't work reliably. In some case, make will complain with
> "jobserver unavailable: using -j1.  Add '+' to parent make rule.".
> Also, it isn't possible to distinguish between the output produced by
> the target "xenversion" and `make`'s own output.
> 
> Instead of running make, this patch "improve" `version.sh` to try to
> guess the output of `make xenversion`.
> 
> In order to have version.sh works in more scenario, it will use
> XEN_EXTRAVERSION and XEN_VENDORVERSION from the environment when
> present. As for the cases were those two variables are overridden by a
> make command line arguments, we export them when invoking version.sh
> via a new $(XEN_FULLVERSION) macro.
> 
> That should hopefully get us to having ./version.sh returning the same
> value that `make xenversion` would.
> 
> This fix GitLab CI's build job "debian-unstable-gcc-arm64".
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> v2:
> - use $(SHELL) to execute ./version.sh
> - get XEN_EXTRAVERSION from the environment if exist
> - use XEN_VENDORVERSION when exist in the environment
> - introduce a new macro $(XEN_FULLVERSION) whose jobs is to export
>   $(XEN_EXTRAVERSION) and $(XEN_VENDORVERSION) in cases where those
>   are overridden by command line argument of make.
>   (we can't just use make's "export" because it doesn't work when
>   parsing the makefile which is when $(shell ) get's executed for
>   POLICY_FILENAME, so it wouldn't work when running `make -C
>   tools/flask/policy XEN_VENDORVERSION=-os`.)
> ---
>  tools/Rules.mk                     |  5 +++++
>  tools/flask/policy/Makefile.common |  2 +-
>  version.sh                         | 18 +++++++++++++++++-
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 2907ed2d3972..b022da3336c4 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -6,6 +6,11 @@ all:
>  -include $(XEN_ROOT)/config/Tools.mk
>  include $(XEN_ROOT)/Config.mk
>  
> +XEN_FULLVERSION=$(shell env \
> +    XEN_EXTRAVERSION=$(XEN_EXTRAVERSION) \
> +    XEN_VENDORVERSION=$(XEN_VENDORVERSION) \
> +    $(SHELL) $(XEN_ROOT)/version.sh --full $(XEN_ROOT)/xen/Makefile)
> +
>  export _INSTALL := $(INSTALL)
>  INSTALL = $(XEN_ROOT)/tools/cross-install
>  
> diff --git a/tools/flask/policy/Makefile.common b/tools/flask/policy/Makefile.common
> index bea5ba4b6a40..e5ed58200e75 100644
> --- a/tools/flask/policy/Makefile.common
> +++ b/tools/flask/policy/Makefile.common
> @@ -35,7 +35,7 @@ OUTPUT_POLICY ?= $(BEST_POLICY_VER)
>  #
>  ########################################
>  
> -POLICY_FILENAME = $(FLASK_BUILD_DIR)/xenpolicy-$(shell $(MAKE) -C $(XEN_ROOT)/xen xenversion --no-print-directory)
> +POLICY_FILENAME = $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
>  POLICY_LOADPATH = /boot
>  
>  # List of policy versions supported by the hypervisor
> diff --git a/version.sh b/version.sh
> index e894ee7e0469..c6a5692c1982 100755
> --- a/version.sh
> +++ b/version.sh
> @@ -1,5 +1,21 @@
>  #!/bin/sh
>  
> +opt_full=false
> +while [ $# -gt 1 ]; do
> +    case "$1" in
> +        --full) opt_full=true ;;
> +        *) break ;;
> +    esac
> +    shift
> +done
> +
>  MAJOR=`grep "export XEN_VERSION" $1 | sed 's/.*=//g' | tr -s " "`
>  MINOR=`grep "export XEN_SUBVERSION" $1 | sed 's/.*=//g' | tr -s " "`
> -printf "%d.%d" $MAJOR $MINOR
> +
> +if $opt_full; then
> +    extraversion=$(grep "export XEN_EXTRAVERSION" $1 | sed 's/^.* ?=\s\+//; s/\$([^)]*)//g; s/ //g')
> +    : ${XEN_EXTRAVERSION:=${extraversion}${XEN_VENDORVERSION}}
> +else
> +    unset XEN_EXTRAVERSION
> +fi
> +printf "%d.%d%s" $MAJOR $MINOR $XEN_EXTRAVERSION
> -- 
> Anthony PERARD
>
Daniel P. Smith Sept. 27, 2021, 12:24 p.m. UTC | #2 
Apologies for any formatting issues, responding via web mail client.

---- On Thu, 09 Sep 2021 10:33:06 -0400 Anthony PERARD <anthony.perard@citrix.com> wrote ----

 > Running $(MAKE) like that in a $(shell ) while parsing the Makefile 
 > doesn't work reliably. In some case, make will complain with 
 > "jobserver unavailable: using -j1.  Add '+' to parent make rule.". 
 > Also, it isn't possible to distinguish between the output produced by 
 > the target "xenversion" and `make`'s own output. 
 >  
 > Instead of running make, this patch "improve" `version.sh` to try to 
 > guess the output of `make xenversion`. 
 >  
 > In order to have version.sh works in more scenario, it will use 
 > XEN_EXTRAVERSION and XEN_VENDORVERSION from the environment when 
 > present. As for the cases were those two variables are overridden by a 
 > make command line arguments, we export them when invoking version.sh 
 > via a new $(XEN_FULLVERSION) macro. 
 >  
 > That should hopefully get us to having ./version.sh returning the same 
 > value that `make xenversion` would. 
 >  
 > This fix GitLab CI's build job "debian-unstable-gcc-arm64". 
 >  
 > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> 
 > --- 
 > v2: 
 > - use $(SHELL) to execute ./version.sh 
 > - get XEN_EXTRAVERSION from the environment if exist 
 > - use XEN_VENDORVERSION when exist in the environment 
 > - introduce a new macro $(XEN_FULLVERSION) whose jobs is to export 
 >  $(XEN_EXTRAVERSION) and $(XEN_VENDORVERSION) in cases where those 
 >  are overridden by command line argument of make. 
 >  (we can't just use make's "export" because it doesn't work when 
 >  parsing the makefile which is when $(shell ) get's executed for 
 >  POLICY_FILENAME, so it wouldn't work when running `make -C 
 >  tools/flask/policy XEN_VENDORVERSION=-os`.) 
 > --- 
 >  tools/Rules.mk                     |  5 +++++ 
 >  tools/flask/policy/Makefile.common |  2 +- 
 >  version.sh                         | 18 +++++++++++++++++- 
 >  3 files changed, 23 insertions(+), 2 deletions(-) 
 >  
 > diff --git a/tools/Rules.mk b/tools/Rules.mk 
 > index 2907ed2d3972..b022da3336c4 100644 
 > --- a/tools/Rules.mk 
 > +++ b/tools/Rules.mk 
 > @@ -6,6 +6,11 @@ all: 
 >  -include $(XEN_ROOT)/config/Tools.mk 
 >  include $(XEN_ROOT)/Config.mk 
 >  
 > +XEN_FULLVERSION=$(shell env \ 
 > +    XEN_EXTRAVERSION=$(XEN_EXTRAVERSION) \ 
 > +    XEN_VENDORVERSION=$(XEN_VENDORVERSION) \ 
 > +    $(SHELL) $(XEN_ROOT)/version.sh --full $(XEN_ROOT)/xen/Makefile) 
 > + 
 >  export _INSTALL := $(INSTALL) 
 >  INSTALL = $(XEN_ROOT)/tools/cross-install 
 >  
 > diff --git a/tools/flask/policy/Makefile.common b/tools/flask/policy/Makefile.common 
 > index bea5ba4b6a40..e5ed58200e75 100644 
 > --- a/tools/flask/policy/Makefile.common 
 > +++ b/tools/flask/policy/Makefile.common 
 > @@ -35,7 +35,7 @@ OUTPUT_POLICY ?= $(BEST_POLICY_VER) 
 >  # 
 >  ######################################## 
 >  
 > -POLICY_FILENAME = $(FLASK_BUILD_DIR)/xenpolicy-$(shell $(MAKE) -C $(XEN_ROOT)/xen xenversion --no-print-directory) 
 > +POLICY_FILENAME = $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION) 
 >  POLICY_LOADPATH = /boot 
 >  
 >  # List of policy versions supported by the hypervisor 
 > diff --git a/version.sh b/version.sh 
 > index e894ee7e0469..c6a5692c1982 100755 
 > --- a/version.sh 
 > +++ b/version.sh 
 > @@ -1,5 +1,21 @@ 
 >  #!/bin/sh 
 >  
 > +opt_full=false 
 > +while [ $# -gt 1 ]; do 
 > +    case "$1" in 
 > +        --full) opt_full=true ;; 
 > +        *) break ;; 
 > +    esac 
 > +    shift 
 > +done 
 > + 
 >  MAJOR=`grep "export XEN_VERSION" $1 | sed 's/.*=//g' | tr -s " "` 
 >  MINOR=`grep "export XEN_SUBVERSION" $1 | sed 's/.*=//g' | tr -s " "` 
 > -printf "%d.%d" $MAJOR $MINOR 
 > + 
 > +if $opt_full; then 
 > +    extraversion=$(grep "export XEN_EXTRAVERSION" $1 | sed 's/^.* ?=\s\+//; s/\$([^)]*)//g; s/ //g') 
 > +    : ${XEN_EXTRAVERSION:=${extraversion}${XEN_VENDORVERSION}} 
 > +else 
 > +    unset XEN_EXTRAVERSION 
 > +fi 
 > +printf "%d.%d%s" $MAJOR $MINOR $XEN_EXTRAVERSION 
 > -- 
 > Anthony PERARD 
 >  
 > 

Provided there are no issues with rest of the changes, I see no issues made to the Flask Policy makefile.

Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>

V/r,
Daniel P. Smith
Apertus Solutions, LLC
Ian Jackson Sept. 29, 2021, 2:29 p.m. UTC | #3 
Anthony PERARD writes ("[XEN PATCH v2] build: add --full to version.sh to guess $(XEN_FULLVERSION)"):
> Running $(MAKE) like that in a $(shell ) while parsing the Makefile
> doesn't work reliably. In some case, make will complain with
> "jobserver unavailable: using -j1.  Add '+' to parent make rule.".
> Also, it isn't possible to distinguish between the output produced by
> the target "xenversion" and `make`'s own output.
> 
> Instead of running make, this patch "improve" `version.sh` to try to
> guess the output of `make xenversion`.
> 
> In order to have version.sh works in more scenario, it will use
> XEN_EXTRAVERSION and XEN_VENDORVERSION from the environment when
> present. As for the cases were those two variables are overridden by a
> make command line arguments, we export them when invoking version.sh
> via a new $(XEN_FULLVERSION) macro.
> 
> That should hopefully get us to having ./version.sh returning the same
> value that `make xenversion` would.
> 
> This fix GitLab CI's build job "debian-unstable-gcc-arm64".
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Ian Jackson <iwj@xenproject.org>
diff mbox series

Patch

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 2907ed2d3972..b022da3336c4 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -6,6 +6,11 @@  all:
 -include $(XEN_ROOT)/config/Tools.mk
 include $(XEN_ROOT)/Config.mk
 
+XEN_FULLVERSION=$(shell env \
+    XEN_EXTRAVERSION=$(XEN_EXTRAVERSION) \
+    XEN_VENDORVERSION=$(XEN_VENDORVERSION) \
+    $(SHELL) $(XEN_ROOT)/version.sh --full $(XEN_ROOT)/xen/Makefile)
+
 export _INSTALL := $(INSTALL)
 INSTALL = $(XEN_ROOT)/tools/cross-install
 
diff --git a/tools/flask/policy/Makefile.common b/tools/flask/policy/Makefile.common
index bea5ba4b6a40..e5ed58200e75 100644
--- a/tools/flask/policy/Makefile.common
+++ b/tools/flask/policy/Makefile.common
@@ -35,7 +35,7 @@  OUTPUT_POLICY ?= $(BEST_POLICY_VER)
 #
 ########################################
 
-POLICY_FILENAME = $(FLASK_BUILD_DIR)/xenpolicy-$(shell $(MAKE) -C $(XEN_ROOT)/xen xenversion --no-print-directory)
+POLICY_FILENAME = $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
 POLICY_LOADPATH = /boot
 
 # List of policy versions supported by the hypervisor
diff --git a/version.sh b/version.sh
index e894ee7e0469..c6a5692c1982 100755
--- a/version.sh
+++ b/version.sh
@@ -1,5 +1,21 @@ 
 #!/bin/sh
 
+opt_full=false
+while [ $# -gt 1 ]; do
+    case "$1" in
+        --full) opt_full=true ;;
+        *) break ;;
+    esac
+    shift
+done
+
 MAJOR=`grep "export XEN_VERSION" $1 | sed 's/.*=//g' | tr -s " "`
 MINOR=`grep "export XEN_SUBVERSION" $1 | sed 's/.*=//g' | tr -s " "`
-printf "%d.%d" $MAJOR $MINOR
+
+if $opt_full; then
+    extraversion=$(grep "export XEN_EXTRAVERSION" $1 | sed 's/^.* ?=\s\+//; s/\$([^)]*)//g; s/ //g')
+    : ${XEN_EXTRAVERSION:=${extraversion}${XEN_VENDORVERSION}}
+else
+    unset XEN_EXTRAVERSION
+fi
+printf "%d.%d%s" $MAJOR $MINOR $XEN_EXTRAVERSION