From f7aa48e53bc0673b9782c70e1b03f1e4c55f788b Mon Sep 17 00:00:00 2001 From: Matt Holt Date: Thu, 14 Apr 2022 20:12:52 -0600 Subject: [PATCH] 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 | 58 +++++++++++++++++++++----- archiver_test.go | 105 +++++++++++++++++++++++++++++++++++++++++++++++ fs.go | 6 +-- 3 files changed, 154 insertions(+), 15 deletions(-) diff --git a/archiver.go b/archiver.go index 309cceba..4ad9d3ad 100644 --- a/archiver.go +++ b/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 } diff --git a/archiver_test.go b/archiver_test.go index 02d349b5..0f5e9f51 100644 --- a/archiver_test.go +++ b/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) + } + } +} diff --git a/fs.go b/fs.go index 492761b7..b9108181 100644 --- a/fs.go +++ b/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)