COMP: Pin proxTV to an immutable commit hash

[hjmjohnson]

proxTV_GIT_TAG named the anchor branch for/itk-proxtv-3.3.0-69062fe5, so advancing that branch changed what ITK builds without any change landing here. Pin the commit hash 0903e7c instead, with the overlay ref in a trailing comment — the form already used by DCMTKGitTag.cmake.

The pinned commit also brings in the GCC warning cleanup merged as InsightSoftwareConsortium/proxTV#1.

Why a hash rather than a tag name

This is a live reference: FetchContent resolves it on every configure. A branch is mutable by design, and a tag — while conventionally immutable — can still be deleted and re-pushed. Only the hash is pinned outright, so it is the right form for anything fetched at build time. Ref names used once by a human (the fork's welcome update procedure, UpdateFromUpstream.sh-style scripts) are unaffected and keep using branch names.

This introduces no new naming convention: DCMTKGitTag.cmake:46 is set(DCMTK_GIT_TAG "554b744…") # for/itk-dcmtk-3.7.0-ccfd10b, and this change makes proxTV match it.

The anchor branch for/itk-proxtv-3.3.0-69062fe5 still exists and still points at 0903e7c; it remains the update target per the fork's welcome procedure. The annotated tag for/itk-proxtv-3.3.0-69062fe5-r1 also points there and is named in the comment, but ITK no longer depends on either name resolving.

What the pinned overlay contains

Three commits on the ITK proxTV overlay, all compiler-warning fixes in the vendored C sources:

• 34af3b0 — guard #pragma omp behind #ifdef _OPENMP and the MSVC warning(suppress:) pragmas behind #ifdef _MSC_VER
• f726e32 — size_t loop counters where compared against size_t bounds (-Wsign-compare), and a -Wmisleading-indentation fix
• 0903e7c — reject n < 2 in more_TV2/morePG_TV2 and npen < 1 in PD_TV/PDR_TV before those values reach memcpy/calloc size arguments (-Wstringop-overflow, -Walloc-size-larger-than)

Upstream base is unchanged: albarji/proxTV 3.3.0, commit 69062fe5. The overlay is a fast-forward, 4c53ae7 → 0903e7c.

The same fixes were offered upstream as albarji/proxTV#63, but upstream has merged nothing since 2018, so the overlay remains the source of truth.

Verification

git ls-remote confirms the branch head, the peeled annotated tag, and this pin are all the same commit, so the build result is unchanged by this PR:

refs/heads/for/itk-proxtv-3.3.0-69062fe5      -> 0903e7c1675d402c3b8be02a06782a08b12f3945
refs/tags/for/itk-proxtv-3.3.0-69062fe5-r1^{} -> 0903e7c1675d402c3b8be02a06782a08b12f3945

CI on this PR is the end-to-end test: it exercises the fetch at the new pin on every platform that builds ITKTotalVariation.

[blowekamp]

I think the "for/itk-XXXX" convention is used in most of our forks and they have similar issues. Human though should be used to update the procedures and documentation as needed to change the convention.

[hjmjohnson]

I think the "for/itk-XXXX" convention is used in most of our forks and they have similar issues. Human though should be used to update the procedures and documentation as needed to change the convention.

@blowekamp. I want to understand your comment on this draft PR. Are you advocating for the proposed "-r1" extention to be documented on the welcome branch documentation on all the itk forks? This draft was intended to spark a discussion, but I did not expect such quick response to the "adds an -rN suffix so it cannot collide with the anchor branch".

I won't have time to do that across all the itk forks, but I'll make a tracking issue to get around to it sometime this fall.

[blowekamp]

Sorry for the short response. I don’t have much time right now either, but I wanted to short cut another convention being introduced without discussion and documentation.

Perhaps for “live” references the actual hash should be used and a comment with the branch name is appropriate.

The clones during a build would be live references This is different than the scripts for third party which only use the reference once so the branch name works and no hash needed.

[hjmjohnson]

We are all too busy right now :(. Thanks for your response. I'll be on vacation until August 18th, but will try to squeeze in a "live" reference update during downtime.

[hjmjohnson]

Switched to the commit hash with the overlay ref in a trailing comment, matching DCMTKGitTag.cmake.

I think this is a short-term fix until we all have time/resources to find a more complete solution.

[greptile-apps]

Greptile Summary

This change replaces proxTV’s moving overlay reference with the immutable commit it currently resolves to. Remote resolution confirmed that for/itk-proxtv-3.3.0-69062fe5-r1 peels to 0903e7c1675d402c3b8be02a06782a08b12f3945, and a fresh shallow checkout of that commit succeeded. No defects were found.

Confidence Score: 5/5

Safe to merge: the pinned dependency revision resolves from the configured upstream repository and checks out successfully.

The changed dependency reference was compared with the existing overlay tag and exercised through a clean shallow fetch and detached checkout; both resolve to the same commit.

Files Needing Attention: No files need further attention.

T-Rex Logs

What T-Rex did

• Queried the proxTV remote for the tag refs/tags/for/itk-proxtv-3.3.0-69062fe5-r1 and its peeled commit, and confirmed the remote advertised 0903e7c1675d402c3b8be02a06782a08b12f3945.
• Fetched the pinned SHA and checked out the identical HEAD, with both commands exiting successfully.

_{Ran code and verified through T-Rex}

_{Reviews (1): Last reviewed commit: "COMP: Pin proxTV to an immutable commit ..." | Re-trigger Greptile}

[blowekamp]

Thank you.

IMHO using hashes and comments is OK, for the long run. But other may have difference experiences and preferences to what has worked well. I can add it to my todo to make that the document and consistent way, if others are in agreement.

[hjmjohnson]

@blowekamp Switched to the commit hash with the overlay ref in a trailing comment, matching DCMTKGitTag.cmake:46.

That's your "live reference" case exactly, and it drops the -rN question entirely — the tag stays on the fork as documentation, but ITK no longer depends on the name resolving, so nothing needs a convention change or a doc sweep across the forks.

0903e7c is the commit the anchor branch and the tag both already point at, so this is a no-op for builds.