Enhance FilesFromDisk (close #331) (#332)

* Enhance FilesFromDisk (close #331)

Now permit suffixing root on disk with separator to enumerate only
its contents and not to create that actual file in archive.

And attempt improving Windows compatibility.

* Fix test on Windows

archiver.go

@@ -49,6 +49,8 @@ func (f File) Stat() (fs.FileInfo, error) { return f.FileInfo, nil }
 // Map keys that specify directories on disk will be walked and added to the
 // archive recursively, rooted at the named directory. They should use the
 // platform's path separator (backslash on Windows; slash on everything else).
+// For convenience, map keys that end in a separator ('/', or '\' on Windows)
+// will enumerate contents only without adding the folder itself to the archive.
 //
 // Map values should typically use slash ('/') as the separator regardless of
 // the platform, as most archive formats standardize on that rune as the
@@ -64,13 +66,6 @@ func (f File) Stat() (fs.FileInfo, error) { return f.FileInfo, nil }
 func FilesFromDisk(options *FromDiskOptions, filenames map[string]string) ([]File, error) {
 	var files []File
 	for rootOnDisk, rootInArchive := range filenames {
-		if rootInArchive == "" {
-			rootInArchive = filepath.Base(rootOnDisk)
-		}
-		if strings.HasSuffix(rootInArchive, "/") {
-			rootInArchive += filepath.Base(rootOnDisk)
-		}
-
 		filepath.WalkDir(rootOnDisk, func(filename string, d fs.DirEntry, err error) error {
 			if err != nil {
 				return err
@@ -81,11 +76,10 @@ func FilesFromDisk(options *FromDiskOptions, filenames map[string]string) ([]Fil
 				return err
 			}
 
-			truncPath := strings.TrimPrefix(filename, rootOnDisk)
-			nameInArchive := path.Join(rootInArchive, filepath.ToSlash(truncPath))
-			var linkTarget string
+			nameInArchive := nameOnDiskToNameInArchive(filename, rootOnDisk, rootInArchive)
 
 			// handle symbolic links
+			var linkTarget string
 			if isSymlink(info) {
 				if options != nil && options.FollowSymlinks {
 					// dereference symlinks
@@ -127,6 +121,50 @@ func FilesFromDisk(options *FromDiskOptions, filenames map[string]string) ([]Fil
 	return files, nil
 }
 
+// nameOnDiskToNameInArchive converts a filename from disk to a name in an archive,
+// respecting rules defined by FilesFromDisk. nameOnDisk is the full filename on disk
+// which is expected to be prefixed by rootOnDisk (according to fs.WalkDirFunc godoc)
+// and which will be placed into a folder rootInArchive in the archive.
+func nameOnDiskToNameInArchive(nameOnDisk, rootOnDisk, rootInArchive string) string {
+	// These manipulations of rootInArchive could be done just once instead of on
+	// every walked file since they don't rely on nameOnDisk which is the only
+	// variable that changes during the walk, but combining all the logic into this
+	// one function is easier to reason about and test. I suspect the performance
+	// penalty is insignificant.
+	if strings.HasSuffix(rootOnDisk, string(filepath.Separator)) {
+		rootInArchive = trimTopDir(rootInArchive)
+	} else if rootInArchive == "" {
+		rootInArchive = filepath.Base(rootOnDisk)
+	}
+	if strings.HasSuffix(rootInArchive, "/") {
+		rootInArchive += filepath.Base(rootOnDisk)
+	}
+	truncPath := strings.TrimPrefix(nameOnDisk, rootOnDisk)
+	return path.Join(rootInArchive, filepath.ToSlash(truncPath))
+}
+
+// trimTopDir strips the top or first directory from the path.
+// It expects a forward-slashed path.
+//
+// For example, "a/b/c" => "b/c".
+func trimTopDir(dir string) string {
+	if pos := strings.Index(dir, "/"); pos >= 0 {
+		return dir[pos+1:]
+	}
+	return dir
+}
+
+// topDir returns the top or first directory in the path.
+// It expects a forward-slashed path.
+//
+// For example, "a/b/c" => "a".
+func topDir(dir string) string {
+	if pos := strings.Index(dir, "/"); pos >= 0 {
+		return dir[:pos]
+	}
+	return dir
+}
+
 // noAttrFileInfo is used to zero out some file attributes (issue #280).
 type noAttrFileInfo struct{ fs.FileInfo }
 

archiver_test.go

@@ -2,6 +2,8 @@ package archiver
 
 import (
 	"reflect"
+	"runtime"
+	"strings"
 	"testing"
 )
 
@@ -118,3 +120,106 @@ func TestSkipList(t *testing.T) {
 		}
 	}
 }
+
+func TestNameOnDiskToNameInArchive(t *testing.T) {
+	for i, tc := range []struct {
+		windows       bool   // only run this test on Windows
+		rootOnDisk    string // user says they want to archive this file/folder
+		nameOnDisk    string // the walk encounters a file with this name (with rootOnDisk as a prefix)
+		rootInArchive string // file should be placed in this dir within the archive (rootInArchive becomes a prefix)
+		expect        string // final filename in archive
+	}{
+		{
+			rootOnDisk:    "a",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: "",
+			expect:        "a/b/c",
+		},
+		{
+			rootOnDisk:    "a/b",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: "",
+			expect:        "b/c",
+		},
+		{
+			rootOnDisk:    "a/b/",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: "",
+			expect:        "c",
+		},
+		{
+			rootOnDisk:    "a/b/",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: ".",
+			expect:        "c",
+		},
+		{
+			rootOnDisk:    "a/b/c",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: "",
+			expect:        "c",
+		},
+		{
+			rootOnDisk:    "a/b",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: "foo",
+			expect:        "foo/c",
+		},
+		{
+			rootOnDisk:    "a",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: "foo",
+			expect:        "foo/b/c",
+		},
+		{
+			rootOnDisk:    "a",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: "foo/",
+			expect:        "foo/a/b/c",
+		},
+		{
+			rootOnDisk:    "a/",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: "foo",
+			expect:        "foo/b/c",
+		},
+		{
+			rootOnDisk:    "a/",
+			nameOnDisk:    "a/b/c",
+			rootInArchive: "foo",
+			expect:        "foo/b/c",
+		},
+		{
+			windows:       true,
+			rootOnDisk:    `C:\foo`,
+			nameOnDisk:    `C:\foo\bar`,
+			rootInArchive: "",
+			expect:        "foo/bar",
+		},
+		{
+			windows:       true,
+			rootOnDisk:    `C:\foo`,
+			nameOnDisk:    `C:\foo\bar`,
+			rootInArchive: "subfolder",
+			expect:        "subfolder/bar",
+		},
+	} {
+		if !strings.HasPrefix(tc.nameOnDisk, tc.rootOnDisk) {
+			t.Fatalf("Test %d: Invalid test case! Filename (on disk) will have rootOnDisk as a prefix according to the fs.WalkDirFunc godoc.", i)
+		}
+		if tc.windows && runtime.GOOS != "windows" {
+			t.Logf("Test %d: Skipping test that is only compatible with Windows", i)
+			continue
+		}
+		if !tc.windows && runtime.GOOS == "windows" {
+			t.Logf("Test %d: Skipping test that is not compatible with Windows", i)
+			continue
+		}
+
+		actual := nameOnDiskToNameInArchive(tc.nameOnDisk, tc.rootOnDisk, tc.rootInArchive)
+		if actual != tc.expect {
+			t.Errorf("Test %d: Got '%s' but expected '%s' (nameOnDisk=%s rootOnDisk=%s rootInArchive=%s)",
+				i, actual, tc.expect, tc.nameOnDisk, tc.rootOnDisk, tc.rootInArchive)
+		}
+	}
+}

fs.go

@@ -453,11 +453,7 @@ func (f ArchiveFS) ReadDir(name string) ([]fs.DirEntry, error) {
 			// so as we traverse deeper, we need to implicitly find subfolders within
 			// this current directory and add fake entries to the output
 			remainingPath := strings.TrimPrefix(file.NameInArchive, name)
-			nextDir := remainingPath // if current path is "a" and name is "a/b", this becomes "/b"
-			if pos := strings.Index(remainingPath, "/"); pos >= 0 {
-				// if current path is "a" and name is longer than "a/b/..." this limits to "/b"
-				nextDir = remainingPath[:pos]
-			}
+			nextDir := topDir(remainingPath)        // if name in archive is "a/b/c" and root is "a", this becomes "b" (the implied folder to add)
 			implicitDir := path.Join(name, nextDir) // the full path of the implied directory
 
 			// create fake entry only if no entry currently exists (don't overwrite a real entry)