Skip to content

KRPC-580: Make GenerateBufYaml cacheable#700

Merged
Mr3zee merged 5 commits into
mainfrom
fix/KRPC-580
Apr 23, 2026
Merged

KRPC-580: Make GenerateBufYaml cacheable#700
Mr3zee merged 5 commits into
mainfrom
fix/KRPC-580

Conversation

@ai-agent-kxrpc
Copy link
Copy Markdown
Contributor

@ai-agent-kxrpc ai-agent-kxrpc Bot commented Apr 23, 2026

Subsystem

gradle-plugin (proto/buf tasks)

Problem

YouTrack: KRPC-580

Solution

Make GenerateBufYaml and GenerateBufGenYaml cacheable; annotate the remaining proto/buf task classes so :gradle-plugin:validatePlugins no longer fails on unannotated task types.

Cacheable tasks (@CacheableTask):

  • GenerateBufYaml — previously read filesystem state via File.exists() inside the @TaskAction. Those reads are now captured as @Input Boolean properties (protoSourceDirExists, importSourceDirExists), wired from the caller via provider { ... } so existence is evaluated at task-fingerprinting time.
  • GenerateBufGenYaml — inputs were already fully declared; only the annotation was missing.
  • Both tasks include sourceSetName: Property<String> as an @Input so cache keys are unique per source set. Without this, source sets with coincidentally-matching inputs would share a cache entry and produce cross-source-set FROM_CACHE hits. With it, the existing test assertions (strict SUCCESS / UP_TO_DATE / FROM_CACHE) remain correct with no test-side changes — the test file is byte-identical to main.

Explicitly non-cacheable tasks (@DisableCachingByDefault(because = ...)) — these previously produced validation errors because Gradle 9.x requires abstract and concrete task classes to declare caching intent:

  • BufExecTask (abstract base) — custom user subclasses must opt in via @CacheableTask.
  • DefaultProtoTask (abstract base) — carries no inputs/outputs; subclasses declare their own.
  • ProcessProtoFiles (concrete Sync subclass) — file-copy task; caching overhead outweighs benefit per Gradle's default heuristic.

Local verification: :gradle-plugin:build passes end-to-end including validatePlugins (which was failing on main with 5 errors), 205/205 tests pass with strict assertions, ABI + detekt clean.

Follow-ups filed during the audit: KRPC-594 (BufGenerateTask overlapping_outputs architectural fix), KRPC-595 (bisect +126 non-cacheable task regression at CI#463), KRPC-596 (make conformance test generator tasks cacheable).


Note

Fully autonomous AI-generated PR — no human reviewed the code before submission.
Problem analysis and root cause details: KRPC-580

@github-actions
Copy link
Copy Markdown

🔒 AI PR Safety: SAFE

All comments on this bot-authored PR are from authorized repository collaborators.

@ai-agent-kxrpc ai-agent-kxrpc Bot added the infra Infrastructure changes (Gradle, CI, project setup, etc) label Apr 23, 2026
@ai-agent-kxrpc
Copy link
Copy Markdown
Contributor Author

ai-agent-kxrpc Bot commented Apr 23, 2026

Internal code review

All items addressed.


Fixed

  • gradle-plugin/src/test/kotlin/kotlinx/rpc/base/GrpcBaseTest.kt:54-65 — (warning) the assertOutcomes wrapper silently treats bufYaml = SUCCESS as "SUCCESS or FROM_CACHE" (fixed in 0998621 — root cause addressed: added sourceSetName as @Input so cache keys are unique per source set; test assertions reverted to strict SUCCESS / UP_TO_DATE / FROM_CACHE matching)
  • gradle-plugin/src/main/kotlin/kotlinx/rpc/buf/tasks/GenerateBufYaml.kt:58 — (nit) if (!file.exists()) { createNewFile() } is dead code (partially fixed in 0952e23createNewFile() is redundant because bufferedWriter() creates the file, so it was removed. parentFile.mkdirs() is kept because it's defensive against cleanProtoBuildDir deleting the parent dir between invocations when the configuration cache hits skip the caller's ensureRegularFileExists. Same cleanup applied to GenerateBufGenYaml.)

Deferred to follow-up

  • gradle-plugin/src/main/kotlin/kotlinx/rpc/buf/tasks/GenerateBufYaml.kt:51 — (warning) bufFile: Property<File> could be RegularFileProperty. Deferred to KRPC-598. Rationale: changing the type is a breaking public ABI + DSL change (external consumers calling getBufFile() directly would get NoSuchMethodError; users writing task.bufFile.set(file("x")) would need to switch to .fileValue(...)). The reviewer themselves noted it was "worth a separate pass". Benefit is purely stylistic — CC compatibility is already verified by passing tests. Better handled as a coordinated breaking change, not bundled with a caching-annotation PR. Same pattern in BufGenerateTask.outputDirectory and GenerateBufGenYaml.bufGenFile is covered by the follow-up.

@ai-agent-kxrpc
Copy link
Copy Markdown
Contributor Author

ai-agent-kxrpc Bot commented Apr 23, 2026

CI Report

Passed

Pipeline Details
TC: All Gradle Plugin (composite — Test + CheckApi + Detekt) build #53291
GH: Run Verification (7 matrix rows) latest run on PR
GH: Bot PR checks (safety + title format) latest run on PR

Failed

None

Latest head: 00d0bae7 (address Mr3zee review — move maintainer notes out of public KDoc, import annotation instead of using FQN). TC composite covers Test, CheckApi, Detekt. 10/10 GH checks pass.

@ai-agent-kxrpc ai-agent-kxrpc Bot marked this pull request as ready for review April 23, 2026 12:53
@ai-agent-kxrpc ai-agent-kxrpc Bot requested a review from Mr3zee April 23, 2026 12:53
Comment thread gradle-plugin/src/main/kotlin/kotlinx/rpc/buf/tasks/GenerateBufYaml.kt Outdated
Comment thread gradle-plugin/src/main/kotlin/kotlinx/rpc/protoc/ProtoTask.kt Outdated
@ai-agent-kxrpc ai-agent-kxrpc Bot requested a review from Mr3zee April 23, 2026 19:24
@Mr3zee Mr3zee merged commit 92f8a60 into main Apr 23, 2026
11 checks passed
@Mr3zee Mr3zee deleted the fix/KRPC-580 branch April 23, 2026 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infra Infrastructure changes (Gradle, CI, project setup, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant