ENH: Configure the ObjectFactory autoload symbol name (backport of #6787) - #6795
Conversation
23cbcb4 to
94bf629
Compare
|
In the current build/configuration of 3D Slicer this change I don't think is needed. See: Slicer/Slicer#8947 for the issue. They disabled adding the itkLoad function to the problematic ImageIO in 3DSlicer. There is still SimpleITK and ITKPython which can try to load libraries and check for the We should consider making a more robust solution to the underlying problem. Perhaps adding a ITK version or build number automatically to the name to more broadly prevent the issue with loading itkLoad. |
94bf629 to
328e713
Compare
ObjectFactoryBase looked up the autoload entry point by string literal while plugins defined it by name, so the two agreed only by convention. Both now derive from ITK_LOAD_FUNCTION_NAME, configured once into itkConfigure.h. The name is configurable because extern "C" names carry no C++ namespace, so a build that renames ITK's symbols cannot reach this entry point through the namespace. Setting it in the generated header keeps the loader and every plugin consistent by construction. No behavior change; the default is itkLoad. Co-Authored-By: Bradley Lowekamp <blowekamp@mail.nih.gov>
328e713 to
423c834
Compare
See larger discussion in 6787. This is an enabling set of changes that does not, by itself, provide benefits. It does, however, make future work easier and more self-contained. |
|
Backport of
to
release-5.4.ObjectFactoryBaselooked up the autoload entry point by string literal while plugins define it by name; both now derive fromITK_LOAD_FUNCTION_NAME, configured once intoitkConfigure.h. No behavior change — the default isitkLoad.Byte-identical change set. Every added and removed line matches #6787 exactly — verified by comparing the two patches with context stripped: 53 changed lines on each side, zero differences. Both are 9 files, +39/−14. The only variation in the raw diffs is two context lines at the top-level
CMakeLists.txtinsertion point, because the surrounding text differs between branches (details below).Merge after #6787. This should not land before its
maincounterpart.maincounterpartWhy
release-5.4matters: it is the branch 3D Slicer builds against. This is enabling work with no improvement today — it makes one of four names configurable so independently-distributed ITK builds can eventually coexist in one process, as PyPIitkwheels (py_itk), SimpleITK (simple_itk) and Slicer (slicer_itk). The namespace mechanism question is #6786 and is not decided here.Equivalence with #6787, and the one legitimate difference
Comparing only added/removed lines, ignoring context and hunk headers:
The two raw patches differ in exactly two context lines, in the top-level
CMakeLists.txthunk:mainset(ITK_LIBRARY_NAMESPACE "ITK")/endif()release-5.4configure_file(CMake/ITKConfigVersion.cmake.in ...)ITK_LIBRARY_NAMESPACEdoes not exist onrelease-5.4— it is amain-only feature — so the cache variable lands at the structurally equivalent slot (afterITKConfigVersion, beforeCMAKE_INSTALL_LIBDIR) rather than beside a sibling that is not there. Context lines describe the base, not the change.An earlier revision of this PR also carried an
ITKConfig.cmake.inexport; it was removed from #6787 on review (no CMake logic consumes the load symbol name) and removed here in the same pass, soITKConfig.cmake.inis untouched by both PRs.Reviewer note: which files a static build exercises
The plugin-facing code is only reachable with
BUILD_SHARED_LIBS=ON. ITK guards both factory plugin targets behindif(ITK_BUILD_SHARED_LIBS), so 3 of the 9 changed files are compiled only in a shared build (itkFactoryTestLib.h,itkFactoryTestLib.cxx,itkFileFreeImageIOFactory.cxx), and a static build does not registeritkIOPluginTest. Of the remainder, 2 are CMake and 4 compile in both configurations.Why
extern "C"needs its own mechanismextern "C"names carry no C++ namespace, so no namespace scheme reaches this entry point:VTK hit the same thing and ships
VTK_ABI_NAMESPACE_MANGLE(x)for it.Verification on this branch
macOS 15 arm64, Apple clang via the pixi
cxxenvironment, Release. All four phases run againstrelease-5.4, not inherited from #6787.1.
BUILD_SHARED_LIBS=ON, default — build clean,itkIOPluginTestPASS.plugin export: 000000000000717c T _itkLoad2.
BUILD_SHARED_LIBS=ON,-DITK_LOAD_FUNCTION_NAME=slicer_itkLoad— build clean,itkIOPluginTestPASS.3. Validation — a value that is not a valid C identifier fails configure:
4.
BUILD_SHARED_LIBS=OFF— build clean, 5/5ObjectFactory+itkIOPluginTesttests PASS.release-5.4predates.pre-commit-config.yaml, so thepre-commit run --all-filesgate does not apply on this branch.