FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

build,meta: bump gcc on travis by refack · Pull Request #23778 · nodejs/node · GitHub

/ node Public

build,meta: bump gcc on travis - #23778

Merged
refack merged 1 commit into
nodejs:masterfrom
refack:bump-gcc-on-tavis
Oct 24, 2018
Merged

build,meta: bump gcc on travis#23778
refack merged 1 commit into
nodejs:masterfrom
refack:bump-gcc-on-tavis

Conversation

refack commented Oct 20, 2018
edited
Loading

Copy link
Copy Markdown
Contributor

The version of clang provided in the Travis linux image uses
libstdc++4.8 whice is below our minimal supported version.

Switching to make test -j1 is to avoid races during the test cycle
causes by the main target being "unstable", that is it always builds
some files, and relinks the binary, which is used by the test procedure.

Details
../src/node_crypto.cc:3641:29: error: no matching function for call to 'get'
    return sign->CheckThrow(std::get<Error>(ret));
                            ^~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/utility:142:5: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Int'
    get(std::pair<_Tp1, _Tp2>& __in) noexcept
    ^
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/utility:147:5: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Int'
    get(std::pair<_Tp1, _Tp2>&& __in) noexcept
    ^
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/utility:152:5: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Int'
    get(const std::pair<_Tp1, _Tp2>& __in) noexcept
    ^
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/array:268:5: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Int'
    get(array<_Tp, _Nm>& __arr) noexcept
    ^
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/array:277:5: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Int'
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

refack added build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. regression Issues related to regressions. fast-track PRs that do not need to wait for 48 hours to land. labels Oct 20, 2018

Copy link
Copy Markdown
Collaborator

refack commented Oct 20, 2018

Copy link
Copy Markdown
Contributor Author

/CC @nodejs/build-files @nodejs/testing

Comment thread .travis.yml Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Refs: #23733

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I’d prefer to undo this unless it’s actually causing trouble on Travis – I don’t think we’ve seen this so far.

refack Oct 20, 2018
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It makes the tests consistent with JEnkins, but I'll do it in another PR

Comment thread .travis.yml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I’d prefer to undo this unless it’s actually causing trouble on Travis – I don’t think we’ve seen this so far.

Comment thread .travis.yml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe set these using export CC='ccache gcc-4.9' && CXX='ccache g++-4.9' on the first line of install:, and then leave these lines as they are?

Also, I’d prefer to keep ./configure – firstly, the .travis.yml of a project is sometimes used by devs to figure out how to build it, because it’s usually much more to-the-point than build instructions, and secondly that way we simply test more code than without it (namely the contents of configure).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm suspicious it's not behaving as we expect (hence it's using GCC over clang), so I'd rather has it explicit (like in Jenkins).

Ack on the ./configure

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Testing export in install

Copy link
Copy Markdown
Member

I'm suspicious it's not behaving as we expect (hence it's using GCC over clang), so I'd rather has it explicit (like in Jenkins).

From a quick google search, it seems like the error message is a clang one.

refack commented Oct 20, 2018

Copy link
Copy Markdown
Contributor Author

From a quick google search, it seems like the error message is a clang one.

So it might be just an stdlibc++ issue?

Copy link
Copy Markdown
Member

@refack My guess is that it might be the gcc-provided libstdc++ 4.8 not playing along well with clang? Could be totally wrong on that one, though.

(I also don’t remember the reason for picking clang in the first place, tbh.)

refack commented Oct 20, 2018

Copy link
Copy Markdown
Contributor Author

I also changed the output generator from

make -j2 V=
make -C out BUILDTYPE=Release V=
make[1]: Entering directory `/home/travis/build/nodejs/node/out'
  CXX(host) /home/travis/build/nodejs/node/out/Release/obj.host/v8_libbase/deps/v8/src/base/bits.o
  CXX(host) /home/travis/build/nodejs/node/out/Release/obj.host/v8_libbase/deps/v8/src/base/cpu.o
  CXX(host) /home/travis/build/nodejs/node/out/Release/obj.host/v8_libbase/deps/v8/src/base/debug/stack_trace.o
  CXX(host) /home/travis/build/nodejs/node/out/Release/obj.host/v8_libbase/deps/v8/src/base/division-by-constant.o
  CXX(host) /home/travis/build/nodejs/node/out/Release/obj.host/v8_libbase/deps/v8/src/base/file-utils.o

to

$ make -j2 V=1 | sed -e "s/'.*\$//" -e "s|$PWD||g"
make -C out BUILDTYPE=Release V=1
make[1]: Entering directory `/out
  ccache g++-4.9 -o /out/Release/obj.host/v8_libbase/deps/v8/src/base/bits.o ../deps/v8/src/base/bits.cc 
  ccache g++-4.9 -o /out/Release/obj.host/v8_libbase/deps/v8/src/base/cpu.o ../deps/v8/src/base/cpu.cc 
  ccache g++-4.9 -o /out/Release/obj.host/v8_libbase/deps/v8/src/base/debug/stack_trace.o ../deps/v8/src/base/debug/stack_trace.cc 

(this is planned to change in Jenkins as well nodejs/build#1517)

Copy link
Copy Markdown
Member

@refack One thing I’m noticing here is that the extra apt install took about a minute in https://travis-ci.com/nodejs/node/jobs/153110829 … not the end of the world, but something we might want to consider?

Comment thread .travis.yml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It seems nodejs/build#1517 is still being discussed. I'm not a fan of this change, can we revert it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

ack

refack force-pushed the bump-gcc-on-tavis branch from 5310a62 to aeae42a Compare October 20, 2018 13:33

refack commented Oct 22, 2018

Copy link
Copy Markdown
Contributor Author

One thing I’m noticing here is that the extra apt install took about a minute in travis-ci.com/nodejs/node/jobs/153110829

But for some reason total was 13m
and other job took 14m

While currently the jobs (using clang) take 15m+

refack force-pushed the bump-gcc-on-tavis branch from aeae42a to 9e1081f Compare October 23, 2018 00:09
Comment thread .travis.yml Outdated
refack force-pushed the bump-gcc-on-tavis branch from 9e1081f to 3c9fecb Compare October 23, 2018 02:44
addaleax removed fast-track PRs that do not need to wait for 48 hours to land. meta Issues and PRs related to the general management of the project. labels Oct 24, 2018
Comment thread .travis.yml Outdated
refack self-assigned this Oct 24, 2018

refack commented Oct 24, 2018

Copy link
Copy Markdown
Contributor Author

It's back:

In file included from ../src/bootstrapper.cc:2:
In file included from ../src/env-inl.h:27:
../src/aliased_buffer.h:27:27: error: no template named 'enable_if_t' in namespace 'std'; did you mean 'enable_if'?
          typename = std::enable_if_t<std::is_scalar<NativeT>::value>>
                     ~~~~~^~~~~~~~~~~
                          enable_if
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/type_traits:1766:12: note: 'enable_if' declared here
    struct enable_if
           ^
1 error generated.

refack commented Oct 24, 2018

Copy link
Copy Markdown
Contributor Author

The version of `clang` provided in the Travis linux image uses
libstdc++4.8 whice is below our minimal supported version.

Switching to `make test -j1` is to avoid races during the test cycle
causes by the main target being "unstable", that is it always builds
some files, and relinks the binary, which is used by the test procedure.

PR-URL: nodejs#23778
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
refack force-pushed the bump-gcc-on-tavis branch from a19546f to a2328da Compare October 24, 2018 21:34
refack merged commit a2328da into nodejs:master Oct 24, 2018
refack removed their assignment Oct 24, 2018
refack deleted the bump-gcc-on-tavis branch October 24, 2018 21:38
targos pushed a commit that referenced this pull request Oct 26, 2018
The version of `clang` provided in the Travis linux image uses
libstdc++4.8 whice is below our minimal supported version.

Switching to `make test -j1` is to avoid races during the test cycle
causes by the main target being "unstable", that is it always builds
some files, and relinks the binary, which is used by the test procedure.

PR-URL: #23778
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
targos pushed a commit that referenced this pull request Nov 1, 2018
The version of `clang` provided in the Travis linux image uses
libstdc++4.8 whice is below our minimal supported version.

Switching to `make test -j1` is to avoid races during the test cycle
causes by the main target being "unstable", that is it always builds
some files, and relinks the binary, which is used by the test procedure.

PR-URL: #23778
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
The version of `clang` provided in the Travis linux image uses
libstdc++4.8 whice is below our minimal supported version.

Switching to `make test -j1` is to avoid races during the test cycle
causes by the main target being "unstable", that is it always builds
some files, and relinks the binary, which is used by the test procedure.

PR-URL: #23778
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
The version of `clang` provided in the Travis linux image uses
libstdc++4.8 whice is below our minimal supported version.

Switching to `make test -j1` is to avoid races during the test cycle
causes by the main target being "unstable", that is it always builds
some files, and relinks the binary, which is used by the test procedure.

PR-URL: #23778
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
codebytere mentioned this pull request Nov 27, 2018
rvagg pushed a commit that referenced this pull request Nov 28, 2018
The version of `clang` provided in the Travis linux image uses
libstdc++4.8 whice is below our minimal supported version.

Switching to `make test -j1` is to avoid races during the test cycle
causes by the main target being "unstable", that is it always builds
some files, and relinks the binary, which is used by the test procedure.

PR-URL: #23778
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
The version of `clang` provided in the Travis linux image uses
libstdc++4.8 whice is below our minimal supported version.

Switching to `make test -j1` is to avoid races during the test cycle
causes by the main target being "unstable", that is it always builds
some files, and relinks the binary, which is used by the test procedure.

PR-URL: #23778
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
The version of `clang` provided in the Travis linux image uses
libstdc++4.8 whice is below our minimal supported version.

Switching to `make test -j1` is to avoid races during the test cycle
causes by the main target being "unstable", that is it always builds
some files, and relinks the binary, which is used by the test procedure.

PR-URL: #23778
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
codebytere mentioned this pull request Nov 29, 2018
BethGriggs mentioned this pull request Dec 4, 2018
richardlau mentioned this pull request Mar 30, 2019
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI. regression Issues related to regressions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL