Skip to content

Commit fb8e6ed

Browse files
committed
COMP: Address greptile review on vendored Eigen 5 import
Three findings on Modules/ThirdParty/Eigen3/src/itkeigen, all scoped to the vendored Eigen build tree (USE_SYSTEM_EIGEN=ON consumers go through find_package and never read these files; the system Eigen3Config.cmake ships separately). P1 — Eigen3Config.cmake.in:7 hardcoded "Eigen3Targets.cmake", dropping the @EIGEN3_TARGETS_FILE@ substitution. The ITK section in Modules/ThirdParty/Eigen3/src/itkeigen/CMakeLists.txt sets EIGEN3_TARGETS_FILE=ITKInternalEigen3Targets.cmake (line 972) before calling configure_package_config_file against this template, but the hardcoded form ignored that variable. The generated ITKInternalEigen3Config.cmake would then include "…/Eigen3Targets.cmake" (belonging to eigen_external), and ITKInternalEigen3::Eigen would never be imported for installed consumers. Restore the substitution. P2 — Modules/ThirdParty/Eigen3/src/itkeigen/CMakeLists.txt declared option(ITK_USE_EIGEN_MPL2_ONLY ...) twice in the ITK section: once at line 844 and again at line 952. CMake silently ignores subsequent option() calls when the variable is cached, so behavior is unchanged, but the duplicate (and a slightly different description) is confusing. Drop the second declaration; replace with a one-line pointer back to the canonical declaration site. P2 — Modules/ThirdParty/Eigen3/src/itkeigen/CMakeLists.txt:964 had a commented-out generator expression "# $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/..>". Eigen 5's header tree is fully sourced from CMAKE_CURRENT_SOURCE_DIR (no configured headers land in the binary dir for this module), so the commented line was dead code. Remove it. USE_SYSTEM_EIGEN compatibility for Eigen 3.3+ is preserved: all three edits live under Modules/ThirdParty/Eigen3/src/itkeigen which is only configured when USE_SYSTEM_EIGEN=OFF; the system-Eigen path (find_package(Eigen3 3.3 REQUIRED)) does not read these files.
1 parent 7071c66 commit fb8e6ed

2 files changed

Lines changed: 2 additions & 4 deletions

File tree

Modules/ThirdParty/Eigen3/src/itkeigen/CMakeLists.txt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -949,8 +949,7 @@ add_library (ITKInternalEigen3::Eigen ALIAS eigen_internal)
949949
# This would wrongly enforce EIGEN_MPL2_ONLY to other libraries using Eigen.
950950
# We wrap this definition in ITK_USE_EIGEN_MPL2_ONLY, and only enabling it internally in the dashboards and CI,
951951
# to avoid introducing GPL code from Eigen3 internally in ITK.
952-
option(ITK_USE_EIGEN_MPL2_ONLY "Set compile definition EIGEN_MPL2_ONLY for ITKInternalEigen3." OFF)
953-
mark_as_advanced(ITK_USE_EIGEN_MPL2_ONLY)
952+
# (ITK_USE_EIGEN_MPL2_ONLY is declared once near line 844 of this file.)
954953

955954
if(ITK_USE_EIGEN_MPL2_ONLY)
956955
target_compile_definitions (eigen_internal INTERFACE "EIGEN_MPL2_ONLY")
@@ -961,7 +960,6 @@ endif()
961960
# INSTALL: headers require pre-prend itkeigen/Eigen/X.
962961
target_include_directories (eigen_internal SYSTEM INTERFACE
963962
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/..>
964-
# $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/..>
965963
$<INSTALL_INTERFACE:${INCLUDE_INSTALL_DIR}>
966964
)
967965

Modules/ThirdParty/Eigen3/src/itkeigen/cmake/Eigen3Config.cmake.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@
44
@PACKAGE_INIT@
55

66
if (NOT TARGET Eigen3::Eigen)
7-
include ("${CMAKE_CURRENT_LIST_DIR}/Eigen3Targets.cmake")
7+
include ("${CMAKE_CURRENT_LIST_DIR}/@EIGEN3_TARGETS_FILE@")
88
endif (NOT TARGET Eigen3::Eigen)

0 commit comments

Comments
 (0)