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,meta: bump gcc on travis #23778

Merged
merged 1 commit into from
Oct 24, 2018
Merged

Conversation

refack
Copy link
Contributor

@refack refack commented Oct 20, 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.

../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 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
@refack
Copy link
Contributor Author

refack commented Oct 20, 2018

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

.travis.yml Outdated
script:
- make -j2 test
- make test
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refs: #23733

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@refack refack Oct 20, 2018

Choose a reason for hiding this comment

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

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

.travis.yml Outdated
script:
- make -j2 test
- make test
Copy link
Member

Choose a reason for hiding this comment

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

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

.travis.yml Outdated
- ./configure
- make -j2 V=
- CC='ccache gcc-4.9' && CXX='ccache g++-4.9' python configure.py --verbose
- CC='ccache gcc-4.9' && CXX='ccache g++-4.9' make -j2 V=
Copy link
Member

Choose a reason for hiding this comment

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

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
Contributor Author

Choose a reason for hiding this comment

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

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
Contributor Author

Choose a reason for hiding this comment

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

Testing export in install

@addaleax
Copy link
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
Copy link
Contributor Author

refack commented Oct 20, 2018

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

So it might be just an stdlibc++ issue?

@addaleax
Copy link
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
Copy link
Contributor Author

refack commented Oct 20, 2018

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)

@addaleax
Copy link
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?

.travis.yml Outdated
- ./configure
- make -j2 V=
- make -j2 V=1 | sed -e "s/'.*\$//" -e "s|$PWD||g"
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

@refack
Copy link
Contributor Author

refack commented Oct 22, 2018

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
image

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

.travis.yml Outdated Show resolved Hide resolved
@addaleax 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
.travis.yml Show resolved Hide resolved
@refack refack self-assigned this Oct 24, 2018
@refack
Copy link
Contributor Author

refack commented Oct 24, 2018

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
Copy link
Contributor Author

refack commented Oct 24, 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: nodejs#23778
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@refack refack merged commit a2328da into nodejs:master Oct 24, 2018
@refack refack removed their assignment Oct 24, 2018
@refack 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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@codebytere 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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@codebytere codebytere mentioned this pull request Nov 29, 2018
@BethGriggs BethGriggs mentioned this pull request Dec 4, 2018
@richardlau richardlau mentioned this pull request Mar 30, 2019
2 tasks
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