Skip to content

STYLE: Make pre-commit run --all-files converge - #475

Merged
dzenanz merged 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:fix/precommit-empty-cmakelists
Sep 2, 2026
Merged

STYLE: Make pre-commit run --all-files converge#475
dzenanz merged 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:fix/precommit-empty-cmakelists

Conversation

@hjmjohnson

@hjmjohnson hjmjohnson commented Sep 2, 2026

Copy link
Copy Markdown
Member

pre-commit run --all-files has never converged on this repo: two hooks disagree about src/External/CMakeLists.txt and src/GPU/CMakeLists.txt, each of which contains only a newline. Giving both files a comment explaining why they exist resolves it, and unblocks the gersemi hook that has been unreachable since it was added.

The deadlock

Both directories hold an index.rst but no examples yet, so their CMakeLists.txt is empty. src/CMakeLists.txt still calls add_subdirectory(External) and add_subdirectory(GPU), so the files cannot be deleted — CMake requires them.

On a degenerate zero-content file the two hooks want opposite things:

Hook Wants
gersemi at least a newline (1 byte)
end-of-file-fixer no trailing blank line (0 bytes)

Each run flips the file back, so --all-files reports a failure forever. Any real content breaks the tie, and a comment is the useful kind of content.

Verified locally: run 1 applies the changes, runs 2 and 3 are fully clean.

Second commit: why one file was drifted, and why the style matches ITK

gersemi was onboarded in 1f457f72 (2026-06-16). ComputeCurvatureAnisotropicDiffusion/CMakeLists.txt last changed in f5f72685 (2026-06-12) — four days earlier, so it predates the hook.

It stayed drifted because this config sets fail_fast: true and end-of-file-fixer runs before gersemi. The deadlock above failed EOF on every run, so gersemi has never executed repo-wide since the day it was added. Fixing the empty files is what makes the formatter reachable; this reformat is the backlog that falls out.

The reformat is not a new or local style. This repo and ITK main pin the same gersemi 0.19.3 with the same .gersemi.config (indent 2, line_length 80, list_expansion: favour-expansion). Checked against ITK's own config directly:

$ gersemi --config <ITK-main>/.gersemi.config --check \
    src/Filtering/AnisotropicSmoothing/ComputeCurvatureAnisotropicDiffusion/CMakeLists.txt
# no-op

Same result for the two commented CMakeLists.txt. ITK's config additionally sets definitions: ["CMake/stubs"], which teaches gersemi about ITK-specific commands; this file uses only set, target_link_libraries, install and add_test, which is why the check is a no-op regardless.

Split out of #476 (ITK 6.0 LevelSetNode container fix), which was blocked behind this gate.

…ists

src/External and src/GPU hold no examples yet, but src/CMakeLists.txt
calls add_subdirectory() on both, so each needs a CMakeLists.txt. With
those files empty, gersemi wrote a lone newline and end-of-file-fixer
stripped it back, so `pre-commit run --all-files` never converged.
Documenting why the files exist gives both hooks something to agree on.

Also applies gersemi's pending reformat of
ComputeCurvatureAnisotropicDiffusion/CMakeLists.txt.
@github-actions github-actions Bot added area:Filtering Issues affecting the Filtering module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Style Style changes: no logic impact (indentation, comments, naming) labels Sep 2, 2026
@hjmjohnson
hjmjohnson marked this pull request as ready for review September 2, 2026 14:00
@dzenanz
dzenanz merged commit 0f90b4a into InsightSoftwareConsortium:main Sep 2, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Style Style changes: no logic impact (indentation, comments, naming)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants