Skip to content

COMP: Fix DCMTK build-tree export for external ITK consumers #6543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hjmjohnson wants to merge 3 commits into
base: main
Choose a base branch
from
+35 −20
Open

COMP: Fix DCMTK build-tree export for external ITK consumers #6543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d2d2d8
COMP: Require DCMTK 3.7.0 or later for ITK_USE_SYSTEM_DCMTK
hjmjohnson Jul 2, 2026
f80a0e3
COMP: Link namespaced DCMTK targets in the DCMTK IO modules
hjmjohnson Jul 2, 2026
bcc43b3
COMP: Repair DCMTK build-tree export references for external consumers
hjmjohnson Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions Modules/IO/DCMTK/src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ target_include_directories(ITKIODCMTK PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
target_link_libraries(
ITKIODCMTK
PRIVATE
dcmdata
dcmimgle
dcmimage
dcmjpeg
dcmjpls
oflog
DCMTK::dcmdata
DCMTK::dcmimgle
DCMTK::dcmimage
DCMTK::dcmjpeg
DCMTK::dcmjpls
DCMTK::oflog
)

if(DEFINED DCMTK_HAVE_CONFIG_H_OPTIONAL AND NOT DCMTK_HAVE_CONFIG_H_OPTIONAL)
Expand Down
2 changes: 1 addition & 1 deletion Modules/IO/IOTransformDCMTK/src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ list(REMOVE_ITEM ITK_MODULE_IOTransformDCMTK_PRIVATE_DEPENDS ITKDCMTK)

itk_module_add_library(IOTransformDCMTK ${IOTransformDCMTK_SRCS})

target_link_libraries(IOTransformDCMTK PRIVATE dcmdata)
target_link_libraries(IOTransformDCMTK PRIVATE DCMTK::dcmdata)

# itkDCMTKTransformIO.hxx is a private implementation header in this directory.
target_include_directories(IOTransformDCMTK PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
38 changes: 26 additions & 12 deletions Modules/ThirdParty/DCMTK/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ else(ITK_USE_SYSTEM_DCMTK)

# DCMTK build options; compiler and language-standard settings inherit from ITK's scope.
set(BUILD_APPS OFF)
# Per-library targets only: a single-library build renames targets with a
# suffix the DCMTK IO modules' DCMTK::<lib> link names cannot match.
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "")

@greptile-apps greptile-apps Bot Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Cache value not forced
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "") does not replace an existing cache entry, so a configure that already has BUILD_SINGLE_SHARED_LIBRARY=ON keeps DCMTK in single-library mode. The code below then looks for unsuffixed per-library targets like dcmdata, finds none, and the IO modules now link DCMTK::dcmdata, which is never created.

Suggested change
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "")
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "" FORCE)

@blowekamp blowekamp Jul 3, 2026

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.

Just do set(BUILD_SINGLE_SHARED_LIBRARY OFF) it should not be a cache variable.

@greptile-apps greptile-apps Bot Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed. A plain set(BUILD_SINGLE_SHARED_LIBRARY OFF) in the parent scope will shadow any cache entry of that name when CMake evaluates it in the FetchContent subdirectory, which is the correct and idiomatic way to force options on FetchContent dependencies. The CACHE INTERNAL form doesn't force-overwrite an existing cache entry (that requires FORCE), making it unreliable.

Suggested change
set(BUILD_SINGLE_SHARED_LIBRARY OFF CACHE INTERNAL "")
set(BUILD_SINGLE_SHARED_LIBRARY OFF)

