Skip to content

build: fix compiler version detection#24879

Closed
richardlau wants to merge 2 commits intonodejs:masterfrom
richardlau:fix_compiler_detection
Closed

build: fix compiler version detection#24879
richardlau wants to merge 2 commits intonodejs:masterfrom
richardlau:fix_compiler_detection

Conversation

@richardlau
Copy link
Member

@richardlau richardlau commented Dec 6, 2018

Compiler version tuples should be numeric for tuple comparisons
to work.

Before:

-bash-4.2$ gcc --version
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

-bash-4.2$ ./configure
-bash-4.2$

After:

-bash-4.2$ gcc --version
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

-bash-4.2$ ./configure
WARNING: C++ compiler too old, need g++ 4.9.4 or clang++ 3.4.2 (CXX=g++)
WARNING: warnings were emitted in the configure phase
-bash-4.2$
Checklist

@richardlau
build: fix compiler version detection
b11a7bb 
Compiler version tuples should be numeric for tuple comparisons
to work.
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 6, 2018

@richardlau build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1857/pipeline

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Dec 6, 2018
@richardlau
Copy link
Member Author

richardlau commented Dec 6, 2018

cc @nodejs/build-files @nodejs/python

@richardlau

This comment has been minimized.

Sign in to view
@richardlau
Copy link
Member Author

richardlau commented Dec 6, 2018

New CI: https://ci.nodejs.org/job/node-test-pull-request/19285/

@richardlau
Copy link
Member Author

richardlau commented Dec 6, 2018

Also corrected the version check for AIX, which requires GCC 6.3.0 (https://github.com/nodejs/node/blob/master/BUILDING.md#aix):

-bash-4.4$ g++ --version
g++ (GCC) 4.8.4
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

-bash-4.4$ ./configure
WARNING: C++ compiler too old, need g++ 6.3.0 (CXX=g++)
WARNING: Could not recognize `gas`:
WARNING: warnings were emitted in the configure phase
-bash-4.4$

joyeecheung
joyeecheung approved these changes Dec 6, 2018
View reviewed changes
Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

LGTM if CI is happy

@richardlau
Copy link
Member Author

richardlau commented Dec 6, 2018

arm-fanned seems to have infra issues. Have raised nodejs/build#1626.

bnoordhuis
bnoordhuis approved these changes Dec 6, 2018
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

The bug was introduced in commit 641d4a4 / PR #21183?

@richardlau
Copy link
Member Author

richardlau commented Dec 6, 2018

The bug was introduced in commit 641d4a4 / PR #21183?

Yes, although the previous string-based comparison was probably not quite correct either.

@richardlau
Copy link
Member Author

richardlau commented Dec 7, 2018

Resume CI: https://ci.nodejs.org/job/node-test-pull-request/19293/

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 7, 2018
@Trott
Copy link
Member

Trott commented Dec 8, 2018

Landed in c3dd0d0

@Trott Trott closed this Dec 8, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Dec 8, 2018
@richardlau @Trott
build: fix compiler version detection
c3dd0d0 
Compiler version tuples should be numeric for tuple comparisons
to work.

Also correct check for AIX where the minimum supported GCC is 6.3.0

PR-URL: nodejs#24879
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BethGriggs pushed a commit that referenced this pull request Dec 17, 2018
@richardlau @BethGriggs
build: fix compiler version detection
cc8a805 
Compiler version tuples should be numeric for tuple comparisons
to work.

Also correct check for AIX where the minimum supported GCC is 6.3.0

PR-URL: #24879
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@BethGriggs BethGriggs mentioned this pull request Dec 18, 2018
Merged
@beevelop beevelop mentioned this pull request Dec 18, 2018
Closed
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@richardlau @refack
build: fix compiler version detection
ebe4189 
Compiler version tuples should be numeric for tuple comparisons
to work.

Also correct check for AIX where the minimum supported GCC is 6.3.0

PR-URL: nodejs#24879
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
@richardlau @BethGriggs
build: fix compiler version detection
8d4d396 
Compiler version tuples should be numeric for tuple comparisons
to work.

Also correct check for AIX where the minimum supported GCC is 6.3.0

PR-URL: #24879
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
Merged
BethGriggs pushed a commit that referenced this pull request Feb 20, 2019
@richardlau @BethGriggs
build: fix compiler version detection
c0f316d 
Compiler version tuples should be numeric for tuple comparisons
to work.

Also correct check for AIX where the minimum supported GCC is 6.3.0

PR-URL: #24879
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@richardlau @rvagg
build: fix compiler version detection
89f071f 
Compiler version tuples should be numeric for tuple comparisons
to work.

Also correct check for AIX where the minimum supported GCC is 6.3.0

PR-URL: #24879
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BaochengSu pushed a commit to BaochengSu/node that referenced this pull request Oct 20, 2020
@richardlau @BaochengSu
build: fix compiler version detection
0b3ba49 
Compiler version tuples should be numeric for tuple comparisons
to work.

Also correct check for AIX where the minimum supported GCC is 6.3.0

PR-URL: nodejs#24879
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
(cherry picked from commit c3dd0d0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@richardlau @nodejs-github-bot @Trott @bnoordhuis @joyeecheung @BethGriggs