Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ability to specify a root dir for boost-libs and curl. #361

Closed
wants to merge 1 commit into from

Conversation

akornatskyy
Copy link
Contributor

No description provided.

@akornatskyy
Copy link
Contributor Author

@asekretenko @greggomann : this is another improvement to cmake to be able to build with a custom location of boost-libs, curl and libseccomp.

3rdparty/CMakeLists.txt Outdated Show resolved Hide resolved
@akornatskyy akornatskyy changed the title Added ability to specify a root dir for boost-libs, curl and libseccomp. Added ability to specify a root dir for boost-libs and curl. Apr 28, 2020
Comment on lines +197 to +207
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})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmake comes with FindCURL.cmake and FindBoost.cmake modules.

In case of boost we need only 5 lines of code:

if ("${BOOST_ROOT_DIR}" STREQUAL "")
...
else ()
  add_library(boost INTERFACE)
  target_include_directories(boost INTERFACE ${BOOST_ROOT_DIR}/include)
endif ()

Shadowing FindBoost.cmake with our own version might not be a good idea due to a possible confusion, same applies to using a different name like FindBOOST.cmake. The default implementation is quite sophisticated (2.3K+ SLOC). Plus you don't really get rid of that if statement because it treats boost as an external project. With find_package you will get an extra line and you still need to use add_library, etc.

Same applies to curl, the only code we add is to make a call of FIND_PACKAGE_HELPER and adopt it response to one produced by default FindCURL.cmake.

In case of libseccomp, there was no default find module in cmake, thus it was reasonable to introduce one.

Taking above into account we might consider to get rid of FindLIBARCHIVE.cmake in favor of default one FindLibArchive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants