From 597039f32de0b736435edaa980e55c1d2e1f6644 Mon Sep 17 00:00:00 2001 From: Carlos A Becker Date: Tue, 16 Aug 2022 00:04:27 -0300 Subject: [PATCH 1/3] fix: eventual race condition in artifacts Signed-off-by: Carlos A Becker --- internal/artifact/artifact.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 16ed3d0bd90..0ef61103984 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -259,26 +259,28 @@ func (a Artifact) Format() string { // Artifacts is a list of artifacts. type Artifacts struct { items []*Artifact - lock *sync.Mutex + lock *sync.RWMutex } // New return a new list of artifacts. func New() Artifacts { return Artifacts{ items: []*Artifact{}, - lock: &sync.Mutex{}, + lock: &sync.RWMutex{}, } } // List return the actual list of artifacts. func (artifacts Artifacts) List() []*Artifact { + artifacts.lock.RLock() + defer artifacts.lock.RUnlock() return artifacts.items } // GroupByID groups the artifacts by their ID. func (artifacts Artifacts) GroupByID() map[string][]*Artifact { result := map[string][]*Artifact{} - for _, a := range artifacts.items { + for _, a := range artifacts.List() { id := a.ID() if id == "" { continue @@ -489,7 +491,7 @@ func (artifacts *Artifacts) Filter(filter Filter) Artifacts { } result := New() - for _, a := range artifacts.items { + for _, a := range artifacts.List() { if filter(a) { result.items = append(result.items, a) } @@ -511,7 +513,7 @@ type VisitFn func(a *Artifact) error // Visit executes the given function for each artifact in the list. func (artifacts Artifacts) Visit(fn VisitFn) error { - for _, artifact := range artifacts.items { + for _, artifact := range artifacts.List() { if err := fn(artifact); err != nil { return err } From 2673f808da4b31022ebad1a89abc621d254f1892 Mon Sep 17 00:00:00 2001 From: Carlos A Becker Date: Tue, 16 Aug 2022 00:06:21 -0300 Subject: [PATCH 2/3] fix: locks Signed-off-by: Carlos A Becker --- internal/artifact/artifact.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 0ef61103984..dedde846420 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -259,21 +259,21 @@ func (a Artifact) Format() string { // Artifacts is a list of artifacts. type Artifacts struct { items []*Artifact - lock *sync.RWMutex + lock *sync.Mutex } // New return a new list of artifacts. func New() Artifacts { return Artifacts{ items: []*Artifact{}, - lock: &sync.RWMutex{}, + lock: &sync.Mutex{}, } } // List return the actual list of artifacts. func (artifacts Artifacts) List() []*Artifact { - artifacts.lock.RLock() - defer artifacts.lock.RUnlock() + artifacts.lock.Lock() + defer artifacts.lock.Unlock() return artifacts.items } @@ -293,7 +293,7 @@ func (artifacts Artifacts) GroupByID() map[string][]*Artifact { // GroupByPlatform groups the artifacts by their platform. func (artifacts Artifacts) GroupByPlatform() map[string][]*Artifact { result := map[string][]*Artifact{} - for _, a := range artifacts.items { + for _, a := range artifacts.List() { plat := a.Goos + a.Goarch + a.Goarm + a.Gomips + a.Goamd64 result[plat] = append(result[plat], a) } From ea70c12237473fc99e385701b2e41558891f86f0 Mon Sep 17 00:00:00 2001 From: Carlos A Becker Date: Tue, 16 Aug 2022 00:54:51 -0300 Subject: [PATCH 3/3] fix: tests Signed-off-by: Carlos A Becker --- internal/pipe/aur/aur_test.go | 8 ++++---- internal/pipe/brew/brew_test.go | 8 ++++---- internal/pipe/docker/docker_test.go | 6 +++--- internal/pipe/krew/krew_test.go | 8 ++++---- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/pipe/aur/aur_test.go b/internal/pipe/aur/aur_test.go index 62304cc593e..2dea5ba1c72 100644 --- a/internal/pipe/aur/aur_test.go +++ b/internal/pipe/aur/aur_test.go @@ -465,13 +465,13 @@ func TestRunPipe(t *testing.T) { } func TestRunPipeNoBuilds(t *testing.T) { - ctx := &context.Context{ - TokenType: context.TokenTypeGitHub, - Config: config.Project{ + ctx := context.New( + config.Project{ ProjectName: "foo", AURs: []config.AUR{{}}, }, - } + ) + ctx.TokenType = context.TokenTypeGitHub client := client.NewMock() require.NoError(t, Pipe{}.Default(ctx)) require.Equal(t, ErrNoArchivesFound, runAll(ctx, client)) diff --git a/internal/pipe/brew/brew_test.go b/internal/pipe/brew/brew_test.go index 5ceb02948bd..1056ad14a12 100644 --- a/internal/pipe/brew/brew_test.go +++ b/internal/pipe/brew/brew_test.go @@ -747,9 +747,8 @@ func TestRunPipeForMultipleArmVersions(t *testing.T) { } func TestRunPipeNoBuilds(t *testing.T) { - ctx := &context.Context{ - TokenType: context.TokenTypeGitHub, - Config: config.Project{ + ctx := context.New( + config.Project{ Brews: []config.Homebrew{ { Tap: config.RepoRef{ @@ -759,7 +758,8 @@ func TestRunPipeNoBuilds(t *testing.T) { }, }, }, - } + ) + ctx.TokenType = context.TokenTypeGitHub client := client.NewMock() require.Equal(t, ErrNoArchivesFound, runAll(ctx, client)) require.False(t, client.CreatedFile) diff --git a/internal/pipe/docker/docker_test.go b/internal/pipe/docker/docker_test.go index 3858c74a692..9b7ee7497a8 100644 --- a/internal/pipe/docker/docker_test.go +++ b/internal/pipe/docker/docker_test.go @@ -1252,13 +1252,13 @@ func TestDefaultDockerfile(t *testing.T) { } func TestDraftRelease(t *testing.T) { - ctx := &context.Context{ - Config: config.Project{ + ctx := context.New( + config.Project{ Release: config.Release{ Draft: true, }, }, - } + ) require.False(t, pipe.IsSkip(Pipe{}.Publish(ctx))) } diff --git a/internal/pipe/krew/krew_test.go b/internal/pipe/krew/krew_test.go index 0bcad078d8a..267081b3bd4 100644 --- a/internal/pipe/krew/krew_test.go +++ b/internal/pipe/krew/krew_test.go @@ -731,9 +731,8 @@ func TestRunPipeForMultipleArmVersions(t *testing.T) { } func TestRunPipeNoBuilds(t *testing.T) { - ctx := &context.Context{ - TokenType: context.TokenTypeGitHub, - Config: config.Project{ + ctx := context.New( + config.Project{ Krews: []config.Krew{ { Name: manifestName(t), @@ -746,7 +745,8 @@ func TestRunPipeNoBuilds(t *testing.T) { }, }, }, - } + ) + ctx.TokenType = context.TokenTypeGitHub client := client.NewMock() require.Equal(t, ErrNoArchivesFound, runAll(ctx, client)) require.False(t, client.CreatedFile)