Skip to content

COMP: Use single export() call to fix namespace corruption - #154

Open
blowekamp wants to merge 1 commit into
DCMTK:masterfrom
blowekamp:cmake_single_export
Open

COMP: Use single export() call to fix namespace corruption#154
blowekamp wants to merge 1 commit into
DCMTK:masterfrom
blowekamp:cmake_single_export

Conversation

@blowekamp

Copy link
Copy Markdown

Replacing three APPEND export() calls with one non-APPEND call fixes a namespace corruption bug where CMake prepends the DCMTK:: namespace to dependency targets (e.g. ITK codec targets) that belong to other export sets.

CMake's HandleMissingTarget only searches other registered export sets when not in append mode. In append mode it blindly prepends the current namespace, producing DCMTK::ITK::ITKZLIBModule instead of the correct ITK::ITKZLIBModule in DCMTKTargets.cmake.

The file(WRITE)+3xAPPEND pattern was used only to accumulate targets across multiple calls; combining them into a single export() call is equivalent and avoids the append-mode shortcut.

Replacing three APPEND export() calls with one non-APPEND call fixes
a namespace corruption bug where CMake prepends the DCMTK:: namespace
to dependency targets (e.g. ITK codec targets) that belong to other
export sets.

CMake's HandleMissingTarget only searches other registered export sets
when not in append mode. In append mode it blindly prepends the current
namespace, producing DCMTK::ITK::ITKZLIBModule instead of the correct
ITK::ITKZLIBModule in DCMTKTargets.cmake.

The file(WRITE)+3xAPPEND pattern was used only to accumulate targets
across multiple calls; combining them into a single export() call is
equivalent and avoids the append-mode shortcut.

@hjmjohnson hjmjohnson left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks @blowekamp Reviewed, built local, and confirmed this resolved the linking issues downstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants