Generate the protobuf C++ files and also produce phabricator
compatible headers as well.
Details
Details
nix develop cd native/cpp/CommonCpp/grpc/ rm -rf build mkdir build cd build cmake .. make -j # do initial build # somehow modify the files touch ../protos/blob.proto make -j # assert that only blob is being rebuilt # assert that no files have changed git status # assert that build is idempotent make -j
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
interesting, because I added @generated to the logic, phabricator automatically deems this to be generated
Comment Actions
Are we going to use the same auto-generation logic inside services, which also consumes the codegenned Protobufs?
Comment Actions
Are we going to use the same auto-generation logic inside services, which also consumes the codegenned Protobufs?
This should already be done since add_subdirectory(<path>/grpc) will include this logic in any downstream package (e.g. any service right now).
https://phab.comm.dev/D4778 is an example where android will now call this logic.
Comment Actions
I don't think the steps in the Test Plan are reproducible as written? We don't have any Makefiles in the repo, so presumably there's some cmake command that needs to be run to generate the Makefile?
As an aside, when I tried generating the Makefile with cmake I got the following:
CMake Error at CMakeLists.txt:56 (add_library): Target "comm-tunnelbroker-grpc" links to target "Threads::Threads" but the target was not found. Perhaps a find_package() call is missing for an IMPORTED target, or an ALIAS target is missing?
Comment Actions
I don't think the steps in the Test Plan are reproducible as written? We don't have any Makefiles in the repo, so presumably there's some cmake command that needs to be run to generate the Makefile?
Oops, forgot write cmake invocation
Comment Actions
I don't think the steps in the Test Plan are reproducible as written? We don't have any Makefiles in the repo, so presumably there's some cmake command that needs to be run to generate the Makefile?
Did you call nix develop (I forgot to add this in original test plan).
But that looks related to https://discourse.cmake.org/t/cmake-3-21-rc-1-threads-threads-not-found/3630 , nix dev environment should have cmake 3.23.2
Comment Actions
Repeative to declare MAIN_DEPENEDENCY and DEPENDS to the same thing
Also rebase on master
Comment Actions
I got a protoc version mismatch when building services in a nix develop when testing this diff:
nix develop cd services/backup rm -dfr build cmake -B build . && make -C build -j4
The errors are:
make[2]: Leaving directory '/Users/max/GitHub/comm/services/backup/build' make[2]: Entering directory '/Users/max/GitHub/comm/services/backup/build' make[2]: Leaving directory '/Users/max/GitHub/comm/services/backup/build' make[2]: Entering directory '/Users/max/GitHub/comm/services/backup/build' [ 20%] Building CXX object protos/CMakeFiles/comm-backup-grpc.dir/backup.pb.cc.o [ 24%] Building CXX object protos/CMakeFiles/comm-blob-grpc.dir/blob.pb.cc.o In file included from /Users/max/GitHub/comm/services/backup/build/protos/backup.pb.cc:4: /Users/max/GitHub/comm/services/backup/build/protos/backup.pb.h:17:2: error: This file was generated by an older version of protoc which is #error This file was generated by an older version of protoc which is ^ /Users/max/GitHub/comm/services/backup/build/protos/backup.pb.h:18:2: error: incompatible with your Protocol Buffer headers. Please #error incompatible with your Protocol Buffer headers. Please ^
Inside the nix develop the protoc shows:
protoc --version libprotoc 3.15.8 which protoc /nix/store/r5nv2l4hiraggzawpjhxhbqgnc93p0a9-protobuf-3.15.8/bin/protoc
Seems that protoc version is ok and it's possibly related to the headers pointing.
Comment Actions
I've removed the local protoc using the homebrew uninstall protobuf and it works as expected. Seems that this is related to the ENG-1590 issue and interference with the locally installed dev tools and not related to this certain diff.
Comment Actions
It all boils down to ENG-1588... in order for the iOS build to work right now, we need to keep the user's default $PATH in the Nix environment, and we need to have it first in the order. As a result, things like your local protoc and your local clang will take precedence over the Nix versions.
It might make sense for us to "punt" on the iOS build for now in order to unblock shipping C++ / services Nix support. Then we can work on ENG-1588 and consider it a blocker for iOS support. @jon, what do you think?
Comment Actions
Did you call nix develop (I forgot to add this in original test plan).
Oh yeah that was the issue when I was running CMake within CLion.. didn't open CLion from within Nix shell so it wasn't picking that up.
But now that the CMake commands are listed in test plan I'll give that a shot
Comment Actions
Hm running into the following:
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....
/Users/atul/comm/native/cpp/CommonCpp/grpc/Client.cpp:91:12: error: no member named 'make_unique' in namespace 'std'
std::make_unique<ClientGetReadReactor>(this->stub_.get(), sessionID);
~~~~~^
/Users/atul/comm/native/cpp/CommonCpp/grpc/Client.cpp:91:24: error: 'ClientGetReadReactor' does not refer to a value
std::make_unique<ClientGetReadReactor>(this->stub_.get(), sessionID);
^
/Users/atul/comm/native/cpp/CommonCpp/grpc/ClientGetReadReactor.h:9:7: note: declared here
class ClientGetReadReactor
^
2 errors generated.
make[2]: *** [CMakeFiles/comm-client.dir/Client.cpp.o] Error 1
make[1]: *** [CMakeFiles/comm-client.dir/all] Error 2
make: *** [all] Error 2Patched the diff, made sure I was in the nix shell, etc.
Comment Actions
/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-grpcAh yes, this is because the NativeModules dependencies aren't captured, this is fixed by a later diff... I'll update test plan