RTECO-992 - Restore build-info for jf uv build, clear artifacts only#449
Conversation
jf uv build creates local dist/ files but uploads nothing to Artifactory. PR jfrog#438 skipped build-info entirely which broke TestUvBuild, TestUvArtifactTypeIsExtension and TestUvCustomModule — all expected at least a module to exist. Following Maven's behavior for non-deploy goals: record dependencies (what was resolved to produce the wheel/sdist) but no artifacts (nothing in Artifactory yet). Artifacts are captured later by jf uv publish.
agrasth
requested review from
a team,
bhanurp,
fluxxBot,
itsmeleela,
naveenku-jfrog and
reshmifrog
| // "build" only creates local dist/ files — nothing is uploaded to Artifactory, | ||
| // so it must not save a partial build-info. Saving unenriched deps from "build" | ||
| // would create duplicate dependency entries when merged with a prior "sync" partial. | ||
| } else { |
There was a problem hiding this comment.
The removed comment flagged a dep-duplication risk for workflows that run jf uv sync then jf uv build under the same build name. Now that build enters the collection path, does jf rt bp deduplicate deps across partials? If yes, this is fine - please add a comment confirming. If no, we may want bi.Modules[0].Dependencies = nil in the build case too (same as publish clears deps, build would clear artifacts and deps).
There was a problem hiding this comment.
jf rt bp does NOT deduplicate — it keys by Id+Sha1+Md5+Scopes, so enriched (from sync) and unenriched (from build) deps are treated as different entries. Added Dependencies = nil to the build block to prevent duplicates.
| // for non-deploy goals. | ||
| if cmdName == "build" && len(bi.Modules) > 0 { | ||
| bi.Modules[0].Artifacts = nil | ||
| } |
There was a problem hiding this comment.
Same [0] pattern as the publish case above, fine for now since UV produces a single module, but if we ever support workspace builds this will need a range loop. Consider a // TODO: loop over all modules if workspace support is added comment to match the implicit contract of the publish block.
There was a problem hiding this comment.
Added // TODO: loop over all modules if workspace support is added, will create a ticket for this.
…omments - Clear dependencies for build command: jf rt bp deduplicates via key(Id+Sha1+Md5+Scopes), so an unenriched build dep and an enriched sync dep produce two separate entries for the same package — duplicates. Nil-ing deps in build avoids this when sync ran first under same build name. - Add TODO comments on both publish and build [0] blocks noting they need a range loop if UV workspace support is ever added.
naveenku-jfrog
left a comment
naveenku-jfrog
left a comment
There was a problem hiding this comment.
Please address comments.
4 participants
Problem
PR #438 fixed duplicate dependency entries by skipping build-info entirely for
jf uv build. This was too aggressive — it broke three tests that expected at least a module to exist:TestUvBuild— expects 1 module, 0 artifactsTestUvArtifactTypeIsExtension— expects module existsTestUvCustomModule— expects module with custom IDFix
Restore build-info collection for
jf uv build, following Maven's behavior for non-deploy goals:--module,jf rt bp, custom naming)uv builduploads nothing to Artifactory; artifacts are captured byjf uv publishThis matches how
jf mvn compile/packageworks: dependencies are tracked, artifacts are only recorded when something is actually deployed.Changes
native_uv.go Run(): removed the!= "build"guard souvGetBuildInfois called forbuildnative_uv.go uvGetBuildInfo(): addedcase "build"block that clearsArtifacts(keeps dependencies)Related
jf uv publish)