build: commit some android build and test fixtures#57748
Open
thunder-coding wants to merge 2 commits into
Open
build: commit some android build and test fixtures#57748thunder-coding wants to merge 2 commits into
thunder-coding wants to merge 2 commits into
Conversation
Collaborator
|
Review requested:
|
nodejs-github-bot
added
needs-ci
thunder-coding
force-pushed
the
android-fixes
branch
from
5d4758e to
3087b6f
Compare
thunder-coding
changed the title
thunder-coding
force-pushed
the
android-fixes
branch
2 times, most recently
from
17a21ac to
961976e
Compare
thunder-coding
commented
Apr 4, 2025
| patches = [ | ||
| './android-patches/common.gypi.patch', | ||
| './android-patches/trap-handler.h.patch', | ||
| './android-patches/v8.gyp.patch', |
Contributor
Author
There was a problem hiding this comment.
I believe both v8.gyp.patch and common.gypi.patch should be actually committed rather than being left as patch files. The reason why I made them as patch files is because I find them quite hacky and smelly. Would like to know what approach do you guys prefer.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #57748 +/- ##
==========================================
- Coverage 90.24% 90.24% -0.01%
==========================================
Files 630 630
Lines 184990 184990
Branches 36216 36215 -1
==========================================
- Hits 166948 166936 -12
+ Misses 11003 11001 -2
- Partials 7039 7053 +14 🚀 New features to boost your workflow:
|
Contributor
Author
|
Interesting, MacOS and FreeBSD do not have procfs, but have |
This commit contains some test fixes as well as build fixes for Android which I have been maintaining for Termux since a few years now. The full list of patches can be found at https://github.com/termux/termux-packages/tree/master/packages/nodejs and https://github.com/termux/termux-packages/tree/master/packages/nodejs-lts Not all the patches from the above sources are being committed over here as a some off them are specific to Termux. This does not fix the builds for Android completely, I was not able to successfully build Node locally according to the build instructions for Android, but I was able to build with the patches as in the above two URLs, in a separate environment. The build scripts used to build Node can be found in build.sh in the above directories linked. Here's a summary of what's changed: - Older Android versions doesn't have /dev/stdin, /dev/stdout, or /dev/stderr. Although this seems to have changed now, I can see that they exist on my new phone. Anyways these are specifically listed as not being the part of the POSIX standard. https://pubs.opengroup.org/onlinepubs/9799919799/ (See 2.1.1 Requirements, the last bullet point of 4th point specifically states that these three character special files are non- standard extension). The tests making use of /dev/stdin have been modified to use /proc/self/fd/0 which is guaranteed to exist on all POSIX platforms - Some other tests where Linux kernel behavior is different from other platforms (buffer size tests) have been patched to treat Android as Linux since Android makes use of the Linux kernel under the hood. - tools/v8_gypfiles/toolchain.gypi adds -m32 to host build flags when compiling for 32 bit target architecture. But in common.gypi adding -m64 conflicts with it which may cause compilation errors. Having both -m32 and -m64 may raise errors that are hard to debug. This patch probably needs to be improved but works good enough for Android builds. This probably hasn't been noticed yet because cross-compilations aren't that common for other platforms, Android has no other option but to cross compile. android-patches/common.gypi.patch does fix the builds for 32-bit architectures on 64-bit host, by not not adding -m64 to host build when cross compiling for 32-bit. - android-patches/v8.gyp.patch fixes assembly errors during cross compilation for 32-bit on 64-bit host. This patch is needed for the same reason as for the above patch - android-patches/trap-handler.h.patch has been updated so that it applies cleanly in the new source tree. - libuv does not implement uv_udp_set_source_membership for Android, OpenBSD and FreeBSD. So for these platforms it returns ENOSYS instead of EINVAL in tests. Tests have been patched to reflect this
FreeBSD and MacOS do not have procfs, so use /dev/fd/0
This is still not the POSIX way of doing things. Apparently there exist
no POSIX way of doing this. /dev/stdin, /dev/stdout, and /dev/stderr
are POSIX extensions, so is procfs. Also there is no mention of
/dev/fd/{fd} anywhere at all in the POSIX specification over at
https://doi.org/10.1109/IEEESTD.2024.10555529
thunder-coding
force-pushed
the
android-fixes
branch
from
961976e to
e03ad8c
Compare
Contributor
Author
|
Hey, it's been 3 days since I created this PR and haven't received any reviews. Also I think @nodejs/build should also review this PR (someone from the nodejs org needs to mention this, as I am not part of the org, this doesn't actually notify them) |
Member
|
@thunder-coding ... three days or even a full is not too long. Folks will perform reviews when they are able to do so. @nodejs/build |
jsamol
added a commit
to Acurast/nodejs-mobile
that referenced
this pull request
Aug 6, 2025
jsamol
added a commit
to Acurast/nodejs-mobile
that referenced
this pull request
Aug 6, 2025
jsamol
added a commit
to Acurast/nodejs-mobile
that referenced
this pull request
Aug 6, 2025
lluisemper
added a commit
to lluisemper/node
that referenced
this pull request
Aug 19, 2025
Replace hardcoded Android patch application with JSON configuration to improve maintainability and enable multiple patches. The current patch_android() function only applies a single hardcoded patch, making it difficult to add new patches needed for Android builds. Recent contributions show multiple patches are required for proper Android cross-compilation support. This change: - Creates android-patches.json for patch configuration - Refactors patch_android() to read from JSON - Enables platform-specific patch filtering - Maintains 100% backward compatibility - Prepares infrastructure for additional patches Fixes: TODO comment in android_configure.py line 5 Refs: nodejs#57748
lluisemper
added a commit
to lluisemper/node
that referenced
this pull request
Aug 19, 2025
Replace hardcoded Android patch application with JSON configuration to improve maintainability and enable multiple patches. The current patch_android() function only applies a single hardcoded patch, making it difficult to add new patches needed for Android builds. Recent contributions show multiple patches are required for proper Android cross-compilation support. This change: - Creates android-patches.json for patch configuration - Refactors patch_android() to read from JSON - Enables platform-specific patch filtering - Maintains 100% backward compatibility - Prepares infrastructure for additional patches Implements TODO comment in android_configure.py line 5 Refs: nodejs#57748
lluisemper
added a commit
to lluisemper/node
that referenced
this pull request
Aug 19, 2025
Replace hardcoded Android patch application with JSON configuration to improve maintainability and enable multiple patches. The current patch_android() function only applies a single hardcoded patch, making it difficult to add new patches needed for Android builds. Recent contributions show multiple patches are required for proper Android cross-compilation support. This change: - Creates android-patches.json for patch configuration - Refactors patch_android() to read from JSON - Enables platform-specific patch filtering - Maintains 100% backward compatibility - Prepares infrastructure for additional patches Implements TODO comment in android_configure.py line 5 Refs: nodejs#57748
Valodim
added a commit
to heylogin/nodejs-mobile
that referenced
this pull request
Oct 22, 2025
From nodejs/node#57748 tools/v8_gypfiles/toolchain.gypi adds -m32 to host build flags when compiling for 32 bit target architecture. But in common.gypi adding -m64 conflicts with it which may cause compilation errors. Having both -m32 and -m64 may raise errors that are hard to debug. This patch probably needs to be improved but works good enough for Android builds. This probably hasn't been noticed yet because cross-compilations aren't that common for other platforms, Android has no other option but to cross compile. This commit does fix the builds for 32-bit architectures on 64-bit host, by not not adding -m64 to host build when cross compiling for 32-bit.
Valodim
added a commit
to heylogin/nodejs-mobile
that referenced
this pull request
Oct 23, 2025
From nodejs/node#57748 tools/v8_gypfiles/toolchain.gypi adds -m32 to host build flags when compiling for 32 bit target architecture. But in common.gypi adding -m64 conflicts with it which may cause compilation errors. Having both -m32 and -m64 may raise errors that are hard to debug. This patch probably needs to be improved but works good enough for Android builds. This probably hasn't been noticed yet because cross-compilations aren't that common for other platforms, Android has no other option but to cross compile. This commit does fix the builds for 32-bit architectures on 64-bit host, by not not adding -m64 to host build when cross compiling for 32-bit.
Valodim
added a commit
to heylogin/nodejs-mobile
that referenced
this pull request
Oct 23, 2025
From nodejs/node#57748 tools/v8_gypfiles/toolchain.gypi adds -m32 to host build flags when compiling for 32 bit target architecture. But in common.gypi adding -m64 conflicts with it which may cause compilation errors. Having both -m32 and -m64 may raise errors that are hard to debug. This patch probably needs to be improved but works good enough for Android builds. This probably hasn't been noticed yet because cross-compilations aren't that common for other platforms, Android has no other option but to cross compile. This commit does fix the builds for 32-bit architectures on 64-bit host, by not not adding -m64 to host build when cross compiling for 32-bit.
Valodim
added a commit
to heylogin/nodejs-mobile
that referenced
this pull request
Oct 30, 2025
From nodejs/node#57748 tools/v8_gypfiles/toolchain.gypi adds -m32 to host build flags when compiling for 32 bit target architecture. But in common.gypi adding -m64 conflicts with it which may cause compilation errors. Having both -m32 and -m64 may raise errors that are hard to debug. This patch probably needs to be improved but works good enough for Android builds. This probably hasn't been noticed yet because cross-compilations aren't that common for other platforms, Android has no other option but to cross compile. This commit does fix the builds for 32-bit architectures on 64-bit host, by not not adding -m64 to host build when cross compiling for 32-bit.
Valodim
added a commit
to heylogin/nodejs-mobile
that referenced
this pull request
Nov 4, 2025
From nodejs/node#57748 tools/v8_gypfiles/toolchain.gypi adds -m32 to host build flags when compiling for 32 bit target architecture. But in common.gypi adding -m64 conflicts with it which may cause compilation errors. Having both -m32 and -m64 may raise errors that are hard to debug. This patch probably needs to be improved but works good enough for Android builds. This probably hasn't been noticed yet because cross-compilations aren't that common for other platforms, Android has no other option but to cross compile. This commit does fix the builds for 32-bit architectures on 64-bit host, by not not adding -m64 to host build when cross compiling for 32-bit.
Contributor
|
This pull request has been marked as stale due to 210 days of inactivity. |
Contributor
Author
|
Hey, I can look forward into rebasing these patches. I already do have slightly different set of patches for newer versions, but I need someone to review them. |
gmaclennan
pushed a commit
to gmaclennan/nodejs-mobile
that referenced
this pull request
May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit contains some test fixes as well as build fixes for Android which I have been maintaining for Termux since a few years now. The full list of patches can be found at https://github.com/termux/termux-packages/tree/master/packages/nodejs and https://github.com/termux/termux-packages/tree/master/packages/nodejs-lts Not all the patches from the above sources are being committed over here as a some off them are specific to Termux. This does not fix the builds for Android completely, I was not able to successfully build Node locally according to the build instructions for Android, but I was able to build with the patches as in the above two URLs, in a separate environment. The build scripts used to build Node can be found in build.sh in the above directories linked.
Here's a summary of what's changed:
Older Android versions doesn't have /dev/stdin, /dev/stdout, or /dev/stderr. Although this seems to have changed now, I can see that they exist on my new phone. Anyways these are specifically listed as not being the part of the POSIX standard. https://pubs.opengroup.org/onlinepubs/9799919799/ (See 2.1.1 Requirements, the last bullet point of 4th point specifically states that these three character special files are non- standard extension). The tests making use of /dev/stdin have been modified to use /proc/self/fd/0 which is guaranteed to exist on all POSIX platforms
Some other tests where Linux kernel behavior is different from other platforms (buffer size tests) have been patched to treat Android as Linux since Android makes use of the Linux kernel under the hood.
tools/v8_gypfiles/toolchain.gypi adds -m32 to host build flags when compiling for 32 bit target architecture. But in common.gypi adding -m64 conflicts with it which may cause compilation errors. Having both -m32 and -m64 may raise errors that are hard to debug. This patch probably needs to be improved but works good enough for Android builds. This probably hasn't been noticed yet because cross-compilations aren't that common for other platforms, Android has no other option but to cross compile. android-patches/common.gypi.patch does fix the builds for 32-bit architectures on 64-bit host, by not not adding -m64 to host build when cross compiling for 32-bit.
android-patches/v8.gyp.patch fixes assembly errors during cross compilation for 32-bit on 64-bit host. This patch is needed for the same reason as for the above patch
android-patches/trap-handler.h.patch has been updated so that it applies cleanly in the new source tree.
libuv does not implement uv_udp_set_source_membership for Android, OpenBSD and FreeBSD. So for these platforms it returns ENOSYS instead of EINVAL in tests. Tests have been patched to reflect this
Additionally, I also tried building for FreeBSD in a VM, but the builds failed because I didn't apply the patches used by FreeBSD https://github.com/freebsd/freebsd-ports/tree/main/www/node23