Fix fs.Overlapping and factor fs.SameConfig

fs/operations.go

@@ -252,7 +252,7 @@ func Copy(f Fs, dst Object, remote string, src Object) (err error) {
 		// Try server side copy first - if has optional interface and
 		// is same underlying remote
 		actionTaken = "Copied (server side copy)"
-		if fCopy, ok := f.(Copier); ok && src.Fs().Name() == f.Name() {
+		if fCopy, ok := f.(Copier); ok && SameConfig(src.Fs(), f) {
 			var newDst Object
 			newDst, err = fCopy.Copy(src, remote)
 			if err == nil {
@@ -353,7 +353,7 @@ func Move(fdst Fs, dst Object, remote string, src Object) (err error) {
 		return nil
 	}
 	// See if we have Move available
-	if do, ok := fdst.(Mover); ok && src.Fs().Name() == fdst.Name() {
+	if do, ok := fdst.(Mover); ok && SameConfig(src.Fs(), fdst) {
 		// Delete destination if it exists
 		if dst != nil {
 			err = DeleteFile(dst)
@@ -536,15 +536,34 @@ func readFilesMaps(fdst Fs, fdstIncludeAll bool, fsrc Fs, fsrcIncludeAll bool, d
 	return dstFiles, srcFiles, err
 }
 
+// SameConfig returns true if fdst and fsrc are using the same config
+// file entry
+func SameConfig(fdst, fsrc Info) bool {
+	return fdst.Name() == fsrc.Name()
+}
+
 // Same returns true if fdst and fsrc point to the same underlying Fs
-func Same(fdst, fsrc Fs) bool {
+func Same(fdst, fsrc Info) bool {
-	return fdst.Name() == fsrc.Name() && fdst.Root() == fsrc.Root()
+	return SameConfig(fdst, fsrc) && fdst.Root() == fsrc.Root()
 }
 
 // Overlapping returns true if fdst and fsrc point to the same
-// underlying Fs or they overlap.
+// underlying Fs and they overlap.
-func Overlapping(fdst, fsrc Fs) bool {
+func Overlapping(fdst, fsrc Info) bool {
-	return fdst.Name() == fsrc.Name() && (strings.HasPrefix(fdst.Root(), fsrc.Root()) || strings.HasPrefix(fsrc.Root(), fdst.Root()))
+	if !SameConfig(fdst, fsrc) {
+		return false
+	}
+	// Return the Root with a trailing / if not empty
+	fixedRoot := func(f Info) string {
+		s := strings.Trim(f.Root(), "/")
+		if s != "" {
+			s += "/"
+		}
+		return s
+	}
+	fdstRoot := fixedRoot(fdst)
+	fsrcRoot := fixedRoot(fsrc)
+	return strings.HasPrefix(fdstRoot, fsrcRoot) || strings.HasPrefix(fsrcRoot, fdstRoot)
 }
 
 // checkIdentical checks to see if dst and src are identical

fs/operations_test.go

@@ -22,6 +22,7 @@ package fs_test
 import (
 	"bytes"
 	"flag"
+	"fmt"
 	"io/ioutil"
 	"log"
 	"os"
@@ -769,3 +770,93 @@ func TestCopyFile(t *testing.T) {
 	fstest.CheckItems(t, r.flocal, file1)
 	fstest.CheckItems(t, r.fremote, file2)
 }
+
+// testFsInfo is for unit testing fs.Info
+type testFsInfo struct {
+	name      string
+	root      string
+	stringVal string
+	precision time.Duration
+	hashes    fs.HashSet
+}
+
+// Name of the remote (as passed into NewFs)
+func (i *testFsInfo) Name() string { return i.name }
+
+// Root of the remote (as passed into NewFs)
+func (i *testFsInfo) Root() string { return i.root }
+
+// String returns a description of the FS
+func (i *testFsInfo) String() string { return i.stringVal }
+
+// Precision of the ModTimes in this Fs
+func (i *testFsInfo) Precision() time.Duration { return i.precision }
+
+// Returns the supported hash types of the filesystem
+func (i *testFsInfo) Hashes() fs.HashSet { return i.hashes }
+
+func TestSameConfig(t *testing.T) {
+	a := &testFsInfo{name: "name", root: "root"}
+	for _, test := range []struct {
+		name     string
+		root     string
+		expected bool
+	}{
+		{"name", "root", true},
+		{"name", "rooty", true},
+		{"namey", "root", false},
+		{"namey", "roott", false},
+	} {
+		b := &testFsInfo{name: test.name, root: test.root}
+		actual := fs.SameConfig(a, b)
+		assert.Equal(t, test.expected, actual)
+		actual = fs.SameConfig(b, a)
+		assert.Equal(t, test.expected, actual)
+	}
+}
+
+func TestSame(t *testing.T) {
+	a := &testFsInfo{name: "name", root: "root"}
+	for _, test := range []struct {
+		name     string
+		root     string
+		expected bool
+	}{
+		{"name", "root", true},
+		{"name", "rooty", false},
+		{"namey", "root", false},
+		{"namey", "roott", false},
+	} {
+		b := &testFsInfo{name: test.name, root: test.root}
+		actual := fs.Same(a, b)
+		assert.Equal(t, test.expected, actual)
+		actual = fs.Same(b, a)
+		assert.Equal(t, test.expected, actual)
+	}
+}
+
+func TestOverlapping(t *testing.T) {
+	a := &testFsInfo{name: "name", root: "root"}
+	for _, test := range []struct {
+		name     string
+		root     string
+		expected bool
+	}{
+		{"name", "root", true},
+		{"namey", "root", false},
+		{"name", "rooty", false},
+		{"namey", "rooty", false},
+		{"name", "roo", false},
+		{"name", "root/toot", true},
+		{"name", "root/toot/", true},
+		{"name", "", true},
+		{"name", "/", true},
+	} {
+		b := &testFsInfo{name: test.name, root: test.root}
+		what := fmt.Sprintf("(%q,%q) vs (%q,%q)", a.name, a.root, b.name, b.root)
+		actual := fs.Overlapping(a, b)
+		assert.Equal(t, test.expected, actual, what)
+		actual = fs.Overlapping(b, a)
+		assert.Equal(t, test.expected, actual, what)
+	}
+}

fs/sync.go

@@ -676,7 +676,7 @@ func MoveDir(fdst, fsrc Fs) error {
 	}
 
 	// First attempt to use DirMover if exists, same Fs and no filters are active
-	if fdstDirMover, ok := fdst.(DirMover); ok && fsrc.Name() == fdst.Name() && Config.Filter.InActive() {
+	if fdstDirMover, ok := fdst.(DirMover); ok && SameConfig(fsrc, fdst) && Config.Filter.InActive() {
 		if Config.DryRun {
 			Log(fdst, "Not doing server side directory move as --dry-run")
 			return nil