Fix CMake warnings + drop old style export of library #46
Conversation
#21 Note: it should still be seen if this commit does not break anything: * https://travis-ci.org/SuperElastix/elastix should support the new CMake version requirement * CMP0007, the new way empty list items are supported should be OK with elastix. * CMP0033: The removal of export_library_dependencies should be OK with elastix. cmake_minimum_required is chosen to be in sync with SuperElastix.
Discussed w/ Marius and kasper: Backward compatibility with respect to elxLibraryDepends.cmake (elxLIBRARY_DEPENDS_FILE) no longer needs to be supported.
UseElastix.cmake.in
Outdated
| # include statements allow users to include this file directly for backwards | ||
| # compatibility. | ||
| # ELASTIX_CONFIG_TARGETS_FILE. | ||
| if( Elastix_FOUND ) |
There was a problem hiding this comment.
@kaspermarstal Should the if be removed, now that I removed the entire else clause?
There was a problem hiding this comment.
Marius wondered if there shouldn't be an else-clause, producing an error message when Elastix is not found. But you think that's not necessary, right?
ElastixConfig.cmake.in
Outdated
| set( ELASTIX_HELP_DIR @ELASTIX_HELP_DIR@ ) | ||
|
|
||
| # Maintain backwards compatibility by also exporting old-style target information | ||
| set( ELASTIX_ALL_COMPONENT_LIBS @AllComponentLibs@ ) |
There was a problem hiding this comment.
@kaspermarstal Should this line with ELASTIX_ALL_COMPONENT_LIBS also be removed? But if so, what about @AllComponentLibs@?
There was a problem hiding this comment.
Everything allcomponentlibs-related should be removed :)
There was a problem hiding this comment.
| @@ -1,17 +1,9 @@ | |||
| cmake_minimum_required( VERSION 2.8 ) | |||
| cmake_minimum_required( VERSION 3.5.1 ) | |||
There was a problem hiding this comment.
Is this strictly necessary? If so, fine, if not, it might be annoying to users to work around.
#21 Note: it should still be seen if this commit does not break anything: * https://travis-ci.org/SuperElastix/elastix should support the new CMake version requirement * CMP0007, the new way empty list items are supported should be OK with elastix. * CMP0033: The removal of export_library_dependencies should be OK with elastix. cmake_minimum_required is chosen to be in sync with SuperElastix.
Discussed w/ Marius and kasper: Backward compatibility with respect to elxLibraryDepends.cmake (elxLIBRARY_DEPENDS_FILE) no longer needs to be supported.
…so removed foreach( LIB IN LISTS AllComponentLibs ), as discussed w/ Kasper. Related to #21.
…s discussed w/ Kasper. Related to #21.
…lastix/elastix into ELX-21-Fix-CMake-warnings
kaspermarstal
force-pushed
the
develop
branch
from
a83dd65 to
1f504cf
Compare
|
We could rebase this branch on develop and see if it works on azure. |
|
The CMake warnings are fixed and the old style export of the elastix library has been dropped by other pull requests:
It is unclear whether the SuperElastix Linux link errors (#21 (comment)) would still occur with the latest elastix library version. But we have no clue and no resources to address them any time soon. This pull request (#46) is obsolete anyway now. |
Fixed CMake warnings reported by Jean-Christophe Fillion-Robin (jcfr) at #21
Dropped old style export of Elastix library files.