Skip to content

WIP: ENH: Ingest 10 beta modules inline under Modules/Beta/ (stacked on #6085)#6086

Closed
hjmjohnson wants to merge 2024 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:inline-beta-modules-ingest
Closed

WIP: ENH: Ingest 10 beta modules inline under Modules/Beta/ (stacked on #6085)#6086
hjmjohnson wants to merge 2024 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:inline-beta-modules-ingest

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Stacked on #6085. Grafts 10 remote modules inline under
Modules/Beta/<Name>/, preserving upstream git history and blame. 60
first-parent commits (~2k total including merged-in upstream histories).

Modules ingested (in order): Cuberille, Montage, FastBilateral,
GrowCut, MeshToPolyData, BoneEnhancement, AnalyzeObjectLabelMap,
SimpleITKFilters, LabelErodeDilate, BoneMorphometry.

Per-module commit pattern (6 commits × 10 modules):

  1. COMP: Bump <M> remote to upstream tip <sha>
  2. ENH: Ingest ITK<M> as Modules/Beta/<M> — merge commit with
    --allow-unrelated-histories --no-ff, byte-identical to upstream tip
  3. STYLE: Remove non-ITK artifacts from ingested <M> — mechanical
    cleanup (.github/, .clang-format, pyproject.toml, etc.)
  4. COMP: Fix pre-commit hook failures for <M> — scoped pre-commit
    autofixes (EOF newlines, trailing whitespace, gersemi, shebang+x)
  5. COMP: Remove <M> remote fetch; now inline at Modules/Beta/<M>
    deletes Modules/Remote/<M>.remote.cmake
  6. DOC: Record <M> upstream provenance in beta manifest — writes
    Modules/Beta/<M>.beta.cmake with UPSTREAM_URL/SHA/INGEST_DATE

Local verification: 118/118 beta-module tests pass on a full
reconfigure+build.

Why the graft-then-clean structure

Reviewer concern on #6061 was that consolidating modules erases author
attribution. This PR takes the opposite approach: each ENH: commit is
the raw git merge --allow-unrelated-histories --no-ff of the upstream
tip, producing a merge whose tree is byte-identical to what
itk_fetch_module() would have checked out. Every subsequent change
(STYLE/COMP/DOC) is a mechanical, auditable layer on top.

Blame preservation verified on Cuberille: 17 distinct authors surface
via git blame, walking back to Matt McCormick's 2015 commits in
ITKCuberille.

The pre-commit-fix commit (new step in procedure)

Ingested upstream repos do not satisfy ITK's pre-commit hooks as-is
(EOF newlines, trailing whitespace, gersemi, shebang-executable bits,
etc.). Rather than mangle the ingest merge or the STYLE cleanup, each
module gets a dedicated COMP: Fix pre-commit hook failures for <M>
commit containing only the autofix hunks for that module's tree.

Reproduce per module:

pixi run -e pre-commit pre-commit run \
  --files $(git ls-files Modules/Beta/<M>/)
# Iterate until fixed-point. For shebang-executable failures:
git update-index --chmod=+x <path>

This step is documented in INGESTION_STRATEGY.md (support worktree).

How to verify blame / history preservation
# Commit count under the module roughly matches upstream log:
git log --oneline -- Modules/Beta/Cuberille/ | wc -l

# Blame walks back to original authors, not the ingester:
git blame Modules/Beta/Cuberille/include/itkCuberilleImageToMeshFilter.h

# All unique authors ever to touch the module:
git log --format='%an' -- Modules/Beta/Cuberille/ | sort -u | wc -l

# No itk_fetch_module() entries remain for ingested modules:
git grep -n itk_fetch_module Modules/Remote/
Future work (post-merge)
  • Tier 3 ingests: MorphologicalContourInterpolation, RLEImage, and
    any other pure-C++ remote modules. Same recipe.
  • External-dependency modules deferred: CudaCommon (CUDA),
    VkFFTBackend (Vulkan), IOTransformDCMTK (DCMTK), IOOpenSlide
    (OpenSlide), SCIFIO (Bio-Formats). These need build-time optional
    dependency handling that this PR doesn't address.
  • Post-hoc cull candidates flagged in INGEST_LOG.md:
    SimpleITKFilters (one adapter file), AnalyzeObjectLabelMap (legacy
    format), BoneMorphometry (large doc/ tex artifacts),
    BoneEnhancement (overlaps with existing enhancement filters).
  • Split option: if reviewers prefer, this PR can be split into 10
    per-module PRs. The commit history is already clean per-module; the
    bundled PR is a reviewer-ergonomics choice, not a structural one.
Verification commands (run locally)
# Configure all beta modules ON:
cmake -B build -S . \
  -DModule_Cuberille=ON -DModule_Montage=ON -DModule_FastBilateral=ON \
  -DModule_GrowCut=ON -DModule_MeshToPolyData=ON -DModule_BoneEnhancement=ON \
  -DModule_AnalyzeObjectLabelMap=ON -DModule_SimpleITKFilters=ON \
  -DModule_LabelErodeDilate=ON -DModule_BoneMorphometry=ON

cmake --build build -j$(nproc)

ctest -j4 --test-dir build \
  -R "Cuberille|Montage|FastBilateral|GrowCut|MeshToPolyData|BoneEnhancement|AnalyzeObjectLabelMap|SimpleITKFilters|LabelErodeDilate|BoneMorphometry"
# Expect: 118/118 passed.

thewtex and others added 30 commits September 7, 2024 11:51
…sitionToPyprojectToml

ENH: Transition project configuration to `pyproject.toml`
…sitionToPyprojectToml

ENH: Transition project configuration to `pyproject.toml`
…s_autofix_ci-itk-5.4.0

ENH: Upgrade CI for ITK 5.4.0
chore: Convert from setup.py to pyproject.toml
Simplify and utilize best practices.
Do not try to sync version with ITK.

Remove languages -- we also need C for KWStyle.

Addresses:

FAILED: KWStyle-prefix/src/KWStyle-stamp/KWStyle-configure /home/matt/bin/ITKAnalyzeObjectMap-GCC-Release/KWStyle-prefix/src/KWStyle-stamp/KWStyle-configure
cd /home/matt/bin/ITKAnalyzeObjectMap-GCC-Release/KWStyle-build && /usr/bin/cmake -DCMAKE_MAKE_PROGRAM:FILEPATH=/usr/bin/ninja -DCMAKE_CXX_COMPILER:STRING=/usr/bin/c++ -DCMAKE_CXX_COMPILER_LAUNCHER:FILEPATH= -DCMAKE_C_COMPILER:STRING= -DCMAKE_C_COMPILER_LAUNCHER:FILEPATH= "-DCMAKE_CXX_FLAGS:STRING=  -fno-sized-deallocation -msse2" "-DCMAKE_C_FLAGS:STRING= " -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX:PATH=/usr/local -DBUILD_TESTING:BOOL=OFF -GNinja /home/matt/bin/ITKAnalyzeObjectMap-GCC-Release/KWStyle && /usr/bin/cmake -E touch /home/matt/bin/ITKAnalyzeObjectMap-GCC-Release/KWStyle-prefix/src/KWStyle-stamp/KWStyle-configure
-- The C compiler identification is unknown
CMake Error at CMakeLists.txt:28 (project):
  No CMAKE_C_COMPILER could be found.

  Tell CMake where to find the compiler by setting either the environment
  variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
  the compiler, or to the compiler name if it is in the PATH.

-- Configuring incomplete, errors occurred!
The name needs to be consistent with the module name.
When preparing for the future with ITK by setting
ITK_FUTURE_LEGACY_REMOVE:BOOL=ON
ITK_LEGACY_REMOVEBOOL=ON

The future preferred macro should be used
│ -  itkTypeMacro
│ +  itkOverrideGetNameOfClassMacro
…onsortium/fix-legacy-remove

COMP: Use modern macro for name of class
When preparing for the future with ITK by setting
ITK_FUTURE_LEGACY_REMOVE:BOOL=ON
ITK_LEGACY_REMOVEBOOL=ON

The future preferred macro should be used
│ -  itkTypeMacro
│ +  itkOverrideGetNameOfClassMacro
…acro

Added two new macro's, intended to replace the old 'itkTypeMacro' and
'itkTypeMacroNoParent'.

The main aim is to be clearer about what those macro's do: add a virtual
'GetNameOfClass()' member function and override it. Unlike 'itkTypeMacro',
'itkOverrideGetNameOfClassMacro' does not have a 'superclass' parameter, as it
was not used anyway.

Note that originally 'itkTypeMacro' did not use its 'superclass' parameter
either, looking at commit 699b66c, Will
Schroeder, June 27, 2001:
https://github.com/InsightSoftwareConsortium/ITK/blob/699b66cb04d410e555656828e8892107add38ccb/Code/Common/itkMacro.h#L331-L337
…acro

Added two new macro's, intended to replace the old 'itkTypeMacro' and
'itkTypeMacroNoParent'.

The main aim is to be clearer about what those macro's do: add a virtual
'GetNameOfClass()' member function and override it. Unlike 'itkTypeMacro',
'itkOverrideGetNameOfClassMacro' does not have a 'superclass' parameter, as it
was not used anyway.

Note that originally 'itkTypeMacro' did not use its 'superclass' parameter
either, looking at commit 699b66c, Will
Schroeder, June 27, 2001:
https://github.com/InsightSoftwareConsortium/ITK/blob/699b66cb04d410e555656828e8892107add38ccb/Code/Common/itkMacro.h#L331-L337
Use static constexpr directly now that C++11 conformance
is required by all compilers.

:%s/itkStaticConstMacro *( *\([^,]*\),[ \_s]*\([^,]*\),\_s*\([^)]*\)) */static constexpr \2 \1 = \3/ge

'itkStaticConstMacro(name, type, value)' became unconditionally
identical to 'static constexpr type name = value' with ITK commit
aec9519 "ENH: Update compiler macros (InsightSoftwareConsortium#810)",
maekclena, 7 May 2019.

'itkGetStaticConstMacro(name)' became unconditionally identical to
'(Self::name)' with ITK commit 84e490b
"Removing some outdated compiler conditionals", Hans Johnson, 31 July
2010.

Most 'itkStaticConstMacro' calls were removed by ITK commit 5c14741
For the sake of code readability, a new 'CoordinateType' alias is added for
each nested 'CoordRepType' alias. The old 'CoordRepType' aliases will still be
available with ITK 6.0, but it is recommended to use 'CoordinateType' instead.
The 'CoordRepType' aliases will be removed when 'ITK_FUTURE_LEGACY_REMOVE' is
enabled. Similarly, 'InputCoordinateType', 'OutputCoordinateType', and
'ImagePointCoordinateType' replace 'InputCoordRepType', 'OutputCoordRepType',
and 'ImagePointCoordRepType', respectively.
For the sake of code readability, a new 'CoordinateType' alias is added for
each nested 'CoordRepType' alias. The old 'CoordRepType' aliases will still be
available with ITK 6.0, but it is recommended to use 'CoordinateType' instead.
The 'CoordRepType' aliases will be removed when 'ITK_FUTURE_LEGACY_REMOVE' is
enabled. Similarly, 'InputCoordinateType', 'OutputCoordinateType', and
'ImagePointCoordinateType' replace 'InputCoordRepType', 'OutputCoordRepType',
and 'ImagePointCoordRepType', respectively.
…acro

Added two new macro's, intended to replace the old 'itkTypeMacro' and
'itkTypeMacroNoParent'.

The main aim is to be clearer about what those macro's do: add a virtual
'GetNameOfClass()' member function and override it. Unlike 'itkTypeMacro',
'itkOverrideGetNameOfClassMacro' does not have a 'superclass' parameter, as it
was not used anyway.

Note that originally 'itkTypeMacro' did not use its 'superclass' parameter
either, looking at commit 699b66c, Will
Schroeder, June 27, 2001:
https://github.com/InsightSoftwareConsortium/ITK/blob/699b66cb04d410e555656828e8892107add38ccb/Code/Common/itkMacro.h#L331-L337
Use static constexpr directly now that C++11 conformance
is required by all compilers.

:%s/itkStaticConstMacro *( *\([^,]*\),[ \_s]*\([^,]*\),\_s*\([^)]*\)) */static constexpr \2 \1 = \3/ge

'itkStaticConstMacro(name, type, value)' became unconditionally
identical to 'static constexpr type name = value' with ITK commit
aec9519 "ENH: Update compiler macros (InsightSoftwareConsortium#810)",
maekclena, 7 May 2019.

'itkGetStaticConstMacro(name)' became unconditionally identical to
'(Self::name)' with ITK commit 84e490b
"Removing some outdated compiler conditionals", Hans Johnson, 31 July
2010.

Most 'itkStaticConstMacro' calls were removed by ITK commit 5c14741
…elMap

Grafts the ITKAnalyzeObjectLabelMap remote module into the ITK source tree at
Modules/Beta/AnalyzeObjectLabelMap/, replacing the configure-time fetch previously
declared in Modules/Remote/AnalyzeObjectLabelMap.remote.cmake.

Upstream:          https://github.com/InsightSoftwareConsortium/ITKAnalyzeObjectLabelMap
Upstream SHA:      7b3eb672e3dbd39b47926498a73a9ca0ef5fb3ca
Ingest date:       2026-04-18
Original license:  Apache-2.0
Compliance level:  3 (beta)

Full upstream commit history (101 commits, 11 authors) is
preserved via git merge --allow-unrelated-histories; git log --follow
and git blame walk across the merge boundary to the original authors.

Paths in ingested history are rewritten under Modules/Beta/AnalyzeObjectLabelMap/
via git-filter-repo --to-subdirectory-filter; trees are otherwise
byte-identical to upstream. Non-ITK artifacts shipped by upstream
are removed in a follow-up STYLE: commit.
Removes standalone-repo scaffolding that has no role in the ITK monorepo:

  .clang-format
  .gitattributes
  .github
  CTestConfig.cmake
  pyproject.toml
  LICENSE
No edits to ingested source files; structural cleanup only.
…s/Beta/AnalyzeObjectLabelMap

With AnalyzeObjectLabelMap grafted inline under Modules/Beta/AnalyzeObjectLabelMap/, the configure-time
remote fetch declaration is redundant. The module is discovered via the
normal itk-module.cmake DAG scan in CMake/ITKModuleEnablement.cmake.
Passive metadata consumed by tooling; itk_beta_module_manifest() is a
no-op defined in Modules/Beta/CMakeLists.txt.
Grafts the ITKSimpleITKFilters remote module into the ITK source tree at
Modules/Beta/SimpleITKFilters/, replacing the configure-time fetch previously
declared in Modules/Remote/SimpleITKFilters.remote.cmake.

Upstream:          https://github.com/InsightSoftwareConsortium/ITKSimpleITKFilters
Upstream SHA:      ae7a7d84be75a20494d8bc383061aa064a488a6d
Ingest date:       2026-04-18
Original license:  Apache-2.0
Compliance level:  3 (beta)

Full upstream commit history (117 commits, 9 authors) is
preserved via git merge --allow-unrelated-histories; git log --follow
and git blame walk across the merge boundary to the original authors.

Paths in ingested history are rewritten under Modules/Beta/SimpleITKFilters/
via git-filter-repo --to-subdirectory-filter; trees are otherwise
byte-identical to upstream. Non-ITK artifacts shipped by upstream
are removed in a follow-up STYLE: commit.
Removes standalone-repo scaffolding that has no role in the ITK monorepo:

  .clang-format
  .github
  CTestConfig.cmake
  pyproject.toml
  LICENSE
No edits to ingested source files; structural cleanup only.
…a/SimpleITKFilters

With SimpleITKFilters grafted inline under Modules/Beta/SimpleITKFilters/, the configure-time
remote fetch declaration is redundant. The module is discovered via the
normal itk-module.cmake DAG scan in CMake/ITKModuleEnablement.cmake.
Passive metadata consumed by tooling; itk_beta_module_manifest() is a
no-op defined in Modules/Beta/CMakeLists.txt.
Grafts the ITKLabelErodeDilate remote module into the ITK source tree at
Modules/Beta/LabelErodeDilate/, replacing the configure-time fetch previously
declared in Modules/Remote/LabelErodeDilate.remote.cmake.

Upstream:          https://github.com/InsightSoftwareConsortium/ITKLabelErodeDilate
Upstream SHA:      a9aeab73ae30d9143e4538812789e406de711bf5
Ingest date:       2026-04-18
Original license:  Apache-2.0
Compliance level:  3 (beta)

Full upstream commit history (201 commits, 15 authors) is
preserved via git merge --allow-unrelated-histories; git log --follow
and git blame walk across the merge boundary to the original authors.

Paths in ingested history are rewritten under Modules/Beta/LabelErodeDilate/
via git-filter-repo --to-subdirectory-filter; trees are otherwise
byte-identical to upstream. Non-ITK artifacts shipped by upstream
are removed in a follow-up STYLE: commit.
Removes standalone-repo scaffolding that has no role in the ITK monorepo:

  .clang-format
  .gitattributes
  .github
  CTestConfig.cmake
  pyproject.toml
  LICENSE
No edits to ingested source files; structural cleanup only.
…a/LabelErodeDilate

With LabelErodeDilate grafted inline under Modules/Beta/LabelErodeDilate/, the configure-time
remote fetch declaration is redundant. The module is discovered via the
normal itk-module.cmake DAG scan in CMake/ITKModuleEnablement.cmake.
Passive metadata consumed by tooling; itk_beta_module_manifest() is a
no-op defined in Modules/Beta/CMakeLists.txt.
Grafts the ITKBoneMorphometry remote module into the ITK source tree at
Modules/Beta/BoneMorphometry/, replacing the configure-time fetch previously
declared in Modules/Remote/BoneMorphometry.remote.cmake.

Upstream:          https://github.com/InsightSoftwareConsortium/ITKBoneMorphometry
Upstream SHA:      a81057b078cf4b1b7ee8dcbefe7130894b45c8a7
Ingest date:       2026-04-18
Original license:  Apache-2.0
Compliance level:  3 (beta)

