nodejs packages should be fixed to use more system libraries

[ericonr]

nodejs has a mislocated patch to use shared libuv, but moving it to patches/ and trying to build leads to a build failure in config phase.

nodejs{,-lts,-lts-10} use bundled SSL, but now that we are moving to OpenSSL they can probably be fixed to use system OpenSSL.

[ericonr]

As seen in #26427 (comment) , the build is also failing on armv6l-musl due to libatomic somehow getting lost in the build process.

[m0rg-dev]

It looks like nodejs{,-lts,-lts-10} have been on system OpenSSL since fefffea.

shared-uv.patch is just missing a comma, so (assuming it actually does what it says, still building) it should be pretty easy to integrate. Maybe I'll take a look at the libatomic thing as well.

[m0rg-dev]

nodejs and nodejs-lts both build with the fixed shared-uv.patch and depend on system libuv, as expected. nodejs-lts-10 already uses system libuv, so I'll leave it alone.

Sadly, I don't actually have a machine that's capable of cross-building nodejs for armv6l-musl at the moment... maybe I'll try to stand up a VM later but for now I'm just going to PR the libuv fix and call it a day

[ericonr]

You can create a musl masterdir with ./xbps-src -m masterdir-musl binary-bootstrap x86_64-musl and use it :)

And for libatomic, testing with armv6l (instead of the musl one) should be enough.

[m0rg-dev]

musl isn't the problem - nodejs won't cross-compile unless "$XBPS_WORDSIZE" -ne "$XBPS_TARGET_WORDSIZE", and the only 32-bit Void machine I have is a disassembled Pentium II.

[m0rg-dev]

er, won't cross-compile if "$XBPS_WORDSIZE" -ne "$XBPS_TARGET_WORDSIZE".

[m0rg-dev]

reads README wait that might not actually be a problem

[m0rg-dev]

Okay, cross-compiling for armv6l from an i686 masterdir works. In fact, it works so well that it generates a /usr/bin/node that depends on system libatomic.so.1.

morgan@dev-zone ~/src/void-packages 29034-nodejs-system-libraries
❯ file masterdir-x86/destdir/arm-linux-gnueabihf/nodejs-14.16.0/usr/bin/node
masterdir-x86/destdir/arm-linux-gnueabihf/nodejs-14.16.0/usr/bin/node: ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, BuildID[sha1]=2690603af7da094104bdd93eb17f3b10757d73af, for GNU/Linux 3.2.0, stripped

morgan@dev-zone ~/src/void-packages 29034-nodejs-system-libraries
❯ readelf -d masterdir-x86/destdir/arm-linux-gnueabihf/nodejs-14.16.0/usr/bin/node | grep libatomic
 0x00000001 (NEEDED)                     Shared library: [libatomic.so.1]

I guess I'll try again targeting musl? If it only happens when building on armv6l native then I definitely won't be able to do anything...

[m0rg-dev]

Oh, it was nodejs-lts-10 that wasn't building for armv6l. That one does reproduce the issue.

I can read!

[ericonr]

Well, for what it's worth our builders for 32 bit targets are 64 bit themselves (using 64 bit masterdirs), so for now it won't be packaged either way :/

[m0rg-dev]

Oh, that's fun. I think I have some idea of how you could fix this (adapting ppc32.patch to link with -latomic on arm as well as mips/ppc32, though I don't know enough about libatomic to know if that would cause performance issues on armv7 where you do have native atomic, or enough about Node's build process to know if it's possible to target that more finely than just arm), but if it's not getting packaged anyways does it really matter? Maybe that should be its own issue in case the builders get sorted out?

[m0rg-dev]

Reading ppc32.patch closer, I'm not sure the patch works in the first place - it checks host_arch not target_arch, so it won't build if cross-compiled from a host that supports native atomic8. https://repo.voidlinux.de/bad/ppc-musl/nodejs-lts-10, for example.

[m0rg-dev]

diff --git a/srcpkgs/nodejs-lts-10/patches/ppc32.patch b/srcpkgs/nodejs-lts-10/patches/ppc32.patch
index 343eff5851..da8b6b7c7c 100644
--- a/srcpkgs/nodejs-lts-10/patches/ppc32.patch
+++ b/srcpkgs/nodejs-lts-10/patches/ppc32.patch
@@ -15,7 +15,7 @@
        'msvs_disabled_warnings!': [4244],

        'conditions': [
-+        [ 'host_arch=="mips" or host_arch=="mipsel" or host_arch=="ppc"', {
++        [ 'target_arch=="mips" or target_arch=="mipsel" or target_arch=="ppc" or target_arch=="arm"', {
 +        'link_settings': {
 +          'libraries': [ '-latomic' ],
 +        },
diff --git a/srcpkgs/nodejs-lts-10/template b/srcpkgs/nodejs-lts-10/template
index 3944d359c6..a680db30cc 100644
--- a/srcpkgs/nodejs-lts-10/template
+++ b/srcpkgs/nodejs-lts-10/template
@@ -37,9 +37,7 @@ if [ "$XBPS_WORDSIZE" -ne "$XBPS_TARGET_WORDSIZE" ]; then
        nocross="host and target must have the same pointer size"
 fi

-if [ "$XBPS_TARGET_NO_ATOMIC8" ]; then
-       makedepends+=" libatomic-devel"
-fi
+makedepends+=" libatomic-devel"
 if [ "$XBPS_NO_ATOMIC8" ]; then
        hostmakedepends+=" libatomic-devel"
 fi

Well this seems to work, at least - compiles on armv6l and armv7l, node pulls in libatomic on armv6l and doesn't on armv7l. I'd rather be able to pass $XBPS_TARGET_NO_ATOMIC8 off to the build system directly but (granted, with 5 minutes of looking) there doesn't appear to be a good way to do that.

[ericonr]

I think including the atomic stuff in the same PR is reasonable, if you want. Ideally our linker flags should avoid a runtime dependency on libatomic, since the binaries should end up not using any symbols from it.

[m0rg-dev]

Sure, I can do that.

I did check - that patch doesn't cause a runtime dependency on libatomic when it's not needed.

[ericonr]

Ah, in the interest of your change not being reverted due to lack of understanding, add a comment to the template saying that "our build system patches require libatomic for more archs than just those covered by XBPS_TARGET_NO_ATOMIC8" or something close to that :)

[m0rg-dev]

Does that look good?

[ericonr]

Closed with #30516