-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
src,build: support g++ 4.7.x #3391
Conversation
Requiring g++ 4.8.x and later to build node binaries means that users who download node binaries from nodejs.org may not be able to use them if the C++ runtime library installed on their system is older. This is the case for instance by default for Solaris 11.2. This change works around a g++ bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53613) whose fix is present in g++ 4.8.x, but hasn't been backported to g++ 4.7.x and makes node buildable with g++4.7.x. The resulting binary thus requires a less recent C++ runtime and works on a broader set of systems. Fixes nodejs#3349.
Please note that, if such a change would be accepted, this PR would need to be tested further. More specifically, we'd need to make sure that resulting binaries built by CI machines would fix the issue on Solaris 11.2, and would not cause regressions on other supported platforms. Obviously, going through that pain to "only" solve a problem that can be solved otherwise on a single platform (Solaris 11.2) can seem overkill, but I believe that lowering our toolchain requirements would be beneficial for more users, especially if we consider the v4.x LTS release. It is also possible that users of not-so-recent systems who are now using v0.10 and v0.12 versions may not have hit that problem, but will when they try to upgrade. /cc @nodejs/build @nodejs/lts @nodejs/collaborators |
Also /cc @nodejs/platform-solaris |
But V8 still requires 4.8+, no? https://chromium.googlesource.com/v8/v8/+/master/docs/building_with_gyp.md#GCC-make If its really that trivial I'd like to see them lower their requirements before we do ours since lowering to 4.7 only having to bump it once a new version of v8 is out seems moot. |
Does the LTS cycle factor in to this? Node-4.x builds will be around and still important even after the next hypothetically breaking v8 upgrade, won't they? |
v8 requires g++ 4.8.x+. That is why we did this in the first place, because it wasn't reasonable to attempt to make it buildable by older compilers, afaik. |
@jbergstroem @Fishrock123 V8 doesn't seem to require g++ 4.8, at least on the platforms on which I built nodejs/node's master with g++ 4.7 successfully (Ubuntu 12.04 and SmartOS).
If I understood what @rmg said correctly, which I think is that node Argon will be around for long enough for that potential improvement to matter, then that's precisely my point. However I remember now that the current releases for Linux use the RedHat Developer Toolset to work around that issue already. I just confirmed that by running v4.2.1 on Centos 5.7. That probably currently leaves only Solaris releases susceptible to this problem, which could be solved differently, for instance by adding actual Solaris build machines to the CI platform. Thus I would probably recommend to wait until we have enough evidence that this is a problem on platforms for which it can't be solved differently before moving forward with this change and I'll close that PR unless someone else comes up with an example of such a platform soon. My apologies for bringing that to your attention in the first place, I still need to get used to differences between joyent/node and nodejs/node builds. |
Hmm... see: 48774ec EDIT: ah I see you already got the configure bit. cc @bnoordhuis |
There are bugs in the g++ 4.7 series where it doesn't parse some C++11 constructs or emits the wrong machine code. They may have been fixed in 4.7.4 but I wouldn't bet on it. |
@bnoordhuis Do you know where we can find more details about these bugs? Also, so far I've been using g++ 4.7.3, not 4.7.4. |
For what it's worth, my Solaris 11.1 system seems to run the 4.2.1/x64 binaries ok (Gets to REPL, I can run process.config, not tested any further. I didn't think I'd pulled much that wasn't default from the repositories when I installed that system (although granted that was some time ago now...) so I'm a little surprised if 11.2 has an older version than mine. |
I was hoping I'd still have them in my browser history but alas. Not that it helps but they're in gcc's bugzilla and IIRC there were one or two in ubuntu's launchpad as well. I remember because I looked into adding 4.7 support last year and was forced to conclude that it was a bad idea. |
Here's a description of what's going on on Solaris with binaries downloaded from nodejs.org: https://gist.github.com/misterdjules/eae9ec70dc1d91fb8dd1. @sxa555 Would you mind pasting the output of the following commands in a gist ?
I'll close that issue for now as it's not clear what's the best strategy moving forward for SmartOS and Solaris binaries, but feel free to continue the discussion. |
@bnoordhuis Doing a bit of research, it seems there are actually more C++11 related issues with g++ 4.8 than with g++ 4.7. Does any of them ring a bell? |
@misterdjules 4.7 is EOL (AFAIK) so I wouldn't expect to see many open bug reports. The few that are still open have probably been overlooked. |
@bnoordhuis Where can I find GCC's support policy? I haven't been able to find any information about some releases being EOL. I was just wondering if any of the existing open issues for gcc 4.7.x looked similar to the issues you had in mind when you mentioned:
|
Here, kind of. I don't think gcc has hard rules but you can see by the release cadence that 4.7 has been effectively retired.
No, none of them ring a bell. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52427 is one but it was fixed. |
@bnoordhuis Right, I had missed the GCC 4.8.5 release in that diagram because it's at the same level as GCC 4.9 releases. It then seems clear that there hasn't been any GCC 4.7 release in a while compared to all the later branches (including 4.8). Unless the current minimum required compiler is broken in a way that incurs regressions for node and it can't be fixed, I don't think bumping the minimum compilers requirements should be done based only on what compilers are in active development. It should also be based on what runtimes we want to support with the binaries available at nodejs.org. Giving up support for older runtimes by bumping the minimum required toolchain will very likely break at least some users, and it should only be done when we have to. In this case, it's not clear why we had to require GCC > 4.7. It seems that it would help to have a detailed matrix of the runtimes/OSes/platforms supported by the binaries available on nodejs.org. We could use that to communicate potential breakage when we want to bump the compiler/runtime requirements, or at least to understand exactly how bumping the minimum required compilers would impact the list of supported platforms. |
@misterdjules #4088 reported earlier today was a build issue with 4.7. I think most people that would be using 4.7 if it was supported would be using 4.7.2 from September 2012 because that is what debian wheezy ships as its gcc-4.7 package. Not only is it not supported by upstream, it's not even the latest patch release for the 4.7 series... |
@bnoordhuis #4088 is the build issue that is fixed by this PR. |
It seems that travis-ci has gcc 4.6.3, at least on its older infrastructure, so this blocks me from shipping "install from source" support in |
@ljharb is there no way to update the gcc in the bootstrapping process of travis? |
@thealphanerd I recall something like this working: addons:
apt:
sources:
- ubuntu-toolchain-r-test
packages:
- gcc-4.8
- g++-4.8 |
@thealphanerd i've been pointed to something like that ^ so i will definitely try it. It also turns out that 48774ec was added in v1.0.3, and I'm testing with v1.0.2 - otherwise I would have understood this failure much, much sooner :-) |
Requiring g++ 4.8.x and later to build node binaries means that users
who download node binaries from nodejs.org may not be able to use them
if the C++ runtime library installed on their system is older. This is
the case for instance by default for Solaris 11.2.
This change works around a g++ bug
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53613) whose fix is
present in g++ 4.8.x, but hasn't been backported to g++ 4.7.x and makes
node buildable with g++4.7.x. The resulting binary thus requires a less
recent C++ runtime and works on a broader set of systems.
Fixes #3349.