Skip to content

ENH: Add migration sentinels for between-tag downstream gating#6665

Closed
hjmjohnson wants to merge 3 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:enh-migration-sentinels
Closed

ENH: Add migration sentinels for between-tag downstream gating#6665
hjmjohnson wants to merge 3 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:enh-migration-sentinels

Conversation

@hjmjohnson

@hjmjohnson hjmjohnson commented Jul 19, 2026

Copy link
Copy Markdown
Member

Downstream projects build against ITK main between tags, which can be two years apart, and cannot tell whether a specific ITK change is present. This adds migration sentinels: one Markdown file per downstream-visible change, collected into ITK_MIGRATION_SENTINELS and exported through ITKConfig.cmake, so a downstream project can make a fine-grained decision at configure time.

The motivating case is git bisect: a downstream project bisecting ITK to find which change broke it must decide, at each arbitrary commit, which set of baseline images to test against. Sentinels are files in the source tree, so they read correctly in a detached HEAD or a shallow clone — states where git describe fails outright and git rev-list --count silently returns a wrong answer.

This is intended only for fine-grained configure-time decisions between tagged releases. It is not a general capability-query API, and it is self-expiring: at each tag the sentinels are deleted and the version arm of the downstream condition takes over.

For maintainers: each release gains two steps (expire-itk, then expire-downstream on consumers). This PR does not add them to a release checklist — point me at the right file and I will.

How a downstream project uses it
find_package(ITK REQUIRED)
#NOTE:  this works even before the 6.1.0 tag is generated!!! 
if(ITK_VERSION VERSION_GREATER_EQUAL 6.1.0
   OR ITK_MIGRATION_PR6532 IN_LIST ITK_MIGRATION_SENTINELS)
  set(BASELINE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/Baseline/post-6532)
else()
  set(BASELINE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/Baseline/legacy)
endif()

Both arms are required. The sentinel arm is true between tags; the version arm is true afterwards, once sentinels have been expired.

Against an ITK that predates this mechanism, ITK_MIGRATION_SENTINELS is simply unset, IN_LIST yields false, and the legacy branch is taken — no guard, no error. That backward-compatibility path is why this is a list variable rather than a function: CMake's if() cannot invoke a function, and calling one that older ITK does not define is a hard Unknown CMake command error.

Why not a version number

Every version-based approach was tried first and fails:

  • There is no v6.0.0 tag, and itkVersion.cmake reports 6.0.0 at v6.0a01, v6.0b01, v6.0b02 and current main alike — so version comparison cannot separate any ITK 6 change from any other.
  • In a --depth 1 clone, which CI uses, git describe --tags fails with "No names found" and git rev-list --count HEAD returns 1 — silently wrong, which is worse than an error.
  • Prerelease tags are not valid CMake version comparands, so VERSION_GREATER_EQUAL 6.0b02 is not usable.
ccache

The sentinel list must never reach a compiled header. Modules/Core/Common/src/itkConfigure.h.in is included nearly everywhere; a value that changes on most merges would change every translation unit's preprocessed output and destroy ccache hit rates on every build machine.

The list is therefore exported through ITKConfig.cmake only, and MigrationSentinelConfigureHeaderGuard asserts that the generated itkConfigure.h contains no sentinel — the constraint is enforced by a test, not by a comment.

Contents and tooling
  • CMake/MigrationSentinels/ — 33 sentinels, pre-populated from the ITK 6 migration guide's history (46 commits mapped to 37 PRs; 4 excluded as guide maintenance rather than migrations — a typo fix, a master-to-main prose update, a rewrite of an existing migration's wording, and the commit that created the file).
  • CMake/itkMigrationSentinels.cmake — collector; uses CONFIGURE_DEPENDS so adding a sentinel triggers a reconfigure.
  • Utilities/Maintenance/MigrationSentinels.pyvalidate (CI-enforced), expire-itk, expire-downstream.
  • Documentation/docs/contributing/migration_sentinels.md — contributor guide.
  • Six tests under Utilities/MigrationSentinelTest/, covering collection, the four-state downstream idiom, the ccache guard, a synthetic find_package(ITK) consumer, and both the accept and reject paths of the validator.
Verification
Check Result
Full ITK build 5599/5599 targets, 0 failures
Full test suite 4683/4683 passed
Sentinel tests 6/6 passed
CONFIGURE_DEPENDS re-trigger adding a sentinel auto-ran CMake and the value reached ITKConfig.cmake; removal reverted it
Sentinels exported 33 reach ITKConfig.cmake
pre-commit run --all-files clean

Additive only: no existing behavior changes, and the net diff deletes nothing.

@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation area:Python wrapping Python bindings for a class area:Documentation Issues affecting the Documentation module labels Jul 19, 2026
@hjmjohnson
hjmjohnson force-pushed the enh-migration-sentinels branch from 938d307 to 26b9351 Compare July 19, 2026 15:38
@hjmjohnson
hjmjohnson requested a review from thewtex July 19, 2026 16:17
Downstream projects build against ITK main between tags, which can be two
years apart, and cannot tell whether a specific change is present. Collect
one Markdown file per downstream-visible change from CMake/MigrationSentinels
into ITK_MIGRATION_SENTINELS and export it through ITKConfig.cmake.

