Page MenuHomePhabricator

[Native] Cleanup grpc/CMakeLists.txt
ClosedPublic

Authored by jon on Aug 30 2022, 2:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 4:42 AM
Unknown Object (File)
Thu, Nov 21, 11:07 PM
Unknown Object (File)
Sat, Nov 16, 5:07 AM
Unknown Object (File)
Wed, Nov 13, 4:40 AM
Unknown Object (File)
Sun, Nov 3, 7:50 AM
Unknown Object (File)
Oct 27 2024, 5:58 AM
Unknown Object (File)
Oct 22 2024, 4:49 PM
Unknown Object (File)
Oct 13 2024, 11:48 PM
Subscribers

Details

Summary

When adding the the CMakeLists.txt, I wasn't able to
reference the other sibling projects. This change is to remove
the target_include_directories entries in favor
of using target_link_library which is the more canonical way
to reference other projects now that they exist.

Depends on D4987

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

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul requested changes to this revision.Aug 30 2022, 3:25 PM
-- Build files have been written to: /Users/atul/comm/native/cpp/CommonCpp/grpc/build
[ 18%] Generate protobuf files for blob
[ 18%] Generate protobuf files for tunnelbroker
[ 18%] Generate protobuf files for backup
[ 31%] Building CXX object CMakeFiles/comm-blob-grpc.dir/blob.grpc.pb.cc.o
[ 31%] Building CXX object CMakeFiles/comm-blob-grpc.dir/blob.pb.cc.o
[ 37%] Building CXX object CMakeFiles/comm-backup-grpc.dir/backup.grpc.pb.cc.o
[ 43%] Building CXX object CMakeFiles/comm-backup-grpc.dir/backup.pb.cc.o
[ 56%] Building CXX object CMakeFiles/comm-tunnelbroker-grpc.dir/tunnelbroker.grpc.pb.cc.o
[ 56%] Building CXX object CMakeFiles/comm-tunnelbroker-grpc.dir/tunnelbroker.pb.cc.o
[ 62%] Linking CXX static library libcomm-blob-grpc.a
[ 62%] Built target comm-blob-grpc
[ 68%] Linking CXX static library libcomm-tunnelbroker-grpc.a
[ 68%] Built target comm-tunnelbroker-grpc
[ 75%] Building CXX object CMakeFiles/comm-client.dir/ClientGetReadReactor.cpp.o
[ 87%] Building CXX object CMakeFiles/comm-client.dir/GRPCStreamHostObject.cpp.o
[ 87%] Building CXX object CMakeFiles/comm-client.dir/Client.cpp.o
In file included from /Users/atul/comm/native/cpp/CommonCpp/grpc/GRPCStreamHostObject.cpp:2:
In file included from /Users/atul/comm/native/cpp/CommonCpp/grpc/../NativeModules/InternalModules/GlobalNetworkSingleton.h:3:
/Users/atul/comm/native/cpp/CommonCpp/grpc/../NativeModules/InternalModules/../../Tools/WorkerThread.h:3:10: fatal error: 'folly/MPMCQueue.h' file not found
#include <folly/MPMCQueue.h>
         ^~~~~~~~~~~~~~~~~~~
[ 93%] Linking CXX static library libcomm-backup-grpc.a
[ 93%] Built target comm-backup-grpc
1 error generated.
make[2]: *** [CMakeFiles/comm-client.dir/GRPCStreamHostObject.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/comm-client.dir/all] Error 2
make: *** [all] Error 2
This revision now requires changes to proceed.Aug 30 2022, 3:25 PM

/Users/atul/comm/native/cpp/CommonCpp/grpc/build

The purpose of this revisions is to remove grpc's knowledge of the responsibilities of the other projects. The grpc client doesn't use folly, native modules does. Please run the build from CommonCpp. CMake doesn't handle sibling dependencies well, and the canonical way to handle these scenarios is from the parent directory. I chose not to do the canonical thing originally because I would have had to package all of the projects under CommonCpp at the same time; and that would have been a hard no-go with the revision process we have.

Copied from D4986:

This is by design. The idea is that people just need to reference CommonCpp and pulling out the needed project. Since they are all so coupled, it's not much of a loss. Being able to build the project from the directory itself was just to be able to get the majority of the CMake logic merged. The target_include_directories worked because we only used code located in headers, if we wanted to use headers and source files then the changes here are the correct way to reference other projects.

Thanks for the explanation, built as expected

This revision is now accepted and ready to land.Aug 31 2022, 1:21 PM
This revision was automatically updated to reflect the committed changes.