fix(mr): bound conflict paths with exact-sized copies
- commit
- f09b4fb25aba68e46dc33cc6637af6750f1c0946
- author
- brice · · 2026-10-19 14:52 UTC
- committer
- brice · · 2026-10-19 18:30 UTC
- parent
- f8b1d20
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +30 -2
M internal/domain/mergerequest.go +12 -2
| @@ -540,10 +540,20 @@ type MergeTreeResult struct { | |||
| 540 | 540 | // set when some were dropped. Adapters return the raw result; the bound is | |
| 541 | 541 | // the domain's. | |
| 542 | 542 | func (r MergeTreeResult) Bounded() MergeTreeResult { | |
| 543 | - | if len(r.ConflictPaths) > MaxConflictPaths { | |
| 544 | - | r.ConflictPaths = r.ConflictPaths[:MaxConflictPaths:MaxConflictPaths] | |
| 543 | + | kept := r.ConflictPaths | |
| 544 | + | if len(kept) > MaxConflictPaths { | |
| 545 | + | kept = kept[:MaxConflictPaths] | |
| 545 | 546 | r.Truncated = true | |
| 546 | 547 | } | |
| 548 | + | if kept != nil { | |
| 549 | + | // Exact-sized copies: neither the backing array nor the git output the | |
| 550 | + | // paths were cut from stays referenced. | |
| 551 | + | out := make([]string, len(kept)) | |
| 552 | + | for i, p := range kept { | |
| 553 | + | out[i] = strings.Clone(p) | |
| 554 | + | } | |
| 555 | + | r.ConflictPaths = out | |
| 556 | + | } | |
| 547 | 557 | return r | |
| 548 | 558 | } | |
| 549 | 559 | ||
| @@ -540,10 +540,20 @@ type MergeTreeResult struct { | |||||
| 540 | // set when some were dropped. Adapters return the raw result; the bound is | 540 | // set when some were dropped. Adapters return the raw result; the bound is | ||
| 541 | // the domain's. | 541 | // the domain's. | ||
| 542 | func (r MergeTreeResult) Bounded() MergeTreeResult { | 542 | func (r MergeTreeResult) Bounded() MergeTreeResult { | ||
| 543 | - | if len(r.ConflictPaths) > MaxConflictPaths { | 543 | + | kept := r.ConflictPaths |
| 544 | - | r.ConflictPaths = r.ConflictPaths[:MaxConflictPaths:MaxConflictPaths] | 544 | + | if len(kept) > MaxConflictPaths { |
| 545 | + | kept = kept[:MaxConflictPaths] | |||
| 545 | r.Truncated = true | 546 | r.Truncated = true | ||
| 546 | } | 547 | } | ||
| 548 | + | if kept != nil { | |||
| 549 | + | // Exact-sized copies: neither the backing array nor the git output the | |||
| 550 | + | // paths were cut from stays referenced. | |||
| 551 | + | out := make([]string, len(kept)) | |||
| 552 | + | for i, p := range kept { | |||
| 553 | + | out[i] = strings.Clone(p) | |||
| 554 | + | } | |||
| 555 | + | r.ConflictPaths = out | |||
| 556 | + | } | |||
| 547 | return r | 557 | return r | ||
| 548 | } | 558 | } | ||
| 549 | 559 | ||||
M internal/domain/mergetree_bound_test.go +18 -0
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | "testing" | |
| 8 | 8 | ||
| 9 | 9 | "github.com/stretchr/testify/assert" | |
| 10 | + | "github.com/stretchr/testify/require" | |
| 10 | 11 | ||
| 11 | 12 | "bnema.dev/ermite/internal/domain" | |
| 12 | 13 | ) | |
| @@ -33,3 +34,20 @@ func TestBoundBehind(t *testing.T) { | |||
| 33 | 34 | assert.Equal(t, domain.MaxBehindCount, n) | |
| 34 | 35 | assert.True(t, capped) | |
| 35 | 36 | } | |
| 37 | + | ||
| 38 | + | func TestMergeTreeResultBoundedCopiesThePaths(t *testing.T) { | |
| 39 | + | var paths []string | |
| 40 | + | for i := 0; i < domain.MaxConflictPaths+2; i++ { | |
| 41 | + | paths = append(paths, strconv.Itoa(i)) | |
| 42 | + | } | |
| 43 | + | got := domain.MergeTreeResult{ConflictPaths: paths}.Bounded() | |
| 44 | + | require.Len(t, got.ConflictPaths, domain.MaxConflictPaths) | |
| 45 | + | assert.Equal(t, len(got.ConflictPaths), cap(got.ConflictPaths)) | |
| 46 | + | assert.NotSame(t, &paths[0], &got.ConflictPaths[0], "a new array, not the git output's") | |
| 47 | + | assert.Equal(t, paths[:domain.MaxConflictPaths], got.ConflictPaths) | |
| 48 | + | ||
| 49 | + | got = domain.MergeTreeResult{ConflictPaths: paths[:3:10]}.Bounded() | |
| 50 | + | assert.Equal(t, 3, cap(got.ConflictPaths)) | |
| 51 | + | assert.False(t, got.Truncated) | |
| 52 | + | assert.Nil(t, domain.MergeTreeResult{}.Bounded().ConflictPaths) | |
| 53 | + | } | |
| @@ -7,6 +7,7 @@ import ( | |||||
| 7 | "testing" | 7 | "testing" | ||
| 8 | 8 | ||||
| 9 | "github.com/stretchr/testify/assert" | 9 | "github.com/stretchr/testify/assert" | ||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 10 | 11 | ||||
| 11 | "bnema.dev/ermite/internal/domain" | 12 | "bnema.dev/ermite/internal/domain" | ||
| 12 | ) | 13 | ) | ||
| @@ -33,3 +34,20 @@ func TestBoundBehind(t *testing.T) { | |||||
| 33 | assert.Equal(t, domain.MaxBehindCount, n) | 34 | assert.Equal(t, domain.MaxBehindCount, n) | ||
| 34 | assert.True(t, capped) | 35 | assert.True(t, capped) | ||
| 35 | } | 36 | } | ||
| 37 | + | ||||
| 38 | + | func TestMergeTreeResultBoundedCopiesThePaths(t *testing.T) { | |||
| 39 | + | var paths []string | |||
| 40 | + | for i := 0; i < domain.MaxConflictPaths+2; i++ { | |||
| 41 | + | paths = append(paths, strconv.Itoa(i)) | |||
| 42 | + | } | |||
| 43 | + | got := domain.MergeTreeResult{ConflictPaths: paths}.Bounded() | |||
| 44 | + | require.Len(t, got.ConflictPaths, domain.MaxConflictPaths) | |||
| 45 | + | assert.Equal(t, len(got.ConflictPaths), cap(got.ConflictPaths)) | |||
| 46 | + | assert.NotSame(t, &paths[0], &got.ConflictPaths[0], "a new array, not the git output's") | |||
| 47 | + | assert.Equal(t, paths[:domain.MaxConflictPaths], got.ConflictPaths) | |||
| 48 | + | ||||
| 49 | + | got = domain.MergeTreeResult{ConflictPaths: paths[:3:10]}.Bounded() | |||
| 50 | + | assert.Equal(t, 3, cap(got.ConflictPaths)) | |||
| 51 | + | assert.False(t, got.Truncated) | |||
| 52 | + | assert.Nil(t, domain.MergeTreeResult{}.Bounded().ConflictPaths) | |||
| 53 | + | } | |||