Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: use Xenial and gcc 6 on Travis #26720

Closed
wants to merge 3 commits into from

Conversation

richardlau
Copy link
Member

@richardlau richardlau commented Mar 17, 2019

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@richardlau richardlau added wip Issues and PRs that are still a work in progress. semver-major PRs that contain breaking changes and should be released in the next major version. labels Mar 17, 2019
@targos
Copy link
Member

targos commented Mar 17, 2019

I don't think it has to be semver-major. Travis CI results are indicative and we don't rely on them to land PRs.

@addaleax addaleax removed the semver-major PRs that contain breaking changes and should be released in the next major version. label Mar 17, 2019
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

I agree that this isn’t semver-major. You can add dont-land-on labels, if you want, imo.

@richardlau
Copy link
Member Author

I don't think it has to be semver-major. Travis CI results are indicative and we don't rely on them to land PRs.

👍 I was being overly cautious (it's easier to downgrade semverness than increase it).

@richardlau
Copy link
Member Author

Part of the motivation here was to flush out any unexpectedness on Xenial in Travis CI on the assumption (I've not seen any communication from Travis about this) that Trusty gets sunset when it goes End-of-Life in April and I figured I might as well bump the compiler version based on #26714 while I was here.

@richardlau richardlau added build Issues and PRs related to build files or the CI. and removed wip Issues and PRs that are still a work in progress. labels Mar 17, 2019
@richardlau richardlau changed the title WIP build: use Xenial and gcc 6 on Travis build: use Xenial and gcc 6 on Travis Mar 17, 2019
@richardlau
Copy link
Member Author

richardlau commented Mar 17, 2019

The Xenial images on Travis also have gcc-5.4.0 by default. We could potentially speed up the builds on Node.js < 12 by using the default on Xenial and not apt installing another gcc version.

@richardlau richardlau added the wip Issues and PRs that are still a work in progress. label Mar 19, 2019
@richardlau
Copy link
Member Author

richardlau commented Mar 19, 2019

I've restarted the Travis build for this PR a few times but it's always timed out, which is a problem. I don't know yet if it's either a) the Xenial hosts are slow or b) gcc 6 is slow or c) since this is a compiler level change nothing is ccached making the build slow (if this is the case I'm not sure how we fix as it sort of requires a successful build to populate the ccache) or d) some combination of all the above.

My plan is to break up this PR so we test changing one thing at a time (i.e. either the Xenial change on its own or the gcc 6 update on its own) and see if we can work out which one is the problem.

@richardlau
Copy link
Member Author

(As an FYI the Travis timeout for jobs is ~50mins.)

@targos
Copy link
Member

targos commented Mar 19, 2019

It's probably c) caching issue.

When we push fixes to canary-base, Travis always fails in a timeout. Latest example: https://travis-ci.com/nodejs/node/jobs/186092202

@richardlau
Copy link
Member Author

It's probably c) caching issue.

When we push fixes to canary-base, Travis always fails in a timeout. Latest example: https://travis-ci.com/nodejs/node/jobs/186092202

Do the timeouts ever fix themselves? Or is Travis just ignored for canary-base?

@targos
Copy link
Member

targos commented Mar 19, 2019

It's probably c) caching issue.
When we push fixes to canary-base, Travis always fails in a timeout. Latest example: https://travis-ci.com/nodejs/node/jobs/186092202

Do the timeouts ever fix themselves? Or is Travis just ignored for canary-base?

I don't think they ever do, because each V8 update usually requires to recompile everything.

@richardlau richardlau force-pushed the travis12 branch 2 times, most recently from bb4c0e7 to d9bbf0c Compare March 21, 2019 03:49
@richardlau
Copy link
Member Author

I temporarily commented out the script to run tests (so the Travis job just built the binaries) and the build completed in 45 mins: https://travis-ci.com/nodejs/node/jobs/186568593

Uncommented the script and restarted the Travis build to see if it now picks up the ccache and is faster/completes: https://travis-ci.com/nodejs/node/jobs/186560878

@richardlau richardlau removed the wip Issues and PRs that are still a work in progress. label Mar 21, 2019
@richardlau
Copy link
Member Author

Build completed in 14mins once the ccache was created (by a previous build that didn't run tests to avoid the timeout). We may need to do a similar trick when landing this.

We're hitting the Travis job timeout of 50mins if built with a new
compiler (as there is no ccache). Temporarily disable the running
of tests so the Travis job can complete within the timeout and
populate the ccache.
Restore running tests on Travis once the ccache is populated.
richardlau added a commit that referenced this pull request Mar 22, 2019
Restore running tests on Travis once the ccache is populated.

PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@richardlau
Copy link
Member Author

https://travis-ci.com/nodejs/node/jobs/186890380 succeeded (:tada:) so we should now have a ccache.

Landed db40b4a. Travis: https://travis-ci.com/nodejs/node/jobs/186898338

@richardlau
Copy link
Member Author

We're all good now. Let's hope nothing happens to the ccache 🤞.

Landed in ecf98b0...db40b4a.

@richardlau richardlau closed this Mar 22, 2019
targos pushed a commit to targos/node that referenced this pull request Mar 27, 2019
PR-URL: nodejs#26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Mar 27, 2019
We're hitting the Travis job timeout of 50mins if built with a new
compiler (as there is no ccache). Temporarily disable the running
of tests so the Travis job can complete within the timeout and
populate the ccache.

PR-URL: nodejs#26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Mar 27, 2019
Restore running tests on Travis once the ccache is populated.

PR-URL: nodejs#26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2019
PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2019
We're hitting the Travis job timeout of 50mins if built with a new
compiler (as there is no ccache). Temporarily disable the running
of tests so the Travis job can complete within the timeout and
populate the ccache.

PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2019
Restore running tests on Travis once the ccache is populated.

PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@richardlau richardlau mentioned this pull request Mar 28, 2019
4 tasks
BethGriggs pushed a commit that referenced this pull request Apr 17, 2019
PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 17, 2019
We're hitting the Travis job timeout of 50mins if built with a new
compiler (as there is no ccache). Temporarily disable the running
of tests so the Travis job can complete within the timeout and
populate the ccache.

PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 17, 2019
Restore running tests on Travis once the ccache is populated.

PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@BethGriggs BethGriggs mentioned this pull request May 1, 2019
MylesBorins pushed a commit that referenced this pull request May 16, 2019
PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
We're hitting the Travis job timeout of 50mins if built with a new
compiler (as there is no ccache). Temporarily disable the running
of tests so the Travis job can complete within the timeout and
populate the ccache.

PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
Restore running tests on Travis once the ccache is populated.

PR-URL: #26720
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants