Skip to content

Commit

Permalink
corrosion_install: fix reconfiguring breaking EXPORTs (#555)
Browse files Browse the repository at this point in the history
* 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 *`

* 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.

* 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.

---------

Co-authored-by: Gtker <[email protected]>
Co-authored-by: Jonathan Schwender <[email protected]>
  • Loading branch information
3 people authored Sep 3, 2024
1 parent c117c37 commit 97164ee
Showing 1 changed file with 19 additions and 17 deletions.
36 changes: 19 additions & 17 deletions cmake/Corrosion.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -1101,15 +1101,16 @@ ANCHOR: corrosion-install
and is not officially released yet. Feedback and Suggestions are welcome.
```cmake
corrosion_install(TARGETS <target1> ... <targetN> [EXPORT <export-name]
corrosion_install(TARGETS <target1> ... <targetN> [EXPORT <export-name>]
[[ARCHIVE|LIBRARY|RUNTIME|PUBLIC_HEADER]
[DESTINATION <dir>]
[PERMISSIONS <permissions...>]
[CONFIGURATIONS [Debug|Release|<other-configuration>]]
] [...])
```
* **TARGETS**: Target or targets to install.
* **EXPORT**: Creates an export that can be installed with `install(EXPORT)`. Also creates a file at ${CMAKE_BINARY_DIR}/corrosion/<export-name>Corrosion.cmake that must be included in the installed config file.
* **EXPORT**: Creates an export that can be installed with `install(EXPORT)`. <export-name> must be globally unique.
Also creates a file at ${CMAKE_BINARY_DIR}/corrosion/<export-name>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.
Expand Down Expand Up @@ -1169,6 +1170,10 @@ function(corrosion_install)
list(REMOVE_AT ARGN 0) # Pop <export-name>
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
Expand Down Expand Up @@ -1308,8 +1313,7 @@ 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}
file(APPEND "${EXPORT_FILE_PATH}"
"
add_library(${INSTALL_TARGET}-static STATIC IMPORTED)
set_target_properties(${INSTALL_TARGET}-static
Expand All @@ -1319,8 +1323,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)
Expand Down Expand Up @@ -1360,30 +1362,30 @@ 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}
"
file(APPEND "${EXPORT_FILE_PATH}"
"
add_library(${INSTALL_TARGET}-shared SHARED IMPORTED)
set_target_properties(${INSTALL_TARGET}-shared
PROPERTIES
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 "${EXPORT_FILE_PATH}"
"
set_target_properties(${INSTALL_TARGET}-shared
PROPERTIES
IMPORTED_IMPLIB \"\${PACKAGE_PREFIX_DIR}/${DESTINATION}/${COR_IMPLIB_FILE_NAME}\"
)"
)
endif()
)
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
Expand Down

0 comments on commit 97164ee

Please sign in to comment.