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: log the found compiler version if too old #30028

Closed
wants to merge 3 commits into from

Conversation

@richardlau
Copy link
Member

richardlau commented Oct 18, 2019

configure will log a warning if the detected compiler is not new
enough. Take some of the guesswork out of it by also logging the
version of the compiler that was detected.

Before:

-bash-4.2$ ./configure
Node configure: Found Python 2.7.5...
WARNING: C++ compiler too old, need g++ 6.3.0 or clang++ 8.0.0 (CXX=g++)
INFO: Using floating patch "tools/icu/patches/64/source/common/putil.cpp" from "tools/icu"
INFO: Using floating patch "tools/icu/patches/64/source/i18n/dtptngen.cpp" from "tools/icu"
WARNING: warnings were emitted in the configure phase
INFO: configure completed successfully
-bash-4.2$

After:

-bash-4.2$ ./configure
Node configure: Found Python 2.7.5...
WARNING: C++ compiler too old, need g++ 6.3.0 or clang++ 8.0.0 (CXX=g++, 4.8.5)
INFO: Using floating patch "tools/icu/patches/64/source/common/putil.cpp" from "tools/icu"
INFO: Using floating patch "tools/icu/patches/64/source/i18n/dtptngen.cpp" from "tools/icu"
WARNING: warnings were emitted in the configure phase
INFO: configure completed successfully
-bash-4.2$
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
`configure` will log a warning if the detected compiler is not new
enough. Take some of the guesswork out of it by also logging the
version of the compiler that was detected.
@nodejs-github-bot

This comment was marked as outdated.

@mscdex

This comment has been minimized.

Copy link
Contributor

mscdex commented Oct 18, 2019

Perhaps the output would be more clear/readable if the information in parentheses was moved to instead come after the words 'C++ compiler', like:

...
WARNING: C++ compiler (CXX=g++, 4.8.5) too old, need g++ 6.3.0 or clang++ 8.0.0
...
@targos

This comment has been minimized.

Copy link
Member

targos commented Oct 18, 2019

V8 CI to get info for #30020

https://ci.nodejs.org/job/node-test-commit-v8-linux/2564/

Edit: it's (CXX=g++, 4.9.4)

@richardlau

This comment has been minimized.

Copy link
Member Author

richardlau commented Oct 18, 2019

Perhaps the output would be more clear/readable if the information in parentheses was moved to instead come after the words 'C++ compiler', like:

...
WARNING: C++ compiler (CXX=g++, 4.8.5) too old, need g++ 6.3.0 or clang++ 8.0.0
...

Done:

--bash-4.2$ ./configure
Node configure: Found Python 2.7.5...
WARNING: C++ compiler (CXX=g++, 4.8.5) too old, need g++ 6.3.0 or clang++ 8.0.0
INFO: Using floating patch "tools/icu/patches/64/source/common/putil.cpp" from "tools/icu"
INFO: Using floating patch "tools/icu/patches/64/source/i18n/dtptngen.cpp" from "tools/icu"
WARNING: warnings were emitted in the configure phase
INFO: configure completed successfully
-bash-4.2$
@nodejs-github-bot

This comment has been minimized.

@lpinca
lpinca approved these changes Oct 18, 2019
@nodejs-github-bot

This comment has been minimized.

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Oct 22, 2019

Landed in 639085e

@Trott Trott closed this Oct 22, 2019
Trott added a commit that referenced this pull request Oct 22, 2019
`configure` will log a warning if the detected compiler is not new
enough. Take some of the guesswork out of it by also logging the
version of the compiler that was detected.

PR-URL: #30028
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@nodejs-github-bot

This comment has been minimized.

MylesBorins added a commit that referenced this pull request Oct 23, 2019
`configure` will log a warning if the detected compiler is not new
enough. Take some of the guesswork out of it by also logging the
version of the compiler that was detected.

PR-URL: #30028
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
MylesBorins added a commit that referenced this pull request Oct 23, 2019
`configure` will log a warning if the detected compiler is not new
enough. Take some of the guesswork out of it by also logging the
version of the compiler that was detected.

PR-URL: #30028
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 23, 2019
targos added a commit that referenced this pull request Nov 8, 2019
`configure` will log a warning if the detected compiler is not new
enough. Take some of the guesswork out of it by also logging the
version of the compiler that was detected.

PR-URL: #30028
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
targos added a commit that referenced this pull request Nov 10, 2019
`configure` will log a warning if the detected compiler is not new
enough. Take some of the guesswork out of it by also logging the
version of the compiler that was detected.

PR-URL: #30028
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
targos added a commit that referenced this pull request Nov 11, 2019
`configure` will log a warning if the detected compiler is not new
enough. Take some of the guesswork out of it by also logging the
version of the compiler that was detected.

PR-URL: #30028
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.