Skip to content

Add disableProtobufVersionCheck to the compiler { } DSL - #95

Open
alexander-yevsyukov wants to merge 11 commits into
masterfrom
disable-protobuf-version-check
Open

alexander-yevsyukov wants to merge 11 commits into
masterfrom
disable-protobuf-version-check

Conversation

@alexander-yevsyukov

@alexander-yevsyukov alexander-yevsyukov commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Why

The Protobuf Java runtime refuses to load generated code that is not compatible with it —
for example, code generated by a newer protoc. Since #90, the Compiler runs on the highest
Protobuf runtime requested by the CLI and by its plugins, which avoids the failure in most
cases. It can still happen, e.g. when a build forces a Protobuf runtime older than the one
the Compiler plugins were built with.

The only off-switch is the TEMPORARILY_DISABLE_PROTOBUF_VERSION_CHECK environment variable,
which so far had to be exported for the build or set on each LaunchSpineCompiler task by hand.

What

A convenience shortcut in the compiler { } DSL:

spine {
    compiler {
        disableProtobufVersionCheck.set(true)
    }
}

The property controls the variable in the environment of every LaunchSpineCompiler fork:

Setting Environment of the Compiler process
not set (default) inherited from the build, unchanged
true variable set to true — check off
false variable removed — check on, even if the build exports it

