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.

Sign up for GitHub

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

Jump to bottom

build: fix compiler version detection #24879

Closed
wants to merge 2 commits into from
Closed

build: fix compiler version detection #24879

wants to merge 2 commits into from

Conversation

richardlau
Copy link
Member

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

@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

cc @nodejs/build-files @nodejs/python

@richardlau

This comment has been minimized.

Sign in to view
@richardlau
Copy link
Member Author

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

@richardlau
Copy link
Member Author

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

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

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

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

@joyeecheung joyeecheung joyeecheung approved these changes

@bnoordhuis bnoordhuis bnoordhuis 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