COMP: Expose itkeigen subdir in ITKEigen3_INCLUDE_DIRS for external consumers#6223
Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom May 6, 2026
Conversation
…onsumers
ITK ships its vendored Eigen3 headers at
Modules/ThirdParty/Eigen3/src/itkeigen/Eigen/<header>
ITK code accesses these via the ITK_EIGEN(<header>) macro
defined in itk_eigen.h, which expands to
<itkeigen/Eigen/<header>>
When ITK_USE_SYSTEM_EIGEN=OFF, the prior ITKEigen3_INCLUDE_DIRS
contained only ${ITKEigen3_SOURCE_DIR}/src — sufficient for the
macro form (the compiler then appends /itkeigen/Eigen/<header>) but
not for the upstream Eigen convention <Eigen/<header>>, which needs
${ITKEigen3_SOURCE_DIR}/src/itkeigen on the include path.
External consumers that legitimately use the upstream pattern hit
'fatal error: Eigen/Dense: No such file or directory' as soon as
they link against ITK::ITKEigen3Module or ITK::eigen_internal.
This blocks the modern-target-interfaces refactor in
InsightSoftwareConsortium/ITKTotalVariation#57: proxTV's
lapackFunctionsWrap.cpp uses #include <Eigen/Dense> and so cannot
build against the vendored Eigen via either of ITK's exported
imported targets.
Add the itkeigen subdirectory to ITKEigen3_INCLUDE_DIRS so both
include conventions resolve. ITK's own consumers continue to find
<itkeigen/Eigen/<header>> via the first entry; external consumers
now find <Eigen/<header>> via the second. Both entries flow into
INTERFACE_INCLUDE_DIRECTORIES of the IMPORTED ITK::ITKEigen3Module
target, so the fix applies in-tree and to find_package(ITK)
consumers alike.
The Eigen3::Eigen IMPORTED target produced by Eigen's own export
already exposes the correct path, but it is in-source-scope only
(declared as add_library(Eigen3::Eigen ALIAS eigen)) and therefore
invisible to FetchContent sub-builds; relying on it is not an
option for downstream remote modules that pull proxTV via
FetchContent.
This change is independent of the Eigen 5 vendored update in
PR InsightSoftwareConsortium#6176; the architectural mismatch predates that PR and applies
to upstream/main as-is.
dzenanz
approved these changes
May 6, 2026
This comment was marked as resolved.
This comment was marked as resolved.
Member
Author
|
Tracking issue for the SYSTEM-include macro cleanup is now open: #6224 — captures the macro-plumbing diagnosis, the three approaches in increasing scope, and the recommended sequencing (investigate the existing |
1976a3a
into
InsightSoftwareConsortium:main
16 of 18 checks passed
blowekamp
reviewed
May 6, 2026
| # ${ITKEigen3_SOURCE_DIR}/src uses the macro form; external | ||
| # consumers (proxTV, anything else that includes Eigen the | ||
| # standard way) need the parent of <Eigen/X>, which is | ||
| # ${ITKEigen3_SOURCE_DIR}/src/itkeigen. |
Member
There was a problem hiding this comment.
This comment is too long, and not efficient for humans to understand.
Member
|
PR #57 was not blocked because of a compilation error it was blocked because of incompatible ITK versioning infrastructure. |
This was referenced May 7, 2026
Closed
Contributor
|
I haven't bisected yet, but looking at what changed in ITK since yesterday, I suspect this change broke my build: |
Contributor
Contributor
|
So in fact it was MR #6176 I'll move comments there... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
${ITKEigen3_SOURCE_DIR}/src/itkeigentoITKEigen3_INCLUDE_DIRSso external consumers using the upstream Eigen include convention (<Eigen/<header>>) resolve against the vendored Eigen via ITK's IMPORTED targets — currently they getfatal error: Eigen/Dense: No such file or directory. Unblocks InsightSoftwareConsortium/ITKTotalVariation # 57's modern-target-interfaces refactor for proxTV. Independent of and complementary to #6176 (the Eigen 5 update); the architecture issue predates that PR.Root cause
ITK ships its vendored Eigen3 headers at:
ITK's own internal code uses
ITK_EIGEN(<header>)fromitk_eigen.h, which expands to<itkeigen/Eigen/<header>>. With include path${ITKEigen3_SOURCE_DIR}/src(the parent ofitkeigen/), the compiler appends/itkeigen/Eigen/<header>and resolves correctly. ITK proper has always built fine.Modules/ThirdParty/Eigen3/CMakeLists.txt:58(vendored case, before this PR):That single path flowed into
INTERFACE_INCLUDE_DIRECTORIESof bothITK::ITKEigen3ModuleandITK::eigen_internal. External consumers using the upstream<Eigen/<header>>convention got the wrong root and failed to resolve. TheEigen3::EigenIMPORTED target produced by Eigen's own export does have the correct path, but it's declared asadd_library(Eigen3::Eigen ALIAS eigen)— an in-source-scope alias, invisible to FetchContent sub-builds (verified empirically).Fix
Single-block change to
Modules/ThirdParty/Eigen3/CMakeLists.txt:58:ITK's own
<itkeigen/Eigen/<header>>consumers continue to find their headers via the first entry. External consumers using<Eigen/<header>>find theirs via the second. Both flow intoINTERFACE_INCLUDE_DIRECTORIESof the IMPORTEDITK::ITKEigen3Moduleso the fix applies in-tree and tofind_package(ITK)consumers identically.Local verification
Full ITK build with
Module_TotalVariation=ONpinned at ITKTotalVariation # 57's tip (39e849e, which previously failedlapackFunctionsWrap.cpp:11:10: fatal error: Eigen/Dense: No such file or directory):ITK_BUILD_DEFAULT_MODULES=ON, Module_TotalVariation pinned at PR # 57 tip)ninja -j16libitkproxTV-6.0.soproducedbin/TotalVariationTestDriverlinkedctest -R ProxTVitkProxTVImageFilterTestpasseslapackFunctionsWrap.cpp.ocompile flags include-isystem .../Eigen3/src/itkeigen<Eigen/Dense>)-isystem /usr/include/eigen3Compile-commands receipt for
lapackFunctionsWrap.cpp:Relationship to other PRs
upstream/mainhas the same setup. PR ENH: Update vendored Eigen3 to 5.0.1+master; verify USE_SYSTEM 3.3/3.4/5.x #6176 is good to merge as-is. After both this PR and ENH: Update vendored Eigen3 to 5.0.1+master; verify USE_SYSTEM 3.3/3.4/5.x #6176 land, no further coordination is needed.set(proxTV_Eigen_LIBRARIES ITK::ITKEigen3Module)becomes correct as-is, no further changes needed in TotalVariation. The longstanding "silent fall-through to system Eigen" bug in ITKTotalVariationmain(whereset(Eigen3_DIR "${ITKInternalEigen3_DIR}")reads an empty value) becomes moot once # 57 lands, because proxTV no longer needs the brokenEigen3_DIRshim.