| Message ID | 20210908095422.438324-3-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build: attempt to "fix" GitLab CI job "debian-unstable-gcc-arm64" | expand |
On 08.09.2021 11:54, Anthony PERARD wrote: > --- 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-$(shell $(XEN_ROOT)/version.sh --full $(XEN_ROOT)/xen/Makefile) Shell scripts better get invoked by "$(SHELL) <script>", to avoid depending on the script actually being marked as executable (which is impossible on some file systems). > --- a/version.sh > +++ b/version.sh > @@ -1,5 +1,18 @@ > #!/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') > +fi > +printf "%d.%d%s" $MAJOR $MINOR $EXTRAVERSION I guess you want to clear EXTRAVERSION either prior to the "if" or in an "else". With these addressed: Acked-by: Jan Beulich <jbeulich@suse.com> Jan
On Wed, Sep 08, 2021 at 02:27:17PM +0200, Jan Beulich wrote: > On 08.09.2021 11:54, Anthony PERARD wrote: > > --- 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-$(shell $(XEN_ROOT)/version.sh --full $(XEN_ROOT)/xen/Makefile) > > Shell scripts better get invoked by "$(SHELL) <script>", to avoid > depending on the script actually being marked as executable (which is > impossible on some file systems). > > > --- a/version.sh > > +++ b/version.sh > > @@ -1,5 +1,18 @@ > > #!/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') > > +fi > > +printf "%d.%d%s" $MAJOR $MINOR $EXTRAVERSION > > I guess you want to clear EXTRAVERSION either prior to the "if" or > in an "else". Actually, I'm thinking of writing this instead: 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 This would takes care of cases where a packager add XEN_EXTRAVERSION or XEN_VENDORVERSION to the environment, and I think would produce the same result as `make xenversion` in that case. But that leaves the case of running e.g. "make XEN_VENDORVERSION=-neat xenversion" out, hopefully, no one does that. Otherwise, we might need "export" or a new macro in the build system (and that would still leave out someone having "xen/xen-version" file). :-( I think I just found a package overriding XEN_VENDORVERSION with a command line argument, the AUR package. So ./version.sh needs help from the build system: Maybe XEN_FULLVERSION=$(shell env XEN_EXTRAVERSION=$(XEN_EXTRAVERSION) XEN_VENDORVERSION=$(XEN_VENDORVERSION) $(SHELL) $(XEN_ROOT)/version.sh --full $(XEN_ROOT)/xen/Makefile) or probably just exporting those two var in Config.mk or tools/Rules.mk. Cheers,
On 09.09.2021 13:10, Anthony PERARD wrote: > :-( I think I just found a package overriding XEN_VENDORVERSION with a > command line argument, the AUR package. So ./version.sh needs help from > the build system: > > Maybe > > XEN_FULLVERSION=$(shell env XEN_EXTRAVERSION=$(XEN_EXTRAVERSION) XEN_VENDORVERSION=$(XEN_VENDORVERSION) $(SHELL) $(XEN_ROOT)/version.sh --full $(XEN_ROOT)/xen/Makefile) > > or probably just exporting those two var in Config.mk or tools/Rules.mk. Either sounds okay to me, fwiw. Jan
diff --git a/tools/flask/policy/Makefile.common b/tools/flask/policy/Makefile.common index bea5ba4b6a40..7f470cd02861 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-$(shell $(XEN_ROOT)/version.sh --full $(XEN_ROOT)/xen/Makefile) POLICY_LOADPATH = /boot # List of policy versions supported by the hypervisor diff --git a/version.sh b/version.sh index e894ee7e0469..abd19ce79ac8 100755 --- a/version.sh +++ b/version.sh @@ -1,5 +1,18 @@ #!/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') +fi +printf "%d.%d%s" $MAJOR $MINOR $EXTRAVERSION
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.", which will be part of "$(make xenversion)" output. 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`. This only works as long as $(XEN_VENDORVERSION) isn't set, or $(XEN_EXTRAVERSION) isn't overridden. This fix GitLab CI's build job "debian-unstable-gcc-arm64" on which $(shell $(MAKE) xenversion) in "flask/policy/Makefile.common" reliably produce more output than just the xenversion. This patch also allows to run for example `make --debug=a` without breaking the build when flask policy is enabled. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/flask/policy/Makefile.common | 2 +- version.sh | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-)