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: race condition and remove unused code #273

Merged
merged 8 commits into from
Dec 30, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 23 additions & 58 deletions files/files.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
package files

import (
"fmt"
"os"
"path/filepath"
"sort"
"time"

"github.com/goreleaser/fileglob"
"gopkg.in/yaml.v3"

"github.com/goreleaser/nfpm/v2/internal/glob"
)

Expand All @@ -34,44 +31,6 @@ type ContentFileInfo struct {
// Contents list of Content to process.
type Contents []*Content

type simpleContents map[string]string

func (c *Contents) UnmarshalYAML(value *yaml.Node) (err error) {
// nolint:exhaustive
// we do not care about `AliasNode`, `DocumentNode`
switch value.Kind {
case yaml.SequenceNode:
type tmpContents Contents
var tmp tmpContents
if err = value.Decode(&tmp); err != nil {
return err
}
*c = Contents(tmp)
case yaml.MappingNode:
var tmp simpleContents
if err = value.Decode(&tmp); err != nil {
return err
}
for src, dst := range tmp {
*c = append(*c, &Content{
Source: src,
Destination: dst,
})
}
case yaml.ScalarNode:
// TODO: implement issue-43 here and remove the fallthrough
// nolint:gocritic
// ignoring `emptyFallthrough: remove empty case containing only fallthrough to default case`
fallthrough
default:
// nolint:goerr113
// this is temporary so we do not need the a static error
return fmt.Errorf("not implemented")
}

return nil
}

func (c Contents) Len() int {
return len(c)
}
Expand All @@ -91,25 +50,33 @@ func (c Contents) Less(i, j int) bool {
return a.Destination < b.Destination
}

func (c *Content) WithFileInfoDefaults() {
if c.FileInfo == nil {
c.FileInfo = &ContentFileInfo{}
func (c *Content) WithFileInfoDefaults() *Content {
Copy link
Member Author

Choose a reason for hiding this comment

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

this method could be called by multiple threads at once, therefore we probably need to modify and return a copy instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

var cc = &Content{
Source: c.Source,
Destination: c.Destination,
Type: c.Type,
Packager: c.Packager,
FileInfo: c.FileInfo,
}
if cc.FileInfo == nil {
cc.FileInfo = &ContentFileInfo{}
}
if c.FileInfo.Owner == "" {
c.FileInfo.Owner = "root"
if cc.FileInfo.Owner == "" {
cc.FileInfo.Owner = "root"
}
if c.FileInfo.Group == "" {
c.FileInfo.Group = "root"
if cc.FileInfo.Group == "" {
cc.FileInfo.Group = "root"
}
info, err := os.Stat(c.Source)
info, err := os.Stat(cc.Source)
if err == nil {
c.FileInfo.MTime = info.ModTime()
c.FileInfo.Mode = info.Mode()
c.FileInfo.Size = info.Size()
cc.FileInfo.MTime = info.ModTime()
cc.FileInfo.Mode = info.Mode()
cc.FileInfo.Size = info.Size()
}
if c.FileInfo.MTime.IsZero() {
c.FileInfo.MTime = time.Now().UTC()
if cc.FileInfo.MTime.IsZero() {
cc.FileInfo.MTime = time.Now().UTC()
}
return cc
}

// Name to part of the os.FileInfo interface
Expand Down Expand Up @@ -153,8 +120,7 @@ func ExpandContentGlobs(filesSrcDstMap Contents, disableGlobbing bool) (files Co
switch f.Type {
case "ghost", "symlink":
// Ghost and symlink files need to be in the list, but dont glob them because they do not really exist
f.WithFileInfoDefaults()
files = append(files, f)
files = append(files, f.WithFileInfoDefaults())
continue
}

Expand All @@ -181,8 +147,7 @@ func appendGlobbedFiles(globbed map[string]string, origFile *Content, incFiles C
FileInfo: origFile.FileInfo,
Packager: origFile.Packager,
}
newFile.WithFileInfoDefaults()
files = append(files, newFile)
files = append(files, newFile.WithFileInfoDefaults())
}

return files
Expand Down
36 changes: 15 additions & 21 deletions files/files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package files_test

import (
"strings"
"sync"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -24,9 +25,9 @@ contents:
- src: a
dst: b
type: "config|noreplace"
packager: "rpm"
file_info:
mode: 0644
packager: "rpm"
mtime: 2008-01-02T15:04:05Z
`))
dec.KnownFields(true)
Expand All @@ -40,31 +41,24 @@ contents:
}
}

func TestMapperDecode(t *testing.T) {
func TestRace(t *testing.T) {
var config testStruct
dec := yaml.NewDecoder(strings.NewReader(`---
contents:
a: b
a2: b2
- src: a
dst: b
type: symlink
`))
dec.KnownFields(true)
err := dec.Decode(&config)
require.NoError(t, err)
assert.Len(t, config.Contents, 2)
for _, f := range config.Contents {
t.Logf("%+#v\n", f)
assert.Equal(t, f.Packager, "")
assert.Equal(t, f.Type, "")
var wg sync.WaitGroup
for i := 0; i < 10; i++ {
wg.Add(1)
go func() {
defer wg.Done()
_, err := files.ExpandContentGlobs(config.Contents, false)
require.NoError(t, err)
}()
}
}

func TestStringDecode(t *testing.T) {
var config testStruct
dec := yaml.NewDecoder(strings.NewReader(`---
contents: /path/to/a/tgz
`))
dec.KnownFields(true)
err := dec.Decode(&config)
require.Error(t, err)
assert.Equal(t, err.Error(), "not implemented")
wg.Wait()
}
9 changes: 7 additions & 2 deletions nfpm.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,13 @@ func Validate(info *Info) (err error) {
return ErrFieldEmpty{"version"}
}

info.Contents, err = files.ExpandContentGlobs(info.Contents, info.DisableGlobbing)
return err
contents, err := files.ExpandContentGlobs(info.Contents, info.DisableGlobbing)
if err != nil {
return err
}

info.Contents = contents
return nil
}

// WithDefaults set some sane defaults into the given Info.
Expand Down