[libvirt PATCH v2 1/2] ci: Switch to meson build system

Erik Skultety posted 2 patches 5 years, 3 months ago
[libvirt PATCH v2 1/2] ci: Switch to meson build system
Posted by Erik Skultety 5 years, 3 months ago
First add the meson required bits to be able to run the build.
NOTE: inspired by our gitlab-ci.yml

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 ci/Makefile | 10 ++++++++--
 ci/build.sh | 20 +++++---------------
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/ci/Makefile b/ci/Makefile
index c7c8eb9a45..f76600240f 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -35,6 +35,9 @@ CI_CONFIGURE = $(CI_CONT_SRCDIR)/configure
 # Default to using all possible CPUs
 CI_SMP = $(shell getconf _NPROCESSORS_ONLN)
 
+# Any extra arguments to pass to ninja
+CI_NINJA_ARGS =
+
 # Any extra arguments to pass to make
 CI_MAKE_ARGS =
 
@@ -227,6 +230,8 @@ ci-run-command@%: ci-prepare-tree
 		  CI_CONFIGURE="$(CI_CONFIGURE)" \
 		  CI_CONFIGURE_ARGS="$(CI_CONFIGURE_ARGS)" \
 		  CI_MAKE_ARGS="$(CI_MAKE_ARGS)" \
+		  MESON_OPTS="$$MESON_OPTS" \
+		  CI_NINJA_ARGS="$(CI_NINJA_ARGS)" \
 		  $(CI_COMMAND) || exit 1'
 	@test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
 
@@ -236,8 +241,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
@@ -268,4 +273,5 @@ ci-help:
 	@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_NINJA_ARGS=      - extra arguments passed to ninja"
 	@echo
diff --git a/ci/build.sh b/ci/build.sh
index 2da84c080a..21b053a4cd 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -7,26 +7,16 @@
 #
 # to make.
 
-mkdir -p "$CI_CONT_BUILDDIR" || exit 1
-cd "$CI_CONT_BUILDDIR"
+mkdir -p "$CI_CONT_SRCDIR" || exit 1
+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,
-# 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
+meson build --werror $MESON_OPTS || (cat build/meson-logs/meson-log.txt && exit 1)
+ninja -C build $CI_NINJA_ARGS
+
 find -name test-suite.log -delete
 
-make -j"$CI_SMP" $CI_MAKE_ARGS
-
 if test $? != 0; then \
     LOGS=$(find -name test-suite.log)
     if test "$LOGS"; then
-- 
2.26.2

Re: [libvirt PATCH v2 1/2] ci: Switch to meson build system
Posted by Andrea Bolognani 5 years, 2 months ago
On Mon, 2020-11-09 at 12:20 +0100, Erik Skultety wrote:
> First add the meson required bits to be able to run the build.
> NOTE: inspired by our gitlab-ci.yml

This note seems unnecessary.

> +++ b/ci/Makefile
> @@ -227,6 +230,8 @@ ci-run-command@%: ci-prepare-tree
>  		  CI_CONFIGURE="$(CI_CONFIGURE)" \
>  		  CI_CONFIGURE_ARGS="$(CI_CONFIGURE_ARGS)" \
>  		  CI_MAKE_ARGS="$(CI_MAKE_ARGS)" \
> +		  MESON_OPTS="$$MESON_OPTS" \

Please keep this right after CONFIGURE_OPTS and before all the CI_*
variables.

> +++ b/ci/build.sh
> -mkdir -p "$CI_CONT_BUILDDIR" || exit 1
> -cd "$CI_CONT_BUILDDIR"
> +mkdir -p "$CI_CONT_SRCDIR" || exit 1
> +cd "$CI_CONT_SRCDIR"

$CI_CONT_SRCDIR is the source directory, which is guaranteed to exist
because we mount it inside the container as a volume. So you can drop
the first line altogether.

> +meson build --werror $MESON_OPTS || (cat build/meson-logs/meson-log.txt && exit 1)
> +ninja -C build $CI_NINJA_ARGS

We enable -Werror automatically when building from a git clone, which
is always going to be the case when using this scaffoling, so I think
you can leave that option out. I see it's used in the GitLab CI
configuration, so you can maybe keep it in right now and then
consider removing it from both places at the same time.

>  find -name test-suite.log -delete

This should be updated to look for testlog.txt instead, but actually
you might be able to leave it out completely since meson seems to do
a good job at displaying the relevant part of the log if a test
fails? We don't have anything like this in the GitLab CI
configuration, so either it's not needed here either or we should add
it there as well.

Either way, removing the log files right after they've been created
by calling ninja is most likely not what you wanted :)

>  if test $? != 0; then \
>      LOGS=$(find -name test-suite.log)
>      if test "$LOGS"; then

If we determine showing the logs manually is not necessary, this
entire hunk can go.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH v2 1/2] ci: Switch to meson build system
Posted by Erik Skultety 5 years, 2 months ago
On Fri, Nov 20, 2020 at 04:29:18PM +0100, Andrea Bolognani wrote:
> On Mon, 2020-11-09 at 12:20 +0100, Erik Skultety wrote:
> > First add the meson required bits to be able to run the build.
> > NOTE: inspired by our gitlab-ci.yml
> 
> This note seems unnecessary.
> 
> > +++ b/ci/Makefile
> > @@ -227,6 +230,8 @@ ci-run-command@%: ci-prepare-tree
> >  		  CI_CONFIGURE="$(CI_CONFIGURE)" \
> >  		  CI_CONFIGURE_ARGS="$(CI_CONFIGURE_ARGS)" \
> >  		  CI_MAKE_ARGS="$(CI_MAKE_ARGS)" \
> > +		  MESON_OPTS="$$MESON_OPTS" \
> 
> Please keep this right after CONFIGURE_OPTS and before all the CI_*
> variables.
> 
> > +++ b/ci/build.sh
> > -mkdir -p "$CI_CONT_BUILDDIR" || exit 1
> > -cd "$CI_CONT_BUILDDIR"
> > +mkdir -p "$CI_CONT_SRCDIR" || exit 1
> > +cd "$CI_CONT_SRCDIR"
> 
> $CI_CONT_SRCDIR is the source directory, which is guaranteed to exist
> because we mount it inside the container as a volume. So you can drop
> the first line altogether.
> 
> > +meson build --werror $MESON_OPTS || (cat build/meson-logs/meson-log.txt && exit 1)
> > +ninja -C build $CI_NINJA_ARGS
> 
> We enable -Werror automatically when building from a git clone, which
> is always going to be the case when using this scaffoling, so I think
> you can leave that option out. I see it's used in the GitLab CI
> configuration, so you can maybe keep it in right now and then
> consider removing it from both places at the same time.
> 
> >  find -name test-suite.log -delete
> 
> This should be updated to look for testlog.txt instead, but actually
> you might be able to leave it out completely since meson seems to do
> a good job at displaying the relevant part of the log if a test
> fails? We don't have anything like this in the GitLab CI
> configuration, so either it's not needed here either or we should add
> it there as well.

Oh, you're right, the testlog dump is quite useless except for e.g. these two
lines:
"Some tests failed. Run them using:
VIR_TEST_DEBUG=1 VIR_TEST_RANGE=849 /home/eskultety/libvirt/build/tests/qemuxml2argvtest"

So I'll drop the suggested hunks.

Erik