travis.yml: use the same image as Murdock. #10251

jcarrano · 2018-10-25T10:53:50Z

Contribution description

It does not make sense to do the static tests in a diferent image than the compilation tests.

This commit updates the travis config to use the riotbuild image (the same as murdock). This should fix the issues with old doxygen versions reporting issues that are not, and also simplifies the CI maintenance.

Testing procedure

The only way to test it is to trigger Travis . Use #9819 (it is rebased on top of this).

Issues/PRs references

After #9819 everything will start failing the tests because the version of Doxygen in Trusty is from the stone age.

This PR would make #10237 no longer applicable.

miri64 · 2018-10-25T11:01:23Z

This PR would replace #10237.

You sure? That PR introduces lesscpy to travis, this PR switches Travis over to docker. The connection is not really clear to me.

jcarrano · 2018-10-25T11:02:59Z

That PR updates the version of lesscpy, this one switches to an image without lesscpy.

Anyways, the problems of the current Travis config go beyond lesscpy.

miri64 · 2018-10-25T11:04:12Z

Anyways, the problems of the current Travis config go beyond lesscpy.

Sure, I just wasn't sure if you accidentally referenced that PR because as I said: I don't see any connection to #10237.

jcarrano · 2018-10-25T11:04:53Z

~~Why have checks disappeared? I'm only seeing Murdock.~~ Edit: I should have validated the travis file before commiting (d'oh!).

I don't see any connection to #10237.

I'll update the text to make it clearer.

cladmi

I agree with going this direction but was also helping writing the PR so not sure I should being the one approving.
I did some inline remarks for the file.

So I let more others approve or decline this first.

.travis.yml

jcarrano · 2018-10-25T12:12:26Z

According to the Travis docs, build-essential is installed in all images, so I changed it to "c" since that should have the least amount of extra stuff. Now the Travis log has a lot of unnecessary data and the image doesn't seem very lightweight anyways.
Edit: forget about that, now it's using the minimal image.

miri64 · 2018-10-25T12:13:29Z

According to the Travis docs, build-essential is installed in all images, so I changed it to "c" since that should have the least amount of extra stuff. Now the Travis log has a lot of unnecessary data and the image doesn't seem very lightweight anyways.

See my remark in #10251 (comment)

cladmi · 2018-10-29T14:10:42Z

Is it now ok to review or do you still want to change things ?

cladmi · 2018-11-13T14:21:06Z

@aabadie can you have a look at this as you are maintaining the travis build ?

aabadie · 2018-11-13T15:08:53Z

In case of failure in Travis, the command would become "The command "docker run -a STDIN -a STDOUT -a STDERR --rm -u "$(id -u)" -v "${PWD}:/data/riotbuild" -v /etc/localtime:/etc/localtime:ro riot/riotbuild make static-test" exited with 2." which is not super nice.

But this is minor I think and I'm ok with the changes.

