[Native] Cleanup grpc/CMakeLists.txt
ClosedPublic
Actions

Authored by • jon on Aug 30 2022, 2:55 PM.

Details

Reviewers

atul
• abosh
• max

Commits

rCOMM368e8118168c: [Native] Cleanup grpc/CMakeLists.txt

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

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• jon created this revision.Aug 30 2022, 2:55 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptAug 30 2022, 2:55 PM

• jon added a child revision: D4989: [Native] Cleanup NativeModules/CMakeLists.txt.Aug 30 2022, 2:55 PM

Harbormaster completed remote builds in B11728: Diff 16118.Aug 30 2022, 3:05 PM

• jon requested review of this revision.Aug 30 2022, 3:05 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

Closed by commit rCOMM368e8118168c: [Native] Cleanup grpc/CMakeLists.txt (authored by • jon). · Explain WhySep 2 2022, 3:22 PM

This revision was automatically updated to reflect the committed changes.

• jon added a commit: rCOMM368e8118168c: [Native] Cleanup grpc/CMakeLists.txt.