[libvirt] [PATCH v4 0/7] Facilitate running libvirt builds via docker containers

For a while QEMU has provided simple make rules for building QEMU inside standard docker container environments. This provides an equivalent mechanism for libvirt inspired by QEMU's. QEMU actually builds the container images on developer's machines locally. Libvirt already hosts pre-built images on quay.io, so that is used directly as it is quicker to download them than to build them locally. This also ensures the container contents match what the live CI system is using, as opposed to building an image with newer packages. Changed in v4: - Re-add cross-compilation patches now that containers exist - Add CI_ prefix on all variables to avoid clashing with automake defined variables - Fix path given to -f arg for ci-check target - Remove dead code - Fix check for submodule checkout existing - Ensure macOS Homebrew is updated Changed in v3: - Lots of new variables & variable renames - Rename to top level Makefile.ci - Rename make targets to have @ separator for the image - Refactor macOS travis config Changed in v2: - Drop cross-compilation patches temporarily. The cross-compiler images are not yet built on quay.io and rather than creating and bulding many more images manually, I'm working on script to automate setup of quay.io images - Many changes to the make rules to make it possible to use them from Travis CI to match its currently testing setup - Modify Travis CI config to use the new make rules for consistency Daniel P. Berrangé (7): tests: add targets for building libvirt inside Docker containers travis: convert Ubuntu, CentOS & MinGW builds to use new make rules travis: use declarative syntax for Homebrew packages travis: remove display of test-suite.log from macOS travis: put macOS script inline in the macOS matrix entry tests: add cross compiler images to CI test help output tests: perform cross compiler builds on GitLab CI .gitignore | 1 + .gitlab-ci.yml | 74 +++++++++++++++++ .travis.yml | 78 +++++------------ Makefile.am | 2 + Makefile.ci | 222 +++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 321 insertions(+), 56 deletions(-) create mode 100644 .gitlab-ci.yml create mode 100644 Makefile.ci -- 2.20.1

The Travis CI system uses Docker containers for its build environment. These are pre-built and hosted under quay.io/libvirt so that developers can use them for reproducing problems locally. Getting the right Docker command syntax to use them, however, is not entirely easy. This patch addresses that usability issue by introducing some make targets. To run a simple build (aka 'make all') using the Fedora 28 container: make ci-build@fedora-28 To also run unit tests make ci-check@fedora-28 This is just syntax sugar for calling the previous command with a custom make target make ci-build@fedora-28 CI_MAKE_ARGS="check" To do a purely interactive build it is possible to request a shell make ci-shell@fedora-28 To do a MinGW build, it is currently possible to use the fedora-rawhide image and request a different configure script make ci-build@fedora-rawhide CI_CONFIGURE=mingw32-configure It is also possible to do cross compiled builds via the Debian containers make ci-build@debian-9-cross-s390x In all cases the GIT source tree is cloned locally into a 'ci-tree/src' sub-directory which is then exposed to the container at '/src'. It is setup to use a separate build directory so the build takes place in a subdir '/src/build'. A source tree build can be requested instead by passing an empty string CI_VPATH= arg to make. The make rules are kept in a standalone file that is included into the main Makefile.am, so that it is possible to run them without having to invoke autotools first. It is neccessary to disable the gnulib submodule commit check because this fails due to the way we have manually cloned submodule repos as primary git repos with their own .git directory, instead of letting git treat them as submodules in the top level .git directory. make[1]: Entering directory '/src/build' fatal: Not a valid object name origin fatal: run_command returned non-zero status for .gnulib . maint.mk: found non-public submodule commit make: *** [/src/maint.mk:1448: public-submodule-commit] Error 1 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .gitignore | 1 + Makefile.am | 2 + Makefile.ci | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+) create mode 100644 Makefile.ci diff --git a/.gitignore b/.gitignore index c918ec8226..bebf6605e4 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,7 @@ /autom4te.cache /build-aux/* /build/ +/ci-tree/ /confdefs.h /config.cache /config.guess diff --git a/Makefile.am b/Makefile.am index 365b0b3b94..621e9b4f89 100644 --- a/Makefile.am +++ b/Makefile.am @@ -96,3 +96,5 @@ gen-AUTHORS: mv -f $(distdir)/AUTHORS-tmp $(distdir)/AUTHORS && \ rm -f all.list maint.list contrib.list; \ fi + +include Makefile.ci diff --git a/Makefile.ci b/Makefile.ci new file mode 100644 index 0000000000..984cd3c205 --- /dev/null +++ b/Makefile.ci @@ -0,0 +1,210 @@ +# -*- makefile -*- +# vim: filetype=make + +# Figure out name and path to this file. This isn't +# portable but we only care for modern GNU make +CI_MAKEFILE = $(abspath $(firstword $(MAKEFILE_LIST))) + +# The directory holding content on the host that we will +# expose to the container. +CI_SCRATCHDIR = $(shell pwd)/ci-tree + +# The root directory of the libvirt.git checkout +CI_GIT_ROOT = $(shell git rev-parse --show-toplevel) + +# The directory holding the clone of the git repo that +# we will expose to the container +CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src + +# The directory holding the source inside the +# container. ie where we told Docker to expose +# the $(CI_HOST_SRCDIR) directory from the host +CI_CONT_SRCDIR = /src + +# 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 = + +# Avoid pulling submodules over the network by locally +# cloning them +CI_SUBMODULES = .gnulib src/keycodemapdb + +# Location of the Docker images we're going to pull +# Can be useful to overridde to use a locally built +# image instead +CI_IMAGE_PREFIX = quay.io/libvirt/buildenv- + +# Docker defaults to pulling the ':latest' tag but +# if the Docker repo above uses different conventions +# this can override it +CI_IMAGE_TAG = :master + +# We delete the virtual root after completion, set +# to 0 if you need to keep it around for debugging +CI_CLEAN = 1 + +# We'll always freshly clone the virtual root each +# time in case it was not cleaned up before. Set +# to 1 if you want to try restarting a previously +# preserved env +CI_REUSE = 0 + +# We need the container process to run with current host IDs +# so that it can access the passed in build directory +CI_UID = $(shell id -u) +CI_GID = $(shell id -g) + +# Docker doesn't require the IDs you run as to exist in +# the container's /etc/passwd & /etc/group files, but +# if they do not, then libvirt's 'make check' will fail +# many tests. +# +# We do not directly mount /etc/{passwd,group} as Docker +# is liable to mess with SELinux labelling which will +# then prevent the host accessing them. Copying them +# first is safer. +CI_PWDB_MOUNTS = \ + --volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \ + --volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \ + $(NULL) + +# Docker containers can have very large ulimits +# for nofiles - as much as 1048576. This makes +# libvirt very slow at exec'ing programs. +CI_ULIMIT_FILES = 1024 + +# Args to use when cloning a git repo. +# -c stop it complaining about checking out a random hash +# -q stop it displaying progress info for local clone +# --local ensure we don't actually copy files +CI_GIT_ARGS = \ + -c advice.detachedHead=false \ + -q \ + --local \ + $(NULL) + +# Args to use when running the Docker env +# --rm stop inactive containers getting left behind +# --user we execute as the same user & group account +# as dev so that file ownership matches host +# instead of root:root +# --volume to pass in the cloned git repo & config +# --workdir to set cwd to vpath build location +# --ulimit lower files limit for performance reasons +# --interactive +# --tty Ensure we have ability to Ctrl-C the build +CI_DOCKER_ARGS = \ + --rm \ + --user $(CI_UID):$(CI_GID) \ + --interactive \ + --tty \ + $(CI_PWDB_MOUNTS) \ + --volume $(CI_HOST_SRCDIR):$(CI_CONT_SRCDIR):z \ + --workdir $(CI_CONT_SRCDIR) \ + --ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \ + $(NULL) + +ci-check-docker: + @echo -n "Checking if Docker is available and running..." && \ + docker version 1>/dev/null && echo "yes" + +ci-prepare-tree: ci-check-docker + @if test "$(CI_REUSE)" != "1" ; then \ + rm -rf $(CI_SCRATCHDIR) ; \ + fi + @if ! test -d $(CI_SCRATCHDIR) ; then \ + mkdir -p $(CI_HOST_SRCDIR); \ + cp /etc/passwd $(CI_SCRATCHDIR); \ + cp /etc/group $(CI_SCRATCHDIR); \ + echo "Cloning $(CI_GIT_ROOT) to $(CI_HOST_SRCDIR)"; \ + git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT) $(CI_HOST_SRCDIR) || exit 1; \ + for mod in $(CI_SUBMODULES) ; \ + do \ + test -f $(CI_GIT_ROOT)/$$mod/.git || continue ; \ + echo "Cloning $(CI_GIT_ROOT)/$$mod to $(CI_HOST_SRCDIR)/$$mod"; \ + git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT)/$$mod $(CI_HOST_SRCDIR)/$$mod || exit 1; \ + done ; \ + fi + +# $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) +# +# gl_public_submodule_commit= to disable gnulib's submodule check +# which breaks due to way we clone the submodules +ci-build@%: ci-prepare-tree + docker run $(CI_DOCKER_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \ + /bin/bash -c '\ + mkdir -p $(CI_CONT_BUILDDIR) || exit 1 ; \ + cd $(CI_CONT_BUILDDIR) ; \ + NOCONFIGURE=1 $(CI_CONT_SRCDIR)/autogen.sh || exit 1 ; \ + $(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 ; \ + export VIR_TEST_DEBUG=1 ; \ + make -j$(CI_SMP) gl_public_submodule_commit= $(CI_MAKE_ARGS) ; \ + 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' + @test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || : + +ci-check@%: + $(MAKE) -f $(CI_MAKEFILE) ci-build@$* CI_MAKE_ARGS="check" + +ci-shell@%: ci-prepare-tree + docker run $(CI_DOCKER_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) /bin/bash + @test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || : + +ci-help: + @echo "Build libvirt inside Docker containers used for CI" + @echo + @echo "Available targets:" + @echo + @echo " ci-build@\$$IMAGE - run a default 'make'" + @echo " ci-check@\$$IMAGE - run a 'make check'" + @echo " ci-shell@\$$IMAGE - run an interactive shell" + @echo + @echo "Available x86 container images:" + @echo + @echo " centos-7" + @echo " debian-9" + @echo " debian-sid" + @echo " fedora-28" + @echo " fedora-29" + @echo " fedora-rawhide" + @echo " ubuntu-18" + @echo + @echo "Available make variables:" + @echo + @echo " CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion" + @echo " CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content" + @echo -- 2.20.1

On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote: [...]
It is also possible to do cross compiled builds via the Debian containers
make ci-build@debian-9-cross-s390x
You're not advertising cross-compilation images until commit 6/7, so either drop this from the commit message or squash that commit into this one. I see no reason not to do the latter. [...]
+# Docker defaults to pulling the ':latest' tag but +# if the Docker repo above uses different conventions +# this can override it +CI_IMAGE_TAG = :master
This should probably contain only "master", since that's the tag itself and ":" is just a separator. We can change it later. [...]
+ci-prepare-tree: ci-check-docker + @if test "$(CI_REUSE)" != "1" ; then \ + rm -rf $(CI_SCRATCHDIR) ; \ + fi
The equivalent code for CI_CLEAN looks like @test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || : Please pick either style and use it consistently throughout.
+ @if ! test -d $(CI_SCRATCHDIR) ; then \ + mkdir -p $(CI_HOST_SRCDIR); \
Please create CI_SCRATCHDIR instead of CI_HOST_SRCDIR here: 'git clone' will then take care of the latter. [...]
+ @echo "Available targets:" + @echo + @echo " ci-build@\$$IMAGE - run a default 'make'" + @echo " ci-check@\$$IMAGE - run a 'make check'" + @echo " ci-shell@\$$IMAGE - run an interactive shell"
Double space between "IMAGE" and "-". [...]
+ @echo "Available make variables:" + @echo + @echo " CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion" + @echo " CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content"
Double space before "-" here as well. There are actually many more variables that users can provide to tweak the build environment or steps, chief among them CI_MAKE_ARGS: we should document at least the ones we expect to be more widely applicable. That's another thing we can deal with later, though. With the small issues pointed out above addressed, and at long last, Reviewed-by: Andrea Bolognani <abologna@redhat.com> Thanks for bearing with me :) -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Apr 10, 2019 at 12:52:12PM +0200, Andrea Bolognani wrote:
On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote: [...]
It is also possible to do cross compiled builds via the Debian containers
make ci-build@debian-9-cross-s390x
You're not advertising cross-compilation images until commit 6/7, so either drop this from the commit message or squash that commit into this one. I see no reason not to do the latter.
Yeah we can squash it in now.
[...]
+# Docker defaults to pulling the ':latest' tag but +# if the Docker repo above uses different conventions +# this can override it +CI_IMAGE_TAG = :master
This should probably contain only "master", since that's the tag itself and ":" is just a separator. We can change it later.
FWIW, I included the ":" because it makes it easier with using images without a tag. Unfortunately doesn't doesn't treat "foo:" as being the same as "foo" - it raises an error in the former case :-( We could do some munging to conditionally add the ":" I guess Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Change the Travis CI configuration to invoke the new ci-build@$IMAGE target instead of directly running Docker. This guarantees that when a developer runs ci-build@$IMAGE locally, the container build setup is identical to that used in Travis CI, with exception of the host kernel and Docker version. Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 46 ++++++++++------------------------------------ 1 file changed, 10 insertions(+), 36 deletions(-) diff --git a/.travis.yml b/.travis.yml index 54de7dd7ad..90a52c7f08 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,24 +11,30 @@ matrix: - docker env: - IMAGE="ubuntu-18" - - DOCKER_CMD="$LINUX_CMD" + - MAKE_ARGS="syntax-check distcheck" + script: + - make -f Makefile.ci ci-build@$IMAGE MAKE_ARGS="$MAKE_ARGS" - services: - docker env: - IMAGE="centos-7" - - DOCKER_CMD="$LINUX_CMD" + - MAKE_ARGS="syntax-check distcheck" + script: + - make -f Makefile.ci ci-build@$IMAGE MAKE_ARGS="$MAKE_ARGS" - services: - docker env: - IMAGE="fedora-rawhide" - MINGW="mingw32" - - DOCKER_CMD="$MINGW_CMD" + script: + - make -f Makefile.ci ci-build@$IMAGE CONFIGURE="$MINGW-configure" - services: - docker env: - IMAGE="fedora-rawhide" - MINGW="mingw64" - - DOCKER_CMD="$MINGW_CMD" + script: + - make -f Makefile.ci ci-build@$IMAGE CONFIGURE="$MINGW-configure" - compiler: clang language: c os: osx @@ -37,43 +43,11 @@ matrix: script: /bin/sh -xc "$MACOS_CMD" -script: - - docker run - -v $(pwd):/build - -w /build - -e VIR_TEST_DEBUG="$VIR_TEST_DEBUG" - -e MINGW="$MINGW" - "quay.io/libvirt/buildenv-$IMAGE:master" - /bin/sh -xc "$DOCKER_CMD" - git: submodules: true env: global: - - VIR_TEST_DEBUG=1 - - LINUX_CMD=" - ./autogen.sh && - make -j3 syntax-check && - make -j3 distcheck || - ( - echo '=== LOG FILE(S) START ==='; - find -name test-suite.log | xargs cat; - echo '=== LOG FILE(S) END ==='; - exit 1 - ) - " - - MINGW_CMD=" - NOCONFIGURE=1 ./autogen.sh && - \$MINGW-configure && - make -j3 || - ( - echo '=== LOG FILE(S) START ==='; - find -name test-suite.log | xargs cat; - echo '=== LOG FILE(S) END ==='; - exit 1 - ) - " # We can't run 'distcheck' or 'syntax-check' because they fail on # macOS, but doing 'install' and 'dist' gives us some useful coverage - MACOS_CMD=" -- 2.20.1

On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote: [...]
+ script: + - make -f Makefile.ci ci-build@$IMAGE MAKE_ARGS="$MAKE_ARGS"
Because of the changes made to Makefile.ci since v3, you need to use CI_MAKE_ARGS="$MAKE_ARGS" here and...
+ script: + - make -f Makefile.ci ci-build@$IMAGE CONFIGURE="$MINGW-configure"
... CI_CONFIGURE="$MINGW-configure" here instead. With that fixed, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

Instead of running custom commands use the new declarative syntax for listing extra Homebrew packages. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 90a52c7f08..47757ccfc9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,15 @@ branches: except: - /^.*-maint$/ +addons: + homebrew: + update: true + packages: + - ccache + - rpcgen + - xz + - yajl + matrix: include: - services: @@ -51,8 +60,6 @@ env: # We can't run 'distcheck' or 'syntax-check' because they fail on # macOS, but doing 'install' and 'dist' gives us some useful coverage - MACOS_CMD=" - brew update && - brew install ccache rpcgen xz yajl && ./autogen.sh --prefix=\$(pwd)/install-root && make -j3 && make -j3 install && -- 2.20.1

On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote: [...]
+addons: + homebrew: + update: true + packages: + - ccache
Looking at https://docs.travis-ci.com/user/caching/#ccache-cache I'm pretty sure we're not taking advantage of ccache support at all... We might want to look into that. Either way, the commit itself is a straightforward (and correct) conversion, so Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

We are not running "make check" on macOS, so the commands to cat the test-suite.log are not useful. Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index 47757ccfc9..1528121dd3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -63,14 +63,7 @@ env: ./autogen.sh --prefix=\$(pwd)/install-root && make -j3 && make -j3 install && - make -j3 dist || - ( - echo '=== LOG FILE(S) START ==='; - find -name test-suite.log | xargs cat; - echo '=== LOG FILE(S) END ==='; - exit 1 - ) - " + make -j3 dist" notifications: irc: -- 2.20.1

On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote: [...]
./autogen.sh --prefix=\$(pwd)/install-root && make -j3 && make -j3 install && - make -j3 dist || - ( - echo '=== LOG FILE(S) START ==='; - find -name test-suite.log | xargs cat; - echo '=== LOG FILE(S) END ==='; - exit 1 - ) - " + make -j3 dist"
Woops, you should not be removing that double quote, as the entire script is stored inside an environment variable! Please make sure to fix that before pushing. -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Apr 10, 2019 at 01:03:32PM +0200, Andrea Bolognani wrote:
On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote: [...]
./autogen.sh --prefix=\$(pwd)/install-root && make -j3 && make -j3 install && - make -j3 dist || - ( - echo '=== LOG FILE(S) START ==='; - find -name test-suite.log | xargs cat; - echo '=== LOG FILE(S) END ==='; - exit 1 - ) - " + make -j3 dist"
Woops, you should not be removing that double quote, as the entire script is stored inside an environment variable! Please make sure to fix that before pushing.
I'm not sure I understand - I removed a " and I also added a " ? Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Thu, 2019-04-11 at 15:59 +0100, Daniel P. Berrangé wrote:
On Wed, Apr 10, 2019 at 01:03:32PM +0200, Andrea Bolognani wrote:
On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote: [...]
./autogen.sh --prefix=\$(pwd)/install-root && make -j3 && make -j3 install && - make -j3 dist || - ( - echo '=== LOG FILE(S) START ==='; - find -name test-suite.log | xargs cat; - echo '=== LOG FILE(S) END ==='; - exit 1 - ) - " + make -j3 dist"
Woops, you should not be removing that double quote, as the entire script is stored inside an environment variable! Please make sure to fix that before pushing.
I'm not sure I understand - I removed a " and I also added a " ?
You're right, your patch will work, I was just confused while looking at the diff. Please just leave the closing double quote on its own line, so that there's no ambiguity over what you're doing. -- Andrea Bolognani / Red Hat / Virtualization

Now that we don't have separate scripts defined for native and mingw builds, there is no point having one for macOS. It can just be inlined at the one place it is needed. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1528121dd3..6a9c98d407 100644 --- a/.travis.yml +++ b/.travis.yml @@ -50,21 +50,13 @@ matrix: env: - PATH="/usr/local/opt/gettext/bin:/usr/local/opt/ccache/libexec:/usr/local/opt/rpcgen/bin:$PATH" script: - /bin/sh -xc "$MACOS_CMD" + # We can't run 'distcheck' or 'syntax-check' because they fail on + # macOS, but doing 'install' and 'dist' gives us some useful coverage + - ./autogen.sh --prefix=$(pwd)/install-root && make -j3 && make -j3 install && make -j3 dist git: submodules: true -env: - global: - # We can't run 'distcheck' or 'syntax-check' because they fail on - # macOS, but doing 'install' and 'dist' gives us some useful coverage - - MACOS_CMD=" - ./autogen.sh --prefix=\$(pwd)/install-root && - make -j3 && - make -j3 install && - make -j3 dist" - notifications: irc: # The channel name "irc.oftc.net#virt" is encrypted against libvirt/libvirt -- 2.20.1

On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote:
Now that we don't have separate scripts defined for native and mingw builds, there is no point having one for macOS. It can just be inlined at the one place it is needed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .travis.yml | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)
Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- Makefile.ci | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Makefile.ci b/Makefile.ci index 984cd3c205..967846a673 100644 --- a/Makefile.ci +++ b/Makefile.ci @@ -203,6 +203,18 @@ ci-help: @echo " fedora-rawhide" @echo " ubuntu-18" @echo + @echo "Available cross-compiler container images:" + @echo + @echo " debian-[9,sid]-cross-aarch64" + @echo " debian-[9,sid]-cross-armv6l" + @echo " debian-[9,sid]-cross-armv7l" + @echo " debian-sid-cross-i686" + @echo " debian-[9,sid]-cross-mips64el" + @echo " debian-[9,sid]-cross-mips" + @echo " debian-[9,sid]-cross-mipsel" + @echo " debian-[9,sid]-cross-ppc64le" + @echo " debian-[9,sid]-cross-s390x" + @echo @echo "Available make variables:" @echo @echo " CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion" -- 2.20.1

On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote: [...]
+ @echo "Available cross-compiler container images:" + @echo + @echo " debian-[9,sid]-cross-aarch64"
All these should look either like debian-{9,sid}-cross-... (shell style, my personal preference) or debian-(9|sid)-cross-... (regex style): the style you've chosen here is not really something I've seen around, better to stick with something more common. With that addressed, and the patch squashed into 1/7, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

GitLab CI provides some shared build runners that use Docker containers. This resource can usefully run cross-compiled builds since all other CI build testing is currently x86 only, and Travis CI is already very busy testing native builds. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .gitlab-ci.yml | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 .gitlab-ci.yml diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml new file mode 100644 index 0000000000..bdacc4f6b8 --- /dev/null +++ b/.gitlab-ci.yml @@ -0,0 +1,74 @@ +.job_template: &job_definition + script: + - mkdir vpath + - cd vpath + - ../autogen.sh $LIBVIRT_CONFIGURE_OPTS + - make -j $(getconf _NPROCESSORS_ONLN) + +deb9crossaarch64: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-aarch64:master + +deb9crossarmv6l: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-armv6l:master + +deb9crossarmv7l: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-armv7l:master + +deb9crossmips64el: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-mips64el:master + +deb9crossmips: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-mips:master + +deb9crossmipsel: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-mipsel:master + +deb9crossppc64le: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-ppc64le:master + +deb9crosss390x: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-s390x:master + +debsidcrossaarch64: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-aarch64:master + +debsidcrossarmv6l: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-armv6l:master + +debsidcrossarmv7l: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-armv7l:master + +debsidcrossi686: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-i686:master + +debsidcrossmips64el: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-mips64el:master + +debsidcrossmips: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-mips:master + +debsidcrossmipsel: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-mipsel:master + +debsidcrossppc64le: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-ppc64le:master + +debsidcrosss390x: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-s390x:master -- 2.20.1

On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote:
GitLab CI provides some shared build runners that use Docker containers. This resource can usefully run cross-compiled builds since all other CI build testing is currently x86 only, and Travis CI is already very busy testing native builds.
Fair warning: this is my first time looking at GitLab CI :) [...]
+ script: + - mkdir vpath + - cd vpath + - ../autogen.sh $LIBVIRT_CONFIGURE_OPTS + - make -j $(getconf _NPROCESSORS_ONLN)
Using $LIBVIRT_CONFIGURE_OPTS will obviously not work, since the variable we bake in the containers is $CONFIGURE_OPTS. Though now that I think about it, I like the prefixed name better :) More generally, I dislike the fact that this is the same, but not quite the same, script as in Makefile.ci and .travis.yml... I assume we can't just use the ci-build targets here, the same way we do on Travis CI, because of how GitLab sets up their environment?
+deb9crossaarch64: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-aarch64:master
You have 17 jobs defined here... Does GitLab not limit the number of concurrent jobs? And if not, why are we even using Travis CI for anything but macOS builds? O:-) About the jobs themselves: the names should match the name of the image, eg. debian-9-cross-armv6l; and I assume there is no way to avoid having to repeat "quay.io/libvirt/buildenv-" and ":master" over and over again, is there? -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Apr 10, 2019 at 02:15:44PM +0200, Andrea Bolognani wrote:
On Wed, 2019-04-03 at 11:41 +0100, Daniel P. Berrangé wrote:
GitLab CI provides some shared build runners that use Docker containers. This resource can usefully run cross-compiled builds since all other CI build testing is currently x86 only, and Travis CI is already very busy testing native builds.
Fair warning: this is my first time looking at GitLab CI :)
[...]
+ script: + - mkdir vpath + - cd vpath + - ../autogen.sh $LIBVIRT_CONFIGURE_OPTS + - make -j $(getconf _NPROCESSORS_ONLN)
Using $LIBVIRT_CONFIGURE_OPTS will obviously not work, since the variable we bake in the containers is $CONFIGURE_OPTS. Though now that I think about it, I like the prefixed name better :)
Yes, of course it won't work and in fact I already had that fixed but somehow I squashed it into a different patch I had locally which wasn't posted in this series as posted.
More generally, I dislike the fact that this is the same, but not quite the same, script as in Makefile.ci and .travis.yml... I assume we can't just use the ci-build targets here, the same way we do on Travis CI, because of how GitLab sets up their environment?
Our Makefile.ci runs docker itself so obviously needs to be invoked from "host" context. We can do that in Travis since the build env runs in a throwaway VM where docker is available as an opt-in. With GitLab though, docker is a builtin feature of their CI so you must tell them upfront what image to run in and your build process will be always inside that. There's no "host" context in GitLab, you are always inside the container.
+deb9crossaarch64: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-aarch64:master
You have 17 jobs defined here... Does GitLab not limit the number of concurrent jobs? And if not, why are we even using Travis CI for anything but macOS builds? O:-)
I'm unclear on what the actual limit is, but it looks much more relaxed that under Travis. It looks to be determined largely by how many projects are currently competing for resources. IOW sometimes you might end up with 1 or 2 jobs running in parlallel. Other times you might get 10 or 15 jobs in parallel. Depends what is available. We should monitor how well GitLab works over a few months, and if it works better we can certainly move the non-macOS jobs off Travis.
About the jobs themselves: the names should match the name of the image, eg. debian-9-cross-armv6l; and I assume there is no way to avoid having to repeat "quay.io/libvirt/buildenv-" and ":master" over and over again, is there?
Not that I've seen so far. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Wed, Apr 03, 2019 at 11:41:41AM +0100, Daniel P. Berrangé wrote:
For a while QEMU has provided simple make rules for building QEMU inside standard docker container environments. This provides an equivalent mechanism for libvirt inspired by QEMU's.
Can we change this so that we don't mention docker everywhere? These are just containers and you can run them using other tools with no docker dependency. I know I can set an alias docker=podman and it will just work, but now that podman, buildah, skopeo and others work on my machine (even non-systemd one) I think it would be nice stop requiring "docker" if it is not needed. And to make Dan Walsh not cringe that much. Nice patch series, I hope I'll get to testing it sooner rather than later. Have a nice day Martin

On Wed, Apr 03, 2019 at 02:29:55PM +0200, Martin Kletzander wrote:
On Wed, Apr 03, 2019 at 11:41:41AM +0100, Daniel P. Berrangé wrote:
For a while QEMU has provided simple make rules for building QEMU inside standard docker container environments. This provides an equivalent mechanism for libvirt inspired by QEMU's.
Can we change this so that we don't mention docker everywhere? These are just containers and you can run them using other tools with no docker dependency. I know I can set an alias docker=podman and it will just work, but now that podman, buildah, skopeo and others work on my machine (even non-systemd one) I think it would be nice stop requiring "docker" if it is not needed. And to make Dan Walsh not cringe that much.
The patch series invokes docker from the Makefile and the CI systems we are targetting with this also use docker, so it is not wrong to refer to docker. Furthermore everyone knows what you mean when you refer to docker, while only a subset of people will have any idea what podman is. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Wed, Apr 03, 2019 at 01:34:04PM +0100, Daniel P. Berrangé wrote:
On Wed, Apr 03, 2019 at 02:29:55PM +0200, Martin Kletzander wrote:
On Wed, Apr 03, 2019 at 11:41:41AM +0100, Daniel P. Berrangé wrote:
For a while QEMU has provided simple make rules for building QEMU inside standard docker container environments. This provides an equivalent mechanism for libvirt inspired by QEMU's.
Can we change this so that we don't mention docker everywhere? These are just containers and you can run them using other tools with no docker dependency. I know I can set an alias docker=podman and it will just work, but now that podman, buildah, skopeo and others work on my machine (even non-systemd one) I think it would be nice stop requiring "docker" if it is not needed. And to make Dan Walsh not cringe that much.
The patch series invokes docker from the Makefile and the CI systems we are targetting with this also use docker, so it is not wrong to refer to docker. Furthermore everyone knows what you mean when you refer to docker, while only a subset of people will have any idea what podman is.
I'm suggesting s/docker containers/containers/ in commit messages and comments and maybe (but I can add that later) testing whether you have at least one of docker/podman installed in the makefile and using that instead of hardcoding the "docker" string in there. Feel free to ignore my suggestion, I just thought it would be nice if we were a bit more inclusive instead of using proprietary eponyms.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Wed, 2019-04-03 at 14:43 +0200, Martin Kletzander wrote:
On Wed, Apr 03, 2019 at 01:34:04PM +0100, Daniel P. Berrangé wrote:
On Wed, Apr 03, 2019 at 02:29:55PM +0200, Martin Kletzander wrote:
On Wed, Apr 03, 2019 at 11:41:41AM +0100, Daniel P. Berrangé wrote:
For a while QEMU has provided simple make rules for building QEMU inside standard docker container environments. This provides an equivalent mechanism for libvirt inspired by QEMU's.
Can we change this so that we don't mention docker everywhere? These are just containers and you can run them using other tools with no docker dependency. I know I can set an alias docker=podman and it will just work, but now that podman, buildah, skopeo and others work on my machine (even non-systemd one) I think it would be nice stop requiring "docker" if it is not needed. And to make Dan Walsh not cringe that much.
The patch series invokes docker from the Makefile and the CI systems we are targetting with this also use docker, so it is not wrong to refer to docker. Furthermore everyone knows what you mean when you refer to docker, while only a subset of people will have any idea what podman is.
I'm suggesting s/docker containers/containers/ in commit messages and comments and maybe (but I can add that later) testing whether you have at least one of docker/podman installed in the makefile and using that instead of hardcoding the "docker" string in there. Feel free to ignore my suggestion, I just thought it would be nice if we were a bit more inclusive instead of using proprietary eponyms.
FWIW I've already tried swapping Docker for Podman and it doesn't quite work yet. Once the current incarnation has been merged, I plan to figure out how to make it work with unpriviledged Podman containers (I'd rather not run priviledged containers on my machine) and along with that I'll also replace references to Dockers with the generic equivalent where it makes sense. As it is now, this only works with Docker so mentioning it seems perfectly reasonable :) -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
Daniel P. Berrangé
-
Martin Kletzander