Repository: goreleaser/nfpm
Pull request #136 (2020-05-13 07:39:11)

refactor: add a method on Info to list files to copy to the package
Since iterating over the files to copy to the package is something that
all package formats need, adding a central place for that logic reduces
duplication.

Also sort the list of files as a step toward making builds more
reproducible.

codecov[bot]: # [Codecov](https://codecov.io/gh/goreleaser/nfpm/pull/136?src=pr&el=h1) Report
> Merging [#136](https://codecov.io/gh/goreleaser/nfpm/pull/136?src=pr&el=desc) into [master](https://codecov.io/gh/goreleaser/nfpm/commit/2768948b4ef621601e4d3593de62186c2362586b&el=desc) will **increase** coverage by `0.86%`.
> The diff coverage is `72.72%`.

[![Impacted file tree graph](https://codecov.io/gh/goreleaser/nfpm/pull/136/graphs/tree.svg?width=650&height=150&src=pr&token=9QKGIxn7wg)](https://codecov.io/gh/goreleaser/nfpm/pull/136?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
+ Coverage   72.44%   73.30%   +0.86%     
==========================================
  Files           6        6              
  Lines         519      517       -2     
==========================================
+ Hits          376      379       +3     
+ Misses         90       87       -3     
+ Partials       53       51       -2     
```


| [Impacted Files](https://codecov.io/gh/goreleaser/nfpm/pull/136?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [deb/deb.go](https://codecov.io/gh/goreleaser/nfpm/pull/136/diff?src=pr&el=tree#diff-ZGViL2RlYi5nbw==) | `72.72% <60.00%> (+0.39%)` | :arrow_up: |
| [rpm/rpm.go](https://codecov.io/gh/goreleaser/nfpm/pull/136/diff?src=pr&el=tree#diff-cnBtL3JwbS5nbw==) | `75.79% <60.00%> (+1.23%)` | :arrow_up: |
| [nfpm.go](https://codecov.io/gh/goreleaser/nfpm/pull/136/diff?src=pr&el=tree#diff-bmZwbS5nbw==) | `89.53% <83.33%> (-1.65%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/goreleaser/nfpm/pull/136?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/goreleaser/nfpm/pull/136?src=pr&el=footer). Last update [2768948...f222494](https://codecov.io/gh/goreleaser/nfpm/pull/136?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/deb/deb.go b/deb/deb.go
index 98328a3..57ab15c 100644
--- a/deb/deb.go
+++ b/deb/deb.go
@@ -19,7 +19,6 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/goreleaser/nfpm"
-	"github.com/goreleaser/nfpm/glob"
 )
 
 // nolint: gochecknoinits
@@ -122,33 +121,20 @@ func createDataTarGz(info *nfpm.Info) (dataTarGz, md5sums []byte, instSize int64
 func createFilesInsideTarGz(info *nfpm.Info, out *tar.Writer, created map[string]bool) (bytes.Buffer, int64, error) {
 	var md5buf bytes.Buffer
 	var instSize int64
-	for _, files := range []map[string]string{
-		info.Files,
-		info.ConfigFiles,
-	} {
-		for srcglob, dstroot := range files {
-			globbed, err := glob.Glob(srcglob, dstroot)
-			if err != nil {
-				return md5buf, 0, err
-			}
-			for src, dst := range globbed {
-				// when used as a lib, target may not be set.
-				// in that case, src will always have the empty sufix, and all
-				// files will be ignored.
-				if info.Target != "" && strings.HasSuffix(src, info.Target) {
-					fmt.Printf("skipping %s because it has the suffix %s", src, info.Target)
-					continue
-				}
-				if err := createTree(out, dst, created); err != nil {
-					return md5buf, 0, err
-				}
-				size, err := copyToTarAndDigest(out, &md5buf, src, dst)
-				if err != nil {
-					return md5buf, 0, err
-				}
-				instSize += size
-			}
+
+	files, err := info.FilesToCopy()
+	if err != nil {
+		return md5buf, 0, err
+	}
+	for _, file := range files {
+		if err := createTree(out, file.Destination, created); err != nil {
+			return md5buf, 0, err
+		}
+		size, err := copyToTarAndDigest(out, &md5buf, file.Source, file.Destination)
+		if err != nil {
+			return md5buf, 0, err
 		}
+		instSize += size
 	}
 	return md5buf, instSize, nil
 }
diff --git a/nfpm.go b/nfpm.go
index 86507c0..d2da046 100644
--- a/nfpm.go
+++ b/nfpm.go
@@ -8,12 +8,15 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"sort"
+	"strings"
 	"sync"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/imdario/mergo"
 	"github.com/pkg/errors"
 
+	"github.com/goreleaser/nfpm/glob"
 	"github.com/goreleaser/nfpm/internal/helpers"
 
 	"gopkg.in/yaml.v2"
@@ -230,3 +233,44 @@ func WithDefaults(info *Info) *Info {
 
 	return info
 }
+
+// FileToCopy describes the source and destination of one file to copy into a
+// package and whether it is a config file.
+type FileToCopy struct {
+	Source      string
+	Destination string
+	Config      bool
+}
+
+// FilesToCopy lists all of the real files to be copied into the package.
+func (info *Info) FilesToCopy() ([]FileToCopy, error) {
+	var files []FileToCopy
+	for i, filesMap := range []map[string]string{info.Files, info.ConfigFiles} {
+		for srcglob, dstroot := range filesMap {
+			globbed, err := glob.Glob(srcglob, dstroot)
+			if err != nil {
+				return nil, err
+			}
+			for src, dst := range globbed {
+				// avoid including a partial file with the name of the target in the target
+				// itself. when used as a lib, target may not be set. in that case, src will
+				// always have the empty sufix, and all files will be ignored.
+				if info.Target != "" && strings.HasSuffix(src, info.Target) {
+					fmt.Printf("skipping %s because it has the suffix %s", src, info.Target)
+					continue
+				}
+
+				files = append(files, FileToCopy{src, dst, i == 1})
+			}
+		}
+	}
+	// sort the files for reproducibility and general cleanliness
+	sort.Slice(files, func(i, j int) bool {
+		a, b := files[i], files[j]
+		if a.Source != b.Source {
+			return a.Source < b.Source
+		}
+		return a.Destination < b.Destination
+	})
+	return files, nil
+}
diff --git a/rpm/rpm.go b/rpm/rpm.go
index e13885d..35791c2 100644
--- a/rpm/rpm.go
+++ b/rpm/rpm.go
@@ -15,7 +15,6 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/goreleaser/nfpm"
-	"github.com/goreleaser/nfpm/glob"
 )
 
 // nolint: gochecknoinits
@@ -216,36 +215,15 @@ func addEmptyDirsRPM(info *nfpm.Info, rpm *rpmpack.RPM) {
 }
 
 func createFilesInsideRPM(info *nfpm.Info, rpm *rpmpack.RPM) error {
-	copyFunc := func(files map[string]string, config bool) error {
-		for srcglob, dstroot := range files {
-			globbed, err := glob.Glob(srcglob, dstroot)
-			if err != nil {
-				return err
-			}
-			for src, dst := range globbed {
-				// when used as a lib, target may not be set.
-				// in that case, src will always have the empty sufix, and all
-				// files will be ignored.
-				if info.Target != "" && strings.HasSuffix(src, info.Target) {
-					fmt.Printf("skipping %s because it has the suffix %s", src, info.Target)
-					continue
-				}
-				err := copyToRPM(rpm, src, dst, config)
-				if err != nil {
-					return err
-				}
-			}
-		}
-
-		return nil
-	}
-	err := copyFunc(info.Files, false)
+	files, err := info.FilesToCopy()
 	if err != nil {
 		return err
 	}
-	err = copyFunc(info.ConfigFiles, true)
-	if err != nil {
-		return err
+	for _, file := range files {
+		err := copyToRPM(rpm, file.Source, file.Destination, file.Config)
+		if err != nil {
+			return err
+		}
 	}
 	return nil
 }


Test patch:
diff --git a/nfpm_test.go b/nfpm_test.go
index 0a583d9..74fab29 100644
--- a/nfpm_test.go
+++ b/nfpm_test.go
@@ -187,6 +187,31 @@ func TestOverrides(t *testing.T) {
 	assert.True(t, reflect.DeepEqual(&config.Info, info))
 }
 
+func TestListFilesToCopy(t *testing.T) {
+	info := &Info{
+		Overridables: Overridables{
+			ConfigFiles: map[string]string{
+				"testdata/whatever.conf": "/whatever",
+			},
+			Files: map[string]string{
+				"testdata/scripts/**/*": "/test",
+			},
+		},
+	}
+
+	files, err := info.FilesToCopy()
+	assert.NoError(t, err)
+
+	// all the input files described in the config in sorted order by source path
+	assert.Equal(t, []FileToCopy{
+		{"testdata/scripts/postinstall.sh", "/test/postinstall.sh", false},
+		{"testdata/scripts/postremove.sh", "/test/postremove.sh", false},
+		{"testdata/scripts/preinstall.sh", "/test/preinstall.sh", false},
+		{"testdata/scripts/preremove.sh", "/test/preremove.sh", false},
+		{"testdata/whatever.conf", "/whatever", true},
+	}, files)
+}
+
 type fakePackager struct{}
 
 func (*fakePackager) Package(info *Info, w io.Writer) error {