The default is "not set" rather than false: a default that removed the variable would
silently break builds relying on the exported variable — including this repo's own functional
tests, which export it for every fixture build (RunnerTuning.tuneRunner()).

  • CompilerSettings (gradle-api) — new disableProtobufVersionCheck: Property<Boolean>,
    documented with its three states, its scope, and an example.
  • Extension — implements it, not set by default.
  • LaunchSpineCompiler — no copy of the setting on the task: applyDefaults() applies
    the extension's value to the environment when the task is configured.
  • gradle-plugin/README.md — a table row and a "Protobuf version check" section.
  • Version 2.0.0-SNAPSHOT.079 (.078 went to Resolve the Protobuf runtime of the Compiler JVM across both classpaths #90).

Design notes

  • Applied when the task is configured. Gradle (verified on 9.7.1 and 9.8.1) converts
    JavaExec environment values with String.valueOf() and does not unpack providers, so
    the value is read in applyDefaults(). The launch tasks are realized in the
    afterEvaluate hooks, after the build script body, so the compiler { } block has run
    by then. The extension property is finalizeValueOnRead(): a change after the tasks
    read it fails loudly instead of being ignored. A user's own
    tasks.withType<LaunchSpineCompiler> { environment(...) } runs later and still wins.
  • Scope. The setting does not affect the Gradle daemon or the Java protoc plugins;
    the docs point users to the environment variable for those.
  • Not a task input. The check does not change the generated code, so toggling
    the setting does not invalidate the build cache.

Testing

  • DisableProtobufVersionCheckSpec (functional, new): reuses the protobuf-runtime-test
    fixture of Resolve the Protobuf runtime of the Compiler JVM across both classpaths #90 with the plugin built on 4.37.0-RC1 and the CLI runtime forced below it,
    so the check really fails in the fork. The fixture gets a keepVersionCheckVariable
    property, so that only the setting under test changes the variable in the fork.
    • false, with the variable exported for the build → the task fails with
      ProtobufRuntimeVersionException;
    • true, with no variable in the environment of the build → the Compiler runs.
  • LaunchTaskEnvironmentSpec (new): the extension's value reaches the task; a change
    after the task is configured is rejected; not set keeps an inherited value; true sets
    the variable; false removes it.
  • ExtensionSpec: not set by default.
  • ./gradlew build dokkaGenerate passes locally.

🤖 Generated with Claude Code

alexander-yevsyukov and others added 3 commits October 19, 2026 20:24
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Setting `compiler { disableProtobufVersionCheck.set(true) }` makes every
`LaunchSpineCompiler` task pass `TEMPORARILY_DISABLE_PROTOBUF_VERSION_CHECK=true`
to the forked Compiler JVM. This is a shortcut for users hitting the Protobuf
gencode/runtime version check, e.g. when their Compiler plugins are built with
a different Protobuf version.

The variable is added in `exec()` because Gradle does not unpack providers
in `JavaExec` environment values, and the task may be configured before the
`compiler { }` block runs. When the flag is `false`, the inherited environment
is left intact, so a variable exported for the build still reaches the fork.
The setting does not affect the Gradle daemon or the `protoc` plugins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 19:26
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-19T19:29:33.472502Z f36d66e PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

…rsion-check

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI previously approved these changes Oct 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approved

Static review found no blocking issues in the focused configuration change and regression coverage; tests were not rerun.

No open issues 🎉

What changed in this PR

Adds an opt-in compiler { } setting to disable Protobuf version checks in the forked Compiler JVM without changing the build environment.

Changes:

  • Adds a default-false property, evaluated when launch tasks execute.
  • Documents its scope and tests late configuration and environment preservation.
  • Bumps the version to 2.0.0-SNAPSHOT.079 and refreshes dependency reports.
File Description
version.gradle.kts Bumps the Compiler version.
gradle-plugin/​src/​test/​kotlin/​io/​spine/​tools/​compiler/​gradle/​plugin/​LaunchTaskEnvironmentSpec.kt Tests environment handling and late configuration.
gradle-plugin/​src/​test/​kotlin/​io/​spine/​tools/​compiler/​gradle/​plugin/​ExtensionSpec.kt Tests the default-false setting.
gradle-plugin/​src/​main/​kotlin/​io/​spine/​tools/​compiler/​gradle/​plugin/​LaunchSpineCompiler.kt Wires the setting and updates the forked process environment.
gradle-plugin/​src/​main/​kotlin/​io/​spine/​tools/​compiler/​gradle/​plugin/​Extension.kt Implements the property with its default.
gradle-plugin/​README.md Documents usage and process scope.
gradle-api/​src/​main/​kotlin/​io/​spine/​tools/​compiler/​gradle/​api/​CompilerSettings.kt Exposes and documents the DSL property.
docs/​dependencies/​pom.xml Updates the reported version.
docs/​dependencies/​dependencies.md Refreshes report versions and timestamps.
.agents/​tasks/​disable-protobuf-version-check.md Records the implementation plan and progress.

Review effort: Balanced

alexander-yevsyukov and others added 2 commits October 19, 2026 20:29
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since `align-protobuf-runtime` (#90), the Compiler runs on the highest
Protobuf runtime requested by the CLI and by its plugins, so a newer plugin
runtime no longer fails the check. The docs now name the remaining case:
a build forcing a runtime older than the one the plugins were built with.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 19:40
Copilot AI dismissed their stale review, a newer Copilot review was requested October 19, 2026 19:40
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI previously approved these changes Oct 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approved

The narrowly scoped implementation preserves default behavior and includes focused regression coverage, with no blocking issues identified.

No open issues 🎉

Review effort: Balanced

alexander-yevsyukov and others added 2 commits October 19, 2026 20:48
`disableProtobufVersionCheck.set(false)` used to leave the environment of
the Compiler process as is, so a variable exported for the build still turned
the check off. Now `false` removes the variable, enforcing the check.

The property is no longer `false` by default but not set: a default that
removed the variable would break builds relying on the exported variable,
including the functional tests of this repo. When not set, the Compiler
process inherits the variable from the build, as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`DisableProtobufVersionCheckSpec` reuses the `protobuf-runtime-test` fixture,
which turns the check back on in the Compiler JVM. The plugin of the fixture is
built with a Protobuf release newer than the CLI's runtime, and the runtime of
the CLI configuration is forced to stay at the CLI's version, so the check
fails for real:

 - `false`, with the variable set as if exported for the build, fails with
   `ProtobufRuntimeVersionException`;
 - `true` lets the Compiler run.

The Protobuf versions and `isNewer()` of `ProtobufRuntimeSpec` become
`internal` for the reuse.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 19:48
Copilot AI dismissed their stale review, a newer Copilot review was requested October 19, 2026 19:48
Copilot AI previously approved these changes Oct 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approved

Static review found no blocking issues, and implementation and test coverage match the documented three-state behavior; tests were not run here.

No open issues 🎉

Review effort: Balanced

@codecov

codecov Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.41%. Comparing base (cb683ca) to head (f8f6717).

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #95      +/-   ##
============================================
+ Coverage     77.20%   77.41%   +0.21%     
- Complexity      698      704       +6     
============================================
  Files           206      207       +1     
  Lines          4066     4104      +38     
  Branches        405      407       +2     
============================================
+ Hits           3139     3177      +38     
  Misses          796      796              
  Partials        131      131              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

alexander-yevsyukov and others added 2 commits October 19, 2026 21:23
`LaunchSpineCompiler` no longer keeps a copy of `disableProtobufVersionCheck`
and no longer amends its environment in `exec()`. `applyDefaults()` applies
the extension's value instead, keeping the whole task state set at
configuration time. The launch tasks are realized in the `afterEvaluate`
hooks, after the build script body, so the `compiler { }` block has run.

The extension property is `finalizeValueOnRead()`, so a change after the
tasks read it fails instead of being silently ignored. A user's own
`environment(...)` call on the task still wins, as it runs later.

The `when` over the setting uses `else` for the unset case, which avoids
an unreachable exhaustiveness branch in the bytecode (reported as partial
coverage).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The setting is now applied by the first configuration action of the task,
so the fixture's own `environment.remove(...)` would override it. The
fixture keeps the variable of the build when `keepVersionCheckVariable`
is set, which `DisableProtobufVersionCheckSpec` does:

 - `false` runs with the variable exported for the build;
 - `true` runs with no variable in the environment of the build.

So in each case only the setting under test changes the outcome.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 20:23
Copilot AI dismissed their stale review, a newer Copilot review was requested October 19, 2026 20:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approved

The focused implementation matches the documented three-state behavior and includes unit and functional coverage; tests were not run during this review.

No open issues 🎉

Review effort: Balanced

@alexander-yevsyukov alexander-yevsyukov moved this from 🏗 In progress to In Review in v2.0 Oct 19, 2026

This branch has not been deployed

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

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

3 participants