From 597362b6cf34947b4d1c36313f8ad22fe4236579 Mon Sep 17 00:00:00 2001 From: Carlos Mastalli Date: Fri, 10 Nov 2023 00:30:00 +0000 Subject: [PATCH 1/3] [catkin] Do not remove catkin file, breaks robostack environment --- cmake_uninstall.cmake.in | 4 ++-- uninstall.cmake | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/cmake_uninstall.cmake.in b/cmake_uninstall.cmake.in index 1dd0999c4..8e1bb56e4 100644 --- a/cmake_uninstall.cmake.in +++ b/cmake_uninstall.cmake.in @@ -19,7 +19,7 @@ IF(NOT EXISTS "@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt") ENDIF(NOT EXISTS "@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt") MESSAGE(STATUS "catkin path: @CMAKE_INSTALL_PREFIX@/.catkin") -IF(EXISTS "@CMAKE_INSTALL_PREFIX@/.catkin") +IF(EXISTS "@CMAKE_INSTALL_PREFIX@/.catkin" AND PACKAGE_CREATES_DOT_CATKIN) MESSAGE(STATUS "Try to remove @CMAKE_INSTALL_PREFIX@/.catkin") EXECUTE_PROCESS( COMMAND @CMAKE_COMMAND@ -E remove "@CMAKE_INSTALL_PREFIX@/.catkin" @@ -27,7 +27,7 @@ IF(EXISTS "@CMAKE_INSTALL_PREFIX@/.catkin") OUTPUT_VARIABLE rm_out ERROR_VARIABLE rm_err ) -ENDIF(EXISTS "@CMAKE_INSTALL_PREFIX@/.catkin") +ENDIF() IF(EXISTS "@CMAKE_CURRRENT_BINARY_DIR@/install_manifest.txt") return() diff --git a/uninstall.cmake b/uninstall.cmake index 4bd98958f..7a4bb8643 100644 --- a/uninstall.cmake +++ b/uninstall.cmake @@ -19,6 +19,23 @@ # Add custom rule to uninstall the package. # macro(_SETUP_PROJECT_UNINSTALL) + # Detect if the .catkin was created previously + if(NOT DEFINED PACKAGE_CREATES_DOT_CATKIN + OR NOT "${PACKAGE_PREVIOUS_INSTALL_PREFIX}" STREQUAL + "${CMAKE_INSTALL_PREFIX}") + set(PACKAGE_PREVIOUS_INSTALL_PREFIX + "${CMAKE_INSTALL_PREFIX}" + CACHE INTERNAL "Cache install prefix given to the package") + if(EXISTS "${CMAKE_INSTALL_PREFIX}/.catkin") + set(PACKAGE_CREATES_DOT_CATKIN + FALSE + CACHE INTERNAL "") + else() + set(PACKAGE_CREATES_DOT_CATKIN + TRUE + CACHE INTERNAL "") + endif() + endif() # FIXME: it is utterly stupid to rely on the install manifest. Can't we just # remember what we install ?! configure_file( From 7602dd625ab055807a9d47e73f9e0638d601c722 Mon Sep 17 00:00:00 2001 From: Pierre Gergondet Date: Wed, 6 Dec 2023 10:17:44 +0900 Subject: [PATCH 2/3] Fix the .catkin uninstall behavior This also adds a test to make sure it works as expected --- _unittests/catkin/CMakeLists.txt | 25 +++++++++++++++++++++++++ _unittests/run_unit_tests.sh | 29 ++++++++++++++++++++++++++++- uninstall.cmake | 6 ++++-- 3 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 _unittests/catkin/CMakeLists.txt diff --git a/_unittests/catkin/CMakeLists.txt b/_unittests/catkin/CMakeLists.txt new file mode 100644 index 000000000..5edb11c64 --- /dev/null +++ b/_unittests/catkin/CMakeLists.txt @@ -0,0 +1,25 @@ +cmake_minimum_required(VERSION 3.10) + +set(PROJECT_NAME jrl-cmakemodules-catkin) +set(PROJECT_VERSION 0.0.0) +set(PROJECT_DESCRIPTION "JRL CMake module - catkin") +set(PROJECT_URL http://jrl-cmakemodules.readthedocs.io) + +include(../../base.cmake) + +project(${PROJECT_NAME} LANGUAGES CXX) + +# This project test the correct behavior of #622 +# +# * If .catkin is already here when the project is configured it is not removed +# * Otherwise it is assumed it is created by the project and removed with the +# uninstall target + +option(FORCE_DOT_CATKIN_CREATION + "Force creation of .catkin file in install destination" OFF) + +if(FORCE_DOT_CATKIN_CREATION) + install( + CODE "execute_process(COMMAND ${CMAKE_COMMAND} -E touch ${CMAKE_INSTALL_PREFIX}/.catkin)" + ) +endif() diff --git a/_unittests/run_unit_tests.sh b/_unittests/run_unit_tests.sh index a66cb48c5..337673d39 100755 --- a/_unittests/run_unit_tests.sh +++ b/_unittests/run_unit_tests.sh @@ -1,6 +1,6 @@ #!/bin/bash -unittests="python cpp dependency" +unittests="python cpp dependency catkin" # Code for running a specific unit test # For unit test foo, function `run_foo` is executed if defined. @@ -23,6 +23,33 @@ function run_cpp() run_default $here/cpp } +function run_catkin() +{ + $CMAKE_BIN ${cmake_options} -DFORCE_DOT_CATKIN_CREATION=ON "${here}/catkin" + make install + if [[ ! -f ${here}/install/.catkin ]]; then + echo ".catkin file should have been created" + exit 1 + fi + make uninstall + if [[ -f ${here}/install/.catkin ]]; then + echo ".catkin file should have been removed" + exit 1 + fi + cd ${here}/build/ + rm -rf ${here}/build/catkin/ + mkdir -p ${here}/build/catkin/ + cd catkin + touch ${here}/install/.catkin + $CMAKE_BIN ${cmake_options} -DFORCE_DOT_CATKIN_CREATION=OFF "${here}/catkin" + make install + make uninstall + if [[ ! -f ${here}/install/.catkin ]]; then + echo ".catkin file should NOT have been removed" + exit 1 + fi +} + # The code below run all the unit tests here="`pwd`" rm -rf build install diff --git a/uninstall.cmake b/uninstall.cmake index 7a4bb8643..d7932b769 100644 --- a/uninstall.cmake +++ b/uninstall.cmake @@ -43,8 +43,10 @@ macro(_SETUP_PROJECT_UNINSTALL) "${CMAKE_CURRENT_BINARY_DIR}/cmake/cmake_uninstall.cmake" IMMEDIATE @ONLY) add_custom_target( - uninstall "${CMAKE_COMMAND}" -P - "${CMAKE_CURRENT_BINARY_DIR}/cmake/cmake_uninstall.cmake") + uninstall + "${CMAKE_COMMAND}" + -DPACKAGE_CREATES_DOT_CATKIN=${PACKAGE_CREATES_DOT_CATKIN} -P + "${CMAKE_CURRENT_BINARY_DIR}/cmake/cmake_uninstall.cmake") configure_file("${CMAKE_CURRENT_LIST_DIR}/cmake_reinstall.cmake.in" "${PROJECT_BINARY_DIR}/cmake/cmake_reinstall.cmake.configured") From 6b33181609b606496b7265c3cf8578d4a6285e39 Mon Sep 17 00:00:00 2001 From: Pierre Gergondet Date: Wed, 6 Dec 2023 10:18:09 +0900 Subject: [PATCH 3/3] [ci] Make sure CI fails if the tests fail Fix the tests, they have been broken since we require CMake >= 3.10 --- .github/workflows/cmake.yml | 2 +- .gitignore | 2 ++ _unittests/cpp/CMakeLists.txt | 2 +- _unittests/dependency/CMakeLists.txt | 2 +- _unittests/python/CMakeLists.txt | 2 +- _unittests/run_unit_tests.sh | 3 +++ 6 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index 96eb72873..4eb51184a 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -8,7 +8,7 @@ jobs: runs-on: [ubuntu-22.04] steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v3 - name: Run project tests run: | diff --git a/.gitignore b/.gitignore index 071c4a8d8..899cf1b11 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ hpp/idl/*.pyc .docs/build +_unittests/build/ +_unittests/install/ diff --git a/_unittests/cpp/CMakeLists.txt b/_unittests/cpp/CMakeLists.txt index 12a540adf..40cd1e4e4 100644 --- a/_unittests/cpp/CMakeLists.txt +++ b/_unittests/cpp/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.2) +cmake_minimum_required(VERSION 3.10) set(PROJECT_NAME jrl-cmakemodules-cpp) set(PROJECT_VERSION 0.0.0) diff --git a/_unittests/dependency/CMakeLists.txt b/_unittests/dependency/CMakeLists.txt index 9c0e0e616..35f7f7949 100644 --- a/_unittests/dependency/CMakeLists.txt +++ b/_unittests/dependency/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.2) +cmake_minimum_required(VERSION 3.10) set(PROJECT_NAME jrl-cmakemodules-dependency) set(PROJECT_VERSION 0.0.0) diff --git a/_unittests/python/CMakeLists.txt b/_unittests/python/CMakeLists.txt index 1d04e1def..97631c3d2 100644 --- a/_unittests/python/CMakeLists.txt +++ b/_unittests/python/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.1) +cmake_minimum_required(VERSION 3.10) # These variables have to be defined before running SETUP_PROJECT set(PROJECT_NAME jrl-cmakemodules-python) diff --git a/_unittests/run_unit_tests.sh b/_unittests/run_unit_tests.sh index 337673d39..a062b1bae 100755 --- a/_unittests/run_unit_tests.sh +++ b/_unittests/run_unit_tests.sh @@ -1,5 +1,8 @@ #!/bin/bash +set -e +set -x + unittests="python cpp dependency catkin" # Code for running a specific unit test