Repository navigation
fix(sdk): don't re-run codegen when attaching sources - #419
Merged
Merged
Conversation
maven-source-plugin's `jar` goal forks the lifecycle through generate-sources, so every build ran the antrun `buf generate` execution twice. Each run hits buf.build (BSR) for remote plugins and module deps, and in xtest CI the second pass is what tripped BSR rate limits. `jar-no-fork` packages the source roots the main lifecycle already registered; the sources jar contents are unchanged. DSPX-5177 Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: preventing repeated code generation when the Maven source JAR is attached. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 OSV Scanner (2.6.0)
-
pom.xml
-
OSV Scanner exited with code 128 without a usable report
-
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the Maven trail,
No extra Buf runs chase its tail.
The source JAR takes the quiet way,
With jar-no-fork in place today.
One hop, one build, then off I go!
Comment @coderabbitai help to get the list of available commands.
|
pflynn-virtru
approved these changes
Oct 15, 2026
Contributor
dmihalcik-virtru
added a commit
to opentdf/tests
that referenced
this pull request
Oct 15, 2026
## Summary java-sdk releases don't attach `cmdline.jar` to their GitHub Releases, and `cmdline` isn't published to Maven Central. So every Java release that xtest uses has been built from source with a full `mvn clean install`. That build's `sdk` module runs `buf generate` against buf.build. With a dozen xtest jobs doing this at the same moment, BSR returns `resource_exhausted: too many requests` and fails the run (e.g. [38632037584](https://github.com/opentdf/tests/actions/runs/38632037584) on #571). This is DSPX-5177. Everything `cmdline` depends on (`sdk-pom`, `sdk`, `sdk-pqc-bc`, `sdk-fips-bc`) **is** on Maven Central. This PR builds just the CLI against those published jars. ### Changes (`otdf-sdk-mgr`) - **`install_java_release`:** when the GitHub Release has no `cmdline.jar`, the installer checks Maven Central for `io.opentdf.platform:sdk:<version>`. If it's there, it shallow-clones the tag into a temp dir and runs `mvn -pl .,cmdline package`, so `sdk*` resolves from Central and **no codegen or BSR calls happen**. - The temp dir is outside `src/` so the java `Makefile` doesn't rebuild it. - The build uses its own local repo, `~/.m2/otdf-sdk-mgr-releases`. java-sdk `main` carries the last release's version (currently `0.19.1`), so an `sdk` that a head build `mvn install`ed into `~/.m2/repository` would otherwise shadow the release. I hit this locally: the v0.19.1 `cmdline` failed to compile against `main`'s sdk. - Any failure (not on Central, clone or build error) still raises `InstallError`, so `setup-cli-tool` falls back to today's full source build. - **`versions resolve java latest`:** no longer returns `head: true` when no release has a `cmdline.jar` asset. It's now an ordinary release row and takes the path above. - Behaviour change: in platform-head cells, `latest` is no longer rebuilt against the platform under test via `PLATFORM_BRANCH`. It's tested as published. - **`install stable java`:** drops the `has_cli` filter, which had made it skip Java entirely. No workflow changes: JDK and Maven are already set up before `setup-cli-tool` in every job that installs Java. ## Expected effect In a run like 38632037584, each job's v0.19.1 build no longer calls BSR. Combined with opentdf/java-sdk#419 (which stops `attach-sources` from running codegen a second time), BSR traffic per run drops from about 96 to about 24 `buf generate` passes. Only real heads still build from source. Building each head once per run is a separate follow-up. ## Testing - Maven Central has `sdk` for every release since 0.7.2 except 0.8.0 and 0.17.0. Those fall back to a source build. - `mvn -pl .,cmdline package` checked against an empty local repo: - v0.19.1 (`latest`): builds, zero `buf` calls. - v0.9.0 (LTS): builds, zero `buf` calls, with Maven 3.9.11 as on the runners. Maven 3.10 rejects that tag's `sdk/pom.xml` for a duplicate plugin declaration in the `fuzz` profile. - End to end: `otdf-sdk-mgr install artifact --sdk java --version v0.19.1 --dist-name v0.19.1` takes the new path, and `cmdline.jar --version` reports `{"version":"0.19.1","tdfSpecVersion":"4.3.0"}`, which `cli.sh supports` gates on. - Added `tests/test_installers.py` and updated `test_resolve.py`. `ruff check`, `ruff format`, `pyright` and `pytest` (144 passed) are all clean. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Java installations now use the published command-line package when available. If it isn’t available, installation can build the CLI using the published SDK release. * Selecting the latest Java version now resolves to the newest stable release, whether or not it includes a prebuilt CLI package. * **Bug Fixes** * Installation reports an error when neither the published package nor the fallback build is available. CI can then fall back to a full source build. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
maven-source-plugin'sjargoal forks the lifecycle throughgenerate-sources. Because of that, everymvn install/packageruns the antrunbuf generateexecution insdk/twice. Each run calls buf.build (BSR) for remote plugins and module deps.In xtest CI this doubles our BSR traffic, and the second pass is the one that trips
resource_exhausted: too many requests. For example, opentdf/tests run 38632037584:This PR switches
attach-sourcestojar-no-fork. That goal packages the source roots the main lifecycle has already registered, includingtarget/generated-sourcesviabuild-helper-maven-plugin, without re-running earlier phases.Part of DSPX-5177. Other mitigations, such as local plugins and building once per xtest run, are tracked separately.
Verification
mvn -B package -DskipTests -pl sdk -ambefore and after:antrun:run (generateSources)plus a>>> source:3.3.1:jar (attach-sources) > generate-sourcesforkantrun:run (generateSources), andsource:3.3.1:jar-no-forkwith no forksdk-*-sources.jaris unchanged: the same 980 entries, anddiff -rof the extracted contents (excludingMETA-INF) is clean.mvn -B package -DskipTestspasses.sdk-pqc-bcandsdk-fips-bcpick upjar-no-forkthroughpluginManagement.Summary by CodeRabbit