I still think switching to CircleCI (with #8729) would make things simpler, with, as a bonus, the possibility to access build artefacts (the generated documentation) from the build job webpage.

cladmi · 2018-11-13T15:21:54Z

We still get the real error message before, it is always the first error that should be looked:

makefiles/tests.inc.mk:3: recipe for target 'static-test' failed
make: *** [static-test] Error 1

The command "docker run -a STDIN -a STDOUT -a STDERR --rm -u "$(id -u)" -v "${PWD}:/data/riotbuild" -v /etc/localtime:/etc/localtime:ro riot/riotbuild make static-test" exited with 2.

It however shows that having a BUILD_IN_DOCKER=1 make static-test shortcut could make sense.

I still think switching to CircleCI (with #8729) would make things simpler, with, as a bonus, the possibility to access build artefacts (the generated documentation) from the build job webpage.

It is still a different change, where only the second proposed argument is still valid, and even for the second argument saving output could also be done by murdock, something I plan to actively push after #10038
Also here the build took 3:30 minutes compared to your announced 7 minutes in CircleCI.

aabadie · 2018-11-13T15:31:01Z

Also here the build took 3:30 minutes compared to your announced 7 minutes in CircleCI.

If you want to go this way, most of the time is spent by circleci by copying the generate documentation to the storage location. Otherwise, the build in itself is only 1m30s. Copying the artifacts takes time because of all the small files the doxygen documentation contains.

Having the documentation generated by Murdock will require extra disk space for storing the generated documentation on a per PR basis with the related infrastructure maintenance. Something not required with CircleCI.

kaspar030 · 2018-11-13T15:32:49Z

Having the documentation generated by Murdock will require extra disk space for storing the generated documentation on a per PR basis with the related infrastructure maintenance. Something not required with CircleCI.

Why not? Because we'd use public CircleCI servers?

aabadie · 2018-11-13T15:41:49Z

Why not? Because we'd use public CircleCI servers?

because RIOT is an open-source project. You have to be logged in CircleCI to access the project artifacts (I use my GitHub account there). From their documentation, it's rather unclear what is the limitation is size or time.
I know a couple of other OSS project using this feature without any trouble. It's pretty useful in fact and very low in terms of maintenance.

aabadie

Nothing special that needs to be changed in this PR. The static tests are passing, except the pr_check, because the git history needs squashing.

ACK, please squash @jcarrano !

It does not make sense to do the static tests in a diferent image than the compilation tests. This commit updates the travis config to use the riotbuild image (the same as murdock). This should fix the issues with old doxygen versions reporting issues that are not, and also simplify the CI maintainance.

miri64 · 2018-11-22T13:21:09Z

And go. I don't expect Codacy to report anything of interest for this PR.

jcarrano mentioned this pull request Oct 25, 2018

tools/doccheck: Detect when make doc fails to run. #9819

Merged

miri64 requested a review from aabadie October 25, 2018 11:01

miri64 added Area: CI Area: Continuous Integration of RIOT components Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Oct 25, 2018

jcarrano requested a review from cladmi October 25, 2018 11:56

cladmi reviewed Oct 25, 2018

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

.travis.yml Outdated Show resolved Hide resolved

jcarrano mentioned this pull request Oct 25, 2018

tools/ci: correcly report flake8 version. #10252

Merged

jcarrano added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Oct 25, 2018

cladmi mentioned this pull request Nov 13, 2018

travis.yml: Use lesscpy from pip. #10237

Closed

aabadie approved these changes Nov 21, 2018

View reviewed changes

jcarrano force-pushed the travis-use-riotdocker branch from c0cbaa0 to 8808096 Compare November 22, 2018 12:36

jcarrano added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Nov 22, 2018

miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 22, 2018

miri64 merged commit 6627e40 into RIOT-OS:master Nov 22, 2018

aabadie added this to the Release 2019.01 milestone Dec 2, 2018

miri64 mentioned this pull request Jan 14, 2019

.travis.yml: provide base branch to static tests #10765

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

travis.yml: use the same image as Murdock. #10251

travis.yml: use the same image as Murdock. #10251

jcarrano commented Oct 25, 2018 •

edited

Loading

miri64 commented Oct 25, 2018

jcarrano commented Oct 25, 2018

miri64 commented Oct 25, 2018

jcarrano commented Oct 25, 2018 •

edited

Loading

cladmi left a comment

jcarrano commented Oct 25, 2018 •

edited

Loading

miri64 commented Oct 25, 2018 •

edited

Loading

cladmi commented Oct 29, 2018

cladmi commented Nov 13, 2018

aabadie commented Nov 13, 2018

cladmi commented Nov 13, 2018

aabadie commented Nov 13, 2018

kaspar030 commented Nov 13, 2018

aabadie commented Nov 13, 2018

aabadie left a comment

miri64 commented Nov 22, 2018

travis.yml: use the same image as Murdock. #10251

travis.yml: use the same image as Murdock. #10251

Conversation

jcarrano commented Oct 25, 2018 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

miri64 commented Oct 25, 2018

jcarrano commented Oct 25, 2018

miri64 commented Oct 25, 2018

jcarrano commented Oct 25, 2018 • edited Loading

cladmi left a comment

Choose a reason for hiding this comment

jcarrano commented Oct 25, 2018 • edited Loading

miri64 commented Oct 25, 2018 • edited Loading

cladmi commented Oct 29, 2018

cladmi commented Nov 13, 2018

aabadie commented Nov 13, 2018

cladmi commented Nov 13, 2018

aabadie commented Nov 13, 2018

kaspar030 commented Nov 13, 2018

aabadie commented Nov 13, 2018

aabadie left a comment

Choose a reason for hiding this comment

miri64 commented Nov 22, 2018

jcarrano commented Oct 25, 2018 •

edited

Loading

jcarrano commented Oct 25, 2018 •

edited

Loading

jcarrano commented Oct 25, 2018 •

edited

Loading

miri64 commented Oct 25, 2018 •

edited

Loading