[ISSUE] nodejs packages should be udpated to use more system libraries

[ISSUE] nodejs packages should be udpated to use more system libraries

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

New issue by ericonr on void-packages repository

https://github.com/void-linux/void-packages/issues/29034

Description:
`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.

Re: nodejs packages should be fixed to use more system libraries

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 338 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-785299763

Comment:
As seen in https://github.com/void-linux/void-packages/issues/26427#issuecomment-777323773 , the build is also failing on armv6l-musl due to `libatomic` somehow getting lost in the build process.

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826356363

Comment:
It looks like `nodejs{,-lts,-lts-10}` have been on system OpenSSL since fefffea5d79d3ff7bca309bcc514bc8648160faf.

`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.

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826366095

Comment:
`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

Re: nodejs packages should be fixed to use more system libraries

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 341 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826367673

Comment:
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.

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 330 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826368116

Comment:
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.

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826368204

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

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 208 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826375338

Comment:
*reads README* wait that might not actually be a problem

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826378154

Comment:
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...

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826380039

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

I can read!

Re: nodejs packages should be fixed to use more system libraries

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826381205

Comment:
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 :/

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826382047

Comment:
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?

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 417 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826387196

Comment:
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.

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 1616 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826402170

Comment:
```diff
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.

Re: nodejs packages should be fixed to use more system libraries

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826920270

Comment:
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.

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826921691

Comment:
Sure, I can do that.

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

Re: nodejs packages should be fixed to use more system libraries

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826924209

Comment:
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 :)

Re: nodejs packages should be fixed to use more system libraries

[digitalagedragon]

[-- Attachment #1: Type: text/plain, Size: 172 bytes --]

New comment by digitalagedragon on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-826926721

Comment:
Does that look good?

Re: nodejs packages should be fixed to use more system libraries

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 161 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/issues/29034#issuecomment-828416723

Comment:
Closed with #30516

Re: [ISSUE] [CLOSED] nodejs packages should be fixed to use more system libraries

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

Closed issue by ericonr on void-packages repository

https://github.com/void-linux/void-packages/issues/29034

Description:
`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.