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

doc: gcc version is at least 4.8.5 at BUILDING.md #11840

Closed
wants to merge 5 commits into from

Conversation

detailyang
Copy link
Contributor

@detailyang detailyang commented Mar 14, 2017

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

This PR is inspired by #10388 when I try to compile the node v6.x with gcc 4.8.0 :(. I check the prerequisites at BUILDING.md and It tell me that gcc version at least is 4.8.0 which is actually out of date (Aslo a typo about or).

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 14, 2017
@bnoordhuis
Copy link
Member

First commit LGTM, thanks. The , or is not a typo though, it's to signal that you need either g++ x.y.z OR clang++ x.y.

@detailyang
Copy link
Contributor Author

Hello.
@bnoordhuis gotcha! Many Thanks :D

targos added a commit to nodejs/build that referenced this pull request Mar 14, 2017
The latest version of 4.8 available for 12.04 is 4.8.4.
It is not able to build V8 from version 5.7 and does not
satisfy the requirements to build Node.js.

Refs: nodejs/node#11840
Refs: nodejs/v8#5
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM, but I see some discussion about requiring 4.9 in other issues.

@bnoordhuis
Copy link
Member

We'll probably drop g++ 4.8 for node 8.0. Bumping the baseline from 4.8.0 to 4.8.5 for <= v7.x is a good idea, though.

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.

LGTM. Can you mention in the commit log that the bump is because of compiler bugs in g++ < 4.8.5?

@detailyang
Copy link
Contributor Author

No Problem. I will do rebase these commits to one commit after cloning this huge repo slowly because of Chinese GFW :(

@bnoordhuis
Copy link
Member

Oh, I suppose that whoever merges this can amend the commit log.

bumping the baseline from gcc 4.8.0 to gcc 4.8.5 because of compiler bugs in g++ < 4.8.5.
@detailyang
Copy link
Contributor Author

detailyang commented Mar 14, 2017

It looks like cannot rebase the PR which have already issued without git push --force (sorry, my fault). Maybe some people can squashing these commits when merging and mention what @bnoordhuis said

bumping the baseline from gcc 4.8.0 to gcc 4.8.5 because of compiler bugs in g++ < 4.8.5.

Many Thanks :D

@detailyang
Copy link
Contributor Author

BTW, 4.8.5 is a good interim version IMO since the centos:7 is using the 4.8.0 as the system default version and other system like fedora:latestdebian:lastest and ubuntu:latest is at least 4.9.0.

jasnell pushed a commit that referenced this pull request Mar 16, 2017
>= 4.8.5 is required because of compiler bugs in earlier versions

PR-URL: #11840
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Mar 16, 2017

Landed in 746339e with amended commit log per @bnoordhuis' request

@jasnell jasnell closed this Mar 16, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Mar 20, 2017
>= 4.8.5 is required because of compiler bugs in earlier versions

PR-URL: nodejs#11840
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
>= 4.8.5 is required because of compiler bugs in earlier versions

PR-URL: nodejs#11840
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

is this also accurate for v6.x?

@gibfahn
Copy link
Member

gibfahn commented Apr 18, 2017

is this also accurate for v6.x?

Based on:

This PR is inspired by #10388 when I try to compile the node v6.x with gcc 4.8.0

I'd say so.

MylesBorins pushed a commit that referenced this pull request May 8, 2017
>= 4.8.5 is required because of compiler bugs in earlier versions

PR-URL: #11840
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 23, 2017
@pmq20
Copy link
Contributor

pmq20 commented Jun 5, 2017

4.8.2 works fine on my old Linux box. Why do we have to make it 5? #13462

@gibfahn
Copy link
Member

gibfahn commented Jun 5, 2017

We'll probably drop g++ 4.8 for node 8.0. Bumping the baseline from 4.8.0 to 4.8.5 for <= v7.x is a good idea, though.

@bnoordhuis I note that this never happened, do you know if it needs to (i.e. will 4.8.5 be okay for the lifetime of v8.x)?

@bnoordhuis
Copy link
Member

@gibfahn C++11 support is not 100% functional in 4.8.x. Updating the requirements is still a good idea, IMO; we don't want to be hampered by compiler bugs. I'll file a pull request.

bnoordhuis added a commit to bnoordhuis/io.js that referenced this pull request Jun 15, 2017
The 4.8.x releases don't fully support C++11.  Update the prerequisites
for node.js 8 so that we won't have to work around compiler bugs in the
future.

To be decided if we should also update the minimum clang version.
I believe clang 3.4.2 properly supports C++11 but am not 100% sure.

Refs: nodejs#11840
jasnell pushed a commit that referenced this pull request Jun 15, 2017
The 4.8.x releases don't fully support C++11.  Update the prerequisites
for node.js 8 so that we won't have to work around compiler bugs in the
future.

To be decided if we should also update the minimum clang version.
I believe clang 3.4.2 properly supports C++11 but am not 100% sure.

PR-URL: #13466
Ref: #11840
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
addaleax pushed a commit that referenced this pull request Jun 17, 2017
The 4.8.x releases don't fully support C++11.  Update the prerequisites
for node.js 8 so that we won't have to work around compiler bugs in the
future.

To be decided if we should also update the minimum clang version.
I believe clang 3.4.2 properly supports C++11 but am not 100% sure.

PR-URL: #13466
Ref: #11840
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
targos added a commit to nodejs/build that referenced this pull request Jun 19, 2017
The latest version of 4.8 available for 12.04 is 4.8.4.
It is not able to build V8 from version 5.7 and does not
satisfy the requirements to build Node.js.

Refs: nodejs/node#11840
Refs: nodejs/v8#5
addaleax pushed a commit that referenced this pull request Jun 21, 2017
The 4.8.x releases don't fully support C++11.  Update the prerequisites
for node.js 8 so that we won't have to work around compiler bugs in the
future.

To be decided if we should also update the minimum clang version.
I believe clang 3.4.2 properly supports C++11 but am not 100% sure.

PR-URL: #13466
Ref: #11840
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
addaleax pushed a commit that referenced this pull request Jun 24, 2017
The 4.8.x releases don't fully support C++11.  Update the prerequisites
for node.js 8 so that we won't have to work around compiler bugs in the
future.

To be decided if we should also update the minimum clang version.
I believe clang 3.4.2 properly supports C++11 but am not 100% sure.

PR-URL: #13466
Ref: #11840
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
rvagg pushed a commit that referenced this pull request Jun 29, 2017
The 4.8.x releases don't fully support C++11.  Update the prerequisites
for node.js 8 so that we won't have to work around compiler bugs in the
future.

To be decided if we should also update the minimum clang version.
I believe clang 3.4.2 properly supports C++11 but am not 100% sure.

PR-URL: #13466
Ref: #11840
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
addaleax pushed a commit that referenced this pull request Jul 11, 2017
The 4.8.x releases don't fully support C++11.  Update the prerequisites
for node.js 8 so that we won't have to work around compiler bugs in the
future.

To be decided if we should also update the minimum clang version.
I believe clang 3.4.2 properly supports C++11 but am not 100% sure.

PR-URL: #13466
Ref: #11840
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
>= 4.8.5 is required because of compiler bugs in earlier versions

PR-URL: nodejs/node#11840
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants