Skip to content

COMP: Add ITKZLIB private dependency to ITKReview#6433

Merged
hjmjohnson merged 3 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:fix-itkreview-zlib-link
Jun 11, 2026
Merged

COMP: Add ITKZLIB private dependency to ITKReview#6433
hjmjohnson merged 3 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:fix-itkreview-zlib-link

Conversation

@hjmjohnson

Copy link
Copy Markdown
Member

Fixes the libITKReview shared-library link failure (undefined _itkzlib_gz* symbols) seen in ITKSphinxExamples python-superbuild CI by adding ITKZLIB to the module's PRIVATE_DEPENDS.

Root cause and verification

itkVoxBoCUBImageIO.cxx includes itk_zlib.h and calls itkzlib_gzopen/gzread/gzwrite/..., but ada67f6 ("COMP: Update ITKReview depends and linkage") moved all module dependencies except ITKIOImageBase to COMPILE_DEPENDS, dropping zlib from the link closure. ITKZLIB was never declared in either list.

The failure only manifests in BUILD_SHARED_LIBS=ON builds (e.g. Python wrapping), which is why ITK's own static-build CI stayed green. First observed in ITKSphinxExamples CI:
https://github.com/InsightSoftwareConsortium/ITKSphinxExamples/actions/runs/27319825769/job/80794743298

Reproduced and fix verified locally on macOS arm64 (AppleClang, Release, shared libs, Module_ITKReview=ON, ITK_BUILD_DEFAULT_MODULES=ON, ITK_LEGACY_REMOVE=ON): link succeeds and otool -L shows @rpath/libitkzlib-6.0.1.dylib.

PRIVATE_DEPENDS matches the pattern used by ITKIOVTK and ITKIOGIPL, since zlib is referenced only from the .cxx, not from module headers.

itkVoxBoCUBImageIO.cxx calls itkzlib_gz* but ITKZLIB was dropped from
the link closure when module dependencies moved to COMPILE_DEPENDS
(ada67f6), leaving libITKReview with undefined zlib symbols in
shared-library builds.

Assisted-by: Claude Code — root-cause analysis from ITKSphinxExamples CI logs
@github-actions github-actions Bot added type:Compiler Compiler support or related warnings area:Registration Issues affecting the Registration module labels Jun 11, 2026
@hjmjohnson

Copy link
Copy Markdown
Member Author

Audited all of ITK for further instances of this issue: ITKReview was the only link-breaking case. A second, compile-time-only gap in Montage is fixed in the follow-up commit.

Audit method and findings

Scanned every non-ThirdParty module's src/ and include/ for includes of vendored third-party headers (itk_zlib.h, itk_png.h, itk_tiff.h, itk_jpeg.h, itk_expat.h, itk_hdf5.h, itk_minc2.h, itk_eigen.h) and cross-checked each hit against the declarations in that module's itk-module.cmake.

Module Header Declared? Severity
Nonunit/Review itk_zlib.h no Link failure in shared builds — fixed by commit 1
Registration/Montage itk_eigen.h (in public itkTileMontage.hxx) no Compile-time only — fixed by commit 2

All other modules declare their third-party dependencies correctly.

Rationale for the Montage commit: Eigen is header-only, so the missing declaration cannot cause a link failure; Montage currently compiles only because ITKEigen3's include path arrives transitively through ITKCommon's dependency chain. That is the same fragile transitive reliance that broke ITKReview, so it is declared explicitly (COMPILE_DEPENDS, since no symbols are linked) for completeness. Verified locally: full build with Module_Montage=ON succeeds.

Comment thread Modules/Registration/Montage/itk-module.cmake Outdated
itkTileMontage.hxx is an installed header that includes itk_eigen.h,
so consumers need the Eigen include path too; declare it under DEPENDS
like every other ITKEigen3 consumer.
@hjmjohnson hjmjohnson force-pushed the fix-itkreview-zlib-link branch from cbc3ea0 to 628a534 Compare June 11, 2026 13:34
@github-actions github-actions Bot added the type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots label Jun 11, 2026
@hjmjohnson hjmjohnson requested a review from blowekamp June 11, 2026 13:52
@hjmjohnson hjmjohnson marked this pull request as ready for review June 11, 2026 13:53
@greptile-apps

This comment was marked as resolved.

Comment thread Modules/Registration/Montage/itk-module.cmake

@blowekamp blowekamp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please just simplify/correct the added documentation for COMPILE_DEPENDS.

Comment thread Modules/Registration/Montage/itk-module.cmake
Comment thread CMake/ITKModuleMacros.cmake Outdated
@hjmjohnson hjmjohnson force-pushed the fix-itkreview-zlib-link branch from a780e38 to eeef992 Compare June 11, 2026 14:34
@blowekamp

Copy link
Copy Markdown
Member

LGTM. Thank for updating the sphinx examples too!

@hjmjohnson hjmjohnson merged commit fcfd098 into InsightSoftwareConsortium:main Jun 11, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Registration Issues affecting the Registration module type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants