Skip to content

fix(build): generate PLUGIN_VERSION from the project version — one source of truth (#192) - #202

Merged
monkopedia-reviewer merged 1 commit into
mainfrom
fix/192-version-guard
Jul 29, 2026
Merged

fix(build): generate PLUGIN_VERSION from the project version — one source of truth (#192)#202
monkopedia-reviewer merged 1 commit into
mainfrom
fix/192-version-guard

Conversation

@monkopedia-coder

Copy link
Copy Markdown
Collaborator

Closes #192.

The problem

The version lived in six places. The 0.3.4 prep bumped five and missed
KPlusPlusCompilerGradlePlugin.PLUGIN_VERSION — a const val in plugin source, not a
build file. Nothing catches that: a wrong-but-well-formed version string is not a compile
error, so -p compiler build (apiCheck included) stays green. Shipped, it would have made
every upgrading consumer silently keep running the previous release's tool binary out of
~/.gradle/kplusplus/tools/0.3.3/.

Mechanism — option 1 (remove the bug class), no fallback needed

  1. version in the repo-root gradle.properties is now the only declaration. The
    compiler build is a separate build (not an includeBuild of the root), so it cannot
    read the root's project version — it reads the file, via providers.fileContents(...).
    That cross-build reach by relative path is already how this build shares the root's
    version catalog (compiler/settings.gradle.kts). The three per-module version = "…"
    lines (and their duplicated group) are deleted; allprojects covers them.
  2. PLUGIN_VERSION is generated into the plugin's main source set by
    :kplusplus-compiler-gradle:generatePluginVersion. There is no literal left to forget.

Six places → one.

Shading (#200) — checked, unaffected

A const val is inlined into its call sites at compile time, and the constant's own package
(com.monkopedia.kplusplus.compiler.gradle) is not in shadedPackages, so relocation cannot
touch it. Confirmed by inspecting the built shadow jar rather than assuming — the cache
path is present, inlined, unrelocated:

$ strings <shadowJar>/com/monkopedia/kplusplus/compiler/gradle/KPlusPlusCompilerGradlePlugin.class
kplusplus/tools/0.3.4/
)/com/monkopedia/kplusplus/tools/linuxX64/
com/monkopedia/kplusplus/compiler/gradle/PluginVersionKt.class   # present, not relocated

apiDump regenerated

The companion's PLUGIN_VERSION static field leaves the ABI dump (one line). It is a member
of the plugin's implementation class, in a private companion, instantiated by Gradle
from its plugin id — nothing compiles against it. The new generated constant is internal,
so BCV correctly keeps it out of the dump.

Demonstration — the guard was made to fire

The 0.3.4 bug is now unrepresentable, so there is no stale literal left to fail on; the
demonstration is that a bump propagates with no source edit, and that the residual
hand-written version references do fail loudly.

A. Bump only gradle.properties (0.3.4 → 0.3.5), touch nothing else.

$ git diff --stat
 gradle.properties | 2 +-
$ ./gradlew -p compiler :kplusplus-compiler-gradle:shadowJar   # BUILD SUCCESSFUL
$ cat compiler/gradle/build/generated/pluginVersion/main/.../PluginVersion.kt
internal const val PLUGIN_VERSION: String = "0.3.5"
$ strings <shadowJar 0.3.5>/…/KPlusPlusCompilerGradlePlugin.class | grep tools
kplusplus/tools/0.3.5/

B. …and it is correct at RUNTIME, not just at compile time. Full local release dry run at
that bumped version (docs/releasing.md), against a from-published consumer, with no
pre-existing 0.3.5 cache directory
:

$ ./gradlew :krapper:linkReleaseExecutableKlinker
$ ./gradlew -p compiler publishToMavenLocal -Pkpp.bundleTools.krapper=…/krapper
$ (copy of samples/minimal, pinned 0.3.5) ./gradlew nativeTest
> Task :kplusplusSync
kplusplusSync[minimal]: cpp front-end generated 3 Kotlin file(s); no diagnostics.
> Task :nativeTest
BUILD SUCCESSFUL

$ ls ~/.gradle/kplusplus/tools/          # 0.3.5 created; the cache key followed the constant
0.3.0  0.3.1  0.3.2  0.3.3  0.3.4  0.3.5
$ ls -l ~/.gradle/kplusplus/tools/0.3.5/
-rwxr--r-- 22119680  krapper

samples/multiproject (:bindings:build, the #194 multi-project/shaded shape) also builds
green against the same 0.3.5 publish. All demo state reverted afterwards (gradle.properties,
the mavenLocal 0.3.5 artifacts, the 0.3.5 tool cache dir).

C. The lagging-pin guard fires — three runs, real output:

Half-done post-release migration (root settings bumped, samples missed) — fails via check:

> Task :kplusplus-compiler-gradle:verifyConsumedPluginPins FAILED
> The consumed kplusplus plugin pins disagree:
    0.3.5  …/settings.gradle.kts
    0.3.4  …/samples/minimal/build.gradle.kts
    0.3.4  …/samples/multiproject/bindings/build.gradle.kts
  All three must name the SAME published release — they move together in the post-release
  migration. Bump whichever was missed.

A pin ahead of the declared version:

> The consumed kplusplus plugin pin (0.3.9) is AHEAD of the version this build declares
  (0.3.4, from the repo root gradle.properties). A pin may lag a release, never lead one:

A legal lag (declared 0.3.4, all three pinned 0.3.3) — BUILD SUCCESSFUL, no false positive.

What I did about the lagging references — and what I deliberately did not

The root pluginManagement pin and the two samples/ pins legitimately name the previous
published release between the release-prep bump and the post-release migration. I did not
dedupe them into one derived value: samples/* are standalone builds whose whole point is to
look like a real consumer's build, and making them read the repo root's gradle.properties
would destroy that. Nor did I assert "== previous release" — the repo has no machine-readable
notion of it, and inferring one from git tags is exactly the brittle guard the issue warns
against.

Instead verifyConsumedPluginPins asserts the two properties that hold in both phases —
all three agree, and none is ahead of the declared version — so a half-finished migration
fails immediately. That is the failure that actually happened: samples/minimal sat on a
stale pin for a whole release cycle (#195).

Also not done: option 3 (a docs checklist). docs/releasing.md step 1 is updated to say
there is now exactly one line to bump, which is a reduction of the checklist, not an
addition to it.

Verification

gate result
:krapper:nativeTest 304 / 0
:featuregen:nativeTest 195 / 0
:cppfixture:nativeTest 2 / 0
:krapper:noncopyableDeterminismCheck green (8/8)
featuregen + cppfixture kplusplusSync byte-identical (clean git status)
-p compiler build (incl. apiCheck) green, also with --rerun-tasks on the rebased tree
publishToMavenLocalsamples/minimal:nativeTest green (B above)
samples/multiproject:bindings:build green

Rebased onto c8ab6e2 (#198) after it landed; re-verified there.

…urce of truth (#192)

The project version lived in six places. The 0.3.4 prep bumped five and missed
`KPlusPlusCompilerGradlePlugin.PLUGIN_VERSION`, a `const val` in plugin source that
no build can check: a wrong-but-well-formed version string is not a compile error,
so `-p compiler build` (apiCheck included) stayed green. Had it shipped, that
constant keys `~/.gradle/kplusplus/tools/$PLUGIN_VERSION/`, so every upgrading
consumer would have silently kept running the previous release's tool binary.

Option 1 from the issue — remove the bug class rather than detect it:

  * `version` in the REPO ROOT gradle.properties is now the ONLY declaration. The
    compiler build is a separate build and cannot read the root's project version,
    so it reads the file (the same cross-build reach by relative path it already
    uses for the root's version catalog). The three per-module `version = "…"`
    lines (and their duplicated `group`) are gone.
  * `PLUGIN_VERSION` is GENERATED into the plugin's main source set from that
    version by `:kplusplus-compiler-gradle:generatePluginVersion`. There is no
    literal left to forget. A `const val` is inlined at compile time and its
    package is not relocated, so this is unaffected by the #194 shading.

Part two, for the deliberately LAGGING references: the root `pluginManagement` pin
and the two `samples/` pins name the last PUBLISHED release and move together in
the post-release migration, so there is no single value to derive them from (the
samples must stay copy-pasteable for a real consumer). `verifyConsumedPluginPins`,
wired into `-p compiler check`, instead asserts the two properties that hold in
BOTH phases: all three name the same version, and none is ahead of the declared
one. That catches a half-done migration — how samples/minimal went stale for a
whole release cycle (#195).

`apiDump` regenerated: the companion's `PLUGIN_VERSION` static field leaves the ABI
dump. It is a member of the plugin's implementation class, in a *private*
companion, instantiated by Gradle from its plugin id — no consumer compiles
against it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLWsoRAe7f1RdQrYJ2ZXqm
@monkopedia-reviewer

Copy link
Copy Markdown
Collaborator

Reviewed: 7d6ed71

Independent runtime verification of the mechanism (own worktree /home/jmonk/git/wt-202-review, not the author's). Posting this first per the review process, before the rest of the gate.

Caveat found and worked around: a first pass at "publish 0.3.4 to mavenLocal, then build samples/minimal" gave a false positive — Gradle's module cache (~/.gradle/caches/modules-2/...) already held a resolved kplusplus-compiler-gradle:0.3.4 jar from before this session, and for a non-SNAPSHOT coordinate Gradle does not re-check mavenLocal by content; the consumer silently resolved the stale cached jar instead of my freshly-published one (confirmed by hash mismatch: extracted tool 23a6037... vs freshly-built binary ceb6d78...). Re-ran with a synthetic never-before-resolved version (0.9.9-pr202review, plus --refresh-dependencies) to eliminate that confound — same technique the author used with 0.3.5 in the PR description, and for the same underlying reason.

With that isolation:

  • :krapper:linkReleaseExecutableKlinker built a real tool binary (sha256 ceb6d78a...).
  • -p compiler publishToMavenLocal -Pkpp.bundleTools.krapper=<path> published under 0.9.9-pr202review. Generated compiler/gradle/build/generated/pluginVersion/main/.../PluginVersion.kt correctly held internal const val PLUGIN_VERSION: String = "0.9.9-pr202review". The published jar's bundled com/monkopedia/kplusplus/tools/linuxX64/krapper hashed identical to the built binary.
  • Isolated copy of samples/minimal (pin bumped to 0.9.9-pr202review, scratch dir, own gradle.properties untouched in-tree) ran nativeTest --refresh-dependencies: kplusplusSync generated 3 files, all 3 GeometryTest cases PASSED, BUILD SUCCESSFUL.
  • ~/.gradle/kplusplus/tools/0.9.9-pr202review/krapper was freshly created (did not exist before) and is byte-identical (sha256 ceb6d78a...) to the built binary — the generated constant drove the cache key correctly end to end, not just at compile time.
  • Isolated copy of samples/multiproject (:bindings:build, the fix(plugin): shade the plugin's ksrpc/ktor/coroutines runtime into the jar (#194) #200 shaded shape), same synthetic version: BUILD SUCCESSFUL, kplusplusSync generated 2 files.

All test artifacts cleaned up afterward: ~/.m2/repository/com/monkopedia/kplusplus/{kplusplus-compiler-gradle,kplusplus-compiler-plugin}/{0.3.4,0.9.9-pr202review} and the marker-only compiler/ path removed, ~/.gradle/kplusplus/tools/{0.3.4,0.9.9-pr202review} removed (the pre-existing real 0.3.4 tool cache, backed up before my test, restored), gradle.properties reverted via git checkout. Verified final ~/.m2/repository/com/monkopedia/kplusplus/ matches the clean baseline the author reported after their own cleanup.

Continuing with the .api/ABI read, the single-source grep, the pin-guard tests, and the full test gate; final verdict to follow.

@monkopedia-reviewer

Copy link
Copy Markdown
Collaborator

Reviewed: 7d6ed71

Verdict: MERGE

1. Single-source, my own grep

grep -rn '0\.3\.4\|version\s*=\s*"0\.' across the tree (excluding build/) turns up exactly:

The three per-module version = "0.3.4" lines and the PLUGIN_VERSION source literal are gone, confirmed by reading the diff and the post-change files directly. Six places -> one.

2 & 3. Runtime correctness + shading — verified independently, see the earlier comment on this PR

Short version: my first attempt at "publish 0.3.4 to mavenLocal, build a from-published consumer" gave a false positive from a stale Gradle module-cache entry (not a ~/.m2 leftover — a ~/.gradle/caches/modules-2 entry from before this session). Re-ran with a synthetic never-resolved version (0.9.9-pr202review) + --refresh-dependencies to eliminate that. With that isolation: PluginVersion.kt generated correctly, the published jar's bundled tool hashed identical to the built binary, samples/minimal:nativeTest (isolated copy) passed 3/3 tests, samples/multiproject:bindings:build (isolated copy, the #200 shaded shape) succeeded, and ~/.gradle/kplusplus/tools/0.9.9-pr202review/krapper was freshly created and byte-identical to the linked binary. All test-published artifacts and caches cleaned up afterward; final ~/.m2/repository/com/monkopedia/kplusplus/ matches the baseline the author left.

4. The .api change — legitimate, not a leak

PLUGIN_VERSION was a const val inside KPlusPlusCompilerGradlePlugin's private companion object. Despite the companion being private, Kotlin still compiled the const val to a public static final field on the outer class — the same quirk that keeps PLUGIN_ID/PLUGIN_GROUP/PLUGIN_NAME/TOOL_NAME public today (unchanged by this PR, still in the .api file). So the field's public-ness was almost certainly accidental in the first place. My own repo-wide grep found zero references to KPlusPlusCompilerGradlePlugin.PLUGIN_VERSION from outside the class. The replacement is a top-level internal const val in a different generated file — correctly excluded from the ABI dump, and apiCheck passes against the updated .api file (confirmed via a fresh -p compiler build, not a cached run).

5. The demonstration — bug is genuinely unconstructible, not just detected

There is no literal left anywhere in source to leave stale — PLUGIN_VERSION doesn't exist as a hand-written declaration any more, only as a generated top-level internal const val. I couldn't construct the original failure mode (bump gradle.properties, "forget" to bump PLUGIN_VERSION) because there is no longer a place to forget. That's a strictly stronger result than a guard, and the author states this precisely in the PR body rather than overclaiming.

6. Lagging pins — reasoning holds, guard verified by hand

I independently reproduced all three of the author's claimed verifyConsumedPluginPins behaviors by editing the three pin files myself and running just that task:

  • Mismatched pins (root bumped to 0.3.5, samples left at 0.3.4) -> FAILED, exact message quoted in the PR body.
  • All three pins ahead of the declared version (0.3.9 vs declared 0.3.4) -> FAILED, exact message quoted.
  • All three pins legally one release behind (0.3.3 vs declared 0.3.4) -> BUILD SUCCESSFUL, no false positive.

All edits reverted afterward (git status clean). The decision not to derive the pins from a single value, and not to hardcode "== previous release," is sound reasoning for the stated reason (samples must look like a real standalone consumer; there's no machine-readable notion of "the previous release" that isn't itself brittle).

7. docs/releasing.md

Re-read in full. Step 1 and the artifact-set description now match the implementation (one place to bump, verifyConsumedPluginPins wired into -p compiler check, "nothing that can be left stale").

Full gate — all reproduced myself, fresh worktree /home/jmonk/git/wt-202-review, JAVA_HOME=21, no -PenableClang

gate result
:krapper:nativeTest 304/0 (summed from JUnit XML)
:featuregen:nativeTest 195/0
:cppfixture:nativeTest 2/0
:krapper:noncopyableDeterminismCheck 8/8 PASS
-p compiler build (incl. apiCheck) green, fresh (non-cached) run
:featuregen:kplusplusSync / :cppfixture:kplusplusSync both byte-identical (git status clean after each)
ktlintCheck --rerun-tasks (root) green
-p compiler ktlintCheck not a task — the compiler build has no ktlint plugin wired (pre-existing, matches prior memory re: krapper_parse .kt ktlint debt); not a gap introduced by this PR

No defects found. This is a real fix that removes the bug class rather than adding a detector, verified correct at runtime (not just compile time) with an independently-constructed test that specifically avoided the caching trap that could have masked a bad result.

Merging.

@monkopedia-reviewer monkopedia-reviewer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed: 7d6ed71 — see detailed comments above. Full gate green, mechanism verified genuinely single-source and runtime-correct including through the #200 shading, .api change judged legitimate, demonstration reproduced (bug is now unconstructible), lagging-pin guard independently reproduced in all three modes. Merging.

@monkopedia-reviewer
monkopedia-reviewer merged commit a65d066 into main Jul 29, 2026
@monkopedia-reviewer
monkopedia-reviewer deleted the fix/192-version-guard branch July 29, 2026 19:38
monkopedia-coder pushed a commit that referenced this pull request Jul 29, 2026
One line — `version` in the root `gradle.properties`. That is the whole change,
and it is the direct payoff of #192/#202: the five other version declarations
(three per-module `version = "..."` lines and the hand-written `PLUGIN_VERSION`
literal in plugin source) no longer exist. During 0.3.4 prep the same operation
touched six places, five were bumped, and the sixth was caught only by a
reviewer's grep.

The three consumed pins (root `settings.gradle.kts`, `samples/minimal`,
`samples/multiproject/bindings`) DELIBERATELY stay at 0.3.4. The repo self-hosts
one release behind, so they can only move once 0.3.5 exists on the Portal.
`verifyConsumedPluginPins` (also from #202) was run and accepts this as a legal
one-release lag.

## What 0.3.5 ships

* **#194 / #200 — the reason for this release.** 0.3.4 is BROKEN for
  multi-project consumers: the plugin picked up whatever kotlinx-coroutines the
  root buildscript classloader owned (1.8.0 via kotlin-compiler-runner) rather
  than the 1.11.0 it resolves, and ktor-io calls a method coroutines 1.9+ moved
  onto the interface -> `NoSuchMethodError: Job.invokeOnCompletion$default`. The
  plugin's ksrpc/ktor/coroutines runtime is now shaded and relocated under
  `com.monkopedia.kplusplus.compiler.shaded`, so it is immune to the buildscript
  classloader's contents. `samples/multiproject` is a new canary reproducing the
  exact topology that shipped broken.
* **#196 / #199** — drop-ledger diagnostics: structurally-expected drops (DEDUP,
  the operator== capability gate) demoted to INFO so the capped WARNING budget
  carries actionable entries. Totals still reported per-severity.
* **#195 / #197** — `samples/minimal:nativeTest` repaired (it had not compiled
  since the context-parameter migration) plus the repo's first PR-triggered CI.
* **#198 / #201** — that CI now asserts a non-zero test count from the JUnit XML
  and uploads it; previously it went green whether 3 tests ran or none.
* **#192 / #202** — this release's own prep, made a one-line operation.

## Verification

Full gate re-run on main @ `a65d066` immediately before the bump:

    krapper       304 / 0
    featuregen    195 / 0
    cppfixture      2 / 0
    noncopyableDeterminismCheck  green
    BUILD SUCCESSFUL in 4m 39s (34/34 tasks executed)

`-p compiler check` green at 0.3.5, including `apiCheck` and the new
`verifyConsumedPluginPins`.

## Follow-up, immediately after this publishes

The post-release migration is one change: bump the three consumed pins
0.3.4 -> 0.3.5 AND delete the root `build.gradle.kts` `apply false` workaround
(#193/#194), whose comment names exactly this condition. It cannot go earlier —
`release.yml` builds `:krapper` on main to cut the release, and until 0.3.5
exists the pin cannot move off the unshaded 0.3.4.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLWsoRAe7f1RdQrYJ2ZXqm
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.

No guard against a partial version bump (version lives in 6 places)

2 participants