fix(release): cut dependent releases for Java framework changes - #595
fix(release): cut dependent releases for Java framework changes#595balajinvda wants to merge 3 commits into
Conversation
A change under src/libraries/java/nv-boot-parent rebuilds cloud-tasks, notary, and api-keys in CI but releases none of them. semantic-release runs per registered subproject with semantic-release-monorepo, which scopes commits to the service directory, so a framework-only commit matches no service path and every dependent reports "no relevant changes". The rebuilt artifacts never leave the CI job. github-release now reads the same per-component bazel-java-ci.json descriptors that .github/workflows/bazel.yml reads for its matrix, so the framework-to-service edge stays declared in exactly one place. When semantic-release computes no version for a java-service subproject and at least one commit touched a java-framework path since that service's last release tag, the script cuts a patch release for it. The bump is always a patch, including for a framework feat:. A framework feature adds no capability to a service that has not adopted it, and the service changelog has nothing to substantiate a minor. A service that does adopt a new framework API does so in a commit under its own directory, which semantic-release turns into the correct bump, and the fan-out does not run at all in that case. Guards: semantic-release wins whenever it computed a version, so a push touching both a framework and a service is never double-tagged; the service must already have a stable release tag to bump from; the framework must have changed since that tag; and dry-run prints the tag and notes and creates nothing. Release notes state that the release is dependency-triggered and list the framework commits so a reader of a GitHub Release with no changes in the service directory can see why the version moved. The publish-mode semantic-release invocation now streams its output instead of discarding it, because the fan-out decision depends on whether semantic-release released anything. Signed-off-by: Balaji Ganesan <bganesan@nvidia.com> Co-authored-by: Balaji Ganesan <bganesan@nvidia.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe release tool discovers Java framework dependencies and creates dependent-service patch releases when semantic-release finds no service-local changes. Tests cover discovery, fan-out, tagging, publishing, dry runs, and outcome routing. ChangesJava framework dependency releases
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant auto_release
participant semantic_release
participant finish_semantic_release
participant publish_framework_dependency_release
participant Git
auto_release->>semantic_release: evaluate service-local changes
semantic_release-->>finish_semantic_release: return release outcome
finish_semantic_release->>publish_framework_dependency_release: handle no-release outcome
publish_framework_dependency_release->>Git: inspect framework commits and service tags
publish_framework_dependency_release->>Git: create and push service patch tag
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-fix-release-java-framework-fanout.docs.buildwithfern.com/nvcf |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tools/ci/test-github-release.py (1)
144-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for a framework commit that creates no release.
Every fan-out test uses
fix(nv-boot): bump shared framework. No test covers a framework-onlydocs:orchore:commit. That is the case flagged attools/ci/github-releaselines 534-540, where such a commit currently triggers a patch release for every dependent Java service. Add a test that commits adocs:change underJAVA_FRAMEWORK_PATHand asserts thatpublish_framework_dependency_releasereturnsFalse. The coding guidelines require tests for changed tool behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/ci/test-github-release.py` around lines 144 - 159, Add a test alongside test_framework_change_fans_out_a_patch_release_to_every_dependent_service that commits a docs-only change under JAVA_FRAMEWORK_PATH, invokes publish_framework_dependency_release, and asserts it returns False. Reuse the existing temporary Java repository setup and framework commit helpers where applicable, without asserting or creating dependent service releases.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/ci/github-release`:
- Around line 622-628: Update resolve_release_outcome so the "released" result
is returned only when exit_code is zero and parse_next_version(output) succeeds.
Preserve the existing no-release and unknown classification behavior, ensuring
non-zero exits with a parsed version are treated as unknown.
- Around line 534-540: Update framework_commits_since to filter its git log
results to release-worthy conventional commit types only, excluding docs, chore,
ci, test, refactor, and other non-releasing changes while preserving the
existing commit format and empty-list skip. Also update the framework fan-out
documentation section to state that only release-worthy framework commits
trigger dependent Java service releases.
---
Nitpick comments:
In `@tools/ci/test-github-release.py`:
- Around line 144-159: Add a test alongside
test_framework_change_fans_out_a_patch_release_to_every_dependent_service that
commits a docs-only change under JAVA_FRAMEWORK_PATH, invokes
publish_framework_dependency_release, and asserts it returns False. Reuse the
existing temporary Java repository setup and framework commit helpers where
applicable, without asserting or creating dependent service releases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 01b7e044-dbb1-4361-b4b8-f9414b0f71e1
📒 Files selected for processing (3)
docs/dev/github-release-process.mdtools/ci/github-releasetools/ci/test-github-release.py
The first pass fanned out on any commit touching a framework path, so a framework-only docs: or chore: commit would have cut a patch release for every dependent Java service. That contradicts the release rules the same script configures semantic-release with, where those types release nothing. Framework commits are now filtered through the configured RELEASE_RULES: feat, fix, perf, and a breaking ! marker fan out; the other declared types and a subject that is not a Conventional Commit do not. The rules are read from RELEASE_RULES rather than restated, so the fan-out and the per-service release rules cannot diverge. Raised in review on PR #595. Signed-off-by: Balaji Ganesan <bganesan@nvidia.com> Co-authored-by: Balaji Ganesan <bganesan@nvidia.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
resolve_release_outcome classified a run as "released" whenever the output contained a next version, ignoring the exit code. A dry-run that printed a version and then died was reported as a clean preview of a tag the publish run may never create, and it never reached dry_run_failures. A non-zero exit is now "unknown" regardless of what the run printed, so it is surfaced in the dry-run preview summary. Publish mode is unaffected: it already raises on a non-zero exit before the outcome is classified. The preview warning no longer claims "no NEXT_VERSION", which was inaccurate for a run that printed one before failing. Raised in review on PR #595. Signed-off-by: Balaji Ganesan <bganesan@nvidia.com> Co-authored-by: Balaji Ganesan <bganesan@nvidia.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sanjay-saxena
left a comment
sanjay-saxena
left a comment
There was a problem hiding this comment.
@balajinvda -- I read the description and it makes sense.
However, the PR description has incorrect versions. These are the current version numbers:
nvct-service-oss: 1.6.2notary-service-oss: 1.8.1nvcf-api-keys-service: 1.6.0
We should make sure that the next patch version or minor version change is based on these versions.
I could not find anywhere in the MR where we have hardcoded the base/current versions. Perhaps, we have them in the nvcf-internal repo where we the CI runs.
|
@sanjay-saxena thanks. You are right that the description was misleading, and the answer to your question is: the base versions are not hardcoded anywhere, and they are not in another repo either. They come from Git tags in this repository. Where the base version comes fromThe chain is entirely inside
So the base version is simply the highest existing tag on The description was wrong, the mechanism is notThe table I published was generated by running the new helpers against No code change is needed for this. Once the anchors are right, the same code computes the right numbers. What has to happen before this can be enabledOne detail worth calling out, because it makes the ordering mandatory rather than cosmetic.
Verified by running the function. So the wrong anchors have to be deleted, not just superseded:
Merging this PR before that is safe. One thing I need you to confirmYou listed api-keys at 1.6.0, but Is |
sanjay-saxena
left a comment
sanjay-saxena
left a comment
There was a problem hiding this comment.
@balajinvda - Latest maven-based tag for api-keys is 1.6.0 as shown here. It's not clear where 1.6.1 is coming from. I don't mind if we use 1.6.1 as the baseline for api-keys.
2 participants
Why
A change to the shared Java framework
src/libraries/java/nv-boot-parentrebuilds every dependent Java service in CI but releases none of them.
The build-side edge already exists.
.github/workflows/bazel.ymldiscoversJava rows from per-component
bazel-java-ci.jsondescriptors, setsjava_framework_changed=trueat line 249 when a changed path sits under acomponent_kind: java-frameworkcomponent, and at line 326 schedules everycomponent_kind: java-servicerow. Socloud-tasks,notary, andapi-keysare built and tested against the new framework.
The release side has no matching edge.
tools/ci/github-release autorunsnpx semantic-releaseper registered subproject withsemantic-release-monorepo(
write_semantic_release_files, line 367), which scopes commits to the servicedirectory. A commit touching only
src/libraries/java/nv-boot-parentmatches noregistered service path, so every dependent reports "no relevant changes" and
cuts nothing.
nv-boot-parentis not itself one of the 42 registeredsubprojects in
tools/ci/github-release-subprojects.json, so nothing isreleased for the framework either.
Net effect: a framework change can alter the behaviour of three services, pass
CI, and ship no new version of any of them. The rebuilt artifacts exist only
inside the CI job.
This is not hypothetical on current
main. HEAD isfeat(nv-boot): discover NGC auth via authenticate challenge (#557), aframework-only commit that landed after the last release tag of all three
dependents.
What changed
tools/ci/github-releasegains a dependency fan-out for Java services.java_ci_components()reads the samesrc/**/bazel-java-ci.jsondescriptorsbazel.ymlreads. The framework-to-service edge stays declared in exactly oneplace; no second list is introduced and no field is added to the generated
release metadata (its generator is not mirrored here).
publish_framework_dependency_release()cuts a patch release through theexisting
publish_tag_for_version()helper, with areasonthat says therelease is dependency-triggered and lists the framework commits.
resolve_release_outcome()/finish_semantic_release()classify asemantic-release run. semantic-release wins whenever it computed a version, so
a push touching both a framework and a service is never double-tagged.
auto_release()calls the fan-out on both paths that end in "nothing torelease": the
only_generated_changes()early return, and thesemantic-release "no relevant changes" result.
npx semantic-releaseinvocation now streams its outputinstead of discarding it, because the fan-out decision depends on whether
semantic-release released anything. A non-zero exit still raises, as before.
Guards, all of which have tests:
java-servicedescriptor are eligible.X.Y.Zrelease tag to bump from.that tag, bounded by
latest_service_tag(), so a rerun over the sameframework commit is a no-op. Release-worthiness is derived from the existing
RELEASE_RULESconstant, so a frameworkdocs:orchore:commit fans outnothing, exactly as it would release nothing inside a service directory.
auto_tagging_enabled=falseare honored by reusingpublish_tag_for_version(), which prints the tag and notes and createsnothing.
Design decision: patch, not minor, even for a framework
feat:The synthesized bump is always a patch, including when the framework commit is a
feat:. Reasons, in order:The service's public surface is unchanged; only its build inputs moved. Minor
would advertise functionality the service does not have.
nothing in that service's changelog to substantiate it.
can never consume a service's minor line or collide with a planned minor.
Minor and major bumps remain owned by commits in the service's own directory.
its own directory. semantic-release computes the correct bump from that
commit, and this fan-out does not run at all in that case.
The same argument applies more strongly to a framework
BREAKING CHANGE: asilent major fan-out would be the most surprising outcome of all, so it is not
implemented. A breaking framework change should be adopted deliberately in each
service.
This behaviour is documented in
docs/dev/github-release-process.mdunder"Java framework dependency releases", so it is not a surprise at the console.
Customer Release Notes
Not customer visible. Release automation only.
Plan Summary
Not applicable. No infrastructure, chart, or deploy changes.
Usage
No new commands. The existing entrypoint covers it:
Behaviour is unchanged for every non-Java subproject and for Java subprojects
whose own directory changed.
Testing
tools/ci/test-github-release.py(run by.github/workflows/build-test.yml)goes from 7 tests to 20, all passing:
New coverage:
dependent service
docs:/chore:/refactor:/ non-conventional commits fan outnothing, and the release notes quote only the release-worthy commits
semantic-release version wins and no extra tag is created
dependency-triggered notes to
create_releasejava-servicedescriptors and registered subprojects are checked for driftagainst the real repo, so a new Java service that is not registered fails the
suite instead of silently never releasing
releases_a_version()matches the configuredRELEASE_RULESfor everydeclared type
produces neither a preview tag nor a dependency fan-out
Also validated against real
mainstate by running the new helpers over theactual checkout.
Read this table as a demonstration of the mechanism against the anchors that
exist on
maintoday, not as the versions these services should be released at.Two of the three anchors are on the wrong version line, which is the merge-order
dependency described under Notes. The "would cut" column is what the fan-out
computes from the current, incorrect anchors.
maintodaysrc/control-plane-services/cloud-tasks/v1.62.2src/control-plane-services/notary/v1.12.0src/control-plane-services/api-keys/v1.6.1Not run: an end-to-end
github-release autoagainstmain, which needs npm andnetwork access to install semantic-release. The semantic-release interaction is
covered by feeding recorded semantic-release output through
finish_semantic_release().Notes
Where the base version comes from
There is no hardcoded base or current version anywhere in this PR, and none in
this repository. The base version is read from Git tags in this repository:
tools/ci/github-release-subprojects.jsongives each service itspath.default_tag_format()(line 121) turns that path into<path>/v${version},and
tag_prefix()(line 132) strips the placeholder to get the tag prefix.latest_service_tag()(line 282) runsgit tag -l "<prefix>*"and returnsthe semver-max match, ordered by
semverish_sort_key()(line 117).publish_framework_dependency_release()(line 598) takes that tag, extractsthe version with
version_from_tag()(line 159), and increments the patchwith
next_patch_version()(line 531).So the base version is whatever the highest existing tag on
mainsays. Theservice
pom.xmlfiles are all0.0.1-SNAPSHOTand are not a version source,and none of these three services has a
VERSIONfile. Nothing in this codepath reads a version from outside this repository.
Merge-order dependency: fix the version anchors first
Because the base version is the semver-max existing tag, a wrong anchor
propagates.
notaryandcloud-tasksare on the wrong version line today:mainsrc/control-plane-services/cloud-tasks/v1.62.2src/control-plane-services/notary/v1.12.0Adding a correct anchor is not sufficient on its own.
semverish_sort_key()compares numerically per component, so with both
v1.12.0and a newv1.8.1present, the max is still
v1.12.0, and with bothv1.62.2and a newv1.6.2present, the max is still
v1.62.2. Verified against the function.Required order before auto-tagging is enabled:
src/control-plane-services/notary/v1.12.0andsrc/control-plane-services/cloud-tasks/v1.62.2.notaryat 1.8.1 andcloud-tasksat 1.6.2.NVCF_GITHUB_AUTO_TAGGING_ENABLED=true.Merging this PR before that is safe:
NVCF_GITHUB_AUTO_TAGGING_ENABLEDdefaultsto
false, sorelease-tags.ymlruns dry-run only and creates no tag and noRelease.
api-keysneeds confirmation, not correction:maincarries bothsrc/control-plane-services/api-keys/v1.6.0andv1.6.1, solatest_service_tag()returns
v1.6.1and the fan-out would cut 1.6.2. Ifv1.6.1is a real releasethat is already correct. If it is a stray anchor and 1.6.0 is current, it should
be deleted along with the other two.
Scope
nv-boot-parentitself is still not a registered subproject and still cuts notag of its own. This PR does not change that; it makes the framework change
reach released artifacts through its dependents. Registering the framework as
its own release line is a separate decision.
which is exactly what
bazel.ymlimplements today. If a per-service frameworkallowlist is ever needed, it belongs in the descriptors so both consumers pick
it up.
References
Fixes #594
Related Merge Requests/Pull Requests
None.
Review follow-ups
framework path, including a framework-only
docs:orchore:. Fixed inb9f41b8: the commit list is now filtered through
RELEASE_RULES.resolve_release_outcome()ignored the exit code when theoutput contained a version, so a dry-run that printed a version and then died
previewed a tag the publish run may never create. Fixed in 6ba76cd: any
non-zero exit is
unknownand lands in the dry-run preview summary. Publishmode is unaffected; it already raises on a non-zero exit.
Dependencies
None. No new third-party packages; standard library only.
Issues
Fixes #594
Checklist