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

Draft: Use GNUInstallDirs for catkin packages #1185

Open
wants to merge 3 commits into
base: noetic-devel
Choose a base branch
from
Open

Draft: Use GNUInstallDirs for catkin packages #1185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a134099
Use GNUInstallDirs for catkin and catkin packages.
mikepurvis Feb 28, 2023
176b906
Accept PREFIX with absolute LIBDIR/DATADIR.
mikepurvis Mar 2, 2023
fa0ccff
Set xxxDIR without the INSTALL_ prefix.
mikepurvis May 12, 2023
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
13 changes: 7 additions & 6 deletions cmake/all.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,14 @@ unset(_CATKIN_CURRENT_PACKAGE)
configure_shared_library_build_settings()

# set global install destinations
set(CATKIN_GLOBAL_BIN_DESTINATION bin)
set(CATKIN_GLOBAL_ETC_DESTINATION etc)
set(CATKIN_GLOBAL_INCLUDE_DESTINATION include)
set(CATKIN_GLOBAL_LIB_DESTINATION lib)
set(CATKIN_GLOBAL_LIBEXEC_DESTINATION lib)
include(GNUInstallDirs)
set(CATKIN_GLOBAL_BIN_DESTINATION ${CMAKE_INSTALL_BINDIR})
set(CATKIN_GLOBAL_ETC_DESTINATION ${CMAKE_INSTALL_SYSCONFDIR})
set(CATKIN_GLOBAL_INCLUDE_DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
set(CATKIN_GLOBAL_LIB_DESTINATION ${CMAKE_INSTALL_LIBDIR})
set(CATKIN_GLOBAL_LIBEXEC_DESTINATION ${CMAKE_INSTALL_LIBEXECDIR})
set(CATKIN_GLOBAL_PYTHON_DESTINATION ${PYTHON_INSTALL_DIR})
set(CATKIN_GLOBAL_SHARE_DESTINATION share)
set(CATKIN_GLOBAL_SHARE_DESTINATION ${CMAKE_INSTALL_DATADIR})

# undefine CATKIN_ENV since it might be set in the cache from a previous build
set(CATKIN_ENV "" CACHE INTERNAL "catkin environment" FORCE)
Expand Down
33 changes: 23 additions & 10 deletions cmake/catkin_package.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,8 @@ function(_catkin_package)
set(DEVELSPACE TRUE)
set(INSTALLSPACE FALSE)

set(PROJECT_SPACE_DIR ${CATKIN_DEVEL_PREFIX})
set(PROJECT_SPACE_LIBDIR ${CATKIN_DEVEL_PREFIX}/lib)
set(PROJECT_SPACE_DATADIR ${CATKIN_DEVEL_PREFIX}/share)
Copy link
Member Author

Choose a reason for hiding this comment

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

These intentionally do not use the GNUInstallDirs vars, for two reasons:

  • The develspace is not a "real" FHS tree and has its own internal standard that shouldn't be impacted by whatever overrides come in from the CMAKE_INSTALL_XXXX variables.
  • If the GNUInstallDirs vars are set to absolute paths (as in the Nix build case), those absolute paths will be appended to the devel prefix, which is almost certainly not what is wanted here.

We could always check for absolute paths and only append in the case where they aren't, but then what to do when they are? I don't think adding branchiness here is worth whatever gains there might be, particularly when the develspace is dropped in ROS 2 anyway.

set(PKG_INCLUDE_PREFIX ${CMAKE_CURRENT_SOURCE_DIR})

# absolute path to include dirs and validate that they are existing either absolute or relative to packages source
Expand Down Expand Up @@ -314,8 +315,8 @@ function(_catkin_package)

# prepend library path of this workspace
set(PKG_CONFIG_LIB_PATHS ${lib_paths})
list(INSERT PKG_CONFIG_LIB_PATHS 0 ${PROJECT_SPACE_DIR}/lib)
set(PKG_CMAKE_DIR ${PROJECT_SPACE_DIR}/share/${PROJECT_NAME}/cmake)
list(INSERT PKG_CONFIG_LIB_PATHS 0 ${PROJECT_SPACE_LIBDIR})
set(PKG_CMAKE_DIR ${PROJECT_SPACE_DATADIR}/${PROJECT_NAME}/cmake)
if("${PROJECT_NAME}" STREQUAL "catkin")
set(PKG_CMAKE_DIR "${catkin_EXTRAS_DIR}")
endif()
Expand Down Expand Up @@ -400,12 +401,24 @@ function(_catkin_package)
set(DEVELSPACE FALSE)
set(INSTALLSPACE TRUE)

set(PROJECT_SPACE_DIR ${CMAKE_INSTALL_PREFIX})
# Guarding the prefix insertion on whether the destination vars are specified as absolute
# paths permits both the GNUInstallDirs vars and CMAKE_PREFIX_PATH to be set.
if(IS_ABSOLUTE ${CATKIN_GLOBAL_LIB_DESTINATION})
set(PROJECT_SPACE_LIBDIR ${CATKIN_GLOBAL_LIB_DESTINATION})
else()
set(PROJECT_SPACE_LIBDIR ${CMAKE_INSTALL_PREFIX}/${CATKIN_GLOBAL_LIB_DESTINATION})
endif()
if(IS_ABSOLUTE ${CATKIN_GLOBAL_SHARE_DESTINATION})
set(PROJECT_SPACE_DATADIR ${CATKIN_GLOBAL_SHARE_DESTINATION})
else()
set(PROJECT_SPACE_DATADIR ${CMAKE_INSTALL_PREFIX}/${CATKIN_GLOBAL_SHARE_DESTINATION})
endif()
set(PKG_INCLUDE_PREFIX "\\\${prefix}")

# absolute path to include dir under install prefix if any include dir is set
set(PROJECT_CMAKE_CONFIG_INCLUDE_DIRS "")
set(PROJECT_PKG_CONFIG_INCLUDE_DIRS "")

foreach(idir ${PROJECT_INCLUDE_DIRS})
# include dirs in source / build / devel space are handled like relative ones
# since these files are supposed to be installed to the include folder in install space
Expand Down Expand Up @@ -433,9 +446,9 @@ function(_catkin_package)
list_append_unique(PROJECT_PKG_CONFIG_INCLUDE_DIRS ${PROJECT_DEPENDENCIES_INCLUDE_DIRS})
endif()

# prepend library path of this workspace
# prepend installed library path of this workspace
set(PKG_CONFIG_LIB_PATHS ${lib_paths})
list(INSERT PKG_CONFIG_LIB_PATHS 0 ${PROJECT_SPACE_DIR}/lib)
list(INSERT PKG_CONFIG_LIB_PATHS 0 ${PROJECT_SPACE_LIBDIR})
# package cmake dir is the folder where the generated pkgConfig.cmake is located
set(PKG_CMAKE_DIR "\${${PROJECT_NAME}_DIR}")

Expand All @@ -448,7 +461,7 @@ function(_catkin_package)
${catkin_EXTRAS_DIR}/em/pkg.pc.em
${CMAKE_CURRENT_BINARY_DIR}/catkin_generated/installspace/${PROJECT_NAME}.pc)
install(FILES ${CMAKE_CURRENT_BINARY_DIR}/catkin_generated/installspace/${PROJECT_NAME}.pc
DESTINATION lib/pkgconfig
DESTINATION ${PROJECT_SPACE_LIBDIR}/pkgconfig
)
endif()

