sync: Implement --suffix-keep-extension for use with --suffix - fixes #3032

docs/content/docs.md

@@ -821,6 +821,16 @@ then the files will have SUFFIX added on to them.
 
 See `--backup-dir` for more info.
 
+### --suffix-keep-extension ###
+
+When using `--suffix`, setting this causes rclone put the SUFFIX
+before the extension of the files that it backs up rather than after.
+
+So let's say we had `--suffix -2019-01-01`, without the flag `file.txt`
+would be backed up to `file.txt-2019-01-01` and with the flag it would
+be backed up to `file-2019-01-01.txt`.  This can be helpful to make
+sure the suffixed files can still be opened.
+
 ### --syslog ###
 
 On capable OSes (not Windows or Plan9) send all log output to syslog.

fs/config.go

@@ -67,6 +67,7 @@ type ConfigInfo struct {
 	DataRateUnit          string
 	BackupDir             string
 	Suffix                string
+	SuffixKeepExtension   bool
 	UseListR              bool
 	BufferSize            SizeSuffix
 	BwLimit               BwTimetable

fs/config/configflags/configflags.go

@@ -68,6 +68,7 @@ func AddFlags(flagSet *pflag.FlagSet) {
 	flags.BoolVarP(flagSet, &fs.Config.NoUpdateModTime, "no-update-modtime", "", fs.Config.NoUpdateModTime, "Don't update destination mod-time if files identical.")
 	flags.StringVarP(flagSet, &fs.Config.BackupDir, "backup-dir", "", fs.Config.BackupDir, "Make backups into hierarchy based in DIR.")
 	flags.StringVarP(flagSet, &fs.Config.Suffix, "suffix", "", fs.Config.Suffix, "Suffix for use with --backup-dir.")
+	flags.BoolVarP(flagSet, &fs.Config.SuffixKeepExtension, "suffix-keep-extension", "", fs.Config.SuffixKeepExtension, "Preserve the extension when using --suffix.")
 	flags.BoolVarP(flagSet, &fs.Config.UseListR, "fast-list", "", fs.Config.UseListR, "Use recursive list if available. Uses more memory but fewer transactions.")
 	flags.Float64VarP(flagSet, &fs.Config.TPSLimit, "tpslimit", "", fs.Config.TPSLimit, "Limit HTTP transactions per second to this.")
 	flags.IntVarP(flagSet, &fs.Config.TPSLimitBurst, "tpslimit-burst", "", fs.Config.TPSLimitBurst, "Max burst of transactions for --tpslimit.")

fs/operations/operations.go

@@ -435,6 +435,20 @@ func CanServerSideMove(fdst fs.Fs) bool {
 	return canMove || canCopy
 }
 
+// SuffixName adds the current --suffix to the remote, obeying
+// --suffix-keep-extension if set
+func SuffixName(remote string) string {
+	if fs.Config.Suffix == "" {
+		return remote
+	}
+	if fs.Config.SuffixKeepExtension {
+		ext := path.Ext(remote)
+		base := remote[:len(remote)-len(ext)]
+		return base + fs.Config.Suffix + ext
+	}
+	return remote + fs.Config.Suffix
+}
+
 // DeleteFileWithBackupDir deletes a single file respecting --dry-run
 // and accumulating stats and errors.
 //
@@ -456,7 +470,7 @@ func DeleteFileWithBackupDir(dst fs.Object, backupDir fs.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 {
-			remoteWithSuffix := dst.Remote() + fs.Config.Suffix
+			remoteWithSuffix := SuffixName(dst.Remote())
 			overwritten, _ := backupDir.NewObject(remoteWithSuffix)
 			_, err = Move(backupDir, overwritten, remoteWithSuffix, dst)
 		}

fs/operations/operations_test.go

@@ -231,6 +231,33 @@ func TestHashSums(t *testing.T) {
 	}
 }
 
+func TestSuffixName(t *testing.T) {
+	origSuffix, origKeepExt := fs.Config.Suffix, fs.Config.SuffixKeepExtension
+	defer func() {
+		fs.Config.Suffix, fs.Config.SuffixKeepExtension = origSuffix, origKeepExt
+	}()
+	for _, test := range []struct {
+		remote  string
+		suffix  string
+		keepExt bool
+		want    string
+	}{
+		{"test.txt", "", false, "test.txt"},
+		{"test.txt", "", true, "test.txt"},
+		{"test.txt", "-suffix", false, "test.txt-suffix"},
+		{"test.txt", "-suffix", true, "test-suffix.txt"},
+		{"test.txt.csv", "-suffix", false, "test.txt.csv-suffix"},
+		{"test.txt.csv", "-suffix", true, "test.txt-suffix.csv"},
+		{"test", "-suffix", false, "test-suffix"},
+		{"test", "-suffix", true, "test-suffix"},
+	} {
+		fs.Config.Suffix = test.suffix
+		fs.Config.SuffixKeepExtension = test.keepExt
+		got := operations.SuffixName(test.remote)
+		assert.Equal(t, test.want, got, fmt.Sprintf("%+v", test))
+	}
+}
+
 func TestCount(t *testing.T) {
 	r := fstest.NewRun(t)
 	defer r.Finalise()

fs/sync/sync.go

@@ -226,7 +226,7 @@ func (s *syncCopyMove) pairChecker(in *pipe, out *pipe, wg *sync.WaitGroup) {
 				} else {
 					// If destination already exists, then we must move it into --backup-dir if required
 					if pair.Dst != nil && s.backupDir != nil {
-						remoteWithSuffix := pair.Dst.Remote() + s.suffix
+						remoteWithSuffix := operations.SuffixName(pair.Dst.Remote())
 						overwritten, _ := s.backupDir.NewObject(remoteWithSuffix)
 						_, err := operations.Move(s.backupDir, overwritten, remoteWithSuffix, pair.Dst)
 						if err != nil {

fs/sync/sync_test.go

@@ -1188,7 +1188,7 @@ func TestSyncOverlap(t *testing.T) {
 }
 
 // Test with BackupDir set
-func testSyncBackupDir(t *testing.T, suffix string) {
+func testSyncBackupDir(t *testing.T, suffix string, suffixKeepExtension bool) {
 	r := fstest.NewRun(t)
 	defer r.Finalise()
 
@@ -1199,16 +1199,18 @@ func testSyncBackupDir(t *testing.T, suffix string) {
 
 	fs.Config.BackupDir = r.FremoteName + "/backup"
 	fs.Config.Suffix = suffix
+	fs.Config.SuffixKeepExtension = suffixKeepExtension
 	defer func() {
 		fs.Config.BackupDir = ""
 		fs.Config.Suffix = ""
+		fs.Config.SuffixKeepExtension = false
 	}()
 
 	// 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", t1)
-	file3 := r.WriteObject("dst/three", "three", t1)
+	file3 := r.WriteObject("dst/three.txt", "three", t1)
 	file2a := r.WriteFile("two", "two", t1)
 	file1a := r.WriteFile("one", "oneA", t2)
 
@@ -1227,13 +1229,17 @@ func testSyncBackupDir(t *testing.T, suffix string) {
 	file1a.Path = "dst/one"
 	// two should be unchanged
 	// three should be moved to the backup dir
-	file3.Path = "backup/three" + suffix
+	if suffixKeepExtension {
+		file3.Path = "backup/three" + suffix + ".txt"
+	} else {
+		file3.Path = "backup/three.txt" + 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)
+	file3a := r.WriteObject("dst/three.txt", "threeA", t2)
 	file1b := r.WriteFile("one", "oneBB", t3)
 	fstest.CheckItems(t, r.Fremote, file1, file2, file3, file1a, file3a)
 
@@ -1248,12 +1254,17 @@ func testSyncBackupDir(t *testing.T, suffix string) {
 	file1b.Path = "dst/one"
 	// two should be unchanged
 	// three should be moved to the backup dir
-	file3a.Path = "backup/three" + suffix
+	if suffixKeepExtension {
+		file3a.Path = "backup/three" + suffix + ".txt"
+	} else {
+		file3a.Path = "backup/three.txt" + 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") }
+func TestSyncBackupDir(t *testing.T)                        { testSyncBackupDir(t, "", false) }
+func TestSyncBackupDirWithSuffix(t *testing.T)              { testSyncBackupDir(t, ".bak", false) }
+func TestSyncBackupDirWithSuffixKeepExtension(t *testing.T) { testSyncBackupDir(t, "-2019-01-01", true) }
 
 // Check we can sync two files with differing UTF-8 representations
 func TestSyncUTFNorm(t *testing.T) {