Skip to content

COMP: Fix COMPILE_DEPENDS → DEPENDS in three Filtering module declara…#6097

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
blowekamp:fix_deps_system_eigen
Apr 21, 2026
Merged

COMP: Fix COMPILE_DEPENDS → DEPENDS in three Filtering module declara…#6097
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
blowekamp:fix_deps_system_eigen

Conversation

@blowekamp
Copy link
Copy Markdown
Member

…tions

ITKImageIntensity, ITKDenoising, and ITKDiffusionTensorImage declared their dependencies with COMPILE_DEPENDS instead of DEPENDS. COMPILE_DEPENDS only populates the legacy include_directories() path and does not call target_link_libraries(), so no CMake target link is established and INTERFACE_INCLUDE_DIRECTORIES cannot propagate transitively.

This breaks builds with system Eigen (ITK_USE_SYSTEM_EIGEN=ON): the Eigen3::Eigen include path is silently dropped, causing failures in headers such as itkSymmetricEigenAnalysis.h. Since these modules genuinely link against their listed dependencies, DEPENDS is semantically correct.

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

…tions

ITKImageIntensity, ITKDenoising, and ITKDiffusionTensorImage declared their
dependencies with COMPILE_DEPENDS instead of DEPENDS. COMPILE_DEPENDS only
populates the legacy include_directories() path and does not call
target_link_libraries(), so no CMake target link is established and
INTERFACE_INCLUDE_DIRECTORIES cannot propagate transitively.

This breaks builds with system Eigen (ITK_USE_SYSTEM_EIGEN=ON): the
Eigen3::Eigen include path is silently dropped, causing failures in headers
such as itkSymmetricEigenAnalysis.h. Since these modules genuinely link
against their listed dependencies, DEPENDS is semantically correct.
@github-actions github-actions Bot added type:Compiler Compiler support or related warnings area:Filtering Issues affecting the Filtering module labels Apr 21, 2026
@blowekamp
Copy link
Copy Markdown
Member Author

This issue was encountered when trying to build the conda-forge libitk-feedstock against the current ITK main. The build uses eigen provided by conda-forge.

@blowekamp blowekamp marked this pull request as ready for review April 21, 2026 17:24
@blowekamp blowekamp requested review from hjmjohnson and thewtex April 21, 2026 17:24
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 21, 2026

Greptile Summary

This PR fixes a build regression when ITK_USE_SYSTEM_EIGEN=ON by correcting three itk-module.cmake declarations that erroneously used COMPILE_DEPENDS instead of DEPENDS. COMPILE_DEPENDS only orders the build but does not call target_link_libraries(), so INTERFACE_INCLUDE_DIRECTORIES (e.g. the Eigen3::Eigen path) never propagated transitively, silently breaking headers like itkSymmetricEigenAnalysis.h.

The three one-word changes in ITKDenoising, ITKDiffusionTensorImage, and ITKImageIntensity are minimal and semantically correct. Note that 56 other itk-module.cmake files across the repository still use COMPILE_DEPENDS; those may or may not be intentional, and a broader audit could be worthwhile as follow-up.

Confidence Score: 5/5

Safe to merge — targeted, minimal fix for a real build failure with no behaviour change in standard builds.

All three changes are single-keyword corrections that restore correct CMake semantics (target linking vs. compile ordering). No API surface, no logic, no data flow is altered. The remaining 56 COMPILE_DEPENDS usages are out of scope for this PR and do not block merging.

No files require special attention; consider a follow-up audit of the 56 remaining COMPILE_DEPENDS usages in the repository.

Important Files Changed

Filename Overview
Modules/Filtering/Denoising/itk-module.cmake Changed COMPILE_DEPENDS → DEPENDS so ITKImageAdaptors, ITKImageGrid, ITKImageStatistics, ITKIOImageBase, and ITKStatistics are properly linked via target_link_libraries() and propagate INTERFACE_INCLUDE_DIRECTORIES transitively.
Modules/Filtering/DiffusionTensorImage/itk-module.cmake Changed COMPILE_DEPENDS → DEPENDS for ITKSpatialObjects, establishing proper CMake target link and transitive include propagation for this module.
Modules/Filtering/ImageIntensity/itk-module.cmake Changed COMPILE_DEPENDS → DEPENDS for ITKImageAdaptors, ITKImageStatistics, ITKImageGrid, and ITKPath — the fix that directly restores the Eigen3::Eigen include path when ITK_USE_SYSTEM_EIGEN=ON.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["itk_module() with COMPILE_DEPENDS\n(before fix)"] -->|"add_dependencies() only"| B["Module is built after dependency\n(ordering guarantee only)"]
    B --> C["target_link_libraries() NOT called"]
    C --> D["INTERFACE_INCLUDE_DIRECTORIES\nnot propagated transitively"]
    D --> E["❌ Eigen3::Eigen include path missing\n→ build failure with ITK_USE_SYSTEM_EIGEN=ON"]

    F["itk_module() with DEPENDS\n(after fix)"] -->|"add_dependencies() + target_link_libraries()"| G["Module is built after dependency\n(ordering guarantee)"]
    G --> H["INTERFACE_INCLUDE_DIRECTORIES\npropagated transitively"]
    H --> I["✅ Eigen3::Eigen include path available\n→ itkSymmetricEigenAnalysis.h compiles correctly"]
Loading

Reviews (1): Last reviewed commit: "COMP: Fix COMPILE_DEPENDS → DEPENDS in t..." | Re-trigger Greptile

@hjmjohnson hjmjohnson merged commit 95d3db3 into InsightSoftwareConsortium:main Apr 21, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module type:Compiler Compiler support or related warnings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants