ITKModuleMacros: _SYSTEM_INCLUDE_DIRS does not support in-source paths (blocks SYSTEM treatment of vendored Eigen)

[hjmjohnson]

CMake/ITKModuleMacros.cmake adds entries from ${itk-module}_SYSTEM_INCLUDE_DIRS into ${itk-module}_SYSTEM_GENEX_INCLUDE_DIRS as raw absolute paths with no $<BUILD_INTERFACE:> / $<INSTALL_INTERFACE:> guards, so the variable cannot be used for in-source paths (export step rejects unguarded absolute paths in INTERFACE_INCLUDE_DIRECTORIES). This blocks the cleanest fix for warning-suppression on vendored Eigen and any future in-source SYSTEM-include need.

Surfaced from PR #6223's Greptile P2 review (warning leak from ${ITKEigen3_SOURCE_DIR}/src/itkeigen exposed as regular -I rather than -isystem).

Reproducer

In Modules/ThirdParty/Eigen3/CMakeLists.txt (vendored branch), replace the regular-include addition with the structurally-correct SYSTEM equivalent:

-  set(ITKEigen3_INCLUDE_DIRS ${ITKEigen3_SOURCE_DIR}/src ${ITKEigen3_SOURCE_DIR}/src/itkeigen)
+  set(ITKEigen3_INCLUDE_DIRS ${ITKEigen3_SOURCE_DIR}/src)
+  set(ITKEigen3_SYSTEM_INCLUDE_DIRS ${ITKEigen3_SOURCE_DIR}/src/itkeigen)

CMake fails the generate step:

CMake Error in Modules/ThirdParty/Eigen3/CMakeLists.txt:
  Target "ITKEigen3Module" INTERFACE_INCLUDE_DIRECTORIES property contains
  path: "/path/to/Modules/ThirdParty/Eigen3/src/itkeigen"

Root cause

CMake/ITKModuleMacros.cmake:264-289 handles _INCLUDE_DIRS and _SYSTEM_INCLUDE_DIRS asymmetrically:

# Lines 264-271: regular includes get $<BUILD_INTERFACE:> + $<INSTALL_INTERFACE:>
foreach(_dir ${${itk-module}_INCLUDE_DIRS})
  list(APPEND ${itk-module}_GENEX_INCLUDE_DIRS "$<BUILD_INTERFACE:${_dir}>")
endforeach()
list(APPEND ${itk-module}_GENEX_INCLUDE_DIRS
  "$<INSTALL_INTERFACE:$<INSTALL_PREFIX>/${${itk-module}_INSTALL_INCLUDE_DIR}>")

# Lines 273-279: SYSTEM includes added raw — comment explicitly opts out
# of install-interface support.
if(${itk-module}_SYSTEM_INCLUDE_DIRS)
  foreach(_dir ${${itk-module}_SYSTEM_INCLUDE_DIRS})
    list(APPEND ${itk-module}_SYSTEM_GENEX_INCLUDE_DIRS ${_dir})
  endforeach()
endif()

Comment at line 273-274 makes the design intent explicit:

System include directories are assumed to be external dependencies that are not installed, and thus do not have separate install interface paths.

So _SYSTEM_INCLUDE_DIRS is structurally a /usr/include-style escape hatch for paths to libraries that exist independently of the ITK install tree. In-source paths (anything under ${ITKEigen3_SOURCE_DIR}/...) violate this assumption: at export time CMake walks INTERFACE_INCLUDE_DIRECTORIES looking for unguarded absolute paths that aren't valid in the install interface and rejects them.

Why this matters now

PR #6223 unblocks proxTV / InsightSoftwareConsortium/ITKTotalVariation#57 by adding ${ITKEigen3_SOURCE_DIR}/src/itkeigen to ITKEigen3_INCLUDE_DIRS so external consumers using <Eigen/<header>> resolve via ITK's vendored Eigen. That works (verified end-to-end), but it exposes the path as -I (regular include) rather than -isystem. Eigen 5's templates emit -Wshadow / -Wmaybe-uninitialized / -Wctad-maybe-unsupported etc. that downstream consumers compiling with -Wall -Wextra will start seeing.

The neighboring vendored declaration in Modules/ThirdParty/Eigen3/src/itkeigen/CMakeLists.txt already uses the correct mechanism for the same physical directory:

target_include_directories (eigen_internal SYSTEM INTERFACE
  $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
  "$<INSTALL_INTERFACE:$<INSTALL_PREFIX>/${ITK3P_INSTALL_INCLUDE_DIR}/itkeigen>;"
)

This is what the macro version should produce when given an in-source SYSTEM path; today it cannot.

By symmetry the long-standing ${ITKEigen3_SOURCE_DIR}/src regular include also probably wants SYSTEM treatment — it leaks the same Eigen warnings to any non-third-party consumer of ITK::ITKEigen3Module (e.g., ITKCommon and every downstream module that ultimately resolves an ITK_EIGEN(<header>) macro to <itkeigen/Eigen/<header>>). It's been regular-include for the entire 4.x/5.x/6.x history and the codebase apparently doesn't trip on the warnings under ITK's own CI flags, but a careful audit would resolve whether it should also be SYSTEM.

Three approaches, in increasing scope

Approach A — Bypass the macro for the specific eigen path

Add the SYSTEM include directly on the eigen_internal target inside Modules/ThirdParty/Eigen3/src/itkeigen/CMakeLists.txt, mirroring the existing ${CMAKE_CURRENT_SOURCE_DIR} line:

