Page MenuHomePhabricator

[services] Export generated code as CMake Project
AbandonedPublic

Authored by jon on Jun 19 2022, 3:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 11:09 AM
Unknown Object (File)
Fri, Nov 22, 10:40 AM
Unknown Object (File)
Fri, Nov 22, 5:19 AM
Unknown Object (File)
Sun, Nov 10, 5:05 AM
Unknown Object (File)
Mon, Oct 28, 2:59 AM
Unknown Object (File)
Sun, Oct 27, 6:36 AM
Unknown Object (File)
Sun, Oct 27, 6:36 AM
Unknown Object (File)
Sun, Oct 27, 6:36 AM

Details

Summary

Export the generated react-native NativeModules
as a CMake Project.

Generating the related files through CMake is a bit more involved,
so just present the generated files with a pretty bow.

Depends on D4301

Test Plan

nix develop && cd native/cpp/ && mkdir build && cd build && cmake .. && make

Diff Detail

Repository
rCOMM Comm
Branch
joringer/add-nix-clean (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 19 2022, 4:06 PM
Harbormaster failed remote builds in B9821: Diff 13585!
Harbormaster returned this revision to the author for changes because remote builds failed.Jun 22 2022, 8:34 PM
Harbormaster failed remote builds in B9911: Diff 13703!

Update with protobuf changes

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 3 2022, 9:25 AM
Harbormaster failed remote builds in B10231: Diff 14118!
jon published this revision for review.Jul 7 2022, 3:38 PM
jon added reviewers: ashoat, atul, varun.

Resigning since I'm usually not a good person to set as a first-pass reviewer. Exceptions here

Not able to test this locally because I can't get nix to work at the moment, but it looks right to me.

Some of these CMakeLists.txt files assume that dependencies like gRPC and OpenSSL and whatever are available on the machine and can be found. This seems like a reasonable assumption when everything is under Nix, but could we include calls to FetchContent (https://cmake.org/cmake/help/latest/module/FetchContent.html) to fetch the dependencies in case they aren't on the machine to keep things more portable?

native/cpp/CommonCpp/_generated/CMakeLists.txt
18 ↗(On Diff #14303)

Can we add a little more detail here before landing?

This revision is now accepted and ready to land.Jul 14 2022, 8:59 PM

Some of these CMakeLists.txt files assume that dependencies like gRPC and OpenSSL and whatever are available on the machine and can be found. This seems like a reasonable assumption when everything is under Nix, but could we include calls to FetchContent (https://cmake.org/cmake/help/latest/module/FetchContent.html) to fetch the dependencies in case they aren't on the machine to keep things more portable?

Yea, that doesn't seem too unreasonable. But there is a long term maintenance burden with doing this, as there's a need to maintain two configuration which may be out-of-sync. However, falling back to pulling from a repo doesn't seem too bad. I can create a task to do so.

native/cpp/CommonCpp/_generated/CMakeLists.txt
18 ↗(On Diff #14303)

yea, I can expand

ashoat requested changes to this revision.Jul 15 2022, 1:01 PM
ashoat added inline comments.
native/cpp/CommonCpp/_generated/CMakeLists.txt
26 ↗(On Diff #14303)

Line longer than 80 chars. See comment here and please prioritize resolving this with a CMake linter so I never have to leave a comment like this again

This revision now requires changes to proceed.Jul 15 2022, 1:01 PM

D4494 was the spiritual successor to this