refactor: unify android runtime c++ layout#45
Open
DjDeveloperr wants to merge 14 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6d556df to
5058e0d
Compare
Moves the imported Android engine-specific Node-API adapters out of the Android test-app C++ tree while preserving them as Android copies. The shared Node-API headers live under NativeScript/napi/common; merging the drifted engine adapters is intentionally left out of this mechanical move.
5058e0d to
4f9c5df
Compare
Switches Android Hermes over to the same Static Hermes header/library surface used by Apple and keeps SHERMES only as a compatibility selector. This absorbs the earlier transient Hermes adapter/header alignment work so reviewers see one Hermes unification step.
4f9c5df to
64966d5
Compare
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
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.
Summary
platforms/android, while sharing Android C++ throughNativeScript/runtime/android,NativeScript/napi/android, andNativeScript/ffi/jni/napiNativeScript/ffi/objcand update generated-output/test wiring for the unified layoutNativeScript/napi/common, while preserving the drifted Android engine adapters underNativeScript/napi/androidfor parityHERMESandSHERMESonto the same Static Hermes (shermes) header/library surface already used by Apple;SHERMESremains only a compatibility engine selectorReview Notes
NativeScript/napi/androiddoes not mean the engine adapters are intended to stay Android-only forever. The common Node-API headers are shared now, but the engine implementations have real drift across Android and Apple, so this PR keeps the imported Android copies intact while moving them out ofplatforms/android.214ca63eis rename-only for Android engine copies: 1,239 renames, 0 insertions, 0 deletions. The old Androidnapi/commonremoval is in the build-wiring commit, and the old Android Hermes header-surface removal is in the Static Hermes commit.690a8a6fnow keeps AndroidHERMES/SHERMESon the imported Android Hermes sources while wiring the rest of the Android C++ layout.c4246a68owns the actual Static Hermes unification.fix(android): align hermes napi adapterscommit is gone as a standalone review step. Its temporaryinclude_shermes/split-header work is folded into the Static Hermes unification commit below.xit/xdescribeusages are pre-existing upstream disabled specs or Jasmine infrastructure, not new engine-specific gates from this PR.Commit Review Order
f87402a7refactor: move objc ffi under platform namespacedec31505refactor: move android jni napi ffi into shared tree20bcb287refactor: move android runtime sources into shared tree214ca63erefactor(android): move napi engine copies into shared tree690a8a6frefactor: wire android build to unified runtime layout8ee9d157fix(apple): update objc ffi generated pathsc4246a68refactor(android): unify static hermes backende1f8982bfix(android): preserve unified runtime behavior793d549dfix(android): stabilize runtime test harness4c4114f0fix: clean up runtime refactor paths1f659acafix(jni): handle static hermes constructor receivers9872c1bbfix(objc): preserve bridge object ownership732c335dfix(quickjs): clear weak refs before forced gc64966d56test(ios): skip app main in scripted runnerValidation
git diff --checknpm run check:ffi-boundariespython3 -m py_compile metadata-generator/build-step-metadata-generator.py690a8a6fboundary check: Android Hermes paths still point at importedsrc/main/cpp/napi/hermesc4246a68boundary check: oldshermes/fbjnireferences are gonegit show --find-renames=99% --name-status --format= 214ca63e | awk '$1=="D"{print}' | wc -l:0git show --shortstat --oneline 214ca63e:1239 files changed, 0 insertions(+), 0 deletions(-)/tmp/ns-runtime-android-engine-matrix-final-20260608-235950:V8-10: 443 specs, 0 failures, 12 skipped, 16 disabledV8-11: 443 specs, 0 failures, 12 skipped, 16 disabledV8-13: 443 specs, 0 failures, 12 skipped, 16 disabledQUICKJS: 443 specs, 0 failures, 12 skipped, 16 disabledQUICKJS_NG: 443 specs, 0 failures, 12 skipped, 16 disabledPRIMJS: 443 specs, 0 failures, 12 skipped, 16 disabled on rerun; initial run hit oneTNS Workers > Should keep the worker alive after errortimeout, logs in/tmp/ns-runtime-android-primjs-rerun-20260609-001236JSC: 442 specs, 0 failures, 36 skipped, 46 disabledHERMES: 442 specs, 0 failures, 52 skipped, 43 disabledSHERMES: 442 specs, 0 failures, 52 skipped, 43 disabledNativeScript/napi-android:V8-10: 443 specs, 0 failures, 12 skipped, 16 disabledPRIMJS: 443 specs, 0 failures, 12 skipped, 16 disablednapi-androidJSC crash atTNS Workers > Should throw exception when no parameter is passedwithNativeScriptException, with no XML result; parity logs in/tmp/ns-runtime-no-fd62-android-matrix-20260609-002813and/tmp/napi-android-standalone-jsc-parity-20260609-003053/tmp/ns-runtime-apple-matrix-final7:v8: 713 specs, 0 failures, 13 skippedhermes: 713 specs, 0 failures, 11 skippedquickjs: 713 specs, 0 failures, 13 skippedjsc: 713 specs, 0 failures, 13 skipped/tmp/ns-runtime-apple-matrix-final7:v8: 713 specs, 0 failures, 10 skippedhermes: 713 specs, 0 failures, 8 skippedquickjs: 713 specs, 0 failures, 10 skippedjsc: 713 specs, 0 failures, 10 skippedprimjs,quickjs_ng,v8-10,v8-11, andv8-13are still unsupported/unwired on Apple in this PR./gradlew ':runtime:buildCMakeDebug[x86]' -Pengine=QUICKJS_NG -PonlyX86::runtime:buildCMakeDebug[x86]completed; the Gradle invocation then failed in the:app:buildMetadatafinalizer path under local JDK 25 (:android-metadata-generator:compileKotlinrejects25.0.2, and:dts-generator:compileJavafails on existing-Werrorwarnings)./gradlew ':runtime:buildCMakeDebug[x86]' -Pengine=QUICKJS -PonlyX86::runtime:buildCMakeDebug[x86]and:runtime:buildCMakeRelWithDebInfo[x86]completed; same unrelated metadata finalizer failures under JDK 25