Consumers use the list variable rather than a function: CMake's if() cannot
invoke a function, and calling one that older ITK does not define is a hard
error. An unset variable simply yields false, so an ITK predating this
mechanism takes the legacy branch with no guard.

The list is exported to CMake only. Embedding it in itkConfigure.h would
change every translation unit's preprocessed output on most merges and
invalidate ccache fleet-wide; a test asserts it stays out.
Add MigrationSentinels.py with three subcommands: validate, which CI runs so
a malformed sentinel cannot merge; expire-itk, which removes every sentinel
at a tag because all of them ship in it; and expire-downstream, which reduces
a consumer's two-arm condition to its version arm once the tag exists.

validate also rejects a misspelled prefix. Such a file is invisible to the
ITK_MIGRATION_*.md glob, so the sentinel would silently never be published.

expire-downstream pins UTF-8 on the files it rewrites. The locale default is
cp1252 on Windows, and the rewrite is in place and not atomic, so an
unpinned encoding can silently corrupt non-ASCII content.
Every pull request that added a downstream-visible change to the ITK 6
migration guide gets a sentinel, so downstream projects can gate on any of
them before a release contains one.

The 6.0 prerelease tags all report ITK_VERSION 6.0.0, so version comparison
cannot separate these changes from each other or from current main; the
sentinel is the only signal that distinguishes them.

Derived by mapping the guide's history to pull requests. Commits that edited
the guide without introducing a migration -- a typo fix, a master-to-main
prose update, a rewrite of an existing migration's wording, and the commit
that created the file -- are excluded.
@hjmjohnson

Copy link
Copy Markdown
Member Author

NOTE: BRAINSia/BRAINSTools@9c86141. Shows how this would be used in downstream projects.

@hjmjohnson
hjmjohnson requested review from blowekamp and dzenanz July 20, 2026 15:30
@hjmjohnson

Copy link
Copy Markdown
Member Author

FYI: I think that the migration guide file (which is a single file that often causes merge conflicts) should be an index to the files in commit 91876b4 , and the individual file should contain the report for the migration inducing change. Perhpas add rigid structure to the new topic based files with "Summary", "Expected Migration Actions", "Diagnosis Report" sections.

@blowekamp

blowekamp commented Jul 20, 2026

Copy link
Copy Markdown
Member

The solution to this problem is more frequent releases, rather than this unusual solution.

Alternatively, perhaps some type of date, or patch number since the prior release could be incorporated is the reported ITK version rather than this.

@hjmjohnson

Copy link
Copy Markdown
Member Author

Thanks @blowekamp — that's a useful pushback I was looking for, and I'd rather get input than decide this myself.

Quick notes on the two alternatives you raised:

More frequent releases — agreed, and orthogonal. Downstream still builds against main between tags, and git bisect lands on untagged commits at any cadence. This is still a major pain point when trying to get downstream fixes in place, and not a complete solution. It effectively stops downstream projects from continuing to A/B test solutions against older and newer ITK versions until the published release is made.

Patch count since prior release — tried it, and it fails in exactly the situations that matter. git rev-list --count HEAD returns 1 in --depth 1 clones (silently wrong), and git describe --tags fails with "No names found" there and in a detached HEAD. Anything computed from git history at configure time has this problem; the value must be committed to the tree.

Date in the reported version — this one works, and I think it's a real competing design rather than a variant. CMake's project() has an unused fourth tweak component. Policy would be: any migration guide entry sets the tweak to its merge date as YYYYMMDD, release tags too, never reset. Downstream becomes:

if(ITK_VERSION_FULL VERSION_GREATER_EQUAL 6.0.0.20260719)

That drops both release-checklist steps (expiry falls out of version ordering), the two-arm idiom, and nearly all the tooling in this PR.

The trade, stated plainly: sentinels answer "is change X present?"; a date answers "is ITK at least as new as the day X landed?" — ordering rather than identity, at day granularity. Two migrations on the same day become indistinguishable. It also requires manually setting the migration day to the day that the PR is merged (or close to it).

I don't have a confident view on whether that loss matters in practice, and it's the crux. My original framing came from cmake_policy — named, individually-queryable changes — but I'm no longer sure the problem warrants that much apparatus.

@thewtex, @dzenanz, @blowekamp — two questions I'd like answered before I build either one:

  1. Is day granularity sufficient for selecting baselines during a bisect, or is per-change identity needed?
  2. Are two extra release-checklist steps an acceptable recurring cost if the answer to (1) is "identity needed"?

@dzenanz

dzenanz commented Jul 20, 2026

Copy link
Copy Markdown
Member

The solution to this problem is more frequent releases, rather than this unusual solution.

Agreed.

Having all migration-related docs in one file is quite useful, even if it is harder to update it.

if(ITK_VERSION_FULL VERSION_GREATER_EQUAL 6.0.0.20260719)

I like this approach. It is simple. Day granularity is quite enough. And this is indeed a workaround for lack of frequent releases. But I see the use in having it. It is quite useful for downstream projects during pre-release testing, no matter how frequent releases are.

@hjmjohnson

Copy link
Copy Markdown
Member Author

Superseded by #6667, which implements the tweak-date design discussed above. Leaving this open for reference until that lands.

@hjmjohnson hjmjohnson closed this Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Documentation Issues affecting the Documentation module area:Python wrapping Python bindings for a class type:Enhancement Improvement of existing methods or implementation 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