Skip to content

COMP: IO modules publicly depend on ImageIOBase#6070

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:fix-io-module-public-depends
Apr 17, 2026
Merged

COMP: IO modules publicly depend on ImageIOBase#6070
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:fix-io-module-public-depends

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Move ITKIOImageBase from PRIVATE_DEPENDS to DEPENDS for 4 IO modules whose public headers #include "itkImageIOBase.h" or inherit from ImageIOBase: DCMTK, GE, LSM, and the Philips REC module.

Follows the same fix applied to VTK in #5850. With PRIVATE_DEPENDS, downstream projects that consume these modules' public headers may not receive the transitive include paths and link targets for ITKIOImageBase.

Header analysis confirming public dependency

Each module was verified to have public headers that directly reference ITKIOImageBase types:

  • DCMTK: itkDCMTKImageIO.h includes itkImageIOBase.h, DCMTKImageIO inherits from ImageIOBase
  • GE: itkGE4ImageIOFactory.h, itkGE5ImageIOFactory.h, itkGEAdwImageIOFactory.h include itkImageIOBase.h
  • LSM: itkLSMImageIOFactory.h includes itkImageIOBase.h
  • Philips REC: itkPhilipsRECImageIO.h includes itkImageIOBase.h, inherits from ImageIOBase

Not included: ITKIOCSV — its public headers do not reference any IOImageBase types. Its PRIVATE_DEPENDS ITKIOImageBase may be vestigial or only needed for module infrastructure, and warrants separate investigation.

Local build verification
  • Configure: passed (ITK 6.0.0, CMake 3.26.6, Ninja, macOS)
  • Build: 1145 incremental targets, 0 errors
  • Tests: 4/4 IO module tests passed (GE, LSM; DCMTK and Philips REC are EXCLUDE_FROM_DEFAULT)
  • Generated module configs verified: ITKIOImageBase appears in PUBLIC_DEPENDS and TRANSITIVE_DEPENDS
  • gersemi pre-commit check: passed

DCMTK, GE, LSM, and PhilipsREC have public headers that
#include itkImageIOBase.h or inherit from ImageIOBase, making
ITKIOImageBase a genuine public dependency. Move it from
PRIVATE_DEPENDS to DEPENDS so downstream consumers receive
the transitive include paths and link targets.

Follows the same pattern as the VTK fix in PR InsightSoftwareConsortium#5850.
@github-actions github-actions Bot added type:Compiler Compiler support or related warnings area:IO Issues affecting the IO module labels Apr 15, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 15, 2026

Greptile Summary

This PR moves ITKIOImageBase from PRIVATE_DEPENDS to DEPENDS in four IO module cmake files (DCMTK, GE, LSM, PhilipsREC), matching the same fix applied to the VTK module in #5850. Header analysis confirms each module has public headers that directly #include "itkImageIOBase.h" or inherit from ImageIOBase, making the public dependency necessary for correct transitive include paths and link targets in downstream consumers.

Confidence Score: 5/5

Safe to merge — all four changes are mechanically correct and verified against public header includes.

No P0 or P1 issues found. Each module's public headers were confirmed to include itkImageIOBase.h, the itk_module() macro's custom parser handles all keyword orderings correctly (including ENABLE_SHARED mid-block in LSM), ITKIOCSV was correctly excluded, and a post-search showed no remaining PRIVATE_DEPENDS ITKIOImageBase in any IO module.

No files require special attention

Important Files Changed

Filename Overview
Modules/IO/DCMTK/itk-module.cmake Moves ITKIOImageBase from PRIVATE_DEPENDS to DEPENDS; confirmed by itkDCMTKImageIO.h, itkDCMTKImageIOFactory.h, and itkDCMTKFileReader.h all including itkImageIOBase.h
Modules/IO/GE/itk-module.cmake Moves ITKIOImageBase from PRIVATE_DEPENDS to DEPENDS and removes now-empty PRIVATE_DEPENDS block; confirmed by itkGE4ImageIOFactory.h, itkGE5ImageIOFactory.h, and itkGEAdwImageIOFactory.h including itkImageIOBase.h
Modules/IO/LSM/itk-module.cmake Moves ITKIOImageBase from PRIVATE_DEPENDS to DEPENDS; ENABLE_SHARED between DEPENDS and PRIVATE_DEPENDS is valid per the custom itk_module() parser which resets _doing="" on ENABLE_SHARED
Modules/IO/PhilipsREC/itk-module.cmake Moves ITKIOImageBase from PRIVATE_DEPENDS to new DEPENDS block; confirmed by itkPhilipsRECImageIO.h and itkPhilipsRECImageIOFactory.h including itkImageIOBase.h

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Before["Before (PRIVATE_DEPENDS)"]
        A1[ITKIODCMTK] -->|PRIVATE| B[ITKIOImageBase]
        A2[ITKIOGE] -->|PRIVATE| B
        A3[ITKIOLSM] -->|PRIVATE| B
        A4[ITKIOPhilipsREC] -->|PRIVATE| B
    end

    subgraph After["After (DEPENDS)"]
        C1[ITKIODCMTK] -->|PUBLIC| D[ITKIOImageBase]
        C2[ITKIOGE] -->|PUBLIC| D
        C3[ITKIOLSM] -->|PUBLIC| D
        C4[ITKIOPhilipsREC] -->|PUBLIC| D
    end

    subgraph Downstream["Downstream Consumer"]
        E[User Module] -->|depends on| C1
        C1 -->|transitive PUBLIC| D
        D -->|include paths + link targets| E
    end
Loading

Reviews (3): Last reviewed commit: "COMP: IO modules publicly depend on Imag..." | Re-trigger Greptile

@hjmjohnson hjmjohnson marked this pull request as draft April 15, 2026 16:36
@hjmjohnson
Copy link
Copy Markdown
Member Author

/azp run ITK.Windows

@hjmjohnson hjmjohnson marked this pull request as ready for review April 15, 2026 20:06
@hjmjohnson hjmjohnson marked this pull request as draft April 17, 2026 13:44
@hjmjohnson hjmjohnson marked this pull request as ready for review April 17, 2026 13:44
@hjmjohnson hjmjohnson requested a review from blowekamp April 17, 2026 13:47
@hjmjohnson
Copy link
Copy Markdown
Member Author

@blowekamp This is an extension of your recent work. I don't fully understand all the implications of this change, but it 'smells' right. The analysis makes sense to me as well.

Copy link
Copy Markdown
Member

@blowekamp blowekamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely fine. As I recall this was discovered in v6 with the testing directories strictly using the target based includes replaced the directory/module wide includes.

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants