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

Remove Catkin dependency while keeping CMake's find_package() feature #133

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Remove Catkin dependency while keeping CMake's find_package() feature #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e220a32
CMake: Serial should be a StaticLib
dbolkensteyn Jul 12, 2016
36c4e9d
Remove Catkin dependency
dbolkensteyn Jul 12, 2016
be625f1
CMake: Increase min version to 2.8.8 for CMakePackageConfigHelpers
dbolkensteyn Jul 12, 2016
be3267b
Travis: Install CMake 3.x
dbolkensteyn Jul 12, 2016
d764d13
Travis: Say yes to add-apt-repository
dbolkensteyn Jul 12, 2016
e685261
Travis: Say yes to apt-get as well...
dbolkensteyn Jul 12, 2016
b94e7dd
Travis: Install CMake differently and support OS X
dbolkensteyn Jul 12, 2016
189db8d
CMake: Keep compat only with same major version & remove CMakePackage…
dbolkensteyn Jul 18, 2016
514acca
Travis: Disable tests temporarly
dbolkensteyn Jul 18, 2016
d6245ab
Travis: Don't install newer CMake version
dbolkensteyn Jul 18, 2016
db7855c
CMake: Fix typo in *Version.cmake filename
dbolkensteyn Jul 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ install:
- make install_deps
- source setup.bash
script:
- make && make test
- make
Copy link
Author

Choose a reason for hiding this comment

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

FIXME: For the time being tests are not built, so they cannot be run

46 changes: 20 additions & 26 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,11 @@
cmake_minimum_required(VERSION 2.8.3)
project(serial)

# Find catkin
find_package(catkin REQUIRED)

if(APPLE)
find_library(IOKIT_LIBRARY IOKit)
find_library(FOUNDATION_LIBRARY Foundation)
endif()

if(UNIX AND NOT APPLE)
# If Linux, add rt and pthread
catkin_package(
LIBRARIES ${PROJECT_NAME}
INCLUDE_DIRS include
DEPENDS rt pthread
)
else()
# Otherwise normal call
catkin_package(
LIBRARIES ${PROJECT_NAME}
INCLUDE_DIRS include
)
endif()

## Sources
set(serial_SRCS
src/serial.cc
Expand All @@ -45,7 +27,7 @@ else()
endif()

## Add serial library
add_library(${PROJECT_NAME} ${serial_SRCS})
add_library(${PROJECT_NAME} STATIC ${serial_SRCS})
if(APPLE)
target_link_libraries(${PROJECT_NAME} ${FOUNDATION_LIBRARY} ${IOKIT_LIBRARY})
elseif(UNIX)
Expand All @@ -54,25 +36,37 @@ else()
target_link_libraries(${PROJECT_NAME} setupapi)
endif()

## Uncomment for example
## Add example project
add_executable(serial_example examples/serial_example.cc)
add_dependencies(serial_example ${PROJECT_NAME})
target_link_libraries(serial_example ${PROJECT_NAME})

## Include headers
include_directories(include)

## Install
set(INSTALL_LIB_DIR lib)
set(INSTALL_INCLUDE_DIR include)
set(INSTALL_CMAKE_DIR share/serial/cmake)

## Install executable
install(TARGETS ${PROJECT_NAME}
ARCHIVE DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
LIBRARY DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
DESTINATION ${INSTALL_LIB_DIR}
EXPORT ${PROJECT_NAME}-targets
Copy link
Author

@dbolkensteyn dbolkensteyn Jul 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Disclaimer: I'm a CMake rookie... and I tried quite a few things before coming up with this solution

More about it here: https://cmake.org/Wiki/CMake/Tutorials/Packaging#Packaging_and_Exporting
And on this thread: https://cmake.org/pipermail/cmake/2009-November/033170.html

What are you putting in the Config? Typically the Config.cmake
file should just include the file produced by install(EXPORT).

)

## Install headers
install(FILES include/serial/serial.h include/serial/v8stdint.h
DESTINATION ${CATKIN_GLOBAL_INCLUDE_DESTINATION}/serial)
DESTINATION ${INSTALL_INCLUDE_DIR}/serial)

## Install CMake files
install(EXPORT ${PROJECT_NAME}-targets DESTINATION ${INSTALL_CMAKE_DIR})

install(FILES ${CMAKE_SOURCE_DIR}/cmake/${PROJECT_NAME}Config.cmake ${CMAKE_SOURCE_DIR}/cmake/${PROJECT_NAME}ConfigVersion.cmake
DESTINATION ${INSTALL_CMAKE_DIR})

## Tests
if(CATKIN_ENABLE_TESTING)
add_subdirectory(tests)
endif()
# FIXME
Copy link
Author

Choose a reason for hiding this comment

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

This PR can't be merged before this is fixed

#if(CATKIN_ENABLE_TESTING)
# add_subdirectory(tests)
#endif()
14 changes: 14 additions & 0 deletions cmake/serialConfig.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Usage:
#
# find_package(serial REQUIRED)
# include_directories(${serial_INCLUDE_DIRS})
# target_link_libraries(<target> serial)

if(serial_CONFIG_INCLUDED)
return()
endif()
set(serial_CONFIG_INCLUDED TRUE)

get_filename_component(SELF_DIR "${CMAKE_CURRENT_LIST_FILE}" PATH)
include(${SELF_DIR}/serial-targets.cmake)
get_filename_component(serial_INCLUDE_DIRS "${SELF_DIR}/../../../include" ABSOLUTE)
21 changes: 21 additions & 0 deletions cmake/serialConfigVersion.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
set(PACKAGE_VERSION "1.2.1")
Copy link
Author

@dbolkensteyn dbolkensteyn Jul 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Currently with Catkin, any newer version of this package is considered compatible.

With this version file, the package is compatible only if:

  1. the requested version number is equal to the installed one
  2. or, the requested version number is inferior to the installed one AND the major version number matches (for "1.2.1", the major version number is "1")

So, if "0.9" is requested and "1.2.1" is installed => mismatch
If "1.1" is requested and "1.2.1" is installed => match

Relying on the major version number for compatibility seems to me like a good idea, and enables breaking changes to be introduced in order to keep this package alive [relates to #102 for instance].


if(PACKAGE_VERSION VERSION_LESS PACKAGE_FIND_VERSION)
set(PACKAGE_VERSION_COMPATIBLE FALSE)
else()
if(PACKAGE_VERSION MATCHES "^([0-9]+)\\.")
set(CVF_VERSION_MAJOR "${CMAKE_MATCH_1}")
else()
set(CVF_VERSION_MAJOR PACKAGE_VERSION)
endif()

if(PACKAGE_FIND_VERSION_MAJOR STREQUAL CVF_VERSION_MAJOR)
set(PACKAGE_VERSION_COMPATIBLE TRUE)
else()
set(PACKAGE_VERSION_COMPATIBLE FALSE)
endif()

if(PACKAGE_FIND_VERSION STREQUAL PACKAGE_VERSION)
set(PACKAGE_VERSION_EXACT TRUE)
endif()
endif()
24 changes: 0 additions & 24 deletions package.xml
View file
Open in desktop

This file was deleted.