You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by GitBox <gi...@apache.org> on 2020/04/27 08:03:45 UTC
[GitHub] [mesos] akornatskyy opened a new pull request #361: Added ability to specify a root dir for boost-libs, curl and libseccomp.
akornatskyy opened a new pull request #361:
URL: https://github.com/apache/mesos/pull/361
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [mesos] akornatskyy commented on pull request #361: Added ability to specify a root dir for boost-libs, curl and libseccomp.
Posted by GitBox <gi...@apache.org>.
akornatskyy commented on pull request #361:
URL: https://github.com/apache/mesos/pull/361#issuecomment-619808069
@asekretenko @greggomann : this is another improvement to cmake to be able to build with a custom location of boost-libs, curl and libseccomp.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [mesos] greggomann commented on a change in pull request #361: Added ability to specify a root dir for boost-libs and curl.
Posted by GitBox <gi...@apache.org>.
greggomann commented on a change in pull request #361:
URL: https://github.com/apache/mesos/pull/361#discussion_r416997093
##########
File path: 3rdparty/CMakeLists.txt
##########
@@ -194,29 +194,34 @@ endfunction()
# Boost: C++ Libraries.
# http://www.boost.org
#######################
-EXTERNAL(boost ${BOOST_VERSION} ${CMAKE_CURRENT_BINARY_DIR})
-add_library(boost INTERFACE)
-add_dependencies(boost ${BOOST_TARGET})
-if (CMAKE_CXX_COMPILER_ID MATCHES GNU OR CMAKE_CXX_COMPILER_ID MATCHES Clang)
- # Headers including Boost 1.65.0 fail to compile with GCC 7.2 and
- # CLang 3.6 without `-Wno-unused-local-typedefs`.
- # TODO(andschwa): Remove this when Boost has a resolution.
- target_compile_options(boost INTERFACE -Wno-unused-local-typedefs)
-endif ()
-target_include_directories(boost INTERFACE ${BOOST_ROOT})
+if ("${BOOST_ROOT_DIR}" STREQUAL "")
+ EXTERNAL(boost ${BOOST_VERSION} ${CMAKE_CURRENT_BINARY_DIR})
+ add_library(boost INTERFACE)
+ add_dependencies(boost ${BOOST_TARGET})
+ if (CMAKE_CXX_COMPILER_ID MATCHES GNU OR CMAKE_CXX_COMPILER_ID MATCHES Clang)
+ # Headers including Boost 1.65.0 fail to compile with GCC 7.2 and
+ # CLang 3.6 without `-Wno-unused-local-typedefs`.
+ # TODO(andschwa): Remove this when Boost has a resolution.
+ target_compile_options(boost INTERFACE -Wno-unused-local-typedefs)
+ endif ()
+ target_include_directories(boost INTERFACE ${BOOST_ROOT})
Review comment:
Hmm actually I'm wondering.... should we do the same thing we do for curl and boost that we do for seccomp, and have a `.cmake` file which defines helpers for those libraries as well?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [mesos] greggomann commented on a change in pull request #361: Added ability to specify a root dir for boost-libs, curl and libseccomp.
Posted by GitBox <gi...@apache.org>.
greggomann commented on a change in pull request #361:
URL: https://github.com/apache/mesos/pull/361#discussion_r416643733
##########
File path: 3rdparty/CMakeLists.txt
##########
@@ -696,7 +701,16 @@ if (ENABLE_SECCOMP_ISOLATOR)
URL ${LIBSECCOMP_URL}
URL_HASH ${LIBSECCOMP_HASH})
else ()
- find_package(LIBSECCOMP REQUIRED)
+ if ("${LIBSECCOMP_ROOT_DIR}" STREQUAL "")
+ find_package(LIBSECCOMP REQUIRED)
+ else ()
+ set(POSSIBLE_LIBSECCOMP_INCLUDE_DIRS ${LIBSECCOMP_ROOT_DIR}/include)
+ set(POSSIBLE_LIBSECCOMP_LIB_DIRS ${LIBSECCOMP_ROOT_DIR}/lib)
+
+ set(LIBSECCOMP_LIBRARY_NAMES seccomp)
+
+ FIND_PACKAGE_HELPER(LIBSECCOMP seccomp.h)
+ endif ()
Review comment:
Check if we can get rid of this?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [mesos] akornatskyy commented on a change in pull request #361: Added ability to specify a root dir for boost-libs and curl.
Posted by GitBox <gi...@apache.org>.
akornatskyy commented on a change in pull request #361:
URL: https://github.com/apache/mesos/pull/361#discussion_r416710336
##########
File path: 3rdparty/CMakeLists.txt
##########
@@ -696,7 +701,16 @@ if (ENABLE_SECCOMP_ISOLATOR)
URL ${LIBSECCOMP_URL}
URL_HASH ${LIBSECCOMP_HASH})
else ()
- find_package(LIBSECCOMP REQUIRED)
+ if ("${LIBSECCOMP_ROOT_DIR}" STREQUAL "")
+ find_package(LIBSECCOMP REQUIRED)
+ else ()
+ set(POSSIBLE_LIBSECCOMP_INCLUDE_DIRS ${LIBSECCOMP_ROOT_DIR}/include)
+ set(POSSIBLE_LIBSECCOMP_LIB_DIRS ${LIBSECCOMP_ROOT_DIR}/lib)
+
+ set(LIBSECCOMP_LIBRARY_NAMES seccomp)
+
+ FIND_PACKAGE_HELPER(LIBSECCOMP seccomp.h)
+ endif ()
Review comment:
yes, done. checked.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [mesos] akornatskyy commented on a change in pull request #361: Added ability to specify a root dir for boost-libs and curl.
Posted by GitBox <gi...@apache.org>.
akornatskyy commented on a change in pull request #361:
URL: https://github.com/apache/mesos/pull/361#discussion_r417074587
##########
File path: 3rdparty/CMakeLists.txt
##########
@@ -194,29 +194,34 @@ endfunction()
# Boost: C++ Libraries.
# http://www.boost.org
#######################
-EXTERNAL(boost ${BOOST_VERSION} ${CMAKE_CURRENT_BINARY_DIR})
-add_library(boost INTERFACE)
-add_dependencies(boost ${BOOST_TARGET})
-if (CMAKE_CXX_COMPILER_ID MATCHES GNU OR CMAKE_CXX_COMPILER_ID MATCHES Clang)
- # Headers including Boost 1.65.0 fail to compile with GCC 7.2 and
- # CLang 3.6 without `-Wno-unused-local-typedefs`.
- # TODO(andschwa): Remove this when Boost has a resolution.
- target_compile_options(boost INTERFACE -Wno-unused-local-typedefs)
-endif ()
-target_include_directories(boost INTERFACE ${BOOST_ROOT})
+if ("${BOOST_ROOT_DIR}" STREQUAL "")
+ EXTERNAL(boost ${BOOST_VERSION} ${CMAKE_CURRENT_BINARY_DIR})
+ add_library(boost INTERFACE)
+ add_dependencies(boost ${BOOST_TARGET})
+ if (CMAKE_CXX_COMPILER_ID MATCHES GNU OR CMAKE_CXX_COMPILER_ID MATCHES Clang)
+ # Headers including Boost 1.65.0 fail to compile with GCC 7.2 and
+ # CLang 3.6 without `-Wno-unused-local-typedefs`.
+ # TODO(andschwa): Remove this when Boost has a resolution.
+ target_compile_options(boost INTERFACE -Wno-unused-local-typedefs)
+ endif ()
+ target_include_directories(boost INTERFACE ${BOOST_ROOT})
Review comment:
sounds reasonable to me, will give it a try.
----------------------------------------------------------------
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:
users@infra.apache.org