Skip to content

Groundwork: case-insensitive template lookup + honest untagged-build diagnostic (issue #10) - #14

Closed
CraigHutchinson wants to merge 1 commit into
masterfrom
groundwork/template-selection-diagnostics
Closed

CraigHutchinson wants to merge 1 commit into
masterfrom
groundwork/template-selection-diagnostics

Conversation

@CraigHutchinson

Copy link
Copy Markdown
Contributor

Draft — exploratory groundwork, not ready to merge. Opened to stage the work for review; see the open items below.

Relates to #10.

What the recently-merged tag-matching fix (#9, de14713) did and didn't cover

It fixed tag filtering and made the untagged/no-tag case fall back to VERSION_FALLBACK instead of an empty header, and distinguished WARNING (tag matched but unparseable) from STATUS in the final summary line. It did not touch the mid-stream message(CHECK_FAIL ...) at the "Parse version" checkpoint, and did not touch template-file lookup at all. Both halves of #10 were still fully open going into this branch.

Part (a) — template lookup

CMake/Version.cmake:491 builds an exact-case path from VERSION_H_FILENAME; :496 does an exact-case existence check and silently falls through to auto-generating the wrong (C-only) template on a miss, with no error. The docs (:40 and Version.hpp.in:5) documented the lowercase "version.hpp" spelling against the shipped, capital-V Version.hpp.in — i.e. the docs themselves encouraged the exact casing that silently breaks. tests/test_cpp23_namespace/CMakeLists.txt:6 sets the lowercase spelling, so that in-repo test itself only passes today because it happens to run on a case-insensitive filesystem.

Reproduced with a genuinely case-sensitive filesystem, not simulated: enabled NTFS's per-directory case-sensitive attribute (fsutil file setCaseSensitiveInfo) on this Windows machine and confirmed two differently-cased files coexist there. Against unmodified master in that directory, with a consumer using the documented lowercase filename: template lookup missed, the wrong template was generated, and the consumer's main.cpp failed to compile with real MSVC errors (C2653, C2065) — this is a real, breaking consumer failure, not just an ugly log line.

Fix: case-insensitive fallback scan of the template directory when the exact-case match misses, with a CHECK_PASS note suggesting the exact case for portability; plus a new VERSION_H_TEMPLATE variable a consumer can set to an explicit path, bypassing the naming convention entirely (this reuses plumbing that already existed — it was already forwarded into the build-time cmake -P re-invocation — so it needed almost no new wiring). An explicit VERSION_H_TEMPLATE that doesn't exist is a hard FATAL_ERROR, not a silent fallback.

Re-ran the same case-sensitive-directory repro against the fix: log shows the case-insensitive match being found, and the consumer compiles, links, and runs, printing the expected version string. The explicit-override path was verified independently with a template at an arbitrary filename.

Part (b) — diagnostic wording

CMake/Version.cmake:402 emitted CHECK_FAIL for the ordinary "fresh clone / untagged branch" case, even though version_applyFallback (:453) then generates valid constants successfully a few lines later and the final summary (:468-471) correctly reports success. Reproduced directly on this (case-insensitive) machine — this half doesn't need a case-sensitive filesystem to demonstrate.

Fix: the "no tag reachable" case is now CHECK_PASS with wording stating the fallback is in use. A tag that matched but failed to parse — a genuine misconfiguration — correctly stays CHECK_FAIL. Doc comments in CMake/Version.cmake and CMake/Version.hpp.in updated to state the exact shipped filename and the new VERSION_H_TEMPLATE escape hatch.

Other verification

  • Existing suite (ctest against tests/): all 24 tests still pass.
  • No-.git/tarball scenario: a consumer tree placed entirely outside any git repo (confirmed via git rev-parse --show-toplevel failing), VERSION_SOURCE_DIR pointed at it — falls back to VERSION_FALLBACK correctly, final summary reads success, consumer compiles and runs printing the fallback version.
  • FetchContent invocation mirroring the actual downstream CPM usage pattern (GIT_TAG <sha>, no manual include()) — configures, builds, and runs successfully against this branch's commit.

Still open before #10 can close

  • A true Linux/ext4 CI run — everything case-sensitive above was validated via NTFS's per-directory case-sensitive mode on Windows, not a real Linux filesystem, per the issue's acceptance criteria.
  • Decide whether to fix tests/test_cpp23_namespace/CMakeLists.txt:6 to the exact-case spelling (now optional, since the fallback handles it — but doing so keeps that test demonstrating the preferred spelling), or add a dedicated test that intentionally uses the wrong case to lock in the fallback behaviour long-term.
  • A git-worktree-based consumer test case — not attempted.
  • Check whether README.md also documents the old lowercase convention; not checked in this pass.
  • General wording/style pass — this was written to prove the mechanism works, not polished for merge.

Local scratch artifacts from validating this (scratch/, build_test/, .scratch_casetest/) are untracked and not part of this PR.

Groundwork for issue #10, not a finished/ship-ready change.

(a) Template selection: VERSION_H_FILENAME's default lookup for
"${VERSION_H_FILENAME}.in" required an exact-case filename match, but the
shipped template is "Version.hpp.in" while docs and consumer code have used
"version.hpp". On a case-sensitive filesystem this missed silently and fell
through to auto-generating the wrong (C-only) fallback template with no
error -- reproduced on this Windows machine using NTFS's per-directory
case-sensitive attribute (fsutil file setCaseSensitiveInfo), including a
consumer target that failed to compile against the resulting header.

Fixed with two additions: (1) the exact-match lookup now falls back to a
case-insensitive scan of the same directory before giving up, so the
existing convention keeps working across filesystems; (2) an explicit
VERSION_H_TEMPLATE variable lets a consumer bypass the filename convention
entirely, erroring at configure time if the given path does not exist. It
reuses plumbing already in place to forward VERSION_H_TEMPLATE into
genCmakeVersion's build-time re-invocation.

(b) Untagged-build diagnostic: the "no tag reachable from HEAD" case (the
ordinary state of a fresh clone) was reported with CHECK_FAIL even though it
always resolves via VERSION_FALLBACK and the build succeeds. Changed to
CHECK_PASS with wording that states the fallback is being used, so a reader
scanning configure output for what broke their build does not stop here.
The genuinely abnormal case -- a tag matched but failed to parse -- keeps
CHECK_FAIL, since that indicates a real VERSION_TAG_PATTERN/parser mismatch.
@CraigHutchinson

Copy link
Copy Markdown
Contributor Author

Superseded by #11, which merged into master while this branch was in progress and closed #10 directly.

Comparing the two: #11 is more complete than this prototype in two ways this session's groundwork pass didn't catch —

  • It handles two differently-cased templates existing simultaneously (hard FATAL_ERROR on ambiguity); this branch's lookup didn't consider that case at all.
  • It fixes a real bug in the old VERSION_H_TEMPLATE handling — the fallback path here still wrote into the package's own directory, which an installed or CPM-fetched copy doesn't own; Make header generation independent of filesystem case and git history #11's fallback writes to VERSION_OUT_DIR instead.

Real merge conflicts exist against current master in the same region of CMake/Version.cmake, and there's no remaining gap in #11 worth carrying this branch's changes forward for — including its own diagnostic-wording fix (issue #10, part b), which #11 already covers equivalently. Closing without merging.

For the record, in case it's useful corroborating evidence: this branch's validation independently reproduced the case-sensitivity defect via NTFS's per-directory case-sensitive mode on Windows (rather than Linux CI) and validated a FetchContent invocation mirroring the CPM GIT_TAG <sha> pattern a real downstream consumer uses — both against #11's fix, both passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant