Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: dedupliate cataloging binary artifacts #2839

Merged
merged 1 commit into from Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Expand Up @@ -28,6 +28,7 @@ require (
github.com/muesli/coral v1.0.0
github.com/muesli/mango-coral v1.0.1
github.com/muesli/roff v0.1.0
github.com/scylladb/go-set v1.0.2
github.com/slack-go/slack v0.10.2
github.com/stretchr/testify v1.7.0
github.com/ulikunitz/xz v0.5.10
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Expand Up @@ -322,6 +322,8 @@ github.com/envoyproxy/protoc-gen-validate v0.6.2/go.mod h1:2t7qjJNvHPx8IjnBOzl9E
github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
github.com/fatih/set v0.2.1 h1:nn2CaJyknWE/6txyUDGwysr3G5QC6xWB/PtVjPBbeaA=
github.com/fatih/set v0.2.1/go.mod h1:+RKtMCH+favT2+3YecHGxcc0b4KyVWA1QWWJUs4E0CI=
github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc=
github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k=
github.com/fortytw2/leaktest v1.3.0/go.mod h1:jDsjWgpAGjm2CA7WthBh/CdZYEPF31XHquHwclZch5g=
Expand Down Expand Up @@ -674,6 +676,8 @@ github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQD
github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/scylladb/go-set v1.0.2 h1:SkvlMCKhP0wyyct6j+0IHJkBkSZL+TDzZ4E7f7BCcRE=
github.com/scylladb/go-set v1.0.2/go.mod h1:DkpGd78rljTxKAnTDPFqXSGxvETQnJyuSOQwsHycqfs=
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
Expand Down
34 changes: 34 additions & 0 deletions internal/artifact/artifact.go
Expand Up @@ -17,6 +17,7 @@ import (
"sync"

"github.com/apex/log"
"github.com/scylladb/go-set/strset"
)

// Type defines the type of an artifact.
Expand Down Expand Up @@ -379,6 +380,39 @@ func ByIDs(ids ...string) Filter {
return Or(filters...)
}

// ByBinaryLikeArtifacts filter artifacts down to artifacts that are Binary, UploadableBinary, or UniversalBinary,
// deduplicating artifacts by path (preferring UploadableBinary over all others). Note: this filter is unique in the
// sense that it cannot act in isolation of the state of other artifacts; the filter requires the whole list of
// artifacts in advance to perform deduplication.
func ByBinaryLikeArtifacts(arts Artifacts) Filter {
// find all of the paths for any uploadable binary artifacts
uploadableBins := arts.Filter(ByType(UploadableBinary)).List()
uploadableBinPaths := strset.New()
for _, a := range uploadableBins {
uploadableBinPaths.Add(a.Path)
}

// we want to keep any matching artifact that is not a binary that already has a path accounted for
// by another uploadable binary. We always prefer uploadable binary artifacts over binary artifacts.
deduplicateByPath := func(a *Artifact) bool {
if a.Type == UploadableBinary {
return true
}
return !uploadableBinPaths.Has(a.Path)
}

return And(
// allow all of the binary-like artifacts as possible...
Or(
ByType(Binary),
ByType(UploadableBinary),
ByType(UniversalBinary),
),
// ... but remove any duplicates found
deduplicateByPath,
)
}

// Or performs an OR between all given filters.
func Or(filters ...Filter) Filter {
return func(a *Artifact) bool {
Expand Down
257 changes: 257 additions & 0 deletions internal/artifact/artifact_test.go
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"github.com/goreleaser/goreleaser/internal/golden"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/sync/errgroup"
)
Expand Down Expand Up @@ -593,3 +594,259 @@ func TestMarshalJSON(t *testing.T) {
require.NoError(t, err)
golden.RequireEqualJSON(t, bts)
}

func Test_ByBinaryLikeArtifacts(t *testing.T) {
tests := []struct {
name string
initial []*Artifact
expected []*Artifact
}{
{
name: "keep all unique paths",
initial: []*Artifact{
{
Path: "binary-path",
Type: Binary,
},
{
Path: "uploadable-binary-path",
Type: UploadableBinary,
},
{
Path: "universal-binary-path",
Type: UniversalBinary,
},
},
expected: []*Artifact{
{
Path: "binary-path",
Type: Binary,
},
{
Path: "uploadable-binary-path",
Type: UploadableBinary,
},
{
Path: "universal-binary-path",
Type: UniversalBinary,
},
},
},
{
name: "duplicate path between binaries ignored (odd configuration)",
initial: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "uploadable-binary-path",
Type: UploadableBinary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
},
expected: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "uploadable-binary-path",
Type: UploadableBinary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
},
},
{
name: "remove duplicate binary",
initial: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
{
Path: "universal-binary-path",
Type: UniversalBinary,
},
},
expected: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
{
Path: "universal-binary-path",
Type: UniversalBinary,
},
},
},
{
name: "remove duplicate universal binary",
initial: []*Artifact{
{
Path: "binary-path",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
},
expected: []*Artifact{
{
Path: "binary-path",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
},
},
{
name: "remove multiple duplicates",
initial: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
},
expected: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
},
},
{
name: "keep duplicate uploadable binaries (odd configuration)",
initial: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
},
expected: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
{
Path: "!!!duplicate!!!",
Type: UploadableBinary,
},
},
},
{
name: "keeps duplicates when there is no uploadable binary",
initial: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
},
expected: []*Artifact{
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: Binary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
{
Path: "!!!duplicate!!!",
Type: UniversalBinary,
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
arts := New()
for _, a := range tt.initial {
arts.Add(a)
}
actual := arts.Filter(ByBinaryLikeArtifacts(arts)).List()
assert.Equal(t, tt.expected, actual)

if t.Failed() {
t.Log("expected:")
for _, a := range tt.expected {
t.Logf(" %s: %s", a.Type.String(), a.Path)
}

t.Log("got:")
for _, a := range actual {
t.Logf(" %s: %s", a.Type.String(), a.Path)
}
}
})
}
}