greggomann commented on a change in pull request #363:
URL: https://github.com/apache/mesos/pull/363#discussion_r424821122
##########
File path: 3rdparty/CMakeLists.txt
##########
@@ -512,6 +523,11 @@ ExternalProject_Add(
URL ${RAPIDJSON_URL}
URL_HASH ${RAPIDJSON_HASH})
+if (ENABLE_INSTALL_MODULE_DEPENDENCIES)
+ install(
+ DIRECTORY
${RAPIDJSON_CMAKE_ROOT}/src/rapidjson-${RAPIDJSON_VERSION}/include
+ DESTINATION .)
Review comment:
Why are there several different install directories for headers: the
root installation directory is used here, others use `MESOS_INSTALL_HEADERS`,
while glog uses `${MESOS_INSTALL_LIBRARIES}/mesos/3rdparty`. If this
inconsistency is really necessary, it would be nice to include some comments
explaining why.
##########
File path: 3rdparty/CMakeLists.txt
##########
@@ -459,6 +459,11 @@ install(
DIRECTORY ${GLOG_INSTALL_DIR}/lib/
DESTINATION ${MESOS_INSTALL_LIBRARIES})
+if (ENABLE_INSTALL_MODULE_DEPENDENCIES)
+ install(
+ DIRECTORY ${GLOG_INSTALL_DIR}/include
+ DESTINATION ${MESOS_INSTALL_LIBRARIES}/mesos/3rdparty)
+endif ()
Review comment:
The autotools build system installs the 3rdparty dependencies by either
running `make install` within the dependency's build directory, or by manually
copying headers into the installation directory.
In the autotools build system, glog is installed with `make install`, as are
picojson, protobuf, and zookeeper. Could you explain how you chose the install
methods you're using below? I'm not familiar with the motivations for the
precise methods used in our autotools `INSTALL_MODULE_DEPENDENCIES`
implementation, so I'm just curious to figure out the best install method here.
IIUC, the equivalent of the `make install` method here would be using
`install(TARGETS ...)` to specify install directories for the 3rdparty builds?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
