Skip to content

Commit

Permalink
Modernize Eigen3 use as an imported target
Browse files Browse the repository at this point in the history
Eigen3 is a header only template library, adopt an updated
FindEigen.cmake that creates an imported target. Note that include
directories from imported targets are automatically treated as system
includes on compilers that support the concept.

The core header only Avogadro library exposes it as part of its
interface, and then we are able to remove a huge amount of duplicated
code. Still need to look at the other side to ensure the exported
targets find all targets like Eigen they depend upon.

Signed-off-by: Marcus D. Hanwell <[email protected]>
  • Loading branch information
cryos committed Apr 19, 2023
1 parent 795e549 commit cc5897a
Show file tree
Hide file tree
Showing 13 changed files with 37 additions and 129 deletions.
6 changes: 0 additions & 6 deletions avogadro/calc/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
find_package(Eigen3 REQUIRED)

# Add as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM "${EIGEN3_INCLUDE_DIR}")

add_library(Calc)

avogadro_headers(Calc
Expand Down
3 changes: 0 additions & 3 deletions avogadro/command/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
find_package(Eigen3 REQUIRED)
include_directories(SYSTEM "${EIGEN3_INCLUDE_DIR}")

add_executable(avocjsontocml cjsontocml.cpp)
target_link_libraries(avocjsontocml Avogadro::IO)

Expand Down
4 changes: 1 addition & 3 deletions avogadro/core/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
find_package(Eigen3 REQUIRED)
# Add as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM ${EIGEN3_INCLUDE_DIR})

if(USE_SPGLIB)
find_package(SPGLIB REQUIRED)
Expand Down Expand Up @@ -34,6 +31,7 @@ target_sources(Headers PUBLIC
utilities.h
vector.h
)
target_link_libraries(Headers INTERFACE Eigen3::Eigen3)
install(TARGETS Headers
EXPORT "AvogadroLibsTargets"
FILE_SET HEADERS DESTINATION "${INSTALL_INCLUDE_DIR}")
Expand Down
4 changes: 1 addition & 3 deletions avogadro/io/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
find_package(Eigen3 REQUIRED)

if(USE_MMTF)
find_package(MMTF REQUIRED)
include_directories(SYSTEM "${MMTF_INCLUDE_DIRS}")
Expand All @@ -19,7 +17,7 @@ endif()

# Add as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM "${EIGEN3_INCLUDE_DIR}"
include_directories(SYSTEM
"${AvogadroLibs_SOURCE_DIR}/thirdparty/pugixml"
"${AvogadroLibs_SOURCE_DIR}/thirdparty/struct"
"${AvogadroLibs_SOURCE_DIR}/thirdparty")
Expand Down
5 changes: 0 additions & 5 deletions avogadro/molequeue/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
find_package(Eigen3 REQUIRED)
# Add as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM ${EIGEN3_INCLUDE_DIR})

if(QT_VERSION EQUAL 6)
find_package(Qt6 COMPONENTS Widgets Network Core5Compat REQUIRED)
else()
Expand Down
5 changes: 0 additions & 5 deletions avogadro/qtgui/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
find_package(Eigen3 REQUIRED)
# Add as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM ${EIGEN3_INCLUDE_DIR})

if(QT_VERSION EQUAL 6)
find_package(Qt6 COMPONENTS Widgets Core5Compat REQUIRED)
else()
Expand Down
5 changes: 0 additions & 5 deletions avogadro/qtopengl/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
find_package(Eigen3 REQUIRED)
# Add as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM ${EIGEN3_INCLUDE_DIR})

if(QT_VERSION EQUAL 6)
find_package(Qt6 COMPONENTS Widgets OpenGLWidgets REQUIRED)
else()
Expand Down
3 changes: 0 additions & 3 deletions avogadro/qtplugins/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
find_package(Eigen3 REQUIRED)
include_directories(SYSTEM ${EIGEN3_INCLUDE_DIR})

find_package(Qt${QT_VERSION} COMPONENTS Widgets Network Concurrent REQUIRED)
if(QT_VERSION EQUAL 6)
find_package(Qt6 COMPONENTS OpenGLWidgets REQUIRED)
Expand Down
4 changes: 1 addition & 3 deletions avogadro/quantumio/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
find_package(Eigen3 REQUIRED)
# Add as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM "${EIGEN3_INCLUDE_DIR}"
"${AvogadroLibs_SOURCE_DIR}/thirdparty")
include_directories(SYSTEM "${AvogadroLibs_SOURCE_DIR}/thirdparty")

add_library(QuantumIO)

Expand Down
5 changes: 0 additions & 5 deletions avogadro/rendering/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
find_package(Eigen3 REQUIRED)
# Add as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM ${EIGEN3_INCLUDE_DIR})

find_package(OpenGL REQUIRED)
find_package(GLEW REQUIRED)
include_directories(SYSTEM ${OPENGL_INCLUDE_DIR} ${GLEW_INCLUDE_DIRS})
Expand Down
5 changes: 0 additions & 5 deletions avogadro/vtk/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
find_package(Eigen3 REQUIRED)

# Add as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM ${EIGEN3_INCLUDE_DIR})
find_package(OpenGL REQUIRED)
find_package(GLEW REQUIRED)
include_directories(SYSTEM ${OPENGL_INCLUDE_DIR} ${GLEW_INCLUDE_DIRS})
Expand Down
113 changes: 33 additions & 80 deletions cmake/FindEigen3.cmake
Original file line number Diff line number Diff line change
@@ -1,80 +1,33 @@
# - Try to find Eigen3 lib
#
# This module supports requiring a minimum version, e.g. you can do
# find_package(Eigen3 3.1.2)
# to require version 3.1.2 or newer of Eigen3.
#
# Once done this will define
#
# EIGEN3_FOUND - system has eigen lib with correct version
# EIGEN3_INCLUDE_DIR - the eigen include directory
# EIGEN3_VERSION - eigen version

# Copyright (c) 2006, 2007 Montel Laurent, <[email protected]>
# Copyright (c) 2008, 2009 Gael Guennebaud, <[email protected]>
# Copyright (c) 2009 Benoit Jacob <[email protected]>
# Redistribution and use is allowed according to the terms of the 2-clause BSD license.

if(NOT Eigen3_FIND_VERSION)
if(NOT Eigen3_FIND_VERSION_MAJOR)
set(Eigen3_FIND_VERSION_MAJOR 2)
endif(NOT Eigen3_FIND_VERSION_MAJOR)
if(NOT Eigen3_FIND_VERSION_MINOR)
set(Eigen3_FIND_VERSION_MINOR 91)
endif(NOT Eigen3_FIND_VERSION_MINOR)
if(NOT Eigen3_FIND_VERSION_PATCH)
set(Eigen3_FIND_VERSION_PATCH 0)
endif(NOT Eigen3_FIND_VERSION_PATCH)

set(Eigen3_FIND_VERSION "${Eigen3_FIND_VERSION_MAJOR}.${Eigen3_FIND_VERSION_MINOR}.${Eigen3_FIND_VERSION_PATCH}")
endif(NOT Eigen3_FIND_VERSION)

macro(_eigen3_check_version)
file(READ "${EIGEN3_INCLUDE_DIR}/Eigen/src/Core/util/Macros.h" _eigen3_version_header)

string(REGEX MATCH "define[ \t]+EIGEN_WORLD_VERSION[ \t]+([0-9]+)" _eigen3_world_version_match "${_eigen3_version_header}")
set(EIGEN3_WORLD_VERSION "${CMAKE_MATCH_1}")
string(REGEX MATCH "define[ \t]+EIGEN_MAJOR_VERSION[ \t]+([0-9]+)" _eigen3_major_version_match "${_eigen3_version_header}")
set(EIGEN3_MAJOR_VERSION "${CMAKE_MATCH_1}")
string(REGEX MATCH "define[ \t]+EIGEN_MINOR_VERSION[ \t]+([0-9]+)" _eigen3_minor_version_match "${_eigen3_version_header}")
set(EIGEN3_MINOR_VERSION "${CMAKE_MATCH_1}")