Full upstream commit history (149 commits, 13 authors) is
preserved via git merge --allow-unrelated-histories; git log --follow
and git blame walk across the merge boundary to the original authors.

Paths in ingested history are rewritten under Modules/Beta/BoneMorphometry/
via git-filter-repo --to-subdirectory-filter; trees are otherwise
byte-identical to upstream. Non-ITK artifacts shipped by upstream
are removed in a follow-up STYLE: commit.
Removes standalone-repo scaffolding that has no role in the ITK monorepo:

  .clang-format
  .gitattributes
  .github
  CTestConfig.cmake
  pyproject.toml
  LICENSE
No edits to ingested source files; structural cleanup only.
…/BoneMorphometry

With BoneMorphometry grafted inline under Modules/Beta/BoneMorphometry/, the configure-time
remote fetch declaration is redundant. The module is discovered via the
normal itk-module.cmake DAG scan in CMake/ITKModuleEnablement.cmake.
Passive metadata consumed by tooling; itk_beta_module_manifest() is a
no-op defined in Modules/Beta/CMakeLists.txt.
Apply pre-commit autofixes scoped to Modules/Beta/Cuberille/: trailing
whitespace, end-of-file newlines, and shebang executable bits.
These are mechanical hook autofixes only; no semantic changes.

Run locally with:
  pixi run -e pre-commit pre-commit run --files $(git ls-files Modules/Beta/Cuberille/)
Apply pre-commit autofixes scoped to Modules/Beta/Montage/: trailing
whitespace, end-of-file newlines, and shebang executable bits where
applicable. These are mechanical hook autofixes only; no semantic
changes to the ingested module.

Reproduce locally with:
  pixi run -e pre-commit pre-commit run \
    --files $(git ls-files Modules/Beta/Montage/)
Apply pre-commit autofixes scoped to Modules/Beta/FastBilateral/: trailing
whitespace, end-of-file newlines, and shebang executable bits where
applicable. These are mechanical hook autofixes only; no semantic
changes to the ingested module.

Reproduce locally with:
  pixi run -e pre-commit pre-commit run \
    --files $(git ls-files Modules/Beta/FastBilateral/)
Apply pre-commit autofixes scoped to Modules/Beta/GrowCut/: trailing
whitespace, end-of-file newlines, and shebang executable bits where
applicable. These are mechanical hook autofixes only; no semantic
changes to the ingested module.

Reproduce locally with:
  pixi run -e pre-commit pre-commit run \
    --files $(git ls-files Modules/Beta/GrowCut/)
Apply pre-commit autofixes scoped to Modules/Beta/MeshToPolyData/: trailing
whitespace, end-of-file newlines, and shebang executable bits where
applicable. These are mechanical hook autofixes only; no semantic
changes to the ingested module.

Reproduce locally with:
  pixi run -e pre-commit pre-commit run \
    --files $(git ls-files Modules/Beta/MeshToPolyData/)
Apply pre-commit autofixes scoped to Modules/Beta/BoneEnhancement/: trailing
whitespace, end-of-file newlines, and shebang executable bits where
applicable. These are mechanical hook autofixes only; no semantic
changes to the ingested module.

Reproduce locally with:
  pixi run -e pre-commit pre-commit run \
    --files $(git ls-files Modules/Beta/BoneEnhancement/)
Apply pre-commit autofixes scoped to Modules/Beta/AnalyzeObjectLabelMap/: trailing
whitespace, end-of-file newlines, and shebang executable bits where
applicable. These are mechanical hook autofixes only; no semantic
changes to the ingested module.

Reproduce locally with:
  pixi run -e pre-commit pre-commit run \
    --files $(git ls-files Modules/Beta/AnalyzeObjectLabelMap/)
Apply pre-commit autofixes scoped to Modules/Beta/SimpleITKFilters/: trailing
whitespace, end-of-file newlines, and shebang executable bits where
applicable. These are mechanical hook autofixes only; no semantic
changes to the ingested module.

Reproduce locally with:
  pixi run -e pre-commit pre-commit run \
    --files $(git ls-files Modules/Beta/SimpleITKFilters/)
Mark utils/perf.sh executable (has shebang). Required by the
check-shebang-scripts-are-executable pre-commit hook.
Mark doc/doubleWordCheck.pl executable (has shebang). Required by the
check-shebang-scripts-are-executable pre-commit hook.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 19, 2026

Too many files changed for review. (2041 files found, 100 file limit)

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.

9 participants