Expand Down Expand Up @@ -495,7 +508,7 @@ function(_catkin_package)
endforeach()
install(FILES
${installable_cfg_extras}
DESTINATION share/${PROJECT_NAME}/cmake
DESTINATION ${PROJECT_SPACE_DATADIR}/${PROJECT_NAME}/cmake
)

if(NOT PROJECT_SKIP_CMAKE_CONFIG_GENERATION)
Expand All @@ -521,12 +534,12 @@ function(_catkin_package)
install(FILES
${CMAKE_CURRENT_BINARY_DIR}/catkin_generated/installspace/${PROJECT_NAME}Config.cmake
${CMAKE_CURRENT_BINARY_DIR}/catkin_generated/installspace/${PROJECT_NAME}Config-version.cmake
DESTINATION share/${PROJECT_NAME}/cmake
DESTINATION ${PROJECT_SPACE_DATADIR}/${PROJECT_NAME}/cmake
)
endif()

# install package.xml
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/package.xml
DESTINATION share/${PROJECT_NAME}
DESTINATION ${PROJECT_SPACE_DATADIR}/${PROJECT_NAME}
)
endfunction()
8 changes: 8 additions & 0 deletions cmake/templates/pkgConfig.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,19 @@ if("@DEVELSPACE@" STREQUAL "TRUE")
set(@PROJECT_NAME@_DEVEL_PREFIX @CATKIN_DEVEL_PREFIX@)
set(@PROJECT_NAME@_INSTALL_PREFIX "")
set(@PROJECT_NAME@_PREFIX ${@PROJECT_NAME@_DEVEL_PREFIX})
set(@PROJECT_NAME@_BINDIR ${@PROJECT_NAME@_DEVEL_PREFIX})
set(@PROJECT_NAME@_LIBDIR ${@PROJECT_NAME@_DEVEL_PREFIX})
set(@PROJECT_NAME@_DATADIR ${@PROJECT_NAME@_DEVEL_PREFIX})
set(@PROJECT_NAME@_INCLUDEDIR ${@PROJECT_NAME@_DEVEL_PREFIX})
else()
set(@PROJECT_NAME@_SOURCE_PREFIX "")
set(@PROJECT_NAME@_DEVEL_PREFIX "")
set(@PROJECT_NAME@_INSTALL_PREFIX @CMAKE_INSTALL_PREFIX@)
set(@PROJECT_NAME@_PREFIX ${@PROJECT_NAME@_INSTALL_PREFIX})
set(@PROJECT_NAME@_BINDIR @CMAKE_INSTALL_BINDIR@)
set(@PROJECT_NAME@_LIBDIR @CMAKE_INSTALL_LIBDIR@)
set(@PROJECT_NAME@_DATADIR @CMAKE_INSTALL_DATADIR@)
set(@PROJECT_NAME@_INCLUDEDIR @CMAKE_INSTALL_INCLUDEDIR@)
endif()

# warn when using a deprecated package
Expand Down
11 changes: 8 additions & 3 deletions cmake/templates/python_distutils_install.sh.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ echo_and_run() { echo "+ $@" ; "$@" ; }

echo_and_run cd "@INSTALL_CMD_WORKING_DIRECTORY@"

# ensure that Python install destination exists
echo_and_run mkdir -p "$DESTDIR@CMAKE_INSTALL_PREFIX@/@PYTHON_INSTALL_DIR@"
# Ensure that Python install destination exists. The .. is necessary since we
# want to use the GNUInstallDirs-aware location variable, but PYTHON_INSTALL_DIR
# already includes the "lib" prefix.
echo_and_run mkdir -p "$DESTDIR@CMAKE_INSTALL_PREFIX@/@CATKIN_GLOBAL_LIB_DESTINATION@/../@PYTHON_INSTALL_DIR@"

# Note that PYTHONPATH is pulled from the environment to support installing
# into one location when some dependencies were installed in another
Expand All @@ -30,4 +32,7 @@ echo_and_run /usr/bin/env \
build --build-base "@CMAKE_CURRENT_BINARY_DIR@" \
install \
--root="${DESTDIR-/}" \
@SETUPTOOLS_ARG_EXTRA@ --prefix="@CMAKE_INSTALL_PREFIX@" --install-scripts="@CMAKE_INSTALL_PREFIX@/@CATKIN_GLOBAL_BIN_DESTINATION@"
@SETUPTOOLS_ARG_EXTRA@ \
--prefix="@CMAKE_INSTALL_PREFIX@" \
--install-scripts="@CMAKE_INSTALL_PREFIX@/@CATKIN_GLOBAL_BIN_DESTINATION@" \
--install-lib="@CMAKE_INSTALL_PREFIX@/@CATKIN_GLOBAL_LIB_DESTINATION@/../@PYTHON_INSTALL_DIR@"