BUG: Restore eigen_internal export to ITK's main targets set (#6239)#6247
BUG: Restore eigen_internal export to ITK's main targets set (#6239)#6247hjmjohnson wants to merge 1 commit intoInsightSoftwareConsortium:mainfrom
Conversation
|
@greptile review |
SimpleITK build verification — before/after differentialReproduced the SimpleITK failure against latest SimpleITK upstream (
Both ITK builds: Pre-flight:
|
find_package(ITK) |
FetchContent fallback | Error cascade | |
|---|---|---|---|
| baseline (no fix) | FAILED | yes — _deps/itk-src/ present |
itksys imported-target collision (exact #6239) |
| PR #6247 | -- ITK found via find_package() |
no | unrelated SimpleITK codegen issues |
The fix resolves the export-set regression cleanly. Whatever else SimpleITK CI surfaces is independent of this PR.
…SoftwareConsortium#6239) Commit 64ddc66 (Eigen 5.0.1 vendored update, 2026-05-01) silently moved eigen_internal out of ITK's main `${ITK3P_INSTALL_EXPORT_NAME}` export set into a standalone `ITKInternalEigen3Targets`. Because `ITKConfig.cmake` only loads `ITKTargets.cmake`, downstream `find_package(ITK)` could no longer resolve `eigen_internal` and any consumer using FetchContent with `FIND_PACKAGE_ARGS` (SimpleITK's pattern) hit a target-collision cascade as CMake fell back to `add_subdirectory`. Re-add the install rule that registers `eigen_internal` in `${ITK3P_INSTALL_EXPORT_NAME}` while keeping the existing standalone `ITKInternalEigen3Targets` install for `find_package(ITKInternalEigen3)` consumers. Both export sets now reference the same target. Issue: InsightSoftwareConsortium#6239
This comment was marked as resolved.
This comment was marked as resolved.
5dfd84a to
c1715d9
Compare
|
How was the ITK eigen for updated? ITK commit 7aa8ae2 indicates set of changes in the eigen fork: Were these changes from the prior eigen commit included in the update to the fork? |
Downstream-consumer end-to-end verification (BRAINSTools + SimpleITK)Built two real-world ITK consumers against the SimpleITK — full build ✅
BRAINSTools — configure clean, build blocked by unrelated module-config issue
|
Restores
eigen_internalto ITK's main${ITK3P_INSTALL_EXPORT_NAME}export set sofind_package(ITK)resolves it for downstream consumers (closes #6239 — SimpleITK CI red since 2026-05-01).Scope note: this addresses only the most immediate regression. The broader Eigen3 design questions (header-path policy #5952 vs #6225, symbol mangling, VNL→Eigen migration, the
_SYSTEM_INCLUDE_DIRSasymmetry #6224) remain in the master tracking issue #6230 and are deliberately not touched here.Root cause (issue #6239)
Commit
64ddc666(Eigen 5.0.1 vendored update, 2026-05-01) silently movedeigen_internalout of ITK's main${ITK3P_INSTALL_EXPORT_NAME}export set into a standaloneITKInternalEigen3Targets. BecauseITKConfig.cmakeonly loadsITKTargets.cmake, downstreamfind_package(ITK)could no longer resolveeigen_internaland any consumer using FetchContent withFIND_PACKAGE_ARGS(SimpleITK's pattern) hit a target-collision cascade as CMake fell back toadd_subdirectory.The fix re-adds the install rule that registers
eigen_internalin the main export set while keeping the standaloneITKInternalEigen3Targetsinstall forfind_package(ITKInternalEigen3)consumers. Both export sets now reference the same target — same pattern as every other ITK-vendored third party (NrrdIO, NIFTI, Expat, OpenJPEG, DICOMParser).Local verification — SimpleITK before/after differential
Reproduced against latest SimpleITK upstream (
853c78a9, 2026-05-09) using two parallel ITK installs at the same configuration (ITK_BUILD_DEFAULT_MODULES=ON+Module_ITKIOTransformMINC=ON+Module_ITKReview=ON, matchingSuperBuild/External_ITK.cmake):7820bbcfb8(no fix)~/src/ITK-eigen-baseline-install~/src/ITK-build-test-tree-eigen/install-fixPre-flight:
eigen_internalin installedITKTargets.cmakeSimpleITK configure against each install:
find_package(ITK)_deps/itk-src/)add_library cannot create target "itksys" because an imported target with the same name already exists(exact #6239 cascade)-- ITK found via find_package()The fix resolves the export-set regression cleanly. Verification comment with full reproduction steps posted to this PR.
What remains for #6230
This PR does not address (deferred per #6230 design discussion 2026-05-07/09):
ITK_EIGEN(X)for ITK internals; PRs ENH: Use standard header approach for eigen e.g. itk_Eigenvalues #5952 (per-module wrappers) and ENH: Migrate Eigen3 includes to canonical <Eigen/x> via Eigen3::Eigen (supersedes #5952) #6225 (canonical<Eigen/X>migration) both pending design resolution.Eigen3::Eigenconsumer exposure — emerging consensus is "no, useITK::ITKEigen3Module" but no code change here.eigen_internal, not_SYSTEM_INCLUDE_DIRSglue. Larger refactor.test/); deferred to avoid precedent-setting in a regression-fix PR.UpdateFromUpstream.shoverlay-patch checklist — was prototyped in an earlier revision of this PR but removed per maintainer preference; documenting the no-mangling decision and the dual-export requirement belongs elsewhere.