target_include_directories (eigen_internal SYSTEM INTERFACE
  $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
  $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/..>          # <-- add for <Eigen/X> path
  "$<INSTALL_INTERFACE:$<INSTALL_PREFIX>/${ITK3P_INSTALL_INCLUDE_DIR}/itkeigen>;"
)

Wait — actually that's the same dir. The new path is ${ITKEigen3_SOURCE_DIR}/src/itkeigen which equals ${CMAKE_CURRENT_SOURCE_DIR} of the inner CMakeLists. So this already exists. Need to verify why the existing SYSTEM INTERFACE on eigen_internal doesn't propagate to external <Eigen/<header>> consumers via ITK::ITKEigen3Module → ITK::eigen_internal link transitively. May be path normalization differences during target export (.../itkeigen vs .../itkeigen/..).

Pros: smallest change; bypasses the broken macro path.
Cons: needs investigation — the existing SYSTEM declaration may already be doing the right thing and PR #6223's regular-include addition may be redundant. Or the export representation reorders paths in a way that breaks resolution.

Approach B — Fix the macro for in-source SYSTEM paths

Extend the SYSTEM-include loop in CMake/ITKModuleMacros.cmake:275-279 to mirror the regular-include treatment:

foreach(_dir ${${itk-module}_SYSTEM_INCLUDE_DIRS})
  list(APPEND ${itk-module}_SYSTEM_GENEX_INCLUDE_DIRS "$<BUILD_INTERFACE:${_dir}>")
endforeach()

(With $<INSTALL_INTERFACE:> only added when an install-side mapping makes sense — the existing comment about external dependencies suggests the install side should typically be empty or find_package-driven.)

Pros: Makes _SYSTEM_INCLUDE_DIRS work for in-source paths, which is what most module authors actually want. Closes the gap between the regular and SYSTEM behaviors.
Cons: touches the module macro every ITK module uses. Need to audit existing _SYSTEM_INCLUDE_DIRS consumers — if any module depends on the raw-path behavior (e.g., to inject a system path that should survive into the install interface), that consumer breaks. Quick grep suggests few in-tree consumers but every third-party module is a potential dependency.

Approach C — Move every Eigen include to SYSTEM, with audit

Once Approach B is in place, also move ${ITKEigen3_SOURCE_DIR}/src from _INCLUDE_DIRS to _SYSTEM_INCLUDE_DIRS for symmetry. The existing <itkeigen/Eigen/<header>> consumers (ITKCommon and friends) start receiving -isystem instead of -I, suppressing whatever warnings might leak. Lower priority because nothing's currently breaking.

Pros: full warning-suppression parity; one consistent SYSTEM treatment for the vendored Eigen.
Cons: changes warning surface for the whole tree. Needs -Werror CI on a representative platform set to surface any latent issues. Possibly trivial in practice but should be its own PR with a clean test cycle.

Recommended sequencing

1. First: investigate Approach A (does the existing eigen_internal SYSTEM INTERFACE already do the right thing once we understand the path-normalization quirk?). Cheap, may resolve the warning concern entirely without touching ITKModuleMacros.cmake.
2. If Approach A is insufficient: do Approach B as a standalone PR. Touch only the macro, audit existing consumers, ensure no regression in install-interface validity.
3. Optionally after B: do Approach C as a separate PR with -Werror CI verification.

PR #6223 ships the regular-_INCLUDE_DIRS form because it's the only one that actually generates today; this issue tracks the SYSTEM-treatment cleanup so it doesn't fall through.

Files / lines of interest

File	Line	Role
CMake/ITKModuleMacros.cmake	264-271	Regular _INCLUDE_DIRS loop with $<BUILD_INTERFACE:>
CMake/ITKModuleMacros.cmake	273-279	SYSTEM _SYSTEM_INCLUDE_DIRS loop without generator-expression guards (the bug)
CMake/ITKModuleMacros.cmake	281-289	THIRD_PARTY consumption of both variables via include_directories(...) and include_directories(SYSTEM ...)
Modules/ThirdParty/Eigen3/CMakeLists.txt	56-69	Where this issue first surfaced; PR #6223 sets _INCLUDE_DIRS for both paths today
Modules/ThirdParty/Eigen3/src/itkeigen/CMakeLists.txt	(the target_include_directories(eigen_internal SYSTEM INTERFACE ...) block)	The existing in-source SYSTEM declaration; reference for what the macro version should produce

[blowekamp]

I am not sure that the change of thought here is correct. But I don't have time to investigate in allotted time.

[blowekamp]

@hjmjohnson These Eigen include related patches and issues are a little sprawling. I think a discussion needs to occur to clarify the requirements and the conventions of the interface modules, and the future goals for third party interface modules. Where is the best place to have this discussion keeping on topic and not having is closed too quickly?

[dzenanz]

Perhaps the forum? Or an issue?

[hjmjohnson]

Per @blowekamp's and @dzenanz's suggestion to consolidate the scattered Eigen-related design conversations, I've opened #6230 (Master tracking — Eigen3 third-party design for ITK 6.x) as a single venue.

The tracking issue surfaces 7 questions (header-path policy, symbol mangling, Eigen3::Eigen target stability, warning-suppression scope, VNL→Eigen migration, discussion venue, and the ITKv6 license to revise historical promises) and pulls in the Discourse history (2018-2023) that documented several decisions never recorded in GitHub.

Suggest holding further design comments on this thread until the umbrella questions in #6230 have a maintainer-quorum answer; otherwise the conversation re-fragments. This thread can stay open as the implementation venue once the direction is decided.