Skip to content

Fix the merge artifact issue#367

Merged
agrasth merged 4 commits intomainfrom
artifactPathFix
Jan 30, 2026
Merged

Fix the merge artifact issue#367
agrasth merged 4 commits intomainfrom
artifactPathFix

Conversation

@agrasth
Copy link
Contributor

@agrasth agrasth commented Jan 27, 2026

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.
  • Appropriate label is added to the PR for auto generate release notes.

Fix Maven SNAPSHOT artifact duplicate entries in build info

Problem

When running jf mvn install followed by jf mvn deploy with the same build name/number, Maven SNAPSHOT artifacts (especially WARs) appeared as duplicates in build info, causing Release Bundle v2 creation to fail:

422 Unprocessable Entity: Unresolvable build artifact: multi3-3.7-SNAPSHOT.war

Root Cause

mergeArtifacts() compared artifacts only by SHA1. Maven SNAPSHOT WARs get new SHA1s on each rebuild (timestamps, bundled dependencies), so the merge logic treated them as different artifacts.

Solution

Updated mergeArtifacts() with a two-priority strategy:

Priority 1: SHA1 Match → Skip

  • Same checksum = same file, already exists
  • Works for deterministic builds (Go, NPM, Gradle reproducible)

Priority 2: Name + Path Match → Replace

  • Controlled by JFROG_CLI_MERGE_BUILD_INFO_ARTIFACTS_BY_NAME env var (default: enabled)
  • If names match AND isSameLogicalArtifact() confirms same directory/repo → replace with newer artifact
  • Handles non-deterministic builds (Maven SNAPSHOTs, Docker images)

No Match → Add as New

  • Different SHA1 and different name/path → add as separate artifact

New Functions

Function Purpose
isSameLogicalArtifact(existing, new) Returns true if same directory and same repo
extractPathDir(path) Extracts directory portion from artifact path

Disable name-based merging (SHA1 only)

export JFROG_CLI_MERGE_BUILD_INFO_ARTIFACTS_BY_NAME=FALSE

@agrasth agrasth requested a review from bhanurp January 27, 2026 09:35
@agrasth agrasth added the bug Something isn't working label Jan 27, 2026
@agrasth agrasth marked this pull request as ready for review January 28, 2026 10:36
// PRIORITY 2: If SHA1 doesn't match, check by artifact name
// This handles non-deterministic/asymmetric package managers (Maven WAR, Docker images with timestamps)
// where rebuilding produces different checksums but represents the same logical artifact
if !exists && mergeArtifact.Name != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isnt !exists redundant since whenever it is set to true immediately it is set to break. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think without !exists, we'd always check by name, even after finding a SHA1 match, which would cause incorrect double-merging.

}

// Similarly for OriginalDeploymentRepo
if merged.OriginalDeploymentRepo == "" && artifact1.OriginalDeploymentRepo != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this mixing information between 2 different objects? If yes we should only pick either one of them instead copying data from one to another object same goes to path as well.

// PRIORITY 1: Check SHA1 first - if checksums match, artifacts are identical
// This works for deterministic/symmetric package managers (Go, NPM with lockfiles, etc.)
// where the same input always produces the same output checksum
if mergeArtifact.Sha1 != "" {
Copy link
Contributor

@fluxxBot fluxxBot Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't it concerning that sha1 is not present?
should we atleast warn?
can we identify those scenarios where this fields will be empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, we have added a WARN, Thanks!

@agrasth agrasth merged commit 2d0d559 into main Jan 30, 2026
29 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants