build: remove unused libatomic and librt on ppc64, s390x

[sam-github]

The libraries are not necessary for ppc64 or s390x. It does no harm with
some linkers, but devtoolset-6 creates runtime dependencies on all link
libraries, even unused ones.

Fixes: #27377
Fixes: #29718

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

[nodejs-github-bot]

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

[sam-github]

I'll look at removing -lrt as well, it seems to just define the mq_, aio_, timer_, etc. real-time APIs, and I don't think node uses those, and librt.so.2 was one of the new runtime deps in #29718

[nodejs-github-bot]

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

[mhdawson]

LGTM as long as CI is good across platforms

[richardlau]

Re. -lrt, there's also

node/deps/uv/uv.gyp

Line 247 in df0e2e3

'libraries': [ '-ldl', '-lrt' ],

in libuv.

Subsystem would more accurately be build instead of tools.

[sam-github]

@joransiu @miladfarca -- you should look at this, too.

[BridgeAR]

Please +1 to fast-track this. I would like to prepare a patch for this soon.

[sam-github]

@bnoordhuis thoughts on the uv use of -lrt? Did you need it? I hacked in a change on this branch for testing, but I assume we'd want to upstream it to uv.

[nodejs-github-bot]

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

[sam-github]

I'd like to give to tomorrow for people to comment on it, is that fast enough? But if you all think this is fine (even the uv change?), then I've no objection to fast-tracking. We can always fast-track another change, I gues, if it causes other problems. We could also switch back to ubuntu-1404 for the release machine until the issue is resolved.

[joransiu]

LGTM

[nodejs-github-bot]

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

[sam-github]

I'm not sure if I'm ready to fast-track ab7fbf85d280b5beca67fcc0783f318453f3506a without comment from @nodejs/libuv, because its patching deps/uv/uv.gyp -- I opened libuv/libuv#2493 so the patch wouldn't just be floating here forever.

EDIT: after rebase, uv patch is in 6e3b475

[nodejs-github-bot]

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

[Trott]

Based on @sam-github comments, removing fast-track label.

[bnoordhuis]

I commented on libuv/libuv#2493 but for libuv, dropping the librt dependency isn't on the table while we still support glibc < 2.17.

Some kind of build flag to make it conditional is acceptable though.

[sam-github]

Node.js doesn't support glibc < 2.17, @cjihrig @bnoordhuis any objection to just floating a patch on deps/uv?

[sam-github]

Alternatively, does anyone know how to remove the refs to unused link libs? I'm poring through gcc docs, but haven't seen anything yet.

[bnoordhuis]

Alternatively, does anyone know how to remove the refs to unused link libs?

Will this do?

diff --git a/common.gypi b/common.gypi
index 10b4ecfc52..b1b7d1abe6 100644
--- a/common.gypi
+++ b/common.gypi
@@ -349,6 +349,11 @@
           'BUILDING_UV_SHARED=1',
         ],
       }],
+      [ 'OS=="linux"', {
+        'link_settings': {
+          'libraries!': [ '-lrt' ],
+        },
+      }],
       [ 'OS in "linux freebsd openbsd solaris aix"', {
         'cflags': [ '-pthread' ],
         'ldflags': [ '-pthread' ],

[richardlau]

Alternatively, does anyone know how to remove the refs to unused link libs?

Will this do?

diff --git a/common.gypi b/common.gypi
index 10b4ecfc52..b1b7d1abe6 100644
--- a/common.gypi
+++ b/common.gypi
@@ -349,6 +349,11 @@
           'BUILDING_UV_SHARED=1',
         ],
       }],
+      [ 'OS=="linux"', {
+        'link_settings': {
+          'libraries!': [ '-lrt' ],
+        },
+      }],
       [ 'OS in "linux freebsd openbsd solaris aix"', {
         'cflags': [ '-pthread' ],
         'ldflags': [ '-pthread' ],

Maybe avoid putting this in common.gypi as that is still used by node-gyp and thus inherited by addons?

[sam-github]

wrt. #29727 (comment) @bnoordhuis no, it doesn't help, because it adds -lrt, instead of taking it away.... also, that code already exists, its down in deps/uv/uv.gyp, and its the code I'd like to get rid of.

The options I can think of are:

1. Add a new runtime dep during a release line: this is a negative, but it comes with some positives, such as not building on an EOL ubuntu-14.04 and of broadening compatibility with older glibc (because devtools is statically linking in some of the new glibc symbols).
2. Float a patch on deps/uv/uv.gyp: 6e3b475e1a2bc43957
3. Find some way to convince devtools to ignore link libs if they aren't required. I'm in process of investigating this.

@danbev you don't have any insider information, do you, as to why RH's devtools-6 is behaving differently from upstream with respect to

The libraries are not necessary for ppc64 or s390x. It does no harm with some linkers, but devtoolset-6 creates runtime dependencies on all link libraries, even unused ones.

[bnoordhuis]

no, it doesn't help, because it adds -lrt

@sam-github Read carefully, that exclamation mark has significance. :-)

[sam-github]

@bnoordhuis Ah, subtle! OK, I'll rework the last commit on this branch and test it.

@richardlau Which .gyp file would you suggest?

[sam-github]

@richardlau pending comment, I modified tools/v8_gypfiles/v8.gyp, PTAL

[nodejs-github-bot]

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

[richardlau]

@bnoordhuis Ah, subtle! OK, I'll rework the last commit on this branch and test it.

@richardlau Which .gyp file would you suggest?

https://github.com/nodejs/node/blob/master/node.gypi.

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[sam-github]

I did as @richardlau suggested. If this passes CI, I'm ready to land it.

[nodejs-github-bot]

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

[sam-github]

Landed in f3ae3c9 and 7ec558c