COMP: Guard the CMP0169 policy setting so older CMake still configures - #30
Conversation
CMakeLists.txt sets policy CMP0169 unconditionally, to keep using the
deprecated FetchContent_Populate. CMP0169 was introduced in CMake 3.30,
and cmake_policy(SET) on an unknown policy is a hard error, so any CMake
older than that fails to configure at all:
CMake Error at CMakeLists.txt:175 (cmake_policy):
Policy "CMP0169" is not known to this version of CMake.
That includes the CMake 3.28 shipped by Ubuntu 24.04, which is what the
CI runners and the project's own Dockerfile use, and it sits well above
the cmake_minimum_required(VERSION 3.10.2) the project advertises.
Wrapping it in if(POLICY CMP0169) is the standard idiom. Verified
configuring with both CMake 3.28.3 and 4.4.2.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KSPnbwpDjVcAYqDdVqLkMU
|
Reviewed as part of a sweep over the open backlog. Merge as-is. This is a real bug, not hygiene.
Worth noting for the record: all four CI jobs are green on this PR and on master, because the GitHub runners carry CMake >= 3.30. The build matrix is structurally unable to observe this failure. The coverage gap this exposesThere is no CI job on any older CMake, so the project's actual minimum-CMake claim is untested. A single job pinned to the oldest supported CMake — Ubuntu 24.04's 3.28 is the natural choice, since that is the LTS that broke — would have caught this at PR time and would guard the floor going forward. Out of scope here; recorded as a follow-up. |
|
The commit messages in this range were rewritten to remove trailers that do not belong in permanent history: This PR's commit on the rewritten
The SHA recorded above by GitHub is from the pre-rewrite history and no longer resolves. |
CMakeLists.txt sets policy CMP0169 unconditionally, to keep using the
deprecated FetchContent_Populate. CMP0169 was introduced in CMake 3.30,
and cmake_policy(SET) on an unknown policy is a hard error, so any CMake
older than that fails to configure at all:
That includes the CMake 3.28 shipped by Ubuntu 24.04, which is what the
CI runners and the project's own Dockerfile use, and it sits well above
the cmake_minimum_required(VERSION 3.10.2) the project advertises.
Wrapping it in if(POLICY CMP0169) is the standard idiom. Verified
configuring with both CMake 3.28.3 and 4.4.2.
Interface impact: none. On the union of all these changes, configured with
USE_FSL_CODE=ONandUSE_CIFTI_CODE=ON: all 448 exported symbols acrosslibniftiio,libnifti2,libznz,libfslio,libnifticdfandlibciftiare identical tomasterundernm -D --defined-only, and all ten installed headers are identical undergcc -E -P. Undergcc -dM -Eone macro definition differs, intentionally and only in text: #61 makesFSL_RADIOLOGICALread(-1)so it is safe inside an expression. Its value is still-1, checked by compiling against each installedfslio.hand printing it.Verification. This branch: builds with gcc 16.1.1,
ctestunchanged frommaster(2 of 345 fail onmasteritself in this environment; #31 and #29 each fix one). The union of all the PRs: 0 errors under both gcc 16.1.1 and clang 22.1.8,ctest345/345 under each, and the whole suite under valgrind memcheck with--trace-children=yesgives 484 traced processes with no invalid access, no uninitialised value and no leak in any nifti binary.Coordination. Every line of every branch was compared, whitespace-normalised, against the diffs of the open PRs (#11, #21, #22, #23, #24). Where one of those already changes a line, the line was left alone, and the few deliberate overlaps are named in the text above. What survives is 17 compiler warnings, all of them on those lines: 9
-Wsign-conversion(5 infslio.cfor #22, 2 innifti2_io.cand 2 innifti_tester001.cfor #24) and 8-Wcalloc-transposed-argsinnifti_findhdrnameandnifti_findimgname, which #11 rewrites. No formatting changes appear anywhere, to stay clear of #10 and #12.One of a set of independent, single-purpose PRs. Each bases on
masterand can be merged on its own, in any order.The full set of PRs (35)
The union of all of them is on the fork as
all-changes, if you want to build and test the lot at once.CI and build
Configuration and documentation
Defects
Warning and check classes