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.

Sign up for GitHub

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

Jump to bottom

Support multiple build systems such as pure make, catkin, ament, etc #109

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Support multiple build systems such as pure make, catkin, ament, etc #109

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f8fdd06
[libpointmatcher] Added cmake files to support pure make or catkin bu…
YoshuaNava Oct 26, 2020
3cc0712
[package.xml] Changed default buildtool
YoshuaNava Oct 26, 2020
89d1da7
[CMakeLists.txt] File rename and move
YoshuaNava Oct 26, 2020
c9414bb
[cmake] Added catkin cmake file
YoshuaNava Oct 26, 2020
0b57425
[package.xml] Fixed depends
YoshuaNava Oct 26, 2020
641f177
[cmake] Default build tool set to catkin
YoshuaNava Oct 26, 2020
3177e0f
Reduce target cmake version to 2.8.12
YoshuaNava Oct 29, 2020
4bbfee9
Address comments from pull request
YoshuaNava Nov 20, 2020
be08549
Package.xml update
YoshuaNava Nov 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 24 additions & 18 deletions cmake/CatkinBuild.cmake
View file
Open in desktop
Expand Up @@ -11,22 +11,18 @@ find_package(catkin REQUIRED)
find_package(Eigen3 REQUIRED)
find_package(Boost REQUIRED COMPONENTS python)
Copy link
Contributor

Choose a reason for hiding this comment

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

Only when Python bindings are built?

Copy link
Author

Choose a reason for hiding this comment

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

Good point. I can add the Python bindings

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, but isn't this find_package anyways done inside python/CMakeLists.txt? It might be superfluous here...


find_package(OpenMP REQUIRED)
if (OpenMP_FOUND)
add_compile_options("${OpenMP_CXX_FLAGS}")
add_definitions(-DHAVE_OPENMP=${OpenMP_FOUND})
find_package(OpenMP)
if (NOT OpenMP_FOUND)
message("OpenMP was not found. It is highly recommended to build libnabo with OpenMP support.")
endif()

# Catkin package macro
catkin_package(
INCLUDE_DIRS
nabo
${CMAKE_SOURCE_DIR}
YoshuaNava marked this conversation as resolved.
Show resolved Hide resolved
${EIGEN3_INCLUDE_DIR}
LIBRARIES
nabo
DEPENDS
Boost
)

########################
Expand All @@ -37,22 +33,29 @@ add_library(nabo
${NABO_SRC}
${NABO_HEADERS}
)

target_include_directories(nabo
PUBLIC
${CMAKE_SOURCE_DIR}
${CMAKE_SOURCE_DIR}/nabo
SYSTEM
${EIGEN3_INCLUDE_DIR}
${Boost_INCLUDE_DIRS}
${OpenMP_CXX_INCLUDE_DIRS}
target_include_directories(nabo PUBLIC
${CMAKE_SOURCE_DIR}
${CMAKE_SOURCE_DIR}/nabo
)

target_link_libraries(nabo
target_include_directories(nabo SYSTEM PRIVATE
${EIGEN3_INCLUDE_DIR}
Copy link
Contributor

Choose a reason for hiding this comment

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

Eigen is a public dependency (it's in nabo.h). Move it above and add <build_export_depend>eigen</build_export_depend> to package.xml.

${Boost_INCLUDE_DIRS}
Copy link
Contributor

Choose a reason for hiding this comment

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

Boost isn't used in the core library, just for the python bindings. So remove it from here.

${OpenMP_CXX_INCLUDE_DIRS}
)
target_link_libraries(nabo PRIVATE
${catkin_LIBRARIES}
${Boost_LIBRARIES}
Copy link
Contributor

Choose a reason for hiding this comment

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

And, remove it from here, too.

${OpenMP_CXX_LIBRARIES}
)
target_compile_options(nabo PRIVATE
"${OpenMP_CXX_FLAGS}"
)
target_compile_definitions(nabo PRIVATE
-DHAVE_OPENMP=${OpenMP_FOUND}
)

# Python bindings
add_subdirectory(python)
Copy link
Contributor

Choose a reason for hiding this comment

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

Respect LIBNABO_BUILD_PYTHON setting.


#############
## Install ##
Expand All @@ -69,4 +72,7 @@ install(
DIRECTORY
${CMAKE_SOURCE_DIR}/nabo/
DESTINATION ${CATKIN_GLOBAL_INCLUDE_DESTINATION}/nabo
YoshuaNava marked this conversation as resolved.
Show resolved Hide resolved
FILES_MATCHING
PATTERN "*.h"
PATTERN "*.hpp"
)
5 changes: 1 addition & 4 deletions package.xml
View file
Open in desktop
Expand Up @@ -16,10 +16,7 @@

<build_depend>boost</build_depend>
Copy link
Contributor

Choose a reason for hiding this comment

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

If python bindings are not built, then remove it.

<build_depend>eigen</build_depend>
<build_depend>libopenmp-dev</build_depend>

<run_depend>boost</run_depend>
<run_depend>eigen</run_depend>
<build_depend>libomp-dev</build_depend>

<!-- For a pure-cmake build the package must be exported explicitly -->
<!-- Thus the following block of code must be un-commented -->
Expand Down