From bfb9060e6fd81bcd15a63ffe1f4ce60b2d386eb3 Mon Sep 17 00:00:00 2001 From: Gtker Date: Mon, 2 Sep 2024 17:54:03 +0200 Subject: [PATCH 1/3] corrosion_install: Move error checking into outer scope It seems this was accidentally put on the check for a `TARGET ${TARGET_NAME}-static` lib instead of the `TARGET_TYPE STREQUAL *` --- cmake/Corrosion.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/Corrosion.cmake b/cmake/Corrosion.cmake index 2fafda8a..d9b8d7f2 100644 --- a/cmake/Corrosion.cmake +++ b/cmake/Corrosion.cmake @@ -1319,8 +1319,6 @@ set_target_properties(${INSTALL_TARGET}-static " ) endif() - else() - message(FATAL_ERROR "Unknown target type ${TARGET_TYPE} for install target ${INSTALL_TARGET}") endif() if(TARGET ${INSTALL_TARGET}-shared) @@ -1384,6 +1382,8 @@ set_target_properties(${INSTALL_TARGET}-shared endif() endif() endif() + else() + message(FATAL_ERROR "Unknown target type ${TARGET_TYPE} for install target ${INSTALL_TARGET}") endif() # Executables can also have export tables, so they _might_ also need header files From ebdc500abfb9cc43ebe6768cbb1a831cb076795c Mon Sep 17 00:00:00 2001 From: Gtker Date: Mon, 2 Sep 2024 18:05:44 +0200 Subject: [PATCH 2/3] corrosion_install: Add guard and use CMAKE_CURRENT_BINARY_DIR It seems something has been switched up and it's using CMAKE_BINARY_DIR instead of CMAKE_CURRENT_BINARY_DIR. Additionally, without a guard CMake will write the static/shared definitions on every reconfigure, making the config file fail. --- cmake/Corrosion.cmake | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/cmake/Corrosion.cmake b/cmake/Corrosion.cmake index d9b8d7f2..3ba4c6ce 100644 --- a/cmake/Corrosion.cmake +++ b/cmake/Corrosion.cmake @@ -1308,8 +1308,9 @@ function(corrosion_install) if(EXPORT_NAME) get_target_property(COR_FILE_NAME ${INSTALL_TARGET}-static COR_FILE_NAME) - file(APPEND - ${CMAKE_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME} + if(NOT EXISTS ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME}-static-exists) + file(APPEND + ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME} " add_library(${INSTALL_TARGET}-static STATIC IMPORTED) set_target_properties(${INSTALL_TARGET}-static @@ -1317,7 +1318,10 @@ set_target_properties(${INSTALL_TARGET}-static IMPORTED_LOCATION \"\${PACKAGE_PREFIX_DIR}/${DESTINATION}/${COR_FILE_NAME}\" ) " - ) + ) + + file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME}-static-exists) + endif() endif() endif() @@ -1357,9 +1361,10 @@ set_target_properties(${INSTALL_TARGET}-static ) if(EXPORT_NAME) - get_target_property(COR_FILE_NAME ${INSTALL_TARGET}-shared COR_FILE_NAME) - file(APPEND - ${CMAKE_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME} + if(NOT EXISTS ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME}-shared-exists) + get_target_property(COR_FILE_NAME ${INSTALL_TARGET}-shared COR_FILE_NAME) + file(APPEND + ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME} " add_library(${INSTALL_TARGET}-shared SHARED IMPORTED) set_target_properties(${INSTALL_TARGET}-shared @@ -1367,18 +1372,21 @@ set_target_properties(${INSTALL_TARGET}-shared IMPORTED_LOCATION \"\${PACKAGE_PREFIX_DIR}/${DESTINATION}/${COR_FILE_NAME}\" ) " - ) + ) - get_target_property(COR_IMPLIB_FILE_NAME ${INSTALL_TARGET}-shared COR_IMPLIB_FILE_NAME) - if (NOT COR_IMPLIB_FILE_NAME MATCHES .*-NOTFOUND) - file(APPEND - ${CMAKE_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME} + get_target_property(COR_IMPLIB_FILE_NAME ${INSTALL_TARGET}-shared COR_IMPLIB_FILE_NAME) + if (NOT COR_IMPLIB_FILE_NAME MATCHES .*-NOTFOUND) + file(APPEND + ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME} " set_target_properties(${INSTALL_TARGET}-shared PROPERTIES IMPORTED_IMPLIB \"\${PACKAGE_PREFIX_DIR}/${DESTINATION}/${COR_IMPLIB_FILE_NAME}\" )" - ) + ) + endif() + + file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME}-shared-exists) endif() endif() endif() From 6a020156977f05715d1311a034f93d11ba1261a9 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Mon, 2 Sep 2024 20:38:48 +0200 Subject: [PATCH 3/3] corrosion_install: Require EXPORT_NAME to be unique. Switch back to CMAKE_BINARY_DIR as documented by the function signature. If we anyway require a globally unique EXPORT_NAME, then we don't need to use the current binary dir, making the file easier to find. --- cmake/Corrosion.cmake | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/cmake/Corrosion.cmake b/cmake/Corrosion.cmake index 3ba4c6ce..b129b0ab 100644 --- a/cmake/Corrosion.cmake +++ b/cmake/Corrosion.cmake @@ -1101,7 +1101,7 @@ ANCHOR: corrosion-install and is not officially released yet. Feedback and Suggestions are welcome. ```cmake -corrosion_install(TARGETS ... [EXPORT ... [EXPORT ] [[ARCHIVE|LIBRARY|RUNTIME|PUBLIC_HEADER] [DESTINATION ] [PERMISSIONS ] @@ -1109,7 +1109,8 @@ corrosion_install(TARGETS ... [EXPORT Corrosion.cmake that must be included in the installed config file. +* **EXPORT**: Creates an export that can be installed with `install(EXPORT)`. must be globally unique. + Also creates a file at ${CMAKE_BINARY_DIR}/corrosion/Corrosion.cmake that must be included in the installed config file. * **ARCHIVE**/**LIBRARY**/**RUNTIME**/PUBLIC_HEADER: Designates that the following settings only apply to that specific type of object. * **DESTINATION**: The subdirectory within the CMAKE_INSTALL_PREFIX that a specific object should be placed. Defaults to values from GNUInstallDirs. * **PERMISSIONS**: The permissions of files copied into the install prefix. @@ -1169,6 +1170,10 @@ function(corrosion_install) list(REMOVE_AT ARGN 0) # Pop set(EXTRA_TARGETS_EXPORT_NAME ${EXPORT_NAME}Corrosion.cmake) set(EXPORT_NAME EXPORT ${EXPORT_NAME}) + set(EXPORT_FILE_PATH "${CMAKE_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME}") + # Remove first, since otherwise we will append to the file on every reconfigure. + # Assumes that the corrosion_install will only be called once for a given EXPORT_NAME. + file(REMOVE "${EXPORT_FILE_PATH}") endif() else() # Prevent variable set in user code from interfering @@ -1308,9 +1313,7 @@ function(corrosion_install) if(EXPORT_NAME) get_target_property(COR_FILE_NAME ${INSTALL_TARGET}-static COR_FILE_NAME) - if(NOT EXISTS ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME}-static-exists) - file(APPEND - ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME} + file(APPEND "${EXPORT_FILE_PATH}" " add_library(${INSTALL_TARGET}-static STATIC IMPORTED) set_target_properties(${INSTALL_TARGET}-static @@ -1318,10 +1321,7 @@ set_target_properties(${INSTALL_TARGET}-static IMPORTED_LOCATION \"\${PACKAGE_PREFIX_DIR}/${DESTINATION}/${COR_FILE_NAME}\" ) " - ) - - file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME}-static-exists) - endif() + ) endif() endif() @@ -1361,11 +1361,9 @@ set_target_properties(${INSTALL_TARGET}-static ) if(EXPORT_NAME) - if(NOT EXISTS ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME}-shared-exists) - get_target_property(COR_FILE_NAME ${INSTALL_TARGET}-shared COR_FILE_NAME) - file(APPEND - ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME} - " + get_target_property(COR_FILE_NAME ${INSTALL_TARGET}-shared COR_FILE_NAME) + file(APPEND "${EXPORT_FILE_PATH}" +" add_library(${INSTALL_TARGET}-shared SHARED IMPORTED) set_target_properties(${INSTALL_TARGET}-shared PROPERTIES @@ -1376,18 +1374,14 @@ set_target_properties(${INSTALL_TARGET}-shared get_target_property(COR_IMPLIB_FILE_NAME ${INSTALL_TARGET}-shared COR_IMPLIB_FILE_NAME) if (NOT COR_IMPLIB_FILE_NAME MATCHES .*-NOTFOUND) - file(APPEND - ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME} - " + file(APPEND "${EXPORT_FILE_PATH}" +" set_target_properties(${INSTALL_TARGET}-shared PROPERTIES IMPORTED_IMPLIB \"\${PACKAGE_PREFIX_DIR}/${DESTINATION}/${COR_IMPLIB_FILE_NAME}\" )" ) endif() - - file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/corrosion/${EXTRA_TARGETS_EXPORT_NAME}-shared-exists) - endif() endif() endif() else()