Skip to content

Fix CMake warnings + drop old style export of library #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
N-Dekker wants to merge 10 commits into from
Closed

Fix CMake warnings + drop old style export of library #46

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
607bb24
COMP: Set cmake_minimum_required to 3.5.1, removed old policies, to f…
N-Dekker Feb 14, 2018
ff86021
STYLE: Removed code related to elxLibraryDepends.cmake
N-Dekker Feb 16, 2018
b46b05e
STYLE: Removed ELASTIX_ALL_COMPONENT from elxLibraryDepends.cmake. Al…
N-Dekker Feb 20, 2018
9b814e9
STYLE: Now unconditionally include(${ELASTIX_CONFIG_TARGETS_FILE}), a…
N-Dekker Feb 20, 2018
1445324
COMP: Set cmake_minimum_required to 3.5.1, removed old policies, to f…
N-Dekker Feb 14, 2018
c32fc58
STYLE: Removed code related to elxLibraryDepends.cmake
N-Dekker Feb 16, 2018
c745754
STYLE: Removed ELASTIX_ALL_COMPONENT from elxLibraryDepends.cmake. Al…
N-Dekker Feb 20, 2018
dc84f14
STYLE: Now unconditionally include(${ELASTIX_CONFIG_TARGETS_FILE}), a…
N-Dekker Feb 20, 2018
b0d157d
Merge branch 'ELX-21-Fix-CMake-warnings' of https://github.com/SuperE…
N-Dekker Feb 26, 2018
4b14e05
Merge branch 'develop' into ELX-21-Fix-CMake-warnings
N-Dekker Aug 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
cmake_minimum_required( VERSION 2.8 )
cmake_minimum_required( VERSION 3.5.1 )
Copy link
Member

@kaspermarstal kaspermarstal Feb 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this strictly necessary? If so, fine, if not, it might be annoying to users to work around.


project( elastix )

#---------------------------------------------------------------------
cmake_policy( SET CMP0012 NEW )
if( POLICY CMP0033 )
# This policy was introduced in cmake 3, so we need to check for its
# existence. When the minimum cmake version above is upgraded to 3
# this if(POLICY) check can be removed.
cmake_policy( SET CMP0033 OLD )
endif()

cmake_policy( SET CMP0007 OLD )
cmake_policy( SET CMP0042 NEW )

#---------------------------------------------------------------------
Expand Down Expand Up @@ -549,9 +541,7 @@ endif()
# Make it easier to include elastix functionality in other programs.

# Save library dependencies.
# The library dependencies file.
set( elxLIBRARY_DEPENDS_FILE ${elastix_BINARY_DIR}/elxLibraryDepends.cmake )
export_library_dependencies( ${elxLIBRARY_DEPENDS_FILE} )


# The build settings file. (necessary for elastix?)
#set( ITK_BUILD_SETTINGS_FILE ${ITK_BINARY_DIR}/ITKBuildSettings.cmake )
Expand Down
1 change: 0 additions & 1 deletion ElastixConfig.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,3 @@ set( ELASTIX_HELP_DIR @ELASTIX_HELP_DIR@ )

# Maintain backwards compatibility by also exporting old-style target information
set( ELASTIX_ALL_COMPONENT_LIBS @AllComponentLibs@ )
Copy link
Member Author

@N-Dekker N-Dekker Feb 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaspermarstal Should this line with ELASTIX_ALL_COMPONENT_LIBS also be removed? But if so, what about @AllComponentLibs@?

Copy link
Member

@kaspermarstal kaspermarstal Feb 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything allcomponentlibs-related should be removed :)

Copy link
Member Author

@N-Dekker N-Dekker Feb 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this one? https://github.com/SuperElastix/elastix/blob/develop/CMakeLists.txt#L562

set( elxLIBRARY_DEPENDS_FILE @elxLIBRARY_DEPENDS_FILE@ )
7 changes: 1 addition & 6 deletions UseElastix.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,8 @@ include_directories( ${ELASTIX_INCLUDE_DIRS} )
link_directories( ${ELASTIX_LIBRARY_DIRS} )

# If Elastix_FOUND is set, this file is included via find_package() which provides
# ELASTIX_CONFIG_TARGETS_FILE and elxLIBRARY_DEPENDS_FILE. Guarding the following
# include statements allow users to include this file directly for backwards
# compatibility.
# ELASTIX_CONFIG_TARGETS_FILE.
if( Elastix_FOUND )
Copy link
Member Author

@N-Dekker N-Dekker Feb 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaspermarstal Should the if be removed, now that I removed the entire else clause?

Copy link
Member

@kaspermarstal kaspermarstal Feb 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

Copy link
Member Author

@N-Dekker N-Dekker Feb 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

# This file was found by find_package
include( ${ELASTIX_CONFIG_TARGETS_FILE} )
else()
# Include linking dependency info for backwards compatibility
include( ${elxLIBRARY_DEPENDS_FILE} )
endif()
2 changes: 1 addition & 1 deletion dox/externalproject/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Example project for using elastix code from external projects.
PROJECT( elxExternalProject )

CMAKE_MINIMUM_REQUIRED( VERSION 2.8 )
CMAKE_MINIMUM_REQUIRED( VERSION 3.5.1 )

# Find TIK
FIND_PACKAGE( ITK REQUIRED )
Expand Down