Implement --backup-dir - fixes #98

The parameter of backup-dir specifies a remote that all deleted or
overwritten files will be copied to.

docs/content/docs.md

@@ -189,6 +189,24 @@ for bytes, `k` for kBytes, `M` for MBytes and `G` for GBytes may be
 used.  These are the binary units, eg 1, 2\*\*10, 2\*\*20, 2\*\*30
 respectively.
 
+### --backup-dir=DIR ###
+
+When using `sync`, `copy` or `move` any files which would have been
+overwritten or deleted are moved in their original hierarchy into this
+directory.
+
+The remote in use must support server side move or copy and you must
+use the same remote as the destination of the sync.  The backup
+directory must not overlap the destination directory.
+
+For example
+
+    rclone sync /path/to/local remote:current --backup-dir remote:old
+
+will sync `/path/to/local` to `remote:current`, but for any files
+which would have been updated or deleted will be stored in
+`remote:old`.
+
 ### --bwlimit=BANDWIDTH_SPEC ###
 
 This option controls the bandwidth limit. Limits can be specified

fs/config.go

@@ -86,6 +86,7 @@ var (
 	ignoreSize      = BoolP("ignore-size", "", false, "Ignore size when skipping use mod-time or checksum.")
 	noTraverse      = BoolP("no-traverse", "", false, "Don't traverse destination file system on copy.")
 	noUpdateModTime = BoolP("no-update-modtime", "", false, "Don't update destination mod-time if files identical.")
+	backupDir       = StringP("backup-dir", "", "", "Make backups into hierarchy based in DIR.")
 	bwLimit         BwTimetable
 
 	// Key to use for password en/decryption.
@@ -209,6 +210,7 @@ type ConfigInfo struct {
 	NoTraverse         bool
 	NoUpdateModTime    bool
 	DataRateUnit       string
+	BackupDir          string
 }
 
 // Find the config directory
@@ -260,6 +262,7 @@ func LoadConfig() {
 	Config.IgnoreSize = *ignoreSize
 	Config.NoTraverse = *noTraverse
 	Config.NoUpdateModTime = *noUpdateModTime
+	Config.BackupDir = *backupDir
 
 	ConfigPath = *configFile
 

fs/operations.go

@@ -396,26 +396,52 @@ func CanServerSideMove(fdst Fs) bool {
 	return canMove || canCopy
 }
 
-// DeleteFile deletes a single file respecting --dry-run and accumulating stats and errors.
+// deleteFileWithBackupDir deletes a single file respecting --dry-run
-func DeleteFile(dst Object) (err error) {
+// and accumulating stats and errors.
-	if Config.DryRun {
+//
-		Log(dst, "Not deleting as --dry-run")
+// If backupDir is set then it moves the file to there instead of
-	} else {
+// deleting
-		Stats.Checking(dst.Remote())
+func deleteFileWithBackupDir(dst Object, backupDir Fs) (err error) {
-		err = dst.Remove()
+	Stats.Checking(dst.Remote())
-		Stats.DoneChecking(dst.Remote())
+	action, actioned, actioning := "delete", "Deleted", "deleting"
-		if err != nil {
+	if backupDir != nil {
-			Stats.Error()
+		action, actioned, actioning = "move into backup dir", "Moved into backup dir", "moving into backup dir"
-			ErrorLog(dst, "Couldn't delete: %v", err)
-		} else {
-			Debug(dst, "Deleted")
-		}
 	}
+	if Config.DryRun {
+		Log(dst, "Not %s as --dry-run", actioning)
+	} else if backupDir != nil {
+		if !SameConfig(dst.Fs(), backupDir) {
+			err = errors.New("parameter to --backup-dir has to be on the same remote as destination")
+		} else {
+			err = Move(backupDir, nil, dst.Remote(), dst)
+		}
+	} else {
+		err = dst.Remove()
+	}
+	if err != nil {
+		Stats.Error()
+		ErrorLog(dst, "Couldn't %s: %v", action, err)
+	} else {
+		Debug(dst, actioned)
+	}
+	Stats.DoneChecking(dst.Remote())
 	return err
 }
 
-// DeleteFiles removes all the files passed in the channel
+// DeleteFile deletes a single file respecting --dry-run and accumulating stats and errors.
-func DeleteFiles(toBeDeleted ObjectsChan) error {
+//
+// If useBackupDir is set and --backup-dir is in effect then it moves
+// the file to there instead of deleting
+func DeleteFile(dst Object) (err error) {
+	return deleteFileWithBackupDir(dst, nil)
+}
+
+// deleteFilesWithBackupDir removes all the files passed in the
+// channel
+//
+// If backupDir is set the files will be placed into that directory
+// instead of being deleted.
+func deleteFilesWithBackupDir(toBeDeleted ObjectsChan, backupDir Fs) error {
 	var wg sync.WaitGroup
 	wg.Add(Config.Transfers)
 	var errorCount int32
@@ -423,7 +449,7 @@ func DeleteFiles(toBeDeleted ObjectsChan) error {
 		go func() {
 			defer wg.Done()
 			for dst := range toBeDeleted {
-				err := DeleteFile(dst)
+				err := deleteFileWithBackupDir(dst, backupDir)
 				if err != nil {
 					atomic.AddInt32(&errorCount, 1)
 				}
@@ -438,6 +464,11 @@ func DeleteFiles(toBeDeleted ObjectsChan) error {
 	return nil
 }
 
+// DeleteFiles removes all the files passed in the channel
+func DeleteFiles(toBeDeleted ObjectsChan) error {
+	return deleteFilesWithBackupDir(toBeDeleted, nil)
+}
+
 // Read a Objects into add() for the given Fs.
 // dir is the start directory, "" for root
 // If includeAll is specified all files will be added,

fs/sync.go

@@ -41,10 +41,10 @@ type syncCopyMove struct {
 	renameMap      map[string][]Object // dst files by hash - only used by trackRenames
 	renamerWg      sync.WaitGroup      // wait for renamers
 	toBeRenamed    ObjectPairChan      // renamers channel
+	backupDir      Fs                  // place to store overwrites/deletes
 }
 
-func newSyncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) *syncCopyMove {
+func newSyncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) (*syncCopyMove, error) {
-	canServerSideMove := CanServerSideMove(fdst)
 	s := &syncCopyMove{
 		fdst:           fdst,
 		fsrc:           fsrc,
@@ -69,7 +69,7 @@ func newSyncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) *syncCopyMove {
 	}
 	if s.trackRenames {
 		// Don't track renames for remotes without server-side move support.
-		if !canServerSideMove {
+		if !CanServerSideMove(fdst) {
 			ErrorLog(fdst, "Ignoring --track-renames as the destination does not support server-side move or copy")
 			s.trackRenames = false
 		}
@@ -82,7 +82,27 @@ func newSyncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) *syncCopyMove {
 		Debug(s.fdst, "Ignoring --no-traverse with --track-renames")
 		s.noTraverse = false
 	}
-	return s
+	// Make Fs for --backup-dir if required
+	if Config.BackupDir != "" {
+		var err error
+		s.backupDir, err = NewFs(Config.BackupDir)
+		if err != nil {
+			return nil, FatalError(errors.Errorf("Failed to make fs for --backup-dir %q: %v", Config.BackupDir, err))
+		}
+		if !CanServerSideMove(s.backupDir) {
+			return nil, FatalError(errors.New("can't use --backup-dir on a remote which doesn't support server side move or copy"))
+		}
+		if !SameConfig(fdst, s.backupDir) {
+			return nil, FatalError(errors.New("parameter to --backup-dir has to be on the same remote as destination"))
+		}
+		if Overlapping(fdst, s.backupDir) {
+			return nil, FatalError(errors.New("destination and parameter to --backup-dir mustn't overlap"))
+		}
+		if Overlapping(fsrc, s.backupDir) {
+			return nil, FatalError(errors.New("source and parameter to --backup-dir mustn't overlap"))
+		}
+	}
+	return s, nil
 }
 
 // Check to see if have set the abort flag
@@ -264,7 +284,19 @@ func (s *syncCopyMove) pairChecker(in ObjectPairChan, out ObjectPairChan, wg *sy
 			// Check to see if can store this
 			if src.Storable() {
 				if NeedTransfer(pair.dst, pair.src) {
-					out <- pair
+					// If destination already exists, then we must move it into --backup-dir if required
+					if pair.dst != nil && s.backupDir != nil {
+						err := Move(s.backupDir, nil, pair.dst.Remote(), pair.dst)
+						if err != nil {
+							s.processError(err)
+						} else {
+							// If successful zero out the dst as it is no longer there and copy the file
+							pair.dst = nil
+							out <- pair
+						}
+					} else {
+						out <- pair
+					}
 				} else {
 					// If moving need to delete the files we don't need to copy
 					if s.DoMove {
@@ -411,7 +443,7 @@ func (s *syncCopyMove) deleteFiles(checkSrcMap bool) error {
 		}
 		close(toDelete)
 	}()
-	return DeleteFiles(toDelete)
+	return deleteFilesWithBackupDir(toDelete, s.backupDir)
 }
 
 // renameHash makes a string with the size and the hash for rename detection
@@ -655,17 +687,29 @@ func (s *syncCopyMove) run() error {
 
 // Sync fsrc into fdst
 func Sync(fdst, fsrc Fs) error {
-	return newSyncCopyMove(fdst, fsrc, true, false).run()
+	do, err := newSyncCopyMove(fdst, fsrc, true, false)
+	if err != nil {
+		return err
+	}
+	return do.run()
 }
 
 // CopyDir copies fsrc into fdst
 func CopyDir(fdst, fsrc Fs) error {
-	return newSyncCopyMove(fdst, fsrc, false, false).run()
+	do, err := newSyncCopyMove(fdst, fsrc, false, false)
+	if err != nil {
+		return err
+	}
+	return do.run()
 }
 
 // moveDir moves fsrc into fdst
 func moveDir(fdst, fsrc Fs) error {
-	return newSyncCopyMove(fdst, fsrc, false, true).run()
+	do, err := newSyncCopyMove(fdst, fsrc, false, true)
+	if err != nil {
+		return err
+	}
+	return do.run()
 }
 
 // MoveDir moves fsrc into fdst

fs/sync_test.go

@@ -742,3 +742,45 @@ func TestServerSideMoveOverlap(t *testing.T) {
 	err = fs.MoveDir(fremoteMove, r.fremote)
 	assert.EqualError(t, err, fs.ErrorCantMoveOverlapping.Error())
 }
+
+// Test with BackupDir set
+func TestSyncBackupDir(t *testing.T) {
+	r := NewRun(t)
+	defer r.Finalise()
+
+	if !fs.CanServerSideMove(r.fremote) {
+		t.Skip("Skipping test as remote does not support server side move")
+	}
+	r.Mkdir(r.fremote)
+
+	fs.Config.BackupDir = r.fremoteName + "/backup"
+	defer func() {
+		fs.Config.BackupDir = ""
+	}()
+
+	file1 := r.WriteObject("dst/one", "one", t1)
+	file2 := r.WriteObject("dst/two", "two", t2)
+	file3 := r.WriteObject("dst/three", "three", t3)
+	file2a := r.WriteFile("two", "two", t2)
+	file1a := r.WriteFile("one", "oneone", t2)
+
+	fstest.CheckItems(t, r.fremote, file1, file2, file3)
+	fstest.CheckItems(t, r.flocal, file1a, file2a)
+
+	fdst, err := fs.NewFs(r.fremoteName + "/dst")
+	require.NoError(t, err)
+
+	fs.Stats.ResetCounters()
+	err = fs.Sync(fdst, r.flocal)
+	require.NoError(t, err)
+
+	// file1 is overwritten and the old version moved to backup-dir
+	file1.Path = "backup/one"
+	file1a.Path = "dst/one"
+	// file 2 is unchanged
+	// file 3 is deleted (moved to backup dir)
+	file3.Path = "backup/three"
+
+	fstest.CheckItems(t, r.fremote, file1, file2, file3, file1a)
+
+}

fstest/fstest.go

@@ -93,7 +93,7 @@ func (i *Item) CheckHashes(t *testing.T, obj fs.Object) {
 // Check checks all the attributes of the object are correct
 func (i *Item) Check(t *testing.T, obj fs.Object, precision time.Duration) {
 	i.CheckHashes(t, obj)
-	assert.Equal(t, i.Size, obj.Size())
+	assert.Equal(t, i.Size, obj.Size(), fmt.Sprintf("%s: size incorrect", i.Path))
 	i.CheckModTime(t, obj, obj.ModTime(), precision)
 }