Fix move command

* Delete src files which already existed in dst - fixes #751
  * Fix deletion of src file when dst file older

fs/sync.go

@@ -116,27 +116,23 @@ func (s *syncCopyMove) readDstFiles() {
 }
 
 // Check to see if src needs to be copied to dst and if so puts it in out
-func (s *syncCopyMove) checkOne(pair ObjectPair, out ObjectPairChan) {
+//
+// Returns a flag which indicates whether the file needs to be transferred or not.
+func (s *syncCopyMove) checkOne(pair ObjectPair) bool {
 	src, dst := pair.src, pair.dst
 	if dst == nil {
 		Debug(src, "Couldn't find file - need to transfer")
-		out <- pair
-		return
-	}
-	// Check to see if can store this
-	if !src.Storable() {
-		return
+		return true
 	}
 	// If we should ignore existing files, don't transfer
 	if Config.IgnoreExisting {
 		Debug(src, "Destination exists, skipping")
-		return
+		return false
 	}
 	// If we should upload unconditionally
 	if Config.IgnoreTimes {
-		Debug(src, "Uploading unconditionally as --ignore-times is in use")
-		out <- pair
-		return
+		Debug(src, "Transferring unconditionally as --ignore-times is in use")
+		return true
 	}
 	// If UpdateOlder is in effect, skip if dst is newer than src
 	if Config.UpdateOlder {
@@ -154,13 +150,13 @@ func (s *syncCopyMove) checkOne(pair ObjectPair, out ObjectPairChan) {
 		switch {
 		case dt >= modifyWindow:
 			Debug(src, "Destination is newer than source, skipping")
-			return
+			return false
 		case dt <= -modifyWindow:
 			Debug(src, "Destination is older than source, transferring")
 		default:
 			if src.Size() == dst.Size() {
 				Debug(src, "Destination mod time is within %v of source and sizes identical, skipping", modifyWindow)
-				return
+				return false
 			}
 			Debug(src, "Destination mod time is within %v of source but sizes differ, transferring", modifyWindow)
 		}
@@ -168,10 +164,10 @@ func (s *syncCopyMove) checkOne(pair ObjectPair, out ObjectPairChan) {
 		// Check to see if changed or not
 		if Equal(src, dst) {
 			Debug(src, "Unchanged skipping")
-			return
+			return false
 		}
 	}
-	out <- pair
+	return true
 }
 
 // This checks the types of errors returned while copying files
@@ -210,7 +206,18 @@ func (s *syncCopyMove) pairChecker(in ObjectPairChan, out ObjectPairChan, wg *sy
 			}
 			src := pair.src
 			Stats.Checking(src.Remote())
-			s.checkOne(pair, out)
+			// Check to see if can store this
+			if src.Storable() {
+				if s.checkOne(pair) {
+					out <- pair
+				} else {
+					// If moving need to delete the files we don't need to copy
+					if s.DoMove {
+						// Delete src if no error on copy
+						//s.processError(DeleteFile(src))
+					}
+				}
+			}
 			Stats.DoneChecking(src.Remote())
 		case <-s.abort:
 			return
@@ -282,7 +289,7 @@ func (s *syncCopyMove) pairMover(in ObjectPairChan, fdst Fs, wg *sync.WaitGroup)
 			} else if haveMover && src.Fs().Name() == fdst.Name() {
 				// Delete destination if it exists
 				if dst != nil {
-					s.processError(DeleteFile(src))
+					s.processError(DeleteFile(dst))
 				}
 				// Move dst <- src
 				_, err := fdstMover.Move(src, src.Remote())

fs/sync_test.go

@@ -577,22 +577,28 @@ func TestSyncWithUpdateOlder(t *testing.T) {
 func testServerSideMove(t *testing.T, r *Run, fremoteMove fs.Fs, withFilter bool) {
 	file1 := r.WriteBoth("potato2", "------------------------------------------------------------", t1)
 	file2 := r.WriteBoth("empty space", "", t2)
+	file3u := r.WriteBoth("potato3", "------------------------------------------------------------ UPDATED", t2)
 
-	fstest.CheckItems(t, r.fremote, file2, file1)
+	fstest.CheckItems(t, r.fremote, file2, file1, file3u)
 
 	t.Logf("Server side move (if possible) %v -> %v", r.fremote, fremoteMove)
 
 	// Write just one file in the new remote
 	r.WriteObjectTo(fremoteMove, "empty space", "", t2, false)
-	fstest.CheckItems(t, fremoteMove, file2)
+	file3 := r.WriteObjectTo(fremoteMove, "potato3", "------------------------------------------------------------", t1, false)
+	fstest.CheckItems(t, fremoteMove, file2, file3)
 
 	// Do server side move
 	fs.Stats.ResetCounters()
 	err := fs.MoveDir(fremoteMove, r.fremote)
 	require.NoError(t, err)
 
-	fstest.CheckItems(t, r.fremote, file2)
-	fstest.CheckItems(t, fremoteMove, file2, file1)
+	if withFilter {
+		fstest.CheckItems(t, r.fremote, file2)
+	} else {
+		fstest.CheckItems(t, r.fremote)
+	}
+	fstest.CheckItems(t, fremoteMove, file2, file1, file3u)
 
 	// Purge the original before moving
 	require.NoError(t, fs.Purge(r.fremote))
@@ -603,10 +609,10 @@ func testServerSideMove(t *testing.T, r *Run, fremoteMove fs.Fs, withFilter bool
 	require.NoError(t, err)
 
 	if withFilter {
-		fstest.CheckItems(t, r.fremote, file1)
+		fstest.CheckItems(t, r.fremote, file1, file3u)
 		fstest.CheckItems(t, fremoteMove, file2)
 	} else {
-		fstest.CheckItems(t, r.fremote, file2, file1)
+		fstest.CheckItems(t, r.fremote, file2, file1, file3u)
 		fstest.CheckItems(t, fremoteMove)
 	}
 }