[v4] ci: Switch to meson build system

[libvirt PATCH v4] ci: Switch to meson build system

Posted by Erik Skultety 3 years, 5 months ago

Add meson required bits to the ci logic in the repo to be able to run
a meson build in a container.
This patch also drops several environment variables we don't need with
meson anymore.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
v3: https://www.redhat.com/archives/libvir-list/2020-November/msg01293.html
Since v3:
- further review comments
- introduced CI_MESON_ARGS variable

 ci/Makefile | 38 +++++++-------------------------------
 ci/build.sh | 29 ++++++-----------------------
 2 files changed, 13 insertions(+), 54 deletions(-)

diff --git a/ci/Makefile b/ci/Makefile
index c7c8eb9a45..d9d14f82c2 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -20,27 +20,6 @@ CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src
 # the $(CI_HOST_SRCDIR) directory from the host
 CI_CONT_SRCDIR = $(CI_USER_HOME)/libvirt
 
-# Relative directory to perform the build in. This
-# defaults to using a separate build dir, but can be
-# set to empty string for an in-source tree build.
-CI_VPATH = build
-
-# The directory holding the build output inside the
-# container.
-CI_CONT_BUILDDIR = $(CI_CONT_SRCDIR)/$(CI_VPATH)
-
-# Can be overridden with mingw{32,64}-configure if desired
-CI_CONFIGURE = $(CI_CONT_SRCDIR)/configure
-
-# Default to using all possible CPUs
-CI_SMP = $(shell getconf _NPROCESSORS_ONLN)
-
-# Any extra arguments to pass to make
-CI_MAKE_ARGS =
-
-# Any extra arguments to pass to configure
-CI_CONFIGURE_ARGS =
-
 # Script containing environment preparation steps
 CI_PREPARE_SCRIPT = $(CI_ROOTDIR)/prepare.sh
 
@@ -220,13 +199,10 @@ ci-run-command@%: ci-prepare-tree
 		  --login \
 		  --user="#$(CI_UID)" \
 		  --group="#$(CI_GID)" \
-		  CONFIGURE_OPTS="$$CONFIGURE_OPTS" \
+		  MESON_OPTS="$$MESON_OPTS" \
 		  CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)" \
-		  CI_CONT_BUILDDIR="$(CI_CONT_BUILDDIR)" \
-		  CI_SMP="$(CI_SMP)" \
-		  CI_CONFIGURE="$(CI_CONFIGURE)" \
-		  CI_CONFIGURE_ARGS="$(CI_CONFIGURE_ARGS)" \
-		  CI_MAKE_ARGS="$(CI_MAKE_ARGS)" \
+		  CI_MESON_ARGS="$(CI_MESON_ARGS)" \
+		  CI_NINJA_ARGS="$(CI_NINJA_ARGS)" \
 		  $(CI_COMMAND) || exit 1'
 	@test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
 
@@ -236,8 +212,8 @@ ci-shell@%:
 ci-build@%:
 	$(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="$(CI_USER_HOME)/build"
 
-ci-check@%:
-	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
+ci-test@%:
+	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS+=test
 
 ci-list-images:
 	@echo
@@ -266,6 +242,6 @@ ci-help:
 	@echo "    CI_CLEAN=0          - do not delete '$(CI_SCRATCHDIR)' after completion"
 	@echo "    CI_REUSE=1          - re-use existing '$(CI_SCRATCHDIR)' content"
 	@echo "    CI_ENGINE=auto      - container engine to use (podman, docker)"
-	@echo "    CI_CONFIGURE_ARGS=  - extra arguments passed to configure"
-	@echo "    CI_MAKE_ARGS=       - extra arguments passed to make, e.g. space delimited list of targets"
+	@echo "    CI_MESON_ARGS=      - extra arguments passed to meson"
+	@echo "    CI_NINJA_ARGS=      - extra arguments passed to ninja"
 	@echo
diff --git a/ci/build.sh b/ci/build.sh
index 2da84c080a..740b46a935 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -7,32 +7,15 @@
 #
 # to make.
 
-mkdir -p "$CI_CONT_BUILDDIR" || exit 1
-cd "$CI_CONT_BUILDDIR"
+cd "$CI_CONT_SRCDIR"
 
 export VIR_TEST_DEBUG=1
-NOCONFIGURE=1 "$CI_CONT_SRCDIR/autogen.sh" || exit 1
 
-# $CONFIGURE_OPTS is a env that can optionally be set in the container,
+# $MESON_OPTS is an env that can optionally be set in the container,
 # populated at build time from the Dockerfile. A typical use case would
-# be to pass --host/--target args to trigger cross-compilation
-#
-# This can be augmented by make local args in $CI_CONFIGURE_ARGS
-"$CI_CONFIGURE" $CONFIGURE_OPTS $CI_CONFIGURE_ARGS
-if test $? != 0; then
-    test -f config.log && cat config.log
-    exit 1
-fi
-find -name test-suite.log -delete
+# be to pass options to trigger cross-compilation
 
-make -j"$CI_SMP" $CI_MAKE_ARGS
+meson build --werror $MESON_OPTS $CI_MESON_ARGS || \
+(cat build/meson-logs/meson-log.txt && exit 1)
 
-if test $? != 0; then \
-    LOGS=$(find -name test-suite.log)
-    if test "$LOGS"; then
-        echo "=== LOG FILE(S) START ==="
-        cat $LOGS
-        echo "=== LOG FILE(S) END ==="
-    fi
-    exit 1
-fi
+ninja -C build $CI_NINJA_ARGS
-- 
2.26.2

Re: [libvirt PATCH v4] ci: Switch to meson build system

Posted by Andrea Bolognani 3 years, 5 months ago

On Fri, 2020-11-27 at 09:46 +0100, Erik Skultety wrote:
> +++ b/ci/Makefile
> -ci-check@%:
> -	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
> +ci-test@%:
> +	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS+=test

I don't know why this last bit turned from =test in v3 to +=test in
v4, but I don't think it should have. Please change it back.

With that changed,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH v4] ci: Switch to meson build system

Posted by Erik Skultety 3 years, 5 months ago

On Fri, Nov 27, 2020 at 10:44:56AM +0100, Andrea Bolognani wrote:
> On Fri, 2020-11-27 at 09:46 +0100, Erik Skultety wrote:
> > +++ b/ci/Makefile
> > -ci-check@%:
> > -	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
> > +ci-test@%:
> > +	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS+=test
> 
> I don't know why this last bit turned from =test in v3 to +=test in
> v4, but I don't think it should have. Please change it back.

Deliberate decision...the whole point of ci-test is to test, you may want to
add more flags to ninja but still want to test, we don't document anywhere that
the flags user sets override our defaults - I've seen user flags to be appended
to global env variables in Makefiles so I did the same here. If you want me to
change it back without documenting that when user selects a CI target expecting
something to happen turning out completely different because they set a certain
variable according to the help, I can sure do that (I don't care that much),
but it's IMO confusing and this way it's more foolproof.

Erik

Re: [libvirt PATCH v4] ci: Switch to meson build system

Posted by Andrea Bolognani 3 years, 5 months ago

On Fri, 2020-11-27 at 11:28 +0100, Erik Skultety wrote:
> On Fri, Nov 27, 2020 at 10:44:56AM +0100, Andrea Bolognani wrote:
> > On Fri, 2020-11-27 at 09:46 +0100, Erik Skultety wrote:
> > > +++ b/ci/Makefile
> > > -ci-check@%:
> > > -	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
> > > +ci-test@%:
> > > +	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS+=test
> > 
> > I don't know why this last bit turned from =test in v3 to +=test in
> > v4, but I don't think it should have. Please change it back.
> 
> Deliberate decision...the whole point of ci-test is to test, you may want to
> add more flags to ninja but still want to test, we don't document anywhere that
> the flags user sets override our defaults - I've seen user flags to be appended
> to global env variables in Makefiles so I did the same here. If you want me to
> change it back without documenting that when user selects a CI target expecting
> something to happen turning out completely different because they set a certain
> variable according to the help, I can sure do that (I don't care that much),
> but it's IMO confusing and this way it's more foolproof.

Since you're just converting from autotools to meson in this commit,
the underlying behavior should probably not change, so I'd leave it
as it is regardless of whether or not you consider the change to be
for the better.

ci-test was always intended to be a convenient shorthand anyway, the
idea being that you'd use it for the basic case and then switch to
ci-build CI_NINJA_ARGS=... as soon as you started to outgrow it.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH v4] ci: Switch to meson build system

Posted by Erik Skultety 3 years, 5 months ago

On Fri, Nov 27, 2020 at 03:05:49PM +0100, Andrea Bolognani wrote:
> On Fri, 2020-11-27 at 11:28 +0100, Erik Skultety wrote:
> > On Fri, Nov 27, 2020 at 10:44:56AM +0100, Andrea Bolognani wrote:
> > > On Fri, 2020-11-27 at 09:46 +0100, Erik Skultety wrote:
> > > > +++ b/ci/Makefile
> > > > -ci-check@%:
> > > > -	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
> > > > +ci-test@%:
> > > > +	$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS+=test
> > > 
> > > I don't know why this last bit turned from =test in v3 to +=test in
> > > v4, but I don't think it should have. Please change it back.
> > 
> > Deliberate decision...the whole point of ci-test is to test, you may want to
> > add more flags to ninja but still want to test, we don't document anywhere that
> > the flags user sets override our defaults - I've seen user flags to be appended
> > to global env variables in Makefiles so I did the same here. If you want me to
> > change it back without documenting that when user selects a CI target expecting
> > something to happen turning out completely different because they set a certain
> > variable according to the help, I can sure do that (I don't care that much),
> > but it's IMO confusing and this way it's more foolproof.
> 
> Since you're just converting from autotools to meson in this commit,
> the underlying behavior should probably not change, so I'd leave it
> as it is regardless of whether or not you consider the change to be
> for the better.
> 
> ci-test was always intended to be a convenient shorthand anyway, the
> idea being that you'd use it for the basic case and then switch to
> ci-build CI_NINJA_ARGS=... as soon as you started to outgrow it.

Fair enough...I changed it back and pushed.

Thanks,
Erik