Skip to content

BUG: Fix check for orphaned remote modules#6398

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
SimonRit:orphaned_remote
Jun 5, 2026
Merged

BUG: Fix check for orphaned remote modules#6398
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
SimonRit:orphaned_remote

Conversation

@SimonRit

@SimonRit SimonRit commented Jun 5, 2026

Copy link
Copy Markdown

Some remote modules have a repository name which does not match the .remote.cmake file name. A few start with ITK, SCIFIO is named scifio-imageio, WebAssemblyInterface is named ITK-Wasm.

I used the following bash command line in Modules/Remote to check the list of exceptions:

for i in *remote.cmake ; do if ! test -e $(grep GIT_REPOSITORY $i | sed "s/.*\\///g" | sed "s/.git//g" | sed "s/^ITK//g").remote.cmake ; then echo $i ; fi ; done

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

@github-actions github-actions Bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels Jun 5, 2026
@greptile-apps

greptile-apps Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates itkCheckForOrphanedRemoteModules.cmake to correctly handle remote modules whose cloned directory name does not match their .remote.cmake file name. Previously, the check assumed a direct name correspondence; now it strips the ITK prefix from cloned directory names before looking up the cmake file, and adds explicit exceptions for directories that require completely different names (scifio-imageio clones → SCIFIO.remote.cmake, ITK-Wasm clones → WebAssemblyInterface.remote.cmake).

  • Adds string(REGEX REPLACE "^ITK" "" ...) to strip the ITK prefix from directory names, enabling the check to find cmake files like BioCell.remote.cmake for a directory named ITKBioCell.
  • Introduces an explicit exception list (Deprecated, ITK-Wasm, scifio-imageio) to skip directories whose names differ too much from their cmake file names even after the prefix strip.

Confidence Score: 4/5

The scifio-imageio fix is correct and merges cleanly, but the ITK-Wasm exception remains unreachable after the prefix-strip mutation — a concern already raised in review discussion that has not been fully resolved in the current revision.

The scifio-imageio exception correctly catches clones of scifio/scifio-imageio.git before the file-existence check. However, the ^ITK regex turns an ITK-Wasm directory into -Wasm, so the STREQUAL "ITK-Wasm" guard is never entered; a developer with that module checked out would still hit FATAL_ERROR.

CMake/itkCheckForOrphanedRemoteModules.cmake — specifically the interaction between the prefix-strip regex and the ITK-Wasm exception string.

Important Files Changed

Filename Overview
CMake/itkCheckForOrphanedRemoteModules.cmake Adds ITK prefix stripping and an exception list for modules with mismatched directory/cmake names; the scifio-imageio exception is correct, but the ITK-Wasm exception string does not match the stripped name (-Wasm vs ITK-Wasm), a bug flagged in a prior review thread.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Glob all entries in Modules/Remote/*] --> B{Is entry a directory?}
    B -- No --> C[Skip]
    B -- Yes --> D[get_filename_component: extract _name]
    D --> E["string(REGEX REPLACE '^ITK' '' _name)"]
    E --> F{"_name == 'Deprecated'\nOR 'ITK-Wasm'\nOR 'scifio-imageio'?"}
    F -- Yes --> G[continue / skip]
    F -- No --> H{"_remote_dir/_name.remote.cmake\nOR Deprecated/_name.remote.cmake\nexists?"}
    H -- Yes --> I[OK - module is not orphaned]
    H -- No --> J[Append to _orphans list]
    J --> K{Any orphans found?}
    I --> K
    K -- No --> L[Done - no orphans]
    K -- Yes --> M[message FATAL_ERROR with rm commands]
Loading

Reviews (2): Last reviewed commit: "BUG: Fix check for orphaned remote modul..." | Re-trigger Greptile

Comment thread CMake/itkCheckForOrphanedRemoteModules.cmake
Comment thread CMake/itkCheckForOrphanedRemoteModules.cmake
Some remote modules have a repository name which does not match the
.remote.cmake file name. A few start with ITK, SCIFIO is named
scifio-imageio, WebAssemblyInterface is named ITK-Wasm.
@SimonRit

SimonRit commented Jun 5, 2026

Copy link
Copy Markdown
Author

@greptileai review

@hjmjohnson hjmjohnson merged commit 8b6277d into InsightSoftwareConsortium:main Jun 5, 2026
18 of 19 checks passed
@SimonRit SimonRit deleted the orphaned_remote branch June 8, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants