Implement --suffix for use with --backup-dir only #98

This also makes sure we remove files we are about to override in the
--backup-dir properly.

docs/content/docs.md

@@ -193,8 +193,11 @@ respectively.
 
 When using `sync`, `copy` or `move` any files which would have been
 overwritten or deleted are moved in their original hierarchy into this
-directory.  Files with matching paths already in DIR will be
+directory.
-overwritten.
+
+If `--suffix` is set, then the moved files will have the suffix added
+to them.  If there is a file with the same path (after the suffix has
+been added) in DIR, then it will be overwritten.
 
 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
@@ -209,7 +212,8 @@ which would have been updated or deleted will be stored in
 `remote:old`.
 
 If running rclone from a script you might want to use today's date as
-the directory name passed to `--backup-dir` to store the old files.
+the directory name passed to `--backup-dir` to store the old files, or
+you might want to pass `--suffix` with today's date.
 
 ### --bwlimit=BANDWIDTH_SPEC ###
 
@@ -457,6 +461,14 @@ equals 1,048,576 bits/s and not 1,000,000 bits/s.
 
 The default is `bytes`.
 
+### --suffix=SUFFIX ###
+
+This is for use with `--backup-dir` only.  If this isn't set then
+`--backup-dir` will move files with their original name.  If it is set
+then the files will have SUFFIX added on to them.
+
+See `--backup-dir` for more info.
+
 ### --track-renames ###
 
 By default rclone doesn't not keep track of renamed files, so if you

fs/config.go

@@ -87,6 +87,7 @@ var (
 	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.")
+	suffix          = StringP("suffix", "", "", "Suffix for use with --backup-dir.")
 	bwLimit         BwTimetable
 
 	// Key to use for password en/decryption.
@@ -211,6 +212,7 @@ type ConfigInfo struct {
 	NoUpdateModTime    bool
 	DataRateUnit       string
 	BackupDir          string
+	Suffix             string
 }
 
 // Find the config directory
@@ -263,6 +265,7 @@ func LoadConfig() {
 	Config.NoTraverse = *noTraverse
 	Config.NoUpdateModTime = *noUpdateModTime
 	Config.BackupDir = *backupDir
+	Config.Suffix = *suffix
 
 	ConfigPath = *configFile
 
@@ -286,6 +289,10 @@ func LoadConfig() {
 		log.Fatalf(`Can't use --size-only and --ignore-size together.`)
 	}
 
+	if Config.Suffix != "" && Config.BackupDir == "" {
+		log.Fatalf(`Can only use --suffix with --backup-dir.`)
+	}
+
 	// Load configuration file.
 	var err error
 	configData, err = loadConfigFile()

fs/operations.go

@@ -413,7 +413,9 @@ func deleteFileWithBackupDir(dst Object, backupDir Fs) (err error) {
 		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)
+			remoteWithSuffix := dst.Remote() + Config.Suffix
+			overwritten, _ := backupDir.NewObject(remoteWithSuffix)
+			err = Move(backupDir, overwritten, remoteWithSuffix, dst)
 		}
 	} else {
 		err = dst.Remove()

fs/sync.go

@@ -42,6 +42,7 @@ type syncCopyMove struct {
 	renamerWg      sync.WaitGroup      // wait for renamers
 	toBeRenamed    ObjectPairChan      // renamers channel
 	backupDir      Fs                  // place to store overwrites/deletes
+	suffix         string              // suffix to add to files placed in backupDir
 }
 
 func newSyncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) (*syncCopyMove, error) {
@@ -101,6 +102,7 @@ func newSyncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) (*syncCopyMove, er
 		if Overlapping(fsrc, s.backupDir) {
 			return nil, FatalError(errors.New("source and parameter to --backup-dir mustn't overlap"))
 		}
+		s.suffix = Config.Suffix
 	}
 	return s, nil
 }
@@ -286,7 +288,9 @@ func (s *syncCopyMove) pairChecker(in ObjectPairChan, out ObjectPairChan, wg *sy
 				if NeedTransfer(pair.dst, pair.src) {
 					// 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)
+						remoteWithSuffix := pair.dst.Remote() + s.suffix
+						overwritten, _ := s.backupDir.NewObject(remoteWithSuffix)
+						err := Move(s.backupDir, overwritten, remoteWithSuffix, pair.dst)
 						if err != nil {
 							s.processError(err)
 						} else {

fs/sync_test.go

@@ -744,7 +744,7 @@ func TestServerSideMoveOverlap(t *testing.T) {
 }
 
 // Test with BackupDir set
-func TestSyncBackupDir(t *testing.T) {
+func testSyncBackupDir(t *testing.T, suffix string) {
 	r := NewRun(t)
 	defer r.Finalise()
 
@@ -754,15 +754,19 @@ func TestSyncBackupDir(t *testing.T) {
 	r.Mkdir(r.fremote)
 
 	fs.Config.BackupDir = r.fremoteName + "/backup"
+	fs.Config.Suffix = suffix
 	defer func() {
 		fs.Config.BackupDir = ""
+		fs.Config.Suffix = ""
 	}()
 
+	// Make the setup so we have one, two, three in the dest
+	// and one (different), two (same) in the source
 	file1 := r.WriteObject("dst/one", "one", t1)
-	file2 := r.WriteObject("dst/two", "two", t2)
+	file2 := r.WriteObject("dst/two", "two", t1)
-	file3 := r.WriteObject("dst/three", "three", t3)
+	file3 := r.WriteObject("dst/three", "three", t1)
-	file2a := r.WriteFile("two", "two", t2)
+	file2a := r.WriteFile("two", "two", t1)
-	file1a := r.WriteFile("one", "oneone", t2)
+	file1a := r.WriteFile("one", "oneA", t2)
 
 	fstest.CheckItems(t, r.fremote, file1, file2, file3)
 	fstest.CheckItems(t, r.flocal, file1a, file2a)
@@ -774,13 +778,35 @@ func TestSyncBackupDir(t *testing.T) {
 	err = fs.Sync(fdst, r.flocal)
 	require.NoError(t, err)
 
-	// file1 is overwritten and the old version moved to backup-dir
+	// one should be moved to the backup dir and the new one installed
-	file1.Path = "backup/one"
+	file1.Path = "backup/one" + suffix
 	file1a.Path = "dst/one"
-	// file 2 is unchanged
+	// two should be unchanged
-	// file 3 is deleted (moved to backup dir)
+	// three should be moved to the backup dir
-	file3.Path = "backup/three"
+	file3.Path = "backup/three" + suffix
 
 	fstest.CheckItems(t, r.fremote, file1, file2, file3, file1a)
 
+	// Now check what happens if we do it again
+	// Restore a different three and update one in the source
+	file3a := r.WriteObject("dst/three", "threeA", t2)
+	file1b := r.WriteFile("one", "oneBB", t3)
+	fstest.CheckItems(t, r.fremote, file1, file2, file3, file1a, file3a)
+
+	// This should delete three and overwrite one again, checking
+	// the files got overwritten correctly in backup-dir
+	fs.Stats.ResetCounters()
+	err = fs.Sync(fdst, r.flocal)
+	require.NoError(t, err)
+
+	// one should be moved to the backup dir and the new one installed
+	file1a.Path = "backup/one" + suffix
+	file1b.Path = "dst/one"
+	// two should be unchanged
+	// three should be moved to the backup dir
+	file3a.Path = "backup/three" + suffix
+
+	fstest.CheckItems(t, r.fremote, file1b, file2, file3a, file1a)
 }
+func TestSyncBackupDir(t *testing.T)           { testSyncBackupDir(t, "") }
+func TestSyncBackupDirWithSuffix(t *testing.T) { testSyncBackupDir(t, ".bak") }