Rebase on Node v18.10.0#7
Conversation
There was a patch to deps/v8/src/base/platform/platform-posix.cc that no longer seems to apply — it added a iOS condition at line 203 https://github.com/nodejs-mobile/nodejs-mobile/blob/mobile-master/deps/v8/src/base/platform/platform-posix.cc#L203 However I can't see how this is needed now? Original commit: ef3a2ca
Fix for env variables being removed on Android
Fix for Posix on Android < SDK 21
Add version metadata for mobile
Add isIOS and isAndroid
Skip tests on iOS & Android
There are many more workflows in here than there were at 12.19.0 when this was last merged. Some of them may still be useful to put back
|
Hey @gmaclennan, great work, nice to see this moving forward. I wanted to experiment whether squashing all commits into one was possible, so I started the branch tiny-history. Currently it has one commit which got everything from nodejs upstream initial commit up until the |
|
Sounds good @staltz. I realized that v18.10.0 has re-written Android build support, but it's not fully working. Currently the problem I'm hitting is that the v8 build seems to be using system g++ rather than respecting the |
|
@gmaclennan Indeed, nodejs/node@70898b4e67 + nodejs/node@c975c4f674 What if update nodejs-mobile to v16? I know you're trying to get ahead of the curve with v18, but one concern is that nodejs-mobile on v18 would drop support for Android 6.0 and require Android 7.0, which may hurt end users with low-end devices. I believe that moving forwards, once we figure out a nice workflow to update nodejs upstream into this project, then within the next 12 months we can do the v16 => v18 update. For now, it would be good to just get things working with an active LTS. |
|
Yes, that makes sense, especially if v16 can be easier. My thinking had been that since v18 becomes active LTS on Oct 25th, to maybe just leap straight there, but it might be harder. Regarding Android compatibility, that is important. nodejs-mobile currently works with API 21 (Android 5.0) and it would be good to maintain that. Some of Jaime's patches are to enable < API 23 support (e.g. https://github.com/nodejs-mobile/nodejs-mobile/blob/mobile-master/deps/uvwasi/src/uvwasi.c#L1319). Maybe we can keep API 21 compat via some patches. Let me know how building goes with v16. I managed to merge the diff here — not too complicated but there were a couple of files where the patches were no longer necessary, or needed changed because of code changes. It's the build I'm having issues with, I think because of changes to the v8 build process (which shouldn't be an issue because v8 officially supports android, but it doesn't seem to be picking up the toolchain env) |
|
I see that nodejs supports iOS's PR, does it help with nodejs-mobile migration. |
|
I was in the process of updating to v16.18.1 (see branch 16_18_1) but I realized that that version contains the android-configure rewrite which also bumped the minimum Android SDK. So instead, the "conservative" version would be v16.17.1. I'll work on updating nodejs-mobile to 16.17.1 and I'll postpone the android-configure update headaches. |
|
Closing this PR because #9 has been merged |
3 participants
This is an attempt to rebase the changes to nodejs-mobile onto upstream Node v18.10.0. This is using the same approach as #3 by applying the diff
upstream-node-v12.19.0...mobile-masteronto v18.10.0 from upstream. I am going through each file one-by-one and manually resolving differences. Some patches apply cleanly, in other cases sometimes code has been removed, or changed order. Once the patch is applied we can see whether it compiles and then have a starting point for modifying the patches to work. I will keep the task list below updated as I go through each file.Modified
Added
Deleted