ENH: Remove unused ctkBinaryFileDescriptor class - #1454
Merged
Conversation
The ctkBinaryFileDescriptor class wrapped the GNU BFD library (libbfd from binutils) to resolve global symbols in an executable without running it, on unix-like systems only. It was ported to CTK in 2010 from Slicer3's ModuleDescriptionParser library (BinaryFileDescriptor, added in 2007), where it was used by ModuleFactory::ScanForCommandLineModulesByPeeking() to speed up CLI module discovery at application startup: instead of launching each CLI executable with the "--xml" argument, the factory "peeked" into the binary to read the XMLModuleDescription global symbol (and the module logo symbols) directly. The class was added to CTK in anticipation of the Qt-based CLI module machinery being migrated from Slicer, but that "peeking" approach was abandoned (Slicer's qCTKSymbolResolver experiment was retired to the Attic in June 2010, and Slicer 4 CLI factories run executables with "--xml" instead), so nothing in CTK, Slicer, or MITK ever used it. The feature was disabled by default anyway (the WITH_BFD_SHARED and WITH_BFD_STATIC library options were OFF), but it is now removed entirely to avoid licensing complications: libbfd is GPL-licensed, which conflicts with CTK's permissive Apache 2.0 license when linked in.
lassoan
force-pushed
the
remove-ctkbinaryfiledescriptor
branch
from
August 30, 2026 02:33
2bacdb4 to
8477243
Compare
There was a problem hiding this comment.
Pull request overview
Removes the unused ctkBinaryFileDescriptor (libbfd-based) feature and its related CMake detection/options and tests from CTKCore, primarily to avoid GPL licensing complications and unblock Debian packaging.
Changes:
- Removed
ctkBinaryFileDescriptorimplementation and public header fromLibs/Core. - Removed BFD-related build options / detection (including
ctkMacroBFDCheck.cmakeand theTestBFDtry-compile project). - Removed BFD-related tests and their CMake wiring in
Libs/Core/Testing/Cpp.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Libs/Core/Testing/Cpp/ctkBinaryFileDescriptorTestHelper.cpp | Deleted BFD test helper executable source. |
| Libs/Core/Testing/Cpp/ctkBinaryFileDescriptorTest1.cpp | Deleted BFD unit test source. |
| Libs/Core/Testing/Cpp/CMakeLists.txt | Removed BFD-conditional test sources, helper target, and test registration. |
| Libs/Core/target_libraries.cmake | Removed BFD_LIBRARIES from CTKCore link list (now empty, consistent with other libs). |
| Libs/Core/ctkBinaryFileDescriptor.h | Deleted unused public API header. |
| Libs/Core/ctkBinaryFileDescriptor.cpp | Deleted unused libbfd-backed implementation. |
| Libs/Core/ctk_library_options.cmake | Deleted Core library options file that only defined BFD options. |
| Libs/Core/CMakeLists.txt | Removed inclusion of the BFD check macro and conditional compilation of BFD sources. |
| Libs/Core/CMake/TestBFD/TestBFD.cpp | Deleted libbfd compile/link probe source. |
| Libs/Core/CMake/TestBFD/CMakeLists.txt | Deleted libbfd try-compile project CMake. |
| Libs/Core/CMake/ctkMacroBFDCheck.cmake | Deleted BFD availability/options macro and related configuration logic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jcfr
approved these changes
Aug 31, 2026
|
Thanks a lot!! @lassoan ideally a new tag release would be welcome |
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.
Removing unused class that prevents Debian packaging of CTK (https://dfsg-new-queue.debian.org/reviews/ctk).
The ctkBinaryFileDescriptor class wrapped the GNU BFD library (libbfd from binutils) to resolve global symbols in an executable without running it, on unix-like systems only.
It was ported to CTK in 2010 from Slicer3's ModuleDescriptionParser library (BinaryFileDescriptor, added in 2007), where it was used by ModuleFactory::ScanForCommandLineModulesByPeeking() to speed up CLI module discovery at application startup: instead of launching each CLI executable with the "--xml" argument, the factory "peeked" into the binary to read the XMLModuleDescription global symbol (and the module logo symbols) directly.
The class was added to CTK in anticipation of the Qt-based CLI module machinery being migrated from Slicer, but that "peeking" approach was abandoned (Slicer's qCTKSymbolResolver experiment was retired to the Attic in June 2010, and Slicer 4 CLI factories run executables with "--xml" instead), so nothing in CTK, Slicer, or MITK ever used it.
The feature was disabled by default anyway (the WITH_BFD_SHARED and WITH_BFD_STATIC library options were OFF), but it is now removed entirely to avoid licensing complications: libbfd is GPL-licensed, which conflicts with CTK's permissive Apache 2.0 license when linked in.