jrl-umi3218 / jrl-cmakemodules

CMake utility toolbox
https://jrl-cmakemodules.readthedocs.io/en/master/
Other
56 stars 46 forks source link

Issue on cmake_install.cmake with uninstall.cmake #617

Closed olivier-stasse closed 10 months ago

olivier-stasse commented 10 months ago

Steps to reproduce the problem

Let us use a package using jrl-cmake modules as a cmake git submodule, for instance master_board_sdk, and a ros humble distro.

mkdir -p /tmp/test_ws/src
cd /tmp/test_ws/src
git clone https://github.com/open-dynamic-robot-initiative/master-board.git --recursive
cd ..
colcon build --packages-select master_board_sdk --executor sequential

Compiling is ok, and the installation is going fine, but the output displays this :

3 warnings generated.

Usage: cmake --build <dir>             [options] [-- [native-options]]
       cmake --build --preset <preset> [options] [-- [native-options]]
Options:
  <dir>          = Project binary directory to be built.
  --preset <preset>, --preset=<preset>
                 = Specify a build preset.
  --list-presets[=<type>]
                 = List available build presets.
  --parallel [<jobs>], -j [<jobs>]
                 = Build in parallel using the given number of jobs. 
                   If <jobs> is omitted the native build tool's 
                   default number is used.
                   The CMAKE_BUILD_PARALLEL_LEVEL environment variable
                   specifies a default parallel level when this option
                   is not given.
  -t <tgt>..., --target <tgt>...
                 = Build <tgt> instead of default targets.
  --config <cfg> = For multi-configuration tools, choose <cfg>.
  --clean-first  = Build target 'clean' first, then build.
                   (To clean only, use --target 'clean'.)
  --resolve-package-references={on|only|off}
                 = Restore/resolve package references during build.
  -v, --verbose  = Enable verbose output - if supported - including
                   the build commands to be executed. 
  --             = Pass remaining options to the native tool.
---
Finished <<< master_board_sdk [1min 23s]

Analysis

The error :

CMake Error: Invalid value used with --config

disappear if one comments the line

execute_process(COMMAND "/Volumes/Devel/miniconda_env/miniconda3_env/envs/ros_humble_dev_p_3_10_env/bin/cmake" --build "" --config ${CMAKE_INSTALL_CONFIG_NAME} --target uninstall)

in ./build/master_board_sdk/cmake_install.cmake

It seems to be generated by the file uninstall.cmake and more specifically https://github.com/jrl-umi3218/jrl-cmakemodules/blob/47b924b4760b239a75474462c1d6d66f74ba42d8/uninstall.cmake#L55

When not specifying a CMAKE_BUILD_TYPE this line fails.

I have a fix here: https://github.com/olivier-stasse/jrl-cmakemodules/blob/cf58b6616b8c28e337d1b55fd3a1e4708b6711d0/uninstall.cmake#L55

Basically if the configuration is empty, the line is not called. Maybe it would be better to not specify the --config option if the variable is empty.

WDYT ?

gergondet commented 10 months ago

Hi @olivier-stasse

Your suggested patch would disable the auto-uninstall feature in this scenario.

I suggest (not tried):

if(DEFINED CMAKE_CONFIGURATION_TYPES)
  set(UNINSTALL_CONFIG_ARG "--config \${CMAKE_INSTALL_CONFIG_NAME}")
endif()
install(
  CODE "execute_process(COMMAND \"${CMAKE_COMMAND}\" --build \"${PROJECT_BINARY_DIR}\" ${UNINSTALL_CONFIG_ARG} --target uninstall)"
)

Since the --config is only really necessary with multi-config generators (i.e. those that have CMAKE_CONFIGURATION_TYPES defined) and can be omitted otherwise. In a multi-config generator I don't think it's possible to get an empty CMAKE_INSTALL_CONFIG_NAME

olivier-stasse commented 10 months ago

I will try and keep you posted.

olivier-stasse commented 10 months ago

Thanks this is working.

gergondet commented 10 months ago

I'm closing since this is fixed in #618

olivier-stasse commented 10 months ago

Yes. Thx. sorry I should have close it.