Re: [libvirt] [PATCH v2] travis: switch to using Ubuntu 16.04 and 18.04
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