Page MenuHomePhabricator

[Native] Fix native modules CMake build
ClosedPublic

Authored by jon on Dec 29 2022, 12:03 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 8, 11:31 PM
Unknown Object (File)
Nov 9 2024, 2:26 AM
Unknown Object (File)
Nov 9 2024, 2:26 AM
Unknown Object (File)
Nov 9 2024, 2:25 AM
Unknown Object (File)
Nov 9 2024, 2:24 AM
Unknown Object (File)
Nov 9 2024, 1:10 AM
Unknown Object (File)
Nov 4 2024, 5:23 AM
Unknown Object (File)
Nov 4 2024, 5:23 AM
Subscribers

Details

Summary

Update native CMake logic to include addition of the
native_rust_library and another react-native header.

The native modules needs to be references as a CMake target
to avoid usage of GLOB_RECURSIVE in our Android build.

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

Test Plan
cd native/cpp/CommonCpp/
rm -rf build && mkdir build && cd build && cmake .. && make -j

Diff Detail

Repository
rCOMM Comm
Branch
jonringer/fix-native-modules
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

We're moving these out from native/android/app/CMakeLists.txt, yeah? It might make review easier if you try to find a way you can do the additions and removals in the same diff – then it becomes easier to grok what's going on as a reviewer. Not always possible though

We're moving these out from native/android/app/CMakeLists.txt, yeah?

Correct, mention it in the second part of the summary.

It might make review easier if you try to find a way you can do the additions and removals in the same diff – then it becomes easier to grok what's going on as a reviewer. Not always possible though

It's going to be quite large if I do that. I would really like to avoid having 100+ line diffs which no one wants to review.

This compiles for me

native/cpp/CommonCpp/NativeModules/CMakeLists.txt
4–5 ↗(On Diff #20386)

Could this comment be added here? I was wondering why we need the CMAKE_OSX_DEPLOYMENT_TARGET and finally found this comment in our other CMake files.

This revision is now accepted and ready to land.Jan 10 2023, 3:03 AM

Add comment about OSX setting

This revision was automatically updated to reflect the committed changes.