Skip to content
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

nodejs: Bump to 16.x #7505

Merged
14 commits merged into from
Sep 14, 2021
Merged

nodejs: Bump to 16.x #7505

14 commits merged into from
Sep 14, 2021

Conversation

thunder-coding
Copy link
Member

No description provided.

@thunder-coding
Copy link
Member Author

thunder-coding commented Sep 11, 2021

So, let me first explain what changes I have made here and why since this PR is gonna be large

This comment contains problems faced while building node 16.6.1

1. The patches have been shamelessly stolen from suhan-paradkar#8

2. About assembly errors for i868 and arm

This is a known upstream issue about the nodejs toolchain suicidally passing -m32 for host builds of nodejs when cross compiling for 32 bit. See nodejs/node#39736

This needs some special attention

This has been fixed by a temporary patch. x64/push_registers_asm.cc has been patched in a way:

#if defined(V8_TARGET_ARCH_ARM64) || defined(V8_TARGET_ARCH_X86_64)
//original content of the file
#else
// Content of ia32 ASM registers.cc
#endif

This ensures that both cross compile and host builds work. This patch should be removed as soon as upstream fixes it.

3. The toolchain tries to use shared libraries which are passed to configure.py but doesn't pass appropriate linker flags which are fixed by hardcoding host linker to have those linker flags. Luckily it doesn't use the shared libraries compiled for Android.

  1. x86_64 builds are broken as in Cross compile node v14.x or v.15.x for android x86_64 nodejs/node#36287 (comment) This should be fixed by some patches in node.gyp as in the comment. This needs to be done properly

The lines in strikes have been fixed and need no attention

@thunder-coding
Copy link
Member Author

thunder-coding commented Sep 12, 2021

Problems faced while building node 16.9.1

1. arpa_nameser.h include: The GYP files somehow didn't add deps/cares/src/lib as include path

2. libuv didn't compile with epoll.c. This patch needs to be also sent upstream

3. ia32(i686) builds fail with undefined reference to functions defined in <atomic>. For mkcodecache this was fixed by adding -latomic, but for node_mksnapshot, this doesn't fix it.
This was fixed by passing proper -u flags to clang as stated in Grimler's comment below

Besides these problems, problems with node 16.6.1 were also persistent without patches

@thunder-coding
Copy link
Member Author

@xeffyr, I got it working for aarch64 and arm. And I tested the binaries and they worked. And only i686 is failing with undefined references for functions declared in <atomic>, I even added a -latomic flag but it didn't work. And some weird thing is that it shows these errors in a file while doesn't even use any of the methods declared in <atomic>. I need your help with this so that nodejs 16 can be available on Termux as soon as possible

@Grimler91
Copy link
Member

Flag order could matter (see #3092). Try if patching histograp.gyp, adding 'ldflags': [ '-latomic' ], (or similar) there instead works, and else maybe -u __atomic_fetch_add_8 -u __atomic_load_8 -u __atomic_compare_exchange_8 -latomic as suggested in the other issue

We are passing flags to `configure.py` in order to tell the nodejs build
toolchain to use shared libraries. The toolchain seems to mistakenly
also try use shared libraries for host build, but it does not add
respective linker flags (For example `-lz` for zlib). This commit
hardcodes these flags to be always passed to linker by setting linker to
linker + LDFLAGS of shared libraries
This patch also needs to be sent upstream. The nodejs team has done this
for linux but somehow didn't do it for android
I don't think much people use x86_64, and anyways it was already
blacklisted, I might look into it later
The -u flags are required only for i686. So why add them to all archs?
@thunder-coding
Copy link
Member Author

This PR is now ready to merge. The previous CI passed 🥳. I have also declared myself as a maintainer for this package so that this package stays updated

@ghost ghost merged commit d6a3de3 into termux:master Sep 14, 2021
@thunder-coding thunder-coding deleted the nodejs branch September 14, 2021 08:28
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants