From ec3d6b36da7edc2718d7509b82e48c0746888ccd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Gonzalve?= Date: Wed, 5 Oct 2022 20:15:39 +0200 Subject: [PATCH 1/2] Use cannonical library name for eigen --- CMakeLists.txt | 2 -- cmake/HandleEigen.cmake | 26 +++++++++++++++++++++----- gtsam/CMakeLists.txt | 7 ++----- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 74433f333..39d1e4307 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -101,8 +101,6 @@ if(GTSAM_BUILD_PYTHON OR GTSAM_INSTALL_MATLAB_TOOLBOX) # Copy matlab.h to the correct folder. configure_file(${PROJECT_SOURCE_DIR}/wrap/matlab.h ${PROJECT_BINARY_DIR}/wrap/matlab.h COPYONLY) - # Add the include directories so that matlab.h can be found - include_directories("${PROJECT_BINARY_DIR}" "${GTSAM_EIGEN_INCLUDE_FOR_BUILD}") add_subdirectory(wrap) list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/wrap/cmake") diff --git a/cmake/HandleEigen.cmake b/cmake/HandleEigen.cmake index c49eb4f8e..887ae1748 100644 --- a/cmake/HandleEigen.cmake +++ b/cmake/HandleEigen.cmake @@ -16,8 +16,14 @@ endif() if(GTSAM_USE_SYSTEM_EIGEN) find_package(Eigen3 REQUIRED) # need to find again as REQUIRED - # Use generic Eigen include paths e.g. - set(GTSAM_EIGEN_INCLUDE_FOR_INSTALL "${EIGEN3_INCLUDE_DIR}") + add_library(Eigen3::Eigen INTERFACE IMPORTED) + + set_target_properties(Eigen3::Eigen PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES ${EIGEN3_INCLUDE_DIR} + ) + + # The actual include directory (for BUILD cmake target interface): + set(GTSAM_EIGEN_INCLUDE_FOR_BUILD "${EIGEN3_INCLUDE_DIR}/") # check if MKL is also enabled - can have one or the other, but not both! # Note: Eigen >= v3.2.5 includes our patches @@ -30,9 +36,6 @@ if(GTSAM_USE_SYSTEM_EIGEN) if(EIGEN_USE_MKL_ALL AND (EIGEN3_VERSION VERSION_EQUAL 3.3.4)) message(FATAL_ERROR "MKL does not work with Eigen 3.3.4 because of a bug in Eigen. See http://eigen.tuxfamily.org/bz/show_bug.cgi?id=1527. Disable GTSAM_USE_SYSTEM_EIGEN to use GTSAM's copy of Eigen, disable GTSAM_WITH_EIGEN_MKL, or upgrade/patch your installation of Eigen.") endif() - - # The actual include directory (for BUILD cmake target interface): - set(GTSAM_EIGEN_INCLUDE_FOR_BUILD "${EIGEN3_INCLUDE_DIR}") else() # Use bundled Eigen include path. # Clear any variables set by FindEigen3 @@ -46,6 +49,19 @@ else() # The actual include directory (for BUILD cmake target interface): set(GTSAM_EIGEN_INCLUDE_FOR_BUILD "${GTSAM_SOURCE_DIR}/gtsam/3rdparty/Eigen/") + + add_library(gtsam_eigen3 INTERFACE) + + target_include_directories(gtsam_eigen3 INTERFACE + $ + $ + ) + add_library(Eigen3::Eigen ALIAS gtsam_eigen3) + + install(TARGETS gtsam_eigen3 EXPORT GTSAM-exports PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) + + list(APPEND GTSAM_EXPORTED_TARGETS gtsam_eigen3) + set(GTSAM_EXPORTED_TARGETS "${GTSAM_EXPORTED_TARGETS}" PARENT_SCOPE) endif() # Detect Eigen version: diff --git a/gtsam/CMakeLists.txt b/gtsam/CMakeLists.txt index 09f1ea806..d3408ee7f 100644 --- a/gtsam/CMakeLists.txt +++ b/gtsam/CMakeLists.txt @@ -117,12 +117,9 @@ set_target_properties(gtsam PROPERTIES VERSION ${gtsam_version} SOVERSION ${gtsam_soversion}) -# Append Eigen include path, set in top-level CMakeLists.txt to either # system-eigen, or GTSAM eigen path -target_include_directories(gtsam PUBLIC - $ - $ -) +target_link_libraries(gtsam PUBLIC Eigen3::Eigen) + # MKL include dir: if (GTSAM_USE_EIGEN_MKL) target_include_directories(gtsam PUBLIC ${MKL_INCLUDE_DIR}) From 774cfd25b880e1deef5696c5b0ac1b4f2e548cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Gonzalve?= Date: Fri, 7 Oct 2022 18:52:10 +0200 Subject: [PATCH 2/2] Use Eigen Config file for cmake detection Since Eigen 3.3.0, a Config.cmake file is provided, thus no need to rely on a custom one. Moreover, the FindEigen3.cmake used in gtsam was erroneously forcing an include directory when using system version of eigen. This fixes bug #1297 --- cmake/Config.cmake.in | 4 ++ cmake/FindEigen3.cmake | 81 ----------------------------------------- cmake/HandleEigen.cmake | 16 ++++---- 3 files changed, 11 insertions(+), 90 deletions(-) delete mode 100644 cmake/FindEigen3.cmake diff --git a/cmake/Config.cmake.in b/cmake/Config.cmake.in index 89627a172..cc2a7df8f 100644 --- a/cmake/Config.cmake.in +++ b/cmake/Config.cmake.in @@ -21,6 +21,10 @@ else() find_dependency(Boost @BOOST_FIND_MINIMUM_VERSION@ COMPONENTS @BOOST_FIND_MINIMUM_COMPONENTS@) endif() +if(@GTSAM_USE_SYSTEM_EIGEN@) +find_dependency(Eigen3 REQUIRED) +endif() + # Load exports include(${OUR_CMAKE_DIR}/@PACKAGE_NAME@-exports.cmake) diff --git a/cmake/FindEigen3.cmake b/cmake/FindEigen3.cmake deleted file mode 100644 index 9c546a05d..000000000 --- a/cmake/FindEigen3.cmake +++ /dev/null @@ -1,81 +0,0 @@ -# - 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, -# Copyright (c) 2008, 2009 Gael Guennebaud, -# Copyright (c) 2009 Benoit Jacob -# 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) - diff --git a/cmake/HandleEigen.cmake b/cmake/HandleEigen.cmake index 887ae1748..48941b85b 100644 --- a/cmake/HandleEigen.cmake +++ b/cmake/HandleEigen.cmake @@ -1,7 +1,7 @@ ############################################################################### # Option for using system Eigen or GTSAM-bundled Eigen # Default: Use system's Eigen if found automatically: -find_package(Eigen3 QUIET) +find_package(Eigen3 CONFIG QUIET) set(USE_SYSTEM_EIGEN_INITIAL_VALUE ${Eigen3_FOUND}) option(GTSAM_USE_SYSTEM_EIGEN "Find and use system-installed Eigen. If 'off', use the one bundled with GTSAM" ${USE_SYSTEM_EIGEN_INITIAL_VALUE}) unset(USE_SYSTEM_EIGEN_INITIAL_VALUE) @@ -14,16 +14,14 @@ endif() # Switch for using system Eigen or GTSAM-bundled Eigen if(GTSAM_USE_SYSTEM_EIGEN) - find_package(Eigen3 REQUIRED) # need to find again as REQUIRED - - add_library(Eigen3::Eigen INTERFACE IMPORTED) - - set_target_properties(Eigen3::Eigen PROPERTIES - INTERFACE_INCLUDE_DIRECTORIES ${EIGEN3_INCLUDE_DIR} - ) + # Since Eigen 3.3.0 a Eigen3Config.cmake is available so use it. + find_package(Eigen3 CONFIG REQUIRED) # need to find again as REQUIRED # The actual include directory (for BUILD cmake target interface): - set(GTSAM_EIGEN_INCLUDE_FOR_BUILD "${EIGEN3_INCLUDE_DIR}/") + # Note: EIGEN3_INCLUDE_DIR points to some random location on some eigen + # versions. So here I use the target itself to get the proper include + # directory (it is generated by cmake, thus has the correct path) + get_target_property(GTSAM_EIGEN_INCLUDE_FOR_BUILD Eigen3::Eigen INTERFACE_INCLUDE_DIRECTORIES) # check if MKL is also enabled - can have one or the other, but not both! # Note: Eigen >= v3.2.5 includes our patches