Re: [libvirt] [PATCH] travis: switch to using Ubuntu 16.04 and 18.04

On Tue, 2018-06-05 at 18:47 +0100, Daniel P. Berrangé wrote: Neat! I was not aware that was possible. TIL :) Everything after this doesn't belong in the commit message IMHO. We will probably want to do that, since installing packages takes quite a bit of time and using Docker like this apparently causes jobs to serialize, which makes shaving down time all the more important. I'm wondering if using Ubuntu 16.04 and 18.04 as base image is the best option, however. Pavel is trying to get more hardware assigned to us in the CentOS CI environment, and assuming that's successful we will get those distributions added there as well for full-stack testing; Travis CI is more useful for developers to smoke test their patch series before posting, however, and with that in mind I think it would be much better to run the build on the latest CentOS, Fedora, Debian and Ubuntu to give reasonable coverage, leaving up to CentOS CI to catch issues related to interactions with other components or compatibility with older distros after merge. [...] Not sure 'before_install' is the best choice here, I would probably have gone with 'script' instead. On the other hand, it seems like the distinction between job phases is fairly blurry, so it probably doesn't matter after all. I'm pretty sure we don't want this :) Is '--privileged' really needed here? I'm not familiar enough with Docker, but it feels like we shouldn't need it. Feel free to point out exactly why I'm wrong :) Please use $(pwd) instead of `pwd`. It would be great to have the base image name in the environment as well, so that in case the build fails on only one of the Ubuntu versions you could easily tell from a quick look at the build summary which one that is. [...] You got rid of this variable entirely. I think we won't lose any coverage because configure will naturally pick python2 on 16.04 and python3 on 18.04 (I can't check because Travis CI decided to only keep around a truncated log of my test run), but it was kinda nice to have that explicitly in the summary. As long as you can confirm the above is correct and no coverage is lost, though, I'm okay with the removal. You copied the wrong line here, we're going to test upstart twice. I don't see a good reason to change the variable name, however: let's just stick with matching the name expected by distcheck itself. [...] You lost the ccache setting. That's probably okay because we install the package and set up PATH explicitly anyway. [...] This is simpler than the previous approach. I like it :) [...] Use --prefix=\$(pwd)/install-root here for consistency with the macOS part. No space between '-j' and '3'. This won't work if we have more than a single flag, will it? You lost the comment about this list being sorted alphabetically. [...] Put the closing quote on a separate line for neater future diffs. -- Andrea Bolognani / Red Hat / Virtualization