Skip to content

Commit 2d0d559

Browse files
authored
Fix the merge artifact issue (#367)
1 parent d7f367b commit 2d0d559

File tree

4 files changed

+447
-6
lines changed

4 files changed

+447
-6
lines changed

build/build.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,9 @@ func (b *Build) ToBuildInfo() (*entities.BuildInfo, error) {
166166
if err != nil {
167167
return nil, err
168168
}
169+
// Sort generated build infos by Started timestamp (oldest first)
170+
// This ensures artifacts are merged in chronological order
171+
sortBuildInfosByTimestamp(generatedBuildsInfo)
169172
for _, v := range generatedBuildsInfo {
170173
buildInfo.Append(v)
171174
}
@@ -209,6 +212,20 @@ func (b *Build) getGeneratedBuildsInfo() ([]*entities.BuildInfo, error) {
209212
return generatedBuildsInfo, nil
210213
}
211214

215+
// sortBuildInfosByTimestamp sorts build infos by their Started timestamp (oldest first).
216+
// This ensures that when merging, newer artifacts replace older ones correctly.
217+
func sortBuildInfosByTimestamp(buildInfos []*entities.BuildInfo) {
218+
sort.Slice(buildInfos, func(i, j int) bool {
219+
timeI, errI := time.Parse(entities.TimeFormat, buildInfos[i].Started)
220+
timeJ, errJ := time.Parse(entities.TimeFormat, buildInfos[j].Started)
221+
// If parsing fails, maintain original order for those items
222+
if errI != nil || errJ != nil {
223+
return false
224+
}
225+
return timeI.Before(timeJ)
226+
})
227+
}
228+
212229
func (b *Build) SaveBuildInfo(buildInfo *entities.BuildInfo) (err error) {
213230
buildJson, err := json.Marshal(buildInfo)
214231
if err != nil {

build/build_test.go

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
package build
22

33
import (
4-
"github.com/jfrog/build-info-go/entities"
5-
"github.com/stretchr/testify/assert"
64
"os"
75
"testing"
6+
7+
"github.com/jfrog/build-info-go/entities"
8+
"github.com/stretchr/testify/assert"
89
)
910

1011
func TestCollectEnv(t *testing.T) {
@@ -79,3 +80,71 @@ func TestCollectEnv(t *testing.T) {
7980
})
8081
}
8182
}
83+
84+
func TestSortBuildInfosByTimestamp(t *testing.T) {
85+
tests := []struct {
86+
name string
87+
buildInfos []*entities.BuildInfo
88+
expectedOrder []string // Expected order of Started timestamps
89+
}{
90+
{
91+
name: "already sorted",
92+
buildInfos: []*entities.BuildInfo{
93+
{Name: "build1", Started: "2026-01-30T10:00:00.000+0000"},
94+
{Name: "build2", Started: "2026-01-30T11:00:00.000+0000"},
95+
{Name: "build3", Started: "2026-01-30T12:00:00.000+0000"},
96+
},
97+
expectedOrder: []string{
98+
"2026-01-30T10:00:00.000+0000",
99+
"2026-01-30T11:00:00.000+0000",
100+
"2026-01-30T12:00:00.000+0000",
101+
},
102+
},
103+
{
104+
name: "reverse order",
105+
buildInfos: []*entities.BuildInfo{
106+
{Name: "build3", Started: "2026-01-30T12:00:00.000+0000"},
107+
{Name: "build2", Started: "2026-01-30T11:00:00.000+0000"},
108+
{Name: "build1", Started: "2026-01-30T10:00:00.000+0000"},
109+
},
110+
expectedOrder: []string{
111+
"2026-01-30T10:00:00.000+0000",
112+
"2026-01-30T11:00:00.000+0000",
113+
"2026-01-30T12:00:00.000+0000",
114+
},
115+
},
116+
{
117+
name: "mixed order - Maven install then deploy scenario",
118+
buildInfos: []*entities.BuildInfo{
119+
{Name: "deploy", Started: "2026-01-30T10:05:30.000+0000"}, // deploy (later)
120+
{Name: "install", Started: "2026-01-30T10:05:00.000+0000"}, // install (earlier)
121+
},
122+
expectedOrder: []string{
123+
"2026-01-30T10:05:00.000+0000", // install first
124+
"2026-01-30T10:05:30.000+0000", // deploy second
125+
},
126+
},
127+
{
128+
name: "empty list",
129+
buildInfos: []*entities.BuildInfo{},
130+
expectedOrder: []string{},
131+
},
132+
{
133+
name: "single item",
134+
buildInfos: []*entities.BuildInfo{
135+
{Name: "build1", Started: "2026-01-30T10:00:00.000+0000"},
136+
},
137+
expectedOrder: []string{"2026-01-30T10:00:00.000+0000"},
138+
},
139+
}
140+
141+
for _, tc := range tests {
142+
t.Run(tc.name, func(t *testing.T) {
143+
sortBuildInfosByTimestamp(tc.buildInfos)
144+
for i, expected := range tc.expectedOrder {
145+
assert.Equal(t, expected, tc.buildInfos[i].Started,
146+
"Position %d: expected %s, got %s", i, expected, tc.buildInfos[i].Started)
147+
}
148+
})
149+
}
150+
}

entities/buildinfo.go

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,28 @@ package entities
22

33
import (
44
"errors"
5+
"fmt"
6+
"os"
57
"regexp"
68
"sort"
79
"strings"
810
"time"
911

1012
"github.com/jfrog/build-info-go/utils/compareutils"
13+
"github.com/jfrog/gofrog/log"
1114
"golang.org/x/exp/maps"
1215
"golang.org/x/exp/slices"
1316

1417
cdx "github.com/CycloneDX/cyclonedx-go"
1518
"github.com/jfrog/gofrog/stringutils"
1619
)
1720

21+
// MergeArtifactsByNameEnv controls whether artifacts with the same name but different SHA1s
22+
// should be merged (keeping the newer one). This handles non-deterministic builds like Maven SNAPSHOTs.
23+
// Set to "FALSE" to disable name-based merging and only use SHA1 comparison.
24+
// Default: enabled (any value other than "FALSE")
25+
const MergeArtifactsByNameEnv = "JFROG_CLI_MERGE_BUILD_INFO_ARTIFACTS_BY_NAME"
26+
1827
type ModuleType string
1928

2029
const (
@@ -255,20 +264,70 @@ func mergeModules(merge *Module, into *Module) {
255264
}
256265

257266
func mergeArtifacts(mergeArtifacts *[]Artifact, intoArtifacts *[]Artifact) {
258-
for _, mergeArtifact := range *mergeArtifacts {
267+
mergeByNameEnabled := strings.ToUpper(os.Getenv(MergeArtifactsByNameEnv)) != "FALSE"
268+
log.Debug(fmt.Sprintf("Merge artifacts by name enabled: %v", mergeByNameEnabled))
269+
270+
for _, newArtifact := range *mergeArtifacts {
259271
exists := false
260-
for _, artifact := range *intoArtifacts {
261-
if mergeArtifact.Sha1 == artifact.Sha1 {
272+
273+
// PRIORITY 1: Check SHA1 - if checksums match, artifact already exists (skip it)
274+
for _, existingArtifact := range *intoArtifacts {
275+
if newArtifact.Sha1 == existingArtifact.Sha1 {
262276
exists = true
263277
break
264278
}
265279
}
280+
281+
// PRIORITY 2: If SHA1 doesn't match, check by artifact name
282+
// This handles non-deterministic builds (Maven WAR with timestamps, etc.)
283+
// where rebuilding produces different checksums for the same logical artifact.
284+
if !exists && mergeByNameEnabled {
285+
for i, existingArtifact := range *intoArtifacts {
286+
if newArtifact.Name == existingArtifact.Name && isSameLogicalArtifact(existingArtifact, newArtifact) {
287+
// Same logical artifact but different checksum
288+
// Use incoming artifact (from later merge operation)
289+
log.Warn(fmt.Sprintf("Artifact '%s' has different checksums (%s vs %s). Using incoming artifact.",
290+
newArtifact.Name, existingArtifact.Sha1, newArtifact.Sha1))
291+
(*intoArtifacts)[i] = newArtifact
292+
exists = true
293+
break
294+
}
295+
}
296+
}
297+
298+
// No match found - add as new artifact
266299
if !exists {
267-
*intoArtifacts = append(*intoArtifacts, mergeArtifact)
300+
*intoArtifacts = append(*intoArtifacts, newArtifact)
268301
}
269302
}
270303
}
271304

305+
// isSameLogicalArtifact checks if two artifacts represent the same logical artifact.
306+
// Returns true if they are in the same directory and same repository.
307+
func isSameLogicalArtifact(existing, new Artifact) bool {
308+
// Check if paths are in the same directory
309+
if extractPathDir(existing.Path) != extractPathDir(new.Path) {
310+
return false
311+
}
312+
313+
// Check repo - reject if BOTH have repos AND they differ
314+
if existing.OriginalDeploymentRepo != "" && new.OriginalDeploymentRepo != "" &&
315+
existing.OriginalDeploymentRepo != new.OriginalDeploymentRepo {
316+
return false
317+
}
318+
319+
return true
320+
}
321+
322+
// extractPathDir extracts the directory portion from an artifact path
323+
func extractPathDir(path string) string {
324+
lastSlash := strings.LastIndex(path, "/")
325+
if lastSlash == -1 {
326+
return ""
327+
}
328+
return path[:lastSlash]
329+
}
330+
272331
func mergeDependenciesLists(dependenciesToAdd, intoDependencies *[]Dependency) {
273332
for i, dependencyToAdd := range *dependenciesToAdd {
274333
exists := false

0 commit comments

Comments
 (0)