Patch ITK's znzlib to mangle its symbols, as its niftiio already is - #231
Closed
gdevenyi wants to merge 1 commit into
Closed
Patch ITK's znzlib to mangle its symbols, as its niftiio already is#231gdevenyi wants to merge 1 commit into
gdevenyi wants to merge 1 commit into
Conversation
ITK renames its bundled niftiio symbols to itk_* (itk_nifti_mangle.h, included from ITK's nifti1.h) but never did the same for znzlib, so ITKznz exports the plain names: Xznzclose, znzdopen, znzeof, znzflush, znzgetc, znzgets, znzopen, znzprintf, znzputc, znzputs, znzread, znzrewind, znzseek, znztell, znzwrite. Every tool here that links ITK next to libminc -- c3d, elastix, ANTs, the EZminc tools -- therefore carries two definitions of each whenever the other side's znzlib is unmangled. Nothing fails at link time: ITK's copies are pulled into the executable and land in its dynamic symbol table, where they interpose over the other library's. Measured against a shared system nifti_clib: 8 znz names in .dynsym before, none after. Our own nifti is already renamed to minc_*, so today this is latent rather than active. It becomes active the moment anything else in the link brings its own znzlib -- which is exactly what USE_SYSTEM_NIFTI would do. Apply the missing half at the source, mirroring what ITK already does for niftiio: drop itk_znzlib_mangle.h beside znzlib.h and include it from there, at the same point nifti1.h includes its counterpart. The patch step is idempotent and fails loudly if the pinned ITK stops matching, rather than silently doing nothing. Submitted upstream as InsightSoftwareConsortium/ITK#6756. This is the local fallback for as long as that has not landed in the pinned SHA; drop both files and the PATCH_COMMAND once it has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019katBUnczdvUS7WQ4RpVoa
Contributor
Author
|
Superseded — InsightSoftwareConsortium/ITK#6756 merged to #230 bumps the pin instead: Verified against the new tarball:
The upshot is what this PR was for: Deleting the branch; the approach is preserved here if a future pin ever needs to move backwards. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
ITK renames its bundled niftiio symbols to
itk_*—itk_nifti_mangle.h, included from ITK'snifti1.h, 103 names — but never did the same for znzlib.ITKznzexports the plain names:So every tool here that links ITK next to libminc —
c3d,elastix, ANTs, the EZminc tools — carries two definitions of each as soon as the other side's znzlib is unmangled. Nothing fails at link time: ITK's copies are pulled into the executable and land in its dynamic symbol table, where they interpose over the other library's.Measured with ITK
release-4.14's ownznzlib.clinked next to a shared systemnifti_clib:ITKznz.dynsymIs it biting today?
No — and that is worth being precise about. Our own NIfTI is already renamed to
minc_*, so nothing currently in the link exports the plain znz names. This is latent.It becomes active the moment something else brings its own znzlib, which is exactly what
USE_SYSTEM_NIFTIdoes — see #230, which currently has to refuse that combination outright. With this patch (or the upstream one) that restriction can be lifted, which is the point: it is what makes a system NIfTI usable alongside the ITK tools, and therefore what a distribution package would need.Even without
USE_SYSTEM_NIFTIthe two have already drifted, so the collision is not merely theoretical: ITK'sznzseek/znztelltake and returnlongwhilenifti_clib3.x usesznz_off_t, which differ on ILP32 with_FILE_OFFSET_BITS=64(measured: 4 bytes against 8); andsizeof(struct znzptr)differs between builds that disagree onHAVE_ZLIB.The change
Mirrors what ITK already does for niftiio, and what we already do for our own NIfTI in
libminc/cmake-modules/PatchNiftiMangle.cmake:cmake-modules/itk_znzlib_mangle.h— 15#defines, generated from the symbols ITK'sznzlib.cactually exports.cmake-modules/PatchITKZnzMangle.cmake— drops that header beside ITK'sznzlib.hand includes it from there, at the same pointnifti1.hincludesitk_nifti_mangle.h.PATCH_COMMANDon theITKv4ExternalProject_Add, which had none.znzcloseis a macro overXznzclose, which is mangled, so it expands to the mangled name at the call site. ITK's only consumer,itkNiftiImageIO.cxx, reaches znz through<nifti1_io.h>→<znzlib.h>, so it picks the mangling up with no further changes.Verification
Against a tree laid out like the pinned ITK source:
already patchedon a second run (idempotent —PATCH_COMMANDre-runs on re-configure);znzlib.c: 15 unmangled exports become 0, 15itk_*exports appear;znzlib.hever stops matching, the step exits 1 with an explanation rather than silently doing nothing — verified, as is the missing-source case.Not verified here: a full ITK build, which is hours. The patch touches one vendored third-party header and adds one file next to it, and ITK's own consumer path goes through that header.
🤖 Generated with Claude Code
https://claude.ai/code/session_019katBUnczdvUS7WQ4RpVoa