Fix more internal pathing bugs

archiver.go

@@ -13,6 +13,11 @@ import (
 // Archiver is a type that can create an archive file
 // from a list of source file names.
 type Archiver interface {
+	// Archive adds all the files or folders in sources
+	// to an archive to be created at destination. Files
+	// are added to the root of the archive, and directories
+	// are walked and recursively added, preserving folder
+	// structure.
 	Archive(sources []string, destination string) error
 }
 
@@ -233,34 +238,22 @@ func folderNameFromFileName(filename string) string {
 	return base
 }
 
-// makeBaseDir returns the base directory to use for storing files in an
-// archive. topLevelFolder should be the name of the top-level folder of
-// the archive (if there is one), and sourceInfo is the file info obtained
-// by calling os.Stat on the source file or directory to include in the
-// archive.
-func makeBaseDir(topLevelFolder string, sourceInfo os.FileInfo) string {
-	var baseDir string
-	if topLevelFolder != "" {
-		baseDir = topLevelFolder
-	}
-	if sourceInfo.IsDir() {
-		baseDir = path.Join(baseDir, sourceInfo.Name())
-	}
-	return baseDir
-}
-
 // makeNameInArchive returns the filename for the file given by fpath to be used within
-// the archive. sourceInfo is the info obtained by calling os.Stat on source, and baseDir
-// is the base directory obtained by calling makeBaseDir. fpath should be the unaltered
-// file path of the file given to a filepath.WalkFunc.
+// the archive. sourceInfo is the FileInfo obtained by calling os.Stat on source, and baseDir
+// is an optional base directory that becomes the root of the archive. fpath should be the
+// unaltered file path of the file given to a filepath.WalkFunc.
 func makeNameInArchive(sourceInfo os.FileInfo, source, baseDir, fpath string) (string, error) {
-	name := fpath
+	name := filepath.Base(fpath) // start with the file or dir name
 	if sourceInfo.IsDir() {
-		var err error
-		name, err = filepath.Rel(source, fpath)
+		// preserve internal directory structure; that's the path components
+		// between the source directory's leaf and this file's leaf
+		dir, err := filepath.Rel(filepath.Dir(source), filepath.Dir(fpath))
 		if err != nil {
 			return "", err
 		}
+		// prepend the internal directory structure to the leaf name,
+		// and convert path separators to forward slashes as per spec
+		name = path.Join(filepath.ToSlash(dir), name)
 	}
-	return path.Join(baseDir, filepath.ToSlash(name)), nil
+	return path.Join(baseDir, name), nil // prepend the base directory
 }

archiver_test.go

@@ -139,46 +139,12 @@ func TestMultipleTopLevels(t *testing.T) {
 	}
 }
 
-func TestMakeBaseDir(t *testing.T) {
-	for i, tc := range []struct {
-		topLevelFolder string
-		sourceInfo     fakeFileInfo
-		expect         string
-	}{
-		{
-			topLevelFolder: "",
-			sourceInfo:     fakeFileInfo{isDir: false},
-			expect:         "",
-		},
-		{
-			topLevelFolder: "foo",
-			sourceInfo:     fakeFileInfo{isDir: false},
-			expect:         "foo",
-		},
-		{
-			topLevelFolder: "",
-			sourceInfo:     fakeFileInfo{isDir: true, name: "bar"},
-			expect:         "bar",
-		},
-		{
-			topLevelFolder: "foo",
-			sourceInfo:     fakeFileInfo{isDir: true, name: "bar"},
-			expect:         "foo/bar",
-		},
-	} {
-		actual := makeBaseDir(tc.topLevelFolder, tc.sourceInfo)
-		if actual != tc.expect {
-			t.Errorf("Test %d: Expected '%s' but got '%s'", i, tc.expect, actual)
-		}
-	}
-}
-
 func TestMakeNameInArchive(t *testing.T) {
 	for i, tc := range []struct {
 		sourceInfo fakeFileInfo
-		source     string
-		baseDir    string
-		fpath      string
+		source     string // a file path explicitly listed by the user to include in the archive
+		baseDir    string // the base or root directory or path within the archive which contains all other files
+		fpath      string // the file path being walked; if source is a directory, this will be a child path
 		expect     string
 	}{
 		{
@@ -200,21 +166,70 @@ func TestMakeNameInArchive(t *testing.T) {
 			source:     "foo/bar.txt",
 			baseDir:    "",
 			fpath:      "foo/bar.txt",
-			expect:     "foo/bar.txt",
+			expect:     "bar.txt",
 		},
 		{
 			sourceInfo: fakeFileInfo{isDir: false},
 			source:     "foo/bar.txt",
 			baseDir:    "base",
 			fpath:      "foo/bar.txt",
-			expect:     "base/foo/bar.txt",
+			expect:     "base/bar.txt",
 		},
 		{
 			sourceInfo: fakeFileInfo{isDir: true},
 			source:     "foo/bar",
-			baseDir:    "bar",
+			baseDir:    "base",
 			fpath:      "foo/bar",
-			expect:     "bar",
+			expect:     "base/bar",
+		},
+		{
+			sourceInfo: fakeFileInfo{isDir: false},
+			source:     "/absolute/path.txt",
+			baseDir:    "",
+			fpath:      "/absolute/path.txt",
+			expect:     "path.txt",
+		},
+		{
+			sourceInfo: fakeFileInfo{isDir: false},
+			source:     "/absolute/sub/path.txt",
+			baseDir:    "",
+			fpath:      "/absolute/sub/path.txt",
+			expect:     "path.txt",
+		},
+		{
+			sourceInfo: fakeFileInfo{isDir: false},
+			source:     "/absolute/sub/path.txt",
+			baseDir:    "base",
+			fpath:      "/absolute/sub/path.txt",
+			expect:     "base/path.txt",
+		},
+		{
+			sourceInfo: fakeFileInfo{isDir: false},
+			source:     "sub/path.txt",
+			baseDir:    "base/subbase",
+			fpath:      "sub/path.txt",
+			expect:     "base/subbase/path.txt",
+		},
+		{
+			sourceInfo: fakeFileInfo{isDir: true},
+			source:     "sub/dir",
+			baseDir:    "base/subbase",
+			fpath:      "sub/dir/path.txt",
+			expect:     "base/subbase/dir/path.txt",
+		},
+		{
+			sourceInfo: fakeFileInfo{isDir: true},
+			source:     "sub/dir",
+			baseDir:    "base/subbase",
+			fpath:      "sub/dir/sub2/sub3/path.txt",
+			expect:     "base/subbase/dir/sub2/sub3/path.txt",
+		},
+		{
+			sourceInfo: fakeFileInfo{isDir: true},
+			source:     `/absolute/dir`,
+			baseDir:    "base",
+			fpath:      `/absolute/dir/sub1/sub2/file.txt`,
+			expect:     "base/dir/sub1/sub2/file.txt",
 		},
 	} {
 		actual, err := makeNameInArchive(tc.sourceInfo, tc.source, tc.baseDir, tc.fpath)

tar.go

@@ -237,19 +237,14 @@ func (t *Tar) untarFile(f File, to string) error {
 }
 
 func (t *Tar) writeWalk(source, topLevelFolder, destination string) error {
-	sourceAbs, err := filepath.Abs(source)
-	if err != nil {
-		return fmt.Errorf("getting absolute path: %v", err)
-	}
-	sourceInfo, err := os.Stat(sourceAbs)
+	sourceInfo, err := os.Stat(source)
 	if err != nil {
 		return fmt.Errorf("%s: stat: %v", source, err)
 	}
 	destAbs, err := filepath.Abs(destination)
 	if err != nil {
 		return fmt.Errorf("%s: getting absolute path of destination %s: %v", source, destination, err)
 	}
-	baseDir := makeBaseDir(topLevelFolder, sourceInfo)
 
 	return filepath.Walk(source, func(fpath string, info os.FileInfo, err error) error {
 		handleErr := func(err error) error {
@@ -276,7 +271,7 @@ func (t *Tar) writeWalk(source, topLevelFolder, destination string) error {
 		}
 
 		// build the name to be used within the archive
-		nameInArchive, err := makeNameInArchive(sourceInfo, source, baseDir, fpath)
+		nameInArchive, err := makeNameInArchive(sourceInfo, source, topLevelFolder, fpath)
 		if err != nil {
 			return handleErr(err)
 		}

zip.go

@@ -195,19 +195,14 @@ func (z *Zip) extractFile(f File, to string) error {
 }
 
 func (z *Zip) writeWalk(source, topLevelFolder, destination string) error {
-	sourceAbs, err := filepath.Abs(source)
-	if err != nil {
-		return fmt.Errorf("getting absolute path: %v", err)
-	}
-	sourceInfo, err := os.Stat(sourceAbs)
+	sourceInfo, err := os.Stat(source)
 	if err != nil {
 		return fmt.Errorf("%s: stat: %v", source, err)
 	}
 	destAbs, err := filepath.Abs(destination)
 	if err != nil {
 		return fmt.Errorf("%s: getting absolute path of destination %s: %v", source, destination, err)
 	}
-	baseDir := makeBaseDir(topLevelFolder, sourceInfo)
 
 	return filepath.Walk(source, func(fpath string, info os.FileInfo, err error) error {
 		handleErr := func(err error) error {
@@ -235,7 +230,7 @@ func (z *Zip) writeWalk(source, topLevelFolder, destination string) error {
 		}
 
 		// build the name to be used within the archive
-		nameInArchive, err := makeNameInArchive(sourceInfo, source, baseDir, fpath)
+		nameInArchive, err := makeNameInArchive(sourceInfo, source, topLevelFolder, fpath)
 		if err != nil {
 			return handleErr(err)
 		}