Skip to content

Remove core modules from the default tmmdelegate#43939

Closed
RSNara wants to merge 2 commits intofacebook:mainfrom
RSNara:export-D55613024
Closed

Remove core modules from the default tmmdelegate#43939
RSNara wants to merge 2 commits intofacebook:mainfrom
RSNara:export-D55613024

Conversation

@RSNara
Copy link
Copy Markdown
Contributor

@RSNara RSNara commented Apr 5, 2024

Summary:

Problem

If we link the default tmmdelegate with our vr apps, we get this issue:

ld.lld: error: duplicate symbol: facebook::react::NativeDevLoadingViewSpecJSI::NativeDevLoadingViewSpecJSI(facebook::react::JavaTurboModule::InitParams const&)
>>> defined at firsttimenux_v2AppModulesCodegen-generated.cpp:1367 (buck-out/v2/gen/fbsource/bcbe7a50bd5ff29a/arvr/libraries/react-panellib/FirstTimeNux/__firsttimenux_v2AppModulesCodegen-codegen-modules-jni_cpp__/out/firsttimenux_v2AppModulesCodegen-generated.cpp:1367)
>>>            firsttimenux_v2AppModulesCodegen-generated.cpp.pic.o:(facebook::react::NativeDevLoadingViewSpecJSI::NativeDevLoadingViewSpecJSI(facebook::react::JavaTurboModule::InitParams const&)) in archive buck-out/v2/gen/fbsource/bcbe7a50bd5ff29a/arvr/libraries/react-panellib/FirstTimeNux/__firsttimenux_v2AppModulesCodegen-jni__/libfirsttimenux_v2AppModulesCodegen-jni.pic.a
>>> defined at rncore-generated.cpp:606 (buck-out/v2/gen/fbsource/bcbe7a50bd5ff29a/xplat/js/react-native-github/__rncore-codegen-modules-jni_cpp__/out/rncore-generated.cpp:606)
>>>            rncore-generated.cpp.pic.o:(.text._ZN8facebook5react27NativeDevLoadingViewSpecJSIC2ERKNS0_15JavaTurboModule10InitParamsE+0x0) in archive buck-out/v2/gen/fbsource/bcbe7a50bd5ff29a/xplat/js/react-native-github/__rncore-jniAndroid__/librncore-jniAndroid.pic.a

Cause

My best understanding of the problem:

  • Default tmmdelegate links against rncore, which contains codegen for react native's standard library of modules.
  • But, the default delegate also pulls in this appmodules.so library. That library also contains codegen for react native's standard library of modules + the app's modules.

So, two so libraries define the same symbols. Hence the build fails.

Solution

Remove the codegen for react native's standard library of modules from the default tmmdelegate.

Prereq: In open source, also make appmodules.so include the codegen for react native's standard library of modules.

Changelog: [Internal]

Reviewed By: cortinico

Differential Revision: D55613024

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 5, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D55613024

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Apr 5, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,201,430 +16,401
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,578,199 +16,390
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: efb0007
Branch: main

RSNara added 2 commits April 5, 2024 15:13
Summary:
Pull Request resolved: facebook#43939

## Problem
If we link the default tmmdelegate with our vr apps, we get this issue:

```
ld.lld: error: duplicate symbol: facebook::react::NativeDevLoadingViewSpecJSI::NativeDevLoadingViewSpecJSI(facebook::react::JavaTurboModule::InitParams const&)
>>> defined at firsttimenux_v2AppModulesCodegen-generated.cpp:1367 (buck-out/v2/gen/fbsource/bcbe7a50bd5ff29a/arvr/libraries/react-panellib/FirstTimeNux/__firsttimenux_v2AppModulesCodegen-codegen-modules-jni_cpp__/out/firsttimenux_v2AppModulesCodegen-generated.cpp:1367)
>>>            firsttimenux_v2AppModulesCodegen-generated.cpp.pic.o:(facebook::react::NativeDevLoadingViewSpecJSI::NativeDevLoadingViewSpecJSI(facebook::react::JavaTurboModule::InitParams const&)) in archive buck-out/v2/gen/fbsource/bcbe7a50bd5ff29a/arvr/libraries/react-panellib/FirstTimeNux/__firsttimenux_v2AppModulesCodegen-jni__/libfirsttimenux_v2AppModulesCodegen-jni.pic.a
>>> defined at rncore-generated.cpp:606 (buck-out/v2/gen/fbsource/bcbe7a50bd5ff29a/xplat/js/react-native-github/__rncore-codegen-modules-jni_cpp__/out/rncore-generated.cpp:606)
>>>            rncore-generated.cpp.pic.o:(.text._ZN8facebook5react27NativeDevLoadingViewSpecJSIC2ERKNS0_15JavaTurboModule10InitParamsE+0x0) in archive buck-out/v2/gen/fbsource/bcbe7a50bd5ff29a/xplat/js/react-native-github/__rncore-jniAndroid__/librncore-jniAndroid.pic.a
```

## Cause
My best understanding of the problem:
- Default tmmdelegate links against rncore, which contains codegen for react native's standard library of modules.
- But, the default delegate also pulls in this appmodules.so library. That library also contains codegen for react native's standard library of modules + the app's modules.

So, two so libraries define the same symbols. Hence the build fails.

## Solution
Remove the codegen for react native's standard library of modules from the default tmmdelegate.

Prereq: In open source, also make appmodules.so include the codegen for react native's standard library of modules.

Changelog: [Android][Breaking] - Make the app responsible for returning core turbomodule jsi hostobjects

Reviewed By: cortinico

Differential Revision: D55613024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D55613024

@RSNara RSNara force-pushed the export-D55613024 branch from 18fb978 to b6555e4 Compare April 5, 2024 22:16
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Apr 6, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in 7facb32.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 6, 2024

This pull request was successfully merged by @RSNara in 7facb32.

When will my fix make it into a release? | How to file a pick request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants