Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

[Converge] Build related commits #27

Closed
jasnell opened this issue May 22, 2015 · 19 comments
Closed

[Converge] Build related commits #27

jasnell opened this issue May 22, 2015 · 19 comments

Comments

@jasnell
Copy link
Member

jasnell commented May 22, 2015

There are a number of build related commits that need to reconciled. /cc @nodejs/build @misterdjules

@jasnell
Copy link
Member Author

jasnell commented May 22, 2015

See: nodejs/node#26 ... The Intl work also impacted the Build.

@Fishrock123
Copy link
Contributor

src: make build pass with GCC < 4.5

iirc this isn't possible with the v8 we are using, cc @bnoordhuis

@bnoordhuis
Copy link
Member

Correct. 4.8 is the absolute minimum.

@jasnell
Copy link
Member Author

jasnell commented May 22, 2015

Another: c0766eb1a4dac9d9b555be271ef84ad401ba91b0 build: fix use of strict aliasing - nodejs/node-v0.x-archive@c0766eb

@Fishrock123
Copy link
Contributor

Also looks like it should just be dropped, again < 4.8 isn't going to work anyways.

@jasnell
Copy link
Member Author

jasnell commented May 22, 2015

@misterdjules @mhdawson ... do you know of anything that breaks without this?

@Fishrock123
Copy link
Contributor

Well, older systems that use GCC < 4.8 will break trying to build without having GCC upgraded.

@bnoordhuis
Copy link
Member

And, FWIW, the library that was the source of the -Wstrict-aliasing warnings, src/queue.h, was removed in nodejs/node@7e779b4.

@jbergstroem
Copy link
Member

Clang could be a simpler install through package managers on those systems. Avoiding the gcc 4.8 is pretty much impossible if we want to use recent/stable v8's.

@jasnell
Copy link
Member Author

jasnell commented May 27, 2015

Ok, then I think this one is largely decided then. It would require a revert of the io.js changes in order to land. @misterdjules @cjihrig @trevnorris @orangemocha @mhdawson : if any of you think this is going to be a problem, we'll need to make the case to revert before we can proceed. Given that, I'm going to close this issue. We can reopen if new information is presented.

@jasnell jasnell closed this as completed May 27, 2015
@Fishrock123
Copy link
Contributor

@jasnell Hmmm, isn't the 2nd commit still valid? The windows 32 one?

@jasnell
Copy link
Member Author

jasnell commented May 27, 2015

Ah! missed that one. Perhaps, reopening until we can confirm

@jasnell jasnell reopened this May 27, 2015
@bnoordhuis
Copy link
Member

nodejs/node-v0.x-archive@6168fe6 LGTM FWIW. It doesn't apply cleanly but you can get it to merge with a little nudge.

@misterdjules
Copy link

Added more info about nodejs/node-v0.x-archive@6168fe6 in nodejs/node#44 (comment).

