Page MenuHomePhabricator
Differential D6507

[Android] Use native modules from CMake
AbandonedPublic
Actions

Authored by varun on Feb 1 2023, 2:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 5:15 PM
Unknown Object (File)
Fri, Nov 22, 4:42 PM
Unknown Object (File)
Fri, Nov 22, 4:17 PM
Unknown Object (File)
Fri, Nov 22, 11:59 AM
Unknown Object (File)
Wed, Nov 20, 5:05 PM
Unknown Object (File)
Tue, Nov 19, 11:26 PM
Unknown Object (File)
Fri, Nov 15, 12:04 AM
Unknown Object (File)
Mon, Nov 11, 12:49 PM
View All Files
Subscribers
tomek

Details

Reviewers
atul
ashoat
• max
• jon
Summary

Use the CMake native modules target instead of
including the sources directly.

https://linear.app/comm/issue/ENG-776

Test Plan

Android gate passes

yarn react-native run-android

Diff Detail

Repository
rCOMM Comm
Branch
jonringer/android-cmake-sane (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• jon created this revision.Feb 1 2023, 2:01 PM
Herald added a subscriber: tomek. · View Herald TranscriptFeb 1 2023, 2:01 PM
• jon added a parent revision: D6498: [Native] Remove CMake installation logic from native/cpp.Feb 1 2023, 2:02 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Feb 1 2023, 2:19 PM
Harbormaster failed remote builds in B15966: Diff 21802!
• jon updated this revision to Diff 21867.Feb 2 2023, 9:24 AM

Propagate native_rust_library include path

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 2 2023, 9:38 AM
Harbormaster failed remote builds in B16021: Diff 21867!
• jon mentioned this in D6621: [CMake] Resolve rust library include directory.Feb 5 2023, 6:26 PM
• jon edited parent revisions, added: D6621: [CMake] Resolve rust library include directory; removed: D6498: [Native] Remove CMake installation logic from native/cpp.Feb 5 2023, 6:26 PM
• jon updated this revision to Diff 22110.Feb 5 2023, 6:27 PM

Rebase with fix

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 5 2023, 7:03 PM
Harbormaster failed remote builds in B16160: Diff 22110!
• jon updated this revision to Diff 22198.Feb 7 2023, 8:59 AM

Wait on cxxbridge, instead of rust library target

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 7 2023, 9:01 AM
Harbormaster failed remote builds in B16230: Diff 22198!
• jon updated this revision to Diff 22219.Feb 7 2023, 9:14 AM

Rebase on master

Harbormaster completed remote builds in B16247: Diff 22219.Feb 7 2023, 9:59 AM
• jon requested review of this revision.Feb 7 2023, 9:59 AM
ashoat requested changes to this revision.Feb 7 2023, 10:45 AM

Can you modify the Test Plan to include running / playing around with the Android app? I want to make sure it doesn't eg. crash on start, etc.

As always, encourage you to try to reach further in your Test Plan – instead of just testing the immediate / obvious thing, ask yourself how this impacts the product, and test the part of the product that it will impact

This revision now requires changes to proceed.Feb 7 2023, 10:45 AM
• jon added a comment.Feb 9 2023, 10:54 AM

Need to compile the .so's for the native libraries, then make their symbols available to jni with system.loadLibrary()

Can you modify the Test Plan to include running / playing around with the Android app? I want to make sure it doesn't eg. crash on start, etc.

Sorry, the intel mac just goes to 100% fans and does nothing. Switching to the m1 allowed me to make progress; determined that the jni was failing to load symbols as they were previously part of the libcomm_jni_module, but now part of a separate libcomm-native-modules library.

• jon planned changes to this revision.Feb 9 2023, 10:54 AM
• jon mentioned this in rCOMMcf721bda9450: [CMake] Resolve rust library include directory.Feb 20 2023, 6:29 PM
varun commandeered this revision.Jan 15 2024, 12:47 PM
varun abandoned this revision.
varun edited reviewers, added: • jon; removed: varun.

Revision Contents
Changeset List

PathSize
native/
android/
app/
44 lines
cpp/
CommonCpp/
1 line
DatabaseManagers/
3 lines
NativeModules/
1 line

Diff 22110

View Options

native/android/app/CMakeLists.txt

Loading...
View Options

native/cpp/CommonCpp/CMakeLists.txt

Loading...
View Options

native/cpp/CommonCpp/DatabaseManagers/CMakeLists.txt

Loading...
View Options

native/cpp/CommonCpp/NativeModules/CMakeLists.txt

Loading...