Repository: goreleaser/nfpm
Pull request #410 (2021-12-01 09:16:17)

fix: Fix destinations for files containing matchers when globbing is disabled.
This PR fixes a bug where file sources containing matchers result in the files being created inside folders of the same name as the file when globbing is disabled.

**Example:**
```
src: foo/{bar}/config.json
dst: /etc/config.json
```
resulted in this:
```
src: foo/{bar}/config.json
dst: /etc/config.json/config.json
```
when it should have actually just remain unchanged. 


rtpt-erikgeiser: I have no idea why, but the Windows CI task fails due to missing `gcc`. I don't think that's related to this RP.

Patch:
diff --git a/apk/apk.go b/apk/apk.go
index 62e836c..c5eb590 100644
--- a/apk/apk.go
+++ b/apk/apk.go
@@ -414,8 +414,14 @@ func createFilesInsideTarGz(info *nfpm.Info, tw *tar.Writer, created map[string]
 			continue
 		}
 
+		normalizedName := normalizePath(strings.Trim(file.Destination, "/")) + "/"
+
+		if created[normalizedName] {
+			return fmt.Errorf("duplicate directory: %q", normalizedName)
+		}
+
 		err = tw.WriteHeader(&tar.Header{
-			Name:     files.ToNixPath(strings.Trim(file.Destination, "/") + "/"),
+			Name:     normalizedName,
 			Mode:     int64(file.FileInfo.Mode),
 			Typeflag: tar.TypeDir,
 			Format:   tar.FormatGNU,
@@ -427,7 +433,7 @@ func createFilesInsideTarGz(info *nfpm.Info, tw *tar.Writer, created map[string]
 			return err
 		}
 
-		created[strings.TrimPrefix(file.Destination, "/")] = true
+		created[normalizedName] = true
 	}
 
 	for _, file := range info.Contents {
@@ -483,7 +489,7 @@ func copyToTarAndDigest(file *files.Content, tw *tar.Writer, sizep *int64) error
 	// tar.FileInfoHeader only uses file.Mode().Perm() which masks the mode with
 	// 0o777 which we don't want because we want to be able to set the suid bit.
 	header.Mode = int64(file.Mode())
-	header.Name = files.ToNixPath(file.Destination[1:])
+	header.Name = normalizePath(file.Destination)
 	header.Uname = file.FileInfo.Owner
 	header.Gname = file.FileInfo.Group
 	if err = newItemInsideTarGz(tw, contents, header); err != nil {
@@ -494,20 +500,30 @@ func copyToTarAndDigest(file *files.Content, tw *tar.Writer, sizep *int64) error
 	return nil
 }
 
+// normalizePath returns a path separated by slashes without a leading slash.
+func normalizePath(src string) string {
+	return files.ToNixPath(strings.TrimLeft(src, "/"))
+}
+
 // this is needed because the data.tar.gz file should have the empty folders
 // as well, so we walk through the dst and create all subfolders.
 func createTree(tarw *tar.Writer, dst string, created map[string]bool) error {
 	for _, path := range pathsToCreate(dst) {
+		path = normalizePath(path) + "/"
+
 		if created[path] {
 			// skipping dir that was previously created inside the archive
 			// (eg: usr/)
 			continue
 		}
+
 		if err := tarw.WriteHeader(&tar.Header{
-			Name:     files.ToNixPath(path + "/"),
+			Name:     path,
 			Mode:     0o755,
 			Typeflag: tar.TypeDir,
 			Format:   tar.FormatGNU,
+			Uname:    "root",
+			Gname:    "root",
 		}); err != nil {
 			return fmt.Errorf("failed to create folder %s: %w", path, err)
 		}
diff --git a/deb/deb.go b/deb/deb.go
index c9e04b3..98eafa0 100644
--- a/deb/deb.go
+++ b/deb/deb.go
@@ -280,8 +280,14 @@ func createFilesInsideDataTar(info *nfpm.Info, tw *tar.Writer,
 			continue
 		}
 
+		normalizedName := normalizePath(strings.Trim(file.Destination, "/")) + "/"
+
+		if created[normalizedName] {
+			return md5buf, 0, fmt.Errorf("duplicate directory: %q", normalizedName)
+		}
+
 		err = tw.WriteHeader(&tar.Header{
-			Name:     normalizePath(strings.Trim(file.Destination, "/") + "/"),
+			Name:     normalizedName,
 			Mode:     int64(file.FileInfo.Mode),
 			Typeflag: tar.TypeDir,
 			Format:   tar.FormatGNU,
@@ -293,7 +299,7 @@ func createFilesInsideDataTar(info *nfpm.Info, tw *tar.Writer,
 			return md5buf, 0, err
 		}
 
-		created[strings.TrimPrefix(file.Destination, "/")] = true
+		created[normalizedName] = true
 	}
 
 	// create files and implicit directories
@@ -588,12 +594,15 @@ func createTree(tarw *tar.Writer, dst string, created map[string]bool) error {
 			// (eg: usr/)
 			continue
 		}
+
 		if err := tarw.WriteHeader(&tar.Header{
 			Name:     path,
 			Mode:     0o755,
 			Typeflag: tar.TypeDir,
 			Format:   tar.FormatGNU,
 			ModTime:  time.Now(),
+			Uname:    "root",
+			Gname:    "root",
 		}); err != nil {
 			return fmt.Errorf("failed to create folder: %w", err)
 		}
diff --git a/files/files.go b/files/files.go
index e837a4c..bfc8cf7 100644
--- a/files/files.go
+++ b/files/files.go
@@ -8,7 +8,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/goreleaser/fileglob"
 	"github.com/goreleaser/nfpm/v2/internal/glob"
 )
 
@@ -143,11 +142,6 @@ func (c *Content) Sys() interface{} {
 
 // ExpandContentGlobs gathers all of the real files to be copied into the package.
 func ExpandContentGlobs(contents Contents, disableGlobbing bool) (files Contents, err error) {
-	options := []fileglob.OptFunc{fileglob.MatchDirectoryIncludesContents}
-	if disableGlobbing {
-		options = append(options, fileglob.QuoteMeta)
-	}
-
 	for _, f := range contents {
 		var globbed map[string]string
 
@@ -157,7 +151,7 @@ func ExpandContentGlobs(contents Contents, disableGlobbing bool) (files Contents
 			// them because they do not really exist
 			files = append(files, f.WithFileInfoDefaults())
 		default:
-			globbed, err = glob.Glob(f.Source, f.Destination, options...)
+			globbed, err = glob.Glob(f.Source, f.Destination, disableGlobbing)
 			if err != nil {
 				return nil, err
 			}
@@ -205,8 +199,13 @@ func checkNoCollisions(contents Contents) error {
 	for _, elem := range contents {
 		present, ok := alreadyPresent[elem.Destination]
 		if ok && (present.Packager == "" || elem.Packager == "" || present.Packager == elem.Packager) {
+			if elem.Type == "dir" {
+				return fmt.Errorf("cannot add directory %q because it is already present: %w",
+					elem.Destination, ErrContentCollision)
+			}
+
 			return fmt.Errorf(
-				"cannot add %s because %s is already present at the same destination (%s): %w",
+				"cannot add %q because %q is already present at the same destination (%s): %w",
 				elem.Source, present.Source, present.Destination, ErrContentCollision)
 		}
 
diff --git a/files/testdata/{test}/[f]oo b/files/testdata/{test}/[f]oo
new file mode 100644
index 0000000..e69de29
diff --git a/files/testdata/{test}/bar b/files/testdata/{test}/bar
new file mode 100644
index 0000000..e69de29
diff --git a/internal/glob/glob.go b/internal/glob/glob.go
index 5631757..5bc2497 100644
--- a/internal/glob/glob.go
+++ b/internal/glob/glob.go
@@ -51,7 +51,12 @@ func (e ErrGlobNoMatch) Error() string {
 // Glob returns a map with source file path as keys and destination as values.
 // First the longest common prefix (lcp) of all globbed files is found. The destination
 // for each globbed file is then dst joined with src with the lcp trimmed off.
-func Glob(pattern, dst string, options ...fileglob.OptFunc) (map[string]string, error) {
+func Glob(pattern, dst string, ignoreMatchers bool) (map[string]string, error) {
+	options := []fileglob.OptFunc{fileglob.MatchDirectoryIncludesContents}
+	if ignoreMatchers {
+		options = append(options, fileglob.QuoteMeta)
+	}
+
 	if strings.HasPrefix(pattern, "../") {
 		p, err := filepath.Abs(pattern)
 		if err != nil {
@@ -59,21 +64,24 @@ func Glob(pattern, dst string, options ...fileglob.OptFunc) (map[string]string,
 		}
 		pattern = filepath.ToSlash(p)
 	}
+
 	matches, err := fileglob.Glob(pattern, append(options, fileglob.MaybeRootFS)...)
-	if errors.Is(err, os.ErrNotExist) {
-		return nil, err
-	}
 	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return nil, err
+		}
+
 		return nil, fmt.Errorf("glob failed: %s: %w", pattern, err)
 	}
 
 	if len(matches) == 0 {
 		return nil, ErrGlobNoMatch{pattern}
 	}
+
 	files := make(map[string]string)
 	prefix := pattern
 	// the prefix may not be a complete path or may use glob patterns, in that case use the parent directory
-	if _, err := os.Stat(prefix); os.IsNotExist(err) || fileglob.ContainsMatchers(pattern) {
+	if _, err := os.Stat(prefix); os.IsNotExist(err) || (fileglob.ContainsMatchers(pattern) && !ignoreMatchers) {
 		prefix = filepath.Dir(longestCommonPrefix(matches))
 	}
 
@@ -82,13 +90,16 @@ func Glob(pattern, dst string, options ...fileglob.OptFunc) (map[string]string,
 		if f, err := os.Stat(src); err == nil && f.Mode().IsDir() {
 			continue
 		}
+
 		relpath, err := filepath.Rel(prefix, src)
 		if err != nil {
 			// since prefix is a prefix of src a relative path should always be found
-			panic(err)
+			return nil, err
 		}
+
 		globdst := filepath.ToSlash(filepath.Join(dst, relpath))
 		files[src] = globdst
 	}
+
 	return files, nil
 }


Test patch:
diff --git a/apk/apk_test.go b/apk/apk_test.go
index 639ad2c..bf09992 100644
--- a/apk/apk_test.go
+++ b/apk/apk_test.go
@@ -410,6 +410,25 @@ func extractFromTar(t *testing.T, tarFile []byte, fileName string) []byte {
 	return nil
 }
 
+func tarContents(tb testing.TB, tarFile []byte) []string {
+	tb.Helper()
+
+	contents := []string{}
+
+	tr := tar.NewReader(bytes.NewReader(tarFile))
+	for {
+		hdr, err := tr.Next()
+		if errors.Is(err, io.EOF) {
+			break // End of archive
+		}
+		require.NoError(tb, err)
+
+		contents = append(contents, hdr.Name)
+	}
+
+	return contents
+}
+
 func TestAPKConventionalFileName(t *testing.T) {
 	apkName := "default"
 	testCases := []struct {
@@ -516,6 +535,66 @@ func TestDirectories(t *testing.T) {
 	require.Equal(t, h.Typeflag, byte(tar.TypeDir))
 }
 
+func TestNoDuplicateAutocreatedDirectories(t *testing.T) {
+	info := exampleInfo()
+	info.DisableGlobbing = true
+	info.Contents = []*files.Content{
+		{
+			Source:      "../testdata/fake",
+			Destination: "/etc/foo/bar",
+		},
+		{
+			Type:        "dir",
+			Destination: "/etc/foo",
+		},
+	}
+	require.NoError(t, info.Validate())
+
+	expected := map[string]bool{
+		"etc/":        true,
+		"etc/foo/":    true,
+		"etc/foo/bar": true,
+	}
+
+	var buf bytes.Buffer
+	size := int64(0)
+	err := createFilesInsideTarGz(info, tar.NewWriter(&buf), make(map[string]bool), &size)
+	require.NoError(t, err)
+
+	contents := tarContents(t, buf.Bytes())
+
+	if len(expected) != len(contents) {
+		t.Fatalf("contents has %d entries instead of %d: %#v", len(contents), len(expected), contents)
+	}
+
+	for _, entry := range contents {
+		if !expected[entry] {
+			t.Fatalf("unexpected content: %q", entry)
+		}
+	}
+}
+
+func TestNoDuplicateDirectories(t *testing.T) {
+	info := exampleInfo()
+	info.DisableGlobbing = true
+	info.Contents = []*files.Content{
+		{
+			Type:        "dir",
+			Destination: "/etc/foo",
+		},
+		{
+			Type:        "dir",
+			Destination: "/etc/foo/",
+		},
+	}
+	require.NoError(t, info.Validate())
+
+	var buf bytes.Buffer
+	size := int64(0)
+	err := createFilesInsideTarGz(info, tar.NewWriter(&buf), make(map[string]bool), &size)
+	require.Error(t, err)
+}
+
 func TestNoDuplicateContents(t *testing.T) {
 	info := exampleInfo()
 	info.Contents = []*files.Content{
diff --git a/deb/deb_test.go b/deb/deb_test.go
index 189f849..deee52d 100644
--- a/deb/deb_test.go
+++ b/deb/deb_test.go
@@ -947,6 +947,62 @@ func TestDisableGlobbing(t *testing.T) {
 	require.Equal(t, expectedContent, actualContent)
 }
 
+func TestNoDuplicateAutocreatedDirectories(t *testing.T) {
+	info := exampleInfo()
+	info.DisableGlobbing = true
+	info.Contents = []*files.Content{
+		{
+			Source:      "../testdata/fake",
+			Destination: "/etc/foo/bar",
+		},
+		{
+			Type:        "dir",
+			Destination: "/etc/foo",
+		},
+	}
+	require.NoError(t, info.Validate())
+
+	expected := map[string]bool{
+		"./etc/":        true,
+		"./etc/foo/":    true,
+		"./etc/foo/bar": true,
+	}
+
+	dataTarball, _, _, tarballName, err := createDataTarball(info)
+	require.NoError(t, err)
+
+	contents := tarContents(t, inflate(t, tarballName, dataTarball))
+
+	if len(expected) != len(contents) {
+		t.Fatalf("contents has %d entries instead of %d: %#v", len(contents), len(expected), contents)
+	}
+
+	for _, entry := range contents {
+		if !expected[entry] {
+			t.Fatalf("unexpected content: %q", entry)
+		}
+	}
+}
+
+func TestNoDuplicateDirectories(t *testing.T) {
+	info := exampleInfo()
+	info.DisableGlobbing = true
+	info.Contents = []*files.Content{
+		{
+			Type:        "dir",
+			Destination: "/etc/foo",
+		},
+		{
+			Type:        "dir",
+			Destination: "/etc/foo/",
+		},
+	}
+	require.NoError(t, info.Validate())
+
+	_, _, _, _, err := createDataTarball(info)
+	require.Error(t, err)
+}
+
 func TestCompressionAlgorithms(t *testing.T) {
 	testCases := []struct {
 		algorithm       string
@@ -1028,6 +1084,25 @@ func tarContains(tb testing.TB, tarFile []byte, filename string) bool {
 	return false
 }
 
+func tarContents(tb testing.TB, tarFile []byte) []string {
+	tb.Helper()
+
+	contents := []string{}
+
+	tr := tar.NewReader(bytes.NewReader(tarFile))
+	for {
+		hdr, err := tr.Next()
+		if errors.Is(err, io.EOF) {
+			break // End of archive
+		}
+		require.NoError(tb, err)
+
+		contents = append(contents, hdr.Name)
+	}
+
+	return contents
+}
+
 func extractFileHeaderFromTar(tb testing.TB, tarFile []byte, filename string) *tar.Header {
 	tb.Helper()
 
diff --git a/files/files_test.go b/files/files_test.go
index 4ba7c24..288924f 100644
--- a/files/files_test.go
+++ b/files/files_test.go
@@ -2,6 +2,7 @@ package files_test
 
 import (
 	"os"
+	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -198,3 +199,52 @@ func TestCollision(t *testing.T) {
 		require.ErrorIs(t, err, files.ErrContentCollision)
 	})
 }
+
+func TestDisableGlobbing(t *testing.T) {
+	testCases := []files.Content{
+		{
+			Source:      "testdata/{test}/bar",
+			Destination: "/etc/{test}/bar",
+		},
+		{
+			Source:      "testdata/{test}/[f]oo",
+			Destination: "testdata/{test}/[f]oo",
+		},
+		{
+			Source:      "testdata/globtest/a.txt",
+			Destination: "testdata/globtest/a.txt",
+		},
+		{
+			Source:      "testdata/globtest/a.txt",
+			Destination: "/etc/a.txt",
+		},
+	}
+
+	disableGlobbing := true
+
+	for i, testCase := range testCases {
+		content := testCase
+
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			result, err := files.ExpandContentGlobs(files.Contents{&content}, disableGlobbing)
+			if err != nil {
+				t.Fatalf("expand content globs: %v", err)
+			}
+
+			if len(result) != 1 {
+				t.Fatalf("unexpected result length: %d, expected one", len(result))
+			}
+
+			actualContent := result[0]
+
+			// we expect the result content to be identical to the input content
+			if actualContent.Source != content.Source {
+				t.Fatalf("unexpected content source: %q, expected %q", actualContent.Source, content.Source)
+			}
+
+			if actualContent.Destination != content.Destination {
+				t.Fatalf("unexpected content destination: %q, expected %q", actualContent.Destination, content.Destination)
+			}
+		})
+	}
+}
diff --git a/internal/glob/glob_test.go b/internal/glob/glob_test.go
index abfca68..721b3c8 100644
--- a/internal/glob/glob_test.go
+++ b/internal/glob/glob_test.go
@@ -40,7 +40,7 @@ func TestLongestCommonPrefix(t *testing.T) {
 
 func TestGlob(t *testing.T) {
 	t.Run("simple", func(t *testing.T) {
-		files, err := Glob("./testdata/dir_a/dir_*/*", "/foo/bar")
+		files, err := Glob("./testdata/dir_a/dir_*/*", "/foo/bar", false)
 		require.NoError(t, err)
 		require.Len(t, files, 2)
 		require.Equal(t, "/foo/bar/dir_b/test_b.txt", files["testdata/dir_a/dir_b/test_b.txt"])
@@ -51,47 +51,47 @@ func TestGlob(t *testing.T) {
 		pattern := "../../testdata/fake"
 		abs, err := filepath.Abs(pattern)
 		require.NoError(t, err)
-		files, err := Glob(pattern, "/foo/fake")
+		files, err := Glob(pattern, "/foo/fake", false)
 		require.NoError(t, err)
 		require.Len(t, files, 1)
 		require.Equal(t, "/foo/fake", files[filepath.ToSlash(abs)])
 	})
 
 	t.Run("single file", func(t *testing.T) {
-		files, err := Glob("testdata/dir_a/dir_b/*", "/foo/bar")
+		files, err := Glob("testdata/dir_a/dir_b/*", "/foo/bar", false)
 		require.NoError(t, err)
 		require.Len(t, files, 1)
 		require.Equal(t, "/foo/bar/test_b.txt", files["testdata/dir_a/dir_b/test_b.txt"])
 	})
 
 	t.Run("double star", func(t *testing.T) {
-		files, err := Glob("testdata/**/test*.txt", "/foo/bar")
+		files, err := Glob("testdata/**/test*.txt", "/foo/bar", false)
 		require.NoError(t, err)
 		require.Len(t, files, 3)
 		require.Equal(t, "/foo/bar/dir_a/dir_b/test_b.txt", files["testdata/dir_a/dir_b/test_b.txt"])
 	})
 
 	t.Run("nil value", func(t *testing.T) {
-		files, err := Glob("does/not/exist", "/foo/bar")
+		files, err := Glob("does/not/exist", "/foo/bar", false)
 		require.EqualError(t, err, "matching \"./does/not/exist\": file does not exist")
 		require.Nil(t, files)
 	})
 
 	t.Run("no matches", func(t *testing.T) {
-		files, err := Glob("testdata/nothing*", "/foo/bar")
+		files, err := Glob("testdata/nothing*", "/foo/bar", false)
 		require.Nil(t, files)
 		require.EqualError(t, err, "glob failed: testdata/nothing*: no matching files")
 	})
 
 	t.Run("escaped brace", func(t *testing.T) {
-		files, err := Glob("testdata/\\{dir_d\\}/*", "/foo/bar")
+		files, err := Glob("testdata/\\{dir_d\\}/*", "/foo/bar", false)
 		require.NoError(t, err)
 		require.Len(t, files, 1)
 		require.Equal(t, "/foo/bar/test_brace.txt", files["testdata/{dir_d}/test_brace.txt"])
 	})
 
 	t.Run("no glob", func(t *testing.T) {
-		files, err := Glob("testdata/dir_a/dir_b/test_b.txt", "/foo/bar/dest.dat")
+		files, err := Glob("testdata/dir_a/dir_b/test_b.txt", "/foo/bar/dest.dat", false)
 		require.NoError(t, err)
 		require.Len(t, files, 1)
 		require.Equal(t, "/foo/bar/dest.dat", files["testdata/dir_a/dir_b/test_b.txt"])