CMake integration for all 14 examples#245
Conversation
mbarger99
left a comment
mbarger99
left a comment
There was a problem hiding this comment.
I still have more to go through here, but it looks great so far.
|
|
||
| # TODO check GRAVITY_USING_EXTERNAL_PROTOBUF | ||
| set(PUBLIC_JAR "/usr/share/java/protobuf.jar" "${Protobuf_SRC_ROOT_FOLDER}/java/target/protobuf-java-${Protobuf_VERSION}.jar") | ||
| set(PUBLIC_JAR "${JAVA_PROTOBUF_JAR}") |
There was a problem hiding this comment.
You should at least include the second element (Protobuf_SRC_ROOT_FOLDER...) here so that people don't have to specify JAVA_PROTOBUF_JAR if they've already specified Protobuf_SRC_ROOT_FOLDER. @JBBee, is the /user/share location a standard location that we should also keep here? Maybe it would make sense to set the /user/share path as the default for JAVA_PROTOBUF_JAR.
There was a problem hiding this comment.
Done for now, will see what Justin says.
There was a problem hiding this comment.
@mbarger99 @FlinkerBluid sorry for the delay here - let's see, I see this is outdated now. Are you still waiting on any input from me here? If so, I'm not sure I completely understand the question. Feel free to hit me up in the chat.
| EXAMPLE_BIN_DIR=@GravityExamples_BINARY_DIR@ | ||
| SD_BIN_DIR=@ServiceDirectory_BINARY_DIR@ | ||
|
|
||
| export PATH=$PATH:../../../src/components/cpp/bin |
There was a problem hiding this comment.
Instead of specifying paths for every executable below, I think it would be easier to read if we updated the PATH here with those folders, and then invoked them with no path like before. This applies to all the run.sh files.
| SD_BIN_DIR=@ServiceDirectory_BINARY_DIR@ | ||
|
|
||
| export PATH=$PATH:../../../src/components/cpp/bin | ||
| export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:../../../src/api/cpp:../../../src/keyvalue_parser:$ZMQ_LIB_DIR:$PROTOBUF_LIB_DIR |
There was a problem hiding this comment.
I would also update these relative paths to use a cmake var in case they ever move.
| import time | ||
|
|
||
| from gravity import GravityNode, GravityDataProduct, gravity, GravitySubscriber, Log | ||
| from gravity import GravityNode, GravityDataProduct, gravity, GravitySubscriber, SpdLog |
There was a problem hiding this comment.
You made the right fix here to convert the old code to spdlog, but we can further improve this by using SpdLogHandler - that funnels logs from the Python logger to the Gravity log. Would be good to use that here instead.
|
|
||
|
|
||
| gravity_protobuf_generate(APPEND_PATH LANGUAGE python PROTOC_OUT_DIR "${CMAKE_CURRENT_BINARY_DIR}" GENERATE_EXTENSIONS .py OUT_VAR SRCS PROTOS ${PROTO_FILES}) | ||
| gravity_protobuf_generate(APPEND_PATH LANGUAGE python PROTOC_OUT_DIR "${CMAKE_CURRENT_SOURCE_DIR}" GENERATE_EXTENSIONS .py OUT_VAR SRCS PROTOS ${PROTO_FILES}) |
There was a problem hiding this comment.
This change will generate the protobufs in the source directory rather than the binary directory right? We want to keep generated files (aside from run.sh) under build and out of the source tree - is there a need to change this?
| import time | ||
| import gravity | ||
| from gravity import GravityNode, GravityDataProduct, gravity, GravityRequestor, GravityServiceProvider, Log | ||
| from gravity import GravityNode, GravityDataProduct, gravity, GravityRequestor, GravityServiceProvider, SpdLog |
There was a problem hiding this comment.
See SpdLogHandler comment above. Would be good to change that here too.
| set(JAVA_HOME "" CACHE PATH "Path to JDK to use") | ||
| set(Protobuf_SRC_ROOT_FOLDER "" CACHE PATH "Path to Protobuf source build") | ||
| set(ZMQ_HOME "" CACHE PATH "Path to your local ZeroMQ installation") | ||
| set(JAVA_PROTOBUF_JAR "/usr/share/java/protobuf.jar" CACHE PATH "Location of protobuf jar file") |
There was a problem hiding this comment.
This looks good to me - making it a cache variable with a default, you can over-ride from the command line if necessary
3 participants
All 14 examples should now work with CMake. Note: You will have to source your own protobuf jar file, and tell CMake where it is.