Regarding nodejs/node-v0.x-archive@5926526 and nodejs/node-v0.x-archive@c0766eb, the goal for these changes was to be able to run node on Linux systems with older C++ runtimes (like CentOS 5.7 and early 6.x, I don't remember the specific version numbers).

The way io.js solved this was to use the RHEL developer toolset to build a node binary for Linux. This toolset basically makes node links against a C++ runtime that is old enough to run on Linux systems with older C++ runtimes, and adds newer C++ runtime features with a static library linked directly into the node binary. I think this approach is better and I created joyent/node-infra#4 to use it in joyent/node too. @mhdawson expressed some important concerns though here: joyent/node-infra#4 (comment).

In other words: as long as node runs on Linux systems in wide usage that ship an older C++ runtime, we don't need these two changes. We'll need input from @mhdawson to determine what needs to be done on IBM platforms.

@mhdawson
Copy link
Member

mhdawson commented Jul 7, 2015

As stated earlier since the converged stream will have a newer level of V8 and V8 requires a newer compiler the converged stream will have a dependency on a later compiler so leaving out the first PR is not an issue.

I'll still concerned though that we don't have a full answer to the problem. Ignoring the issue of IBM based platforms, based on my understanding end users will only be able to build binaries for xlinux that run on older systems by one of the following:

  1. using Red Hat Enterprise Linux with the Develper toolset
  2. using a using the toolset here http://linux.web.cern.ch/linux/devtoolset/, were it says in bold red "TEST PURPOSE ONLY" on centos (not sure about this).

So for example, if I'm an ubuntu shop running ubuntu 12 I'd either have to install a different platform or figure it out for myself how I could build binaries that would run on ubuntu 12.

@rvagg
Copy link
Member

rvagg commented Jul 8, 2015

Ubuntu 12.04 toolchain is fairly simple via a PPA: https://github.com/nodejs/build/blob/master/setup/ubuntu12.04/ansible-playbook.yaml#L25

Ubuntu 10.04 was just as simple: https://github.com/nodejs/build/blob/6b3867b99f555f3f1bb0b84c81ea20db2ca343b6/setup/ubuntu10.04/ansible-playbook.yaml#L25

Wheezy is a problem, although it's common enough that workarounds are appearing. For the ARMv7 machines I've managed to get gcc4.8 via default backport channels but I can't tell you if that goes for non-arm release lines too. Early on I created a procedure for compiling gcc4.8 on Wheezy, see https://github.com/nodejs/build/tree/master/setup/debian-wheezy-gcc for that.

It's actually easier to compile on non-RHEL/CentOS distros of the same age as it is on RHEL because of the hacky way you have to use SCLs.

Also, I read "TEST PURPOSE ONLY" in the same way as I read "DO NOT MACHINE WASH" on clothing labels, it's all about CYA.

@mhdawson
Copy link
Member

mhdawson commented Jul 8, 2015

Having the info documented on how to get the toolchain you'll need for a given distro level would help. The other question is if we know whether binaries built with gcc48 on ubuntu 12.04 will run on ubuntu 10.04 ? The concern was not so much that you could not get the toolchain to build on a given distro level (but as you mention for Wheezy, its not always straight forward, so documentation would be helpful), but that resulting binaries might not run on earlier distro levels without having to update libstdc++ levels or some other components on the backlevel distro. That's the issue that dev toolset addresses.

@rvagg
Copy link
Member

rvagg commented Jul 8, 2015

So 12.04 binaries won't run on 10.04 but 10.04 is out of LTS so we've moved on anyway. The binaries we produce on CentOS5 will work on other CentOS 5 / RHEL5 / Wheezy-era distros without any additional extras. We build the ARM binaries on Wheezy-based distros for the same reason.

I think I've mentioned this before but it's worth saying again: any use of C++ features so far in V8 (and Node where it's crept in) are limited to those that don't require linking to libstdc++, so all that matters for the target distro is the version of libc. All that matters is where the binary is compiled and CentOS 5 and Wheezy represent the oldest reasonably supported distros out there.

To prove the point, here's a copy of what I've just run against a virgin CentOS 5 box:

$ ssh root@45.55.74.162
The authenticity of host '45.55.74.162 (45.55.74.162)' can't be established.
RSA key fingerprint is cc:35:5b:92:77:ad:e4:17:d2:da:93:52:26:3c:8a:ca.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added '45.55.74.162' (RSA) to the list of known hosts.
[root@centostest ~]# uname -a
Linux centostest 2.6.18-308.1.1.el5 #1 SMP Wed Mar 7 04:16:51 EST 2012 x86_64 x86_64 x86_64 GNU/Linux
[root@centostest ~]# uptime
 22:10:59 up 0 min,  1 user,  load average: 0.44, 0.11, 0.04
# curl -sL https://iojs.org/dist/latest/iojs-v2.3.3-linux-x64.tar.gz | tar -zx --strip-components=1 -C /usr/local/
[root@centostest ~]# node -v
v2.3.3
[root@centostest ~]# node -pe process.versions
{ http_parser: '2.5.0',
  node: '2.3.3',
  v8: '4.2.77.20',
  uv: '1.6.1',
  zlib: '1.2.8',
  ares: '1.10.1-DEV',
  modules: '44',
  openssl: '1.0.2c' }

@rvagg
Copy link
Member

rvagg commented Aug 24, 2015

I think this issue boiled down to the vcbuild.bat change but I believe that's redundant now thanks to the Intl support by @srl295: nodejs/node@1f12e032

I've filed a ticket to make sure Jenkins is doing the right thing by v4, currently I don't think it is: nodejs/node#2516

@rvagg rvagg closed this as completed Aug 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants