On Wed, 2018-06-06 at 17:24 +0100, Daniel P. Berrangé wrote: > The container images provided by Travis only support Ubuntu 14.04, > however, Travis has ability to run docker, which allows the build > script to use arbitrary OS images. This takes advantage of that to > convert the build over to Ubuntu 16.04 and 18.04 > > This is using the official Ubuntu provided images and installing > extra build deps required, as we previously did with Travis container > images. As mentioned during the review of v1, this bit: > With the switch to Docker though, this can be improved, by > building custom Docker images with all the deps pre-installed which > will cut down build time. This can be driven from the package lists > in libvirt-jenkins-ci repo, to remove the duplication. This work > for future improvement though, this just does the minimal conversion > to match what we already do, but with newer distro. doesn't belong to the commit message in my opinion. [...] > + - services: > + - docker > env: > + - IMAGE_NAME=ubuntu:18.04 This can be just IMAGE or even OS. IMAGE is probably better because it sorts very early, thus ensuring it will be displayed prominently in the build summary. > + - PYTHON=python3 Since we're not actually passing this to configure after all (and neither could we, due to the fact that $PYTHON needs to be a full path and we set the variable outside of the container where Python could be installed in a different location than inside it) it doesn't really make a lot of sense to have this. Let's just get rid of it and rely on configure picking up python3 on modern operating systems and python2 otherwise, same way we handle the issue on CentOS CI. [...] > + before_script: I thought we agreed on using script instead of before_script here? > + - docker run > + --privileged > + -v $(pwd):/build > + -w /build > + -e VIR_TEST_DEBUG="$VIR_TEST_DEBUG" > + -e PACKAGES="$PACKAGES" > + -e DISTCHECK_FLAGS="$DISTCHECK_CONFIGURE_FLAGS" We should keep the names of variables consistent between the host and container environment. > + $IMAGE_NAME $IMAGE_NAME (or rather $IMAGE) should be quoted. > + /bin/sh -c "$DOCKER_CMD" Something I didn't spot the first time around: using /bin/sh -xc "$DOCKER_CMD" here will be very helpful to figure out exactly what is happening during the build, so please add that. [...] > + - services: > + - docker > + env: > + - IMAGE_NAME=ubuntu:16.04 > + - PYTHON=python2 > + - DISTCHECK_CONFIGURE_FLAGS="--with-init-script=upstart" > + before_script: > + - docker pull berrange/test You forgot to delete this one :) [...] > - brew update > - brew upgrade > - brew install python ccache rpcgen yajl > + before_script: > + - ./autogen.sh --prefix=$(pwd)/install-root Another bit I missed the first time around (sorry): before these changes, we took advantage of the before_script/script split to avoid typing out the autogen.sh call twice, but that's no longer relevant now, so you can simplify things by folding it into script. [...] > env: > global: > - VIR_TEST_DEBUG=1 > + - DOCKER_CMD=" > + apt-get update && > + apt-get install -y \$PACKAGES && > + ./autogen.sh --prefix=$(pwd)/install-root && You didn't escape '$(pwd)' correctly here, so it will be expanded in the host rather than inside the container. That said, we don't actually call 'make install' for builds happening inside Docker since 'make distcheck' works there, so you might as well drop --prefix altogether. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
