← Back to team overview

widelands-dev team mailing list archive

[Merge] lp:~widelands-dev/widelands/travis-container into lp:widelands

 

The proposal to merge lp:~widelands-dev/widelands/travis-container into lp:widelands has been updated.

Description changed to:

Right, now that things are working, this is now ready for review.

The starting point for this MP was that I read about travis running in containers [1]. This has some advantages for instance responding quicker to new commits and running on more powerful hardware. One hard requirement for running in containers is that the build cannot use sudo, though. More on this later.

One thing I discovered while reading about this, was the functionality to add repos and packages [2] in a more declarative way. IMO this simplifies the setup for how additional repos are added and build dependencies installed, including removing the need for looping until commands succeed and manually running update before installing things. Hopefully this makes it easier to see which repos and packages are in use, and make it easier when updating/adding something. Note that since the compiler varies based on the build, that is still installed manually in the script. I'll come back to this.

As mentioned earlier, I've reduced the sudo usage somewhat, but I think it will be hard to get completely rid of. There's two main issues; pip install and installing compilers. 

Pip
`pip install sphinx` is easier to run as sudo, since it installs the binary in a place where it is instantly available on the PATH. We could work around it by using ~/cache/python/path/to/sphinx when calling it, but that seems rather hacky. Plus I don't know if it might attempt to use other things it expects to be present on the path.

Compilers
The second is the main blocker though. When installing a compiler, we do this based on the type (GCC vs clang) and version number which is set up in environment variables in the build matrix. I haven't really experimented with it (so I might be wrong), but I don't know if these environment variables are set when addons.apt is called. It would also require to add build dependencies akin to:
- if($GCC) g++-$GCC_VERSION 
- if($CLANG) clang++-$CLANG_VERSION 
I'm not sure if that would be better than keeping it in the script like it is today. 

So I haven't pursued sudo-less, container-based travis runs further. The changes here have simplified the script quite a bit, but I am not sure if removing more use of sudo would improve the situation. Plus there's the osx-branch which adds new use of sudo and I don't know if that would block the linux builds from running in containers... :/


There's one possible negative thing about these changes though, and that is that now all additional repos are always added, it is possible to mix packages from multiple sources. I haven't figured out why yet, but have seen that the clang builds (see for instance one of [3]) also install gcc-7-base while installing the compiler. In the past, when only one additional repo was added per build, gcc-7-packages wouldn't be available and an older package would be installed instead. I'm not sure about the consequences here, but it could be the case that clang backed by GCC 7 gives a different result than clang backed by GCC 4.x. I'm not sure how worried I should be about this though :/


Just a quick headsup: I might not be able to respond to comments here the next couple of days.

[1] https://docs.travis-ci.com/user/migrating-from-legacy/
[2] https://docs.travis-ci.com/user/migrating-from-legacy/#Adding-APT-Sources
[3] https://travis-ci.org/widelands/widelands/builds/254399392

-- Original description

WIP, sorry for spamming the list of merge proposals, but this seems like the easiest way to get travis to build my branches.

Should be possible to tell travis about which additional repos and packages we need which means we can skip doing it manually as part of the script.

No, I don't know how this will add up with the osx branch, I guess time will show. Oh, and due to the llvm repos I doubt we can get rid of sudo completely :/ Will add details after more experimentation.

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/travis-container/+merge/327492
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/travis-container into lp:widelands.


References