set(EIGEN3_VERSION ${EIGEN3_WORLD_VERSION}.${EIGEN3_MAJOR_VERSION}.${EIGEN3_MINOR_VERSION})
if(${EIGEN3_VERSION} VERSION_LESS ${Eigen3_FIND_VERSION})
set(EIGEN3_VERSION_OK FALSE)
else(${EIGEN3_VERSION} VERSION_LESS ${Eigen3_FIND_VERSION})
set(EIGEN3_VERSION_OK TRUE)
endif(${EIGEN3_VERSION} VERSION_LESS ${Eigen3_FIND_VERSION})

if(NOT EIGEN3_VERSION_OK)

message(STATUS "Eigen3 version ${EIGEN3_VERSION} found in ${EIGEN3_INCLUDE_DIR}, "
"but at least version ${Eigen3_FIND_VERSION} is required")
endif(NOT EIGEN3_VERSION_OK)
endmacro(_eigen3_check_version)

if (EIGEN3_INCLUDE_DIR)

# in cache already
_eigen3_check_version()
set(EIGEN3_FOUND ${EIGEN3_VERSION_OK})

else (EIGEN3_INCLUDE_DIR)

find_path(EIGEN3_INCLUDE_DIR NAMES signature_of_eigen3_matrix_library
PATHS
${CMAKE_INSTALL_PREFIX}/include
${KDE4_INCLUDE_DIR}
PATH_SUFFIXES eigen3 eigen
)

if(EIGEN3_INCLUDE_DIR)
_eigen3_check_version()
endif(EIGEN3_INCLUDE_DIR)

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(Eigen3 DEFAULT_MSG EIGEN3_INCLUDE_DIR EIGEN3_VERSION_OK)

mark_as_advanced(EIGEN3_INCLUDE_DIR)

endif(EIGEN3_INCLUDE_DIR)
find_path(Eigen3_INCLUDE_DIR
NAMES signature_of_eigen3_matrix_library
PATH_SUFFIXES eigen3 eigen
DOC "Eigen include directory")
mark_as_advanced(Eigen3_INCLUDE_DIR)

if (Eigen3_INCLUDE_DIR)
file(STRINGS "${Eigen3_INCLUDE_DIR}/Eigen/src/Core/util/Macros.h" _Eigen3_version_lines
REGEX "#define[ \t]+EIGEN_(WORLD|MAJOR|MINOR)_VERSION")
string(REGEX REPLACE ".*EIGEN_WORLD_VERSION *\([0-9]*\).*" "\\1" _Eigen3_version_world "${_Eigen3_version_lines}")
string(REGEX REPLACE ".*EIGEN_MAJOR_VERSION *\([0-9]*\).*" "\\1" _Eigen3_version_major "${_Eigen3_version_lines}")
string(REGEX REPLACE ".*EIGEN_MINOR_VERSION *\([0-9]*\).*" "\\1" _Eigen3_version_minor "${_Eigen3_version_lines}")
set(Eigen3_VERSION "${_Eigen3_version_world}.${_Eigen3_version_major}.${_Eigen3_version_minor}")
unset(_Eigen3_version_world)
unset(_Eigen3_version_major)
unset(_Eigen3_version_minor)
unset(_Eigen3_version_lines)
endif ()

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(Eigen3
REQUIRED_VARS Eigen3_INCLUDE_DIR
VERSION_VAR Eigen3_VERSION)

if (Eigen3_FOUND)
set(Eigen3_INCLUDE_DIRS "${Eigen3_INCLUDE_DIR}")

if (NOT TARGET Eigen3::Eigen3)
add_library(Eigen3::Eigen3 INTERFACE IMPORTED)
set_target_properties(Eigen3::Eigen3 PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${Eigen3_INCLUDE_DIR}")
endif ()
endif ()
4 changes: 1 addition & 3 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ include_directories("${AvogadroLibs_BINARY_DIR}/avogadro/core")

# find google test
find_package(GTest REQUIRED)
find_package(Eigen3 REQUIRED)
# Add both as "system headers" to avoid warnings generated by them with
# compilers that support that notion.
include_directories(SYSTEM
${GTEST_INCLUDE_DIRS}
${EIGEN3_INCLUDE_DIR})
${GTEST_INCLUDE_DIRS})

include(CheckIncludeFileCXX)
include(CheckCXXSymbolExists)
Expand Down

0 comments on commit cc5897a

Please sign in to comment.