set(DCMTK_ENABLE_CXX11 ON)
set(DCMTK_FORCE_FPIC_ON_UNIX ON)
set(DCMTK_DEFAULT_DICT builtin)
Expand Down Expand Up @@ -148,32 +151,43 @@ else(ITK_USE_SYSTEM_DCMTK)
# namespaced name in ITKTargets.cmake, matching what find_package(DCMTK)
# yields for downstream consumers.
foreach(_dcmtk_lib IN LISTS _ITKDCMTK_LIB_NAMES)
set(_dcmtk_target "${_dcmtk_lib}${DCMTK_LIBRARY_SUFFIX}")
if(TARGET "${_dcmtk_target}")
if(NOT TARGET "DCMTK::${_dcmtk_target}")
add_library("DCMTK::${_dcmtk_target}" INTERFACE IMPORTED GLOBAL)
target_link_libraries(
"DCMTK::${_dcmtk_target}"
INTERFACE
"${_dcmtk_target}"
)
if(TARGET "${_dcmtk_lib}")
if(NOT TARGET "DCMTK::${_dcmtk_lib}")
add_library("DCMTK::${_dcmtk_lib}" INTERFACE IMPORTED GLOBAL)
target_link_libraries("DCMTK::${_dcmtk_lib}" INTERFACE "${_dcmtk_lib}")
endif()
list(APPEND ITKDCMTK_LIBRARIES "DCMTK::${_dcmtk_target}")
list(APPEND ITKDCMTK_LIBRARIES "DCMTK::${_dcmtk_lib}")
endif()
endforeach()
unset(_dcmtk_lib)
unset(_dcmtk_target)

# Build-tree consumers: locate DCMTKConfig.cmake in ITK's build root.
# Build-tree consumers: locate DCMTKConfig.cmake in ITK's build root. The
# build-tree export was written by export(TARGETS NAMESPACE), which
# mis-prefixes ITK's codec targets as DCMTK::ITK::*Module; rewrite those
# link-interface entries to their real ITK::* names after import.
list(JOIN _ITKDCMTK_LIB_NAMES " " _itkdcmtk_lib_names_spaced)
set(
ITKDCMTK_EXPORT_CODE_BUILD
"
if(NOT ITK_BINARY_DIR)
set(DCMTK_DIR \"${CMAKE_BINARY_DIR}\")
find_package(DCMTK REQUIRED NO_MODULE)
foreach(_itkdcmtk_lib ${_itkdcmtk_lib_names_spaced})
if(TARGET DCMTK::\${_itkdcmtk_lib})
get_target_property(_itkdcmtk_ll DCMTK::\${_itkdcmtk_lib} INTERFACE_LINK_LIBRARIES)
if(_itkdcmtk_ll MATCHES \"DCMTK::ITK::\")
string(REPLACE \"DCMTK::ITK::\" \"ITK::\" _itkdcmtk_ll \"\${_itkdcmtk_ll}\")
set_target_properties(DCMTK::\${_itkdcmtk_lib} PROPERTIES
INTERFACE_LINK_LIBRARIES \"\${_itkdcmtk_ll}\")
endif()
endif()
endforeach()
unset(_itkdcmtk_lib)
unset(_itkdcmtk_ll)
endif()
"
)
unset(_itkdcmtk_lib_names_spaced)
# Install-tree consumers: locate DCMTKConfig.cmake under ITK_INSTALL_PREFIX.
set(
ITKDCMTK_EXPORT_CODE_INSTALL
Expand Down
3 changes: 2 additions & 1 deletion Modules/ThirdParty/DCMTK/itk-module-init.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ option(ITK_USE_SYSTEM_DCMTK "Use an outside build of DCMTK." OFF)
if(ITK_USE_SYSTEM_DCMTK)
# Use local FindDCMTK.cmake.
list(INSERT CMAKE_MODULE_PATH 0 "${CMAKE_CURRENT_LIST_DIR}/CMake")
find_package(DCMTK REQUIRED NO_MODULE)
# 3.7.0 matches the vendored DCMTK and guarantees DCMTK::-namespaced targets.
find_package(DCMTK 3.7.0 REQUIRED NO_MODULE)
else()
# Change default from OFF to ON for portability.
option(
Expand Down
Loading