[libvirt PATCH v2 0/2] Switch our build repo CI recipes in the ci subdirectory to meson

Since v1: - honor MESON_OPTS from our cross containers - remove several more variables - shuffle the patches a bit Erik Skultety (2): ci: Switch to meson build system ci: Drop env variables related to autotools and make ci/Makefile | 37 +++++++------------------------------ ci/build.sh | 20 +++++--------------- 2 files changed, 12 insertions(+), 45 deletions(-) -- 2.26.2

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

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

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

Previous patch switched the build to meson which supports only out-of-tree builds, runs by default in parallel on all available CPUs, and we don't use configure anymore - the corresponding variables are thus longer needed. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/Makefile | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/ci/Makefile b/ci/Makefile index f76600240f..e8832a024e 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -20,30 +20,9 @@ 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 ninja CI_NINJA_ARGS = -# 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 @@ -223,13 +202,7 @@ ci-run-command@%: ci-prepare-tree --login \ --user="#$(CI_UID)" \ --group="#$(CI_GID)" \ - CONFIGURE_OPTS="$$CONFIGURE_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)" \ MESON_OPTS="$$MESON_OPTS" \ CI_NINJA_ARGS="$(CI_NINJA_ARGS)" \ $(CI_COMMAND) || exit 1' @@ -271,7 +244,5 @@ 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_NINJA_ARGS= - extra arguments passed to ninja" @echo -- 2.26.2

On Mon, 2020-11-09 at 12:20 +0100, Erik Skultety wrote:
Previous patch switched the build to meson which supports only out-of-tree builds, runs by default in parallel on all available CPUs, and we don't use configure anymore - the corresponding variables are thus longer needed.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/Makefile | 29 ----------------------------- 1 file changed, 29 deletions(-)
The changes look good, so Reviewed-by: Andrea Bolognani <abologna@redhat.com> to those. However, since you don't have such a clean separation between the two patches (you already drop some autotools-related stuff in first one) I would suggest just squashing them together. -- Andrea Bolognani / Red Hat / Virtualization

On Mon, Nov 09, 2020 at 12:20:51PM +0100, Erik Skultety wrote:
Since v1: - honor MESON_OPTS from our cross containers - remove several more variables - shuffle the patches a bit
Erik Skultety (2): ci: Switch to meson build system ci: Drop env variables related to autotools and make
ci/Makefile | 37 +++++++------------------------------ ci/build.sh | 20 +++++--------------- 2 files changed, 12 insertions(+), 45 deletions(-)
Ping Erik
participants (2)
-
Andrea Bolognani
-
Erik Skultety