fs: Allow sync of a file and a directory with the same name

When sorting fs.DirEntries we sort by DirEntry type and
when synchronizing files let the directories be before objects,
so when the destintation fs doesn't support duplicate names,
we will only lose duplicated object instead of whole directory.

The enables synchronisation to work with a file and a directory of the same name
which is reasonably common on bucket based remotes.

fs/direntries.go

@@ -17,7 +17,7 @@ func (ds DirEntries) Swap(i, j int) {
 
 // Less is part of sort.Interface.
 func (ds DirEntries) Less(i, j int) bool {
-	return ds[i].Remote() < ds[j].Remote()
+	return CompareDirEntries(ds[i], ds[j]) < 0
 }
 
 // ForObject runs the function supplied on every object in the entries
@@ -79,3 +79,28 @@ func DirEntryType(d DirEntry) string {
 	}
 	return fmt.Sprintf("unknown type %T", d)
 }
+
+// CompareDirEntries returns 1 if a > b, 0 if a == b and -1 if a < b
+// If two dir entries have the same name, compare their types (directories are before objects)
+func CompareDirEntries(a, b DirEntry) int {
+	aName := a.Remote()
+	bName := b.Remote()
+
+	if aName > bName {
+		return 1
+	} else if aName < bName {
+		return -1
+	}
+
+	typeA := DirEntryType(a)
+	typeB := DirEntryType(b)
+
+	// same name, compare types
+	if typeA > typeB {
+		return 1
+	} else if typeA < typeB {
+		return -1
+	}
+
+	return 0
+}

fs/direntries_test.go

@@ -0,0 +1,26 @@
+package fs_test
+
+import (
+	"sort"
+	"testing"
+
+	"github.com/ncw/rclone/fs"
+	"github.com/ncw/rclone/fstest/mockdir"
+	"github.com/ncw/rclone/fstest/mockobject"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDirEntriesSort(t *testing.T) {
+	a := mockobject.New("a")
+	aDir := mockdir.New("a")
+	b := mockobject.New("b")
+	bDir := mockdir.New("b")
+	c := mockobject.New("c")
+	cDir := mockdir.New("c")
+	anotherc := mockobject.New("c")
+	dirEntries := fs.DirEntries{bDir, b, aDir, a, c, cDir, anotherc}
+
+	sort.Stable(dirEntries)
+
+	assert.Equal(t, fs.DirEntries{aDir, a, bDir, b, cDir, c, anotherc}, dirEntries)
+}

fs/march/march.go

@@ -213,7 +213,7 @@ func (es matchEntries) Less(i, j int) bool {
 	ei, ej := &es[i], &es[j]
 	if ei.name == ej.name {
 		if ei.leaf == ej.leaf {
-			return ei.entry.Remote() < ej.entry.Remote()
+			return fs.CompareDirEntries(ei.entry, ej.entry) < 0
 		}
 		return ei.leaf < ej.leaf
 	}
@@ -267,6 +267,7 @@ type matchTransformFn func(name string) string
 func matchListings(srcListEntries, dstListEntries fs.DirEntries, transforms []matchTransformFn) (srcOnly fs.DirEntries, dstOnly fs.DirEntries, matches []matchPair) {
 	srcList := newMatchEntries(srcListEntries, transforms)
 	dstList := newMatchEntries(dstListEntries, transforms)
+
 	for iSrc, iDst := 0, 0; ; iSrc, iDst = iSrc+1, iDst+1 {
 		var src, dst fs.DirEntry
 		var srcName, dstName string
@@ -282,34 +283,40 @@ func matchListings(srcListEntries, dstListEntries fs.DirEntries, transforms []ma
 			break
 		}
 		if src != nil && iSrc > 0 {
-			prev := srcList[iSrc-1].name
-			if srcName == prev {
+			prev := srcList[iSrc-1].entry
+			prevName := srcList[iSrc-1].name
+			if srcName == prevName && fs.DirEntryType(prev) == fs.DirEntryType(src) {
 				fs.Logf(src, "Duplicate %s found in source - ignoring", fs.DirEntryType(src))
 				iDst-- // ignore the src and retry the dst
 				continue
-			} else if srcName < prev {
+			} else if srcName < prevName {
 				// this should never happen since we sort the listings
 				panic("Out of order listing in source")
 			}
 		}
 		if dst != nil && iDst > 0 {
-			prev := dstList[iDst-1].name
-			if dstName == prev {
+			prev := dstList[iDst-1].entry
+			prevName := dstList[iDst-1].name
+			if dstName == prevName && fs.DirEntryType(dst) == fs.DirEntryType(prev) {
 				fs.Logf(dst, "Duplicate %s found in destination - ignoring", fs.DirEntryType(dst))
 				iSrc-- // ignore the dst and retry the src
 				continue
-			} else if dstName < prev {
+			} else if dstName < prevName {
 				// this should never happen since we sort the listings
 				panic("Out of order listing in destination")
 			}
 		}
 		if src != nil && dst != nil {
-			if srcName < dstName {
-				dst = nil
-				iDst-- // retry the dst
-			} else if srcName > dstName {
+			// we can't use CompareDirEntries because srcName, dstName could
+			// be different then src.Remote() or dst.Remote()
+			srcType := fs.DirEntryType(src)
+			dstType := fs.DirEntryType(dst)
+			if srcName > dstName || (srcName == dstName && srcType > dstType) {
 				src = nil
-				iSrc-- // retry the src
+				iSrc--
+			} else if srcName < dstName || (srcName == dstName && srcType < dstType) {
+				dst = nil
+				iDst--
 			}
 		}
 		// Debugf(nil, "src = %v, dst = %v", src, dst)

fs/march/march_test.go

@@ -3,10 +3,12 @@
 package march
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
 	"github.com/ncw/rclone/fs"
+	"github.com/ncw/rclone/fstest/mockdir"
 	"github.com/ncw/rclone/fstest/mockobject"
 	"github.com/stretchr/testify/assert"
 )
@@ -43,6 +45,8 @@ func TestMatchListings(t *testing.T) {
 		b    = mockobject.Object("b")
 		c    = mockobject.Object("c")
 		d    = mockobject.Object("d")
+		dirA = mockdir.New("A")
+		dirb = mockdir.New("b")
 	)
 
 	for _, test := range []struct {
@@ -147,7 +151,69 @@ func TestMatchListings(t *testing.T) {
 			},
 			transforms: []matchTransformFn{strings.ToLower},
 		},
+		{
+			what: "File and directory are not duplicates - srcOnly",
+			input: fs.DirEntries{
+				dirA, nil,
+				A, nil,
+			},
+			srcOnly: fs.DirEntries{
+				dirA,
+				A,
+			},
+		},
+		{
+			what: "File and directory are not duplicates - matches",
+			input: fs.DirEntries{
+				dirA, dirA,
+				A, A,
+			},
+			matches: []matchPair{
+				{dirA, dirA},
+				{A, A},
+			},
+		},
+		{
+			what: "Sync with directory #1",
+			input: fs.DirEntries{
+				dirA, nil,
+				A, nil,
+				b, b,
+				nil, c,
+				nil, d,
+			},
+			srcOnly: fs.DirEntries{
+				dirA,
+				A,
+			},
+			dstOnly: fs.DirEntries{
+				c, d,
+			},
+			matches: []matchPair{
+				{b, b},
+			},
+		},
+		{
+			what: "Sync with 2 directories",
+			input: fs.DirEntries{
+				dirA, dirA,
+				A, nil,
+				nil, dirb,
+				nil, b,
+			},
+			srcOnly: fs.DirEntries{
+				A,
+			},
+			dstOnly: fs.DirEntries{
+				dirb,
+				b,
+			},
+			matches: []matchPair{
+				{dirA, dirA},
+			},
+		},
 	} {
+		t.Run(fmt.Sprintf("TestMatchListings-%s", test.what), func(t *testing.T) {
 			var srcList, dstList fs.DirEntries
 			for i := 0; i < len(test.input); i += 2 {
 				src, dst := test.input[i], test.input[i+1]
@@ -159,13 +225,14 @@ func TestMatchListings(t *testing.T) {
 				}
 			}
 			srcOnly, dstOnly, matches := matchListings(srcList, dstList, test.transforms)
-		assert.Equal(t, test.srcOnly, srcOnly, test.what)
-		assert.Equal(t, test.dstOnly, dstOnly, test.what)
-		assert.Equal(t, test.matches, matches, test.what)
+			assert.Equal(t, test.srcOnly, srcOnly, test.what, "srcOnly differ")
+			assert.Equal(t, test.dstOnly, dstOnly, test.what, "dstOnly differ")
+			assert.Equal(t, test.matches, matches, test.what, "matches differ")
 			// now swap src and dst
 			dstOnly, srcOnly, matches = matchListings(dstList, srcList, test.transforms)
-		assert.Equal(t, test.srcOnly, srcOnly, test.what)
-		assert.Equal(t, test.dstOnly, dstOnly, test.what)
-		assert.Equal(t, test.matches, matches, test.what)
+			assert.Equal(t, test.srcOnly, srcOnly, test.what, "srcOnly differ")
+			assert.Equal(t, test.dstOnly, dstOnly, test.what, "dstOnly differ")
+			assert.Equal(t, test.matches, matches, test.what, "matches differ")
+		})
 	}
 }