SK-3002 stop publishing build-internal jars to Maven Central - #404
Closed
Devesh-Skyflow wants to merge 1 commit into
Closed
SK-3002 stop publishing build-internal jars to Maven Central#404Devesh-Skyflow wants to merge 1 commit into
Devesh-Skyflow wants to merge 1 commit into
Conversation
The flowvault/v1.0.0 bundle staged 11 files, two of which are
build-internal and were about to become permanent public classifiers:
skyflow-flowvault-java-1.0.0-tests.jar
skyflow-flowvault-java-1.0.0-with-common.jar
Both were published for the same reason: they are ATTACHED as secondary
artifacts, and deploy uploads every attached artifact.
-with-common is the japicmp comparison jar. skyvault/pom.xml already
described it as "comparison-only", but shadedArtifactAttached is exactly
what attaches it, so the comment's intent was never enforced. Replaced
with shade's outputFile, which writes the jar and neither replaces the
main artifact nor attaches it. The output path is unchanged, so the
japicmp newVersion path needs no edit.
-tests came from the root pom's maven-jar-plugin test-jar execution,
inherited by every module. Nothing in the repo consumes a test-jar
(no <type>test-jar</type> dependency anywhere) and the release build's
tests run in-module, so the execution is removed.
Note excludeArtifacts on central-publishing-maven-plugin is NOT a usable
alternative: its filter compares against Artifact.getArtifactId(), not
the classifier, so listing skyflow-flowvault-java would drop the main
jar too.
Verified with 'mvn clean install' over the whole repo. Every module now
attaches exactly jar + sources + javadoc:
common-1.0.0{,-sources,-javadoc}.jar
skyflow-java-2.1.1{,-sources,-javadoc}.jar
skyflow-flowvault-java-1.0.0{,-sources,-javadoc}.jar
japicmp:cmp still runs and passes for both skyvault and flowvault, and
target/*-with-common.jar is still produced on disk for it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #404 +/- ##
==========================================
Coverage 91.30% 91.30%
- Complexity 0 454 +454
==========================================
Files 157 157
Lines 6392 6392
Branches 850 850
==========================================
Hits 5836 5836
Misses 364 364
Partials 192 192
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
saileshwar-skyflow
approved these changes
Aug 3, 2026
Collaborator
Author
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
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.
Problem
The
flowvault/v1.0.0bundle staged 11 files. Two of them are build-internal and were about to become permanent public classifiers:Both for the same reason: they are attached as secondary artifacts, and
deployuploads every attached artifact.-with-commonThis is the japicmp comparison jar.
skyvault/pom.xmlalready called it "comparison-only" — butshadedArtifactAttachedis precisely what attaches it, so the documented intent was never actually enforced.Replaced with shade's
<outputFile>, which writes the jar and neither replaces the main artifact nor attaches it. The output path is deliberately unchanged, so the japicmpnewVersionpath needs no edit.-testsFrom the root pom's
maven-jar-plugintest-jarexecution, inherited by every module. Nothing in the repo consumes a test-jar — there is no<type>test-jar</type>dependency anywhere — and the release build's tests run in-module, so the execution is removed.Correction:
excludeArtifactsdoes not work for thisI suggested
excludeArtifactsas the fix in #402. That was wrong and I want it on the record. Its filter compares againstArtifact.getArtifactId(), not the classifier — from 0.11.0'sPublishMojobytecode:-testsand-with-commonshare the artifactIdskyflow-flowvault-javawith the main jar, so listing it would have dropped the artifact you are publishing.excludeArtifactsis for skipping a whole module in a multi-module build, not for pruning classifiers.This PR therefore does not depend on #402 and is based directly on
main.Verification
mvn clean installacross the whole repo. Every module now attaches exactly jar + sources + javadoc:No
-tests, no-with-commonattached anywhere.japicmp:cmpstill runs and passes for both modules, andtarget/*-with-common.jaris still produced on disk for it (confirmed on a clean build).Two observations, not changed here
1.
-with-commonis redundant for flowvault. Each module has a second, id-less shade execution that replaces the main artifact with a fat jar includingcommon. So the main jar and the comparison jar are content-identical:common/Credentials.classskyflow-flowvault-java-1.0.0.jarskyflow-flowvault-java-1.0.0-with-common.jaroriginal-skyflow-flowvault-java-1.0.0.jarjapicmp could point at the main jar and the extra shade execution could go entirely. Left alone as a behavioural change beyond this fix.
2. Both module poms declare
maven-shade-plugintwice, which is the source of this warning on every build, already visible in the release logs:Merging them into one declaration with two executions would fix both this and (1).
Timing
Worth merging before clicking Publish on the staged
1.0.0deployment — otherwise those two classifiers are permanent. If the current deployment is already staged with them, it needs a Drop and a re-run rather than a Publish.🤖 Generated with Claude Code