corrosion_install: fix reconfiguring breaking EXPORTs #555
Conversation
cmake/Corrosion.cmake
Outdated
| @@ -1308,19 +1308,21 @@ 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) | |||
There was a problem hiding this comment.
Can we assume that corrosion_install() will only be called once per target? If yes perhaps we could delete the file first.
We would still want to overwrite the file in case $DESTINATION changed.
There was a problem hiding this comment.
I guess we could, but the file is at the EXPORT level and we don't know if other installs put something else in there.
I'm not entirely sure what the completely correct solution is here.
There was a problem hiding this comment.
I pushed a suggestion - If we require that the export name is unique, then we shouldn't have any problems with other corrosion_installs putting information into the same export.
What do you think?
There was a problem hiding this comment.
I think that makes it good enough. The worst that happens is that people need to add another line to their config file.
It seems this was accidentally put on the check for a
`TARGET ${TARGET_NAME}-static` lib instead of the
`TARGET_TYPE STREQUAL *`
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.
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.
jschwe
force-pushed
the
wip-corrosion
branch
from
58acb47 to
6a02015
Compare
jschwe
changed the title
Sorry for the immediate do over, but it seems there were a few mistakes in my last PR #544.
It seems the else was accidentally put on the check for a
TARGET ${TARGET_NAME}-staticlib instead of theTARGET_TYPE STREQUAL *.Additionally, I forgot to put a guard on the
file(APPEND)statements, so every reconfigure would add yet another set ofadd_librarys for both static/shared, leading to it not working.I'm not sure what the correct solution for this is, but the current solution at the very least works.