Travis: Build tests on Ubuntu 18.04 with docker #13215

ken2812221 · 2018-05-11T10:18:04Z

Compile and run tests on Ubuntu 18.04 docker.

maflcko · 2018-05-11T14:42:14Z

I can't see any downside, so Concept ACK.

promag · 2018-05-11T14:45:10Z

Can you elaborate the motivation behind this change?

promag

Concept ACK.

Maybe it should also cache docker layers? Similar to http://circleci.com/docs/1.0/docker/#caching-docker-layers.

Edit: seems to take around 30 to 40 seconds in pulling images and installing extra packages.

promag · 2018-05-11T14:46:40Z

.travis.yml

Remove double space.

maflcko · 2018-05-11T14:54:25Z

@promag I presume the motivation is that gitian will (soon) use bionic, so having a similar configuration run on travis isn't that bad. Also, it untangles us a bit from the travis environments and changes, as we use the vanilla bionic docker. So reproducing issues could potentially be easier or harder because it uses docker ...

I have no strong opinion based on that, but since the new qt "requires" bionic for windows cross builds, I am slightly in favor of this change.

ken2812221 · 2018-05-11T14:57:00Z

Can you elaborate the motivation behind this change?

We will bump our gitian build system to Ubuntu 18.04 since 0.17, so I think we might need to have the same build environment as the gitian build. Also, once we've merged the back-compat code, we could run the executable both on 14.04 and 18.04 to test if the back-compat code would work.

jamesob · 2018-05-11T14:59:47Z

This will likely fix the build on #12873 by (inadvertently) using a more recent version of SSL that doesn't cause the deadlock warning system to flare up.

The job duration times don't seem adversely affected by this change.

Concept ACK

maflcko · 2018-05-11T15:15:19Z

Please remove the [WIP] from the commit title when this is ready for review.

jamesob · 2018-05-11T15:44:45Z

.travis.yml

Small thing, but could we add

- DOCKER_EXEC="docker exec ${DOCKER_ID}"

to cut down on some of the boilerplate below?

jamesob · 2018-05-11T16:38:41Z

utACK 9e7924c

ken2812221 · 2018-05-12T00:36:41Z

Maybe it should also cache docker layers? Similar to http://circleci.com/docs/1.0/docker/#caching-docker-layers.

I've tested this, docker save took more time than apt install, so we don't need to cache it.

promag · 2018-05-12T09:41:57Z

@ken2812221 indeed in travis it's fast.

utACK 9e7924c.

laanwj · 2018-05-14T13:54:18Z

utACK 9e7924c0cb0995f8df6cca5d04b576e5b56038a9

maflcko · 2018-05-17T01:36:02Z

.travis.yml

nit: I'd prefer if we only write the environment variables that are specified in the yaml (global env and job env). Currently this also exports a lot of travis specific junk, which shouldn't be necessary. No strong opinion, though, as I am not aware how to achieve this.

maflcko · 2018-05-17T01:40:35Z

.travis.yml

nits:

Please replace the -v with the more verbose --mount type=bind,src=....,dst=...., which is suggested in the docker docs

You overwrite the CCACHE_DIR some lines above, but don't use that value here. Any reason for that?

Please replace the privliledged flag with just --cap-add SYS_ADMIN and add a comment why this is required or make it specific to the windows build.

maflcko · 2018-05-17T02:46:42Z

.travis.yml

This fails, since it is executed on the host, not via DOCKER_EXEC.

maflcko · 2018-05-17T02:47:48Z

Tested ACK 9e7924c0cb0995f8df6cca5d04b576e5b56038a9 (after fixing the failure)

Also left a couple of nits in the process, you can safely ignore them.

maflcko · 2018-05-17T03:06:57Z

.travis.yml

I assume BOOST_TEST_RANDOM and DISPLAY needs to be passed down as well?

maflcko · 2018-05-17T03:35:25Z

re-ACK 09b26751e0

theuni · 2018-05-17T03:55:38Z

Playing around with this now.

theuni · 2018-05-17T05:46:47Z

.travis.yml

Why is this needed?

In order to set wine-binfmt

theuni · 2018-05-17T06:02:29Z

.travis.yml

Is this still needed? IIRC it was an artifact of needing a non-default wine.

Yes, this is needed. wine32 is only available on i386 platform. Here is the link: https://packages.ubuntu.com/bionic/wine32

theuni · 2018-05-17T06:14:11Z

.travis.yml

This dropped the "travis_wait 50", but I guess we're ok without it now?

travis_wait is incompitable with docker. This is a known issue.

I think we may not need this travis_wait anymore, due to several improvements to the unit tests' runtimes.

@sipa is correct. See #12970 (comment) for background. The current run time (on the windows jobs) in this pull:

i686-w64-mingw32: 245.895077045 s
x86_64-w64-mingw32: 95.965838938 s

theuni · 2018-05-17T06:33:44Z

Thanks for addressing my questions. utACK.

@laanwj I'd like to have a chance to look at the qt/gitian changes as well. Mind giving me until the end of this week to get to them? Ideally we'd get all of those in at once so everything remains in sync.

maflcko · 2018-05-17T16:33:32Z

.travis.yml

I incorrectly assumed that apt is an alias for apt-get and asked you to change it. This was wrong, so feel free to change it back to apt-get. Sorry.

ken2812221 · 2018-05-17T16:44:56Z

Update: also expose CONFIG_SHELL into docker

jamesob · 2018-05-21T15:12:00Z

re-utACK 67cfb45

maflcko · 2018-05-21T15:26:37Z

.travis.yml

nit: Could reuse CONFIG_SHELL as below and skip exporting USE_SHELL or is there a reason to have two separate env vars with the same value?

maflcko · 2018-05-21T15:27:26Z

re-ACK 67cfb4537a

jnewbery

Concept ACK.

Build 4 here: https://travis-ci.org/bitcoin/bitcoin/jobs/380317076 took 36 minutes, which seems to be much longer than normal. I don't know whether that's caused by the changes in this PR or whether it's just natural variance.

ken2812221 · 2018-05-21T17:02:55Z

@jnewbery Because I just modify the enviroment variable on that one and travis read caches based on enviroment variable. So it needs to recompile everything.

jnewbery · 2018-05-21T17:35:24Z

.travis.yml

Can we remove this now?

I think we mirror the pwd for the travis vm and the docker container. Otherwise a cat config.log on the travis vm wouldn't work, I presume.

Oh yeah, it seems this one can be removed. Though, no harm in leaving it in, I guess.

No, cat config.log below would not work then.

ah, makes sense. Thanks

ken2812221 · 2018-05-24T05:29:04Z

How about add DOCKER_EXEC () { docker exec $DOCKER_ID bash -c "cd $PWD && $*"; } as a bash funtion, so we don't need to cd all the time.

maflcko · 2018-05-24T21:46:23Z

re-utACK a182c9d

maflcko · 2018-05-24T21:48:58Z

.travis.yml

bash: CONFIG_SHELL: command not found

maflcko · 2018-05-29T16:51:43Z

re-utACK 59e9688

jamesob

re-utACK 59e9688

jamesob

re-utACK 59e9688

59e9688 Travis: Build tests on Ubuntu 18.04 with docker (Chun Kuan Lee) Pull request description: Compile and run tests on Ubuntu 18.04 docker. Tree-SHA512: 4ae5f0cf666abeff2f3e3f541d33e5c76970c5129e60d0299317d73621fafa6f0f1c6cfe7a7d1089200f29ecb1a0a61a22bc116474eb5226282939e0beb37cb8

fanquake added the Tests label May 11, 2018

ken2812221 changed the title ~~[WIP] Travis: Build tests on Ubuntu 18.04 with docker~~ Travis: Build tests on Ubuntu 18.04 with docker May 11, 2018

promag reviewed May 11, 2018

View reviewed changes

jamesob reviewed May 11, 2018

View reviewed changes

maflcko mentioned this pull request May 14, 2018

depends: Upgrade Qt to 5.9.6 #12971

Merged

laanwj requested a review from theuni May 14, 2018 13:55

maflcko reviewed May 17, 2018

View reviewed changes

.travis.yml Outdated

Copy link

Member

maflcko May 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails, since it is executed on the host, not via DOCKER_EXEC.

maflcko reviewed May 17, 2018

View reviewed changes

.travis.yml Outdated

Copy link

Member

maflcko May 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume BOOST_TEST_RANDOM and DISPLAY needs to be passed down as well?

theuni reviewed May 17, 2018

View reviewed changes

maflcko reviewed May 17, 2018

View reviewed changes

maflcko mentioned this pull request May 21, 2018

[qa] Tests occasionally raise http.client.RemoteDisconnected or BadStatusLine #11777

Closed

maflcko reviewed May 21, 2018

View reviewed changes

jnewbery reviewed May 21, 2018

View reviewed changes

maflcko reviewed May 24, 2018

View reviewed changes

Travis: Build tests on Ubuntu 18.04 with docker

59e9688

jamesob approved these changes May 29, 2018

View reviewed changes

laanwj merged commit 59e9688 into bitcoin:master May 29, 2018

ken2812221 deleted the patch-1 branch May 29, 2018 18:07

sickpig mentioned this pull request Jan 11, 2019

Configure travis to use an Ubuntu 18.04 docker container BitcoinUnlimited/BitcoinUnlimited#1450

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Travis: Build tests on Ubuntu 18.04 with docker #13215

Travis: Build tests on Ubuntu 18.04 with docker #13215

Uh oh!

Conversation

ken2812221 commented May 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented May 11, 2018

Uh oh!

promag commented May 11, 2018

Uh oh!

promag left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented May 11, 2018

Uh oh!

ken2812221 commented May 11, 2018

Uh oh!

jamesob commented May 11, 2018

Uh oh!

maflcko commented May 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesob commented May 11, 2018

Uh oh!

ken2812221 commented May 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented May 12, 2018

Uh oh!

laanwj commented May 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented May 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented May 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theuni commented May 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theuni commented May 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ken2812221 commented May 17, 2018

Uh oh!

jamesob commented May 21, 2018

ken2812221 commented May 11, 2018 •

edited

Loading

promag left a comment •

edited

Loading

ken2812221 commented May 12, 2018 •

edited

Loading

maflcko commented May 17, 2018 •

edited

Loading

ken2812221 commented May 24, 2018 •

edited

Loading