operations: fix use of --suffix without --backup-dir

As part of the original work adding this feature it was overlooked
that this didn't actually work for full rclone copy/sync.

This commit fixes the problem and adds a test to make sure it stays
working.

See: https://forum.rclone.org/t/suffix-not-working-on-folder-upload-via-ssh-sftp/19526

docs/content/docs.md

@@ -1253,11 +1253,17 @@ or with `--backup-dir`. See `--backup-dir` for more info.
 
 For example
 
-    rclone sync -i /path/to/local/file remote:current --suffix .bak
+    rclone copy -i /path/to/local/file remote:current --suffix .bak
 
-will sync `/path/to/local` to `remote:current`, but for any files
+will copy `/path/to/local` to `remote:current`, but for any files
 which would have been updated or deleted have .bak added.
 
+If using `rclone sync` with `--suffix` and without `--backup-dir` then
+it is recommended to put a filter rule in excluding the suffix
+otherwise the `sync` will delete the backup files.
+
+    rclone sync -i /path/to/local/file remote:current --suffix .bak --exclude "*.bak"
+
 ### --suffix-keep-extension ###
 
 When using `--suffix`, setting this causes rclone put the SUFFIX

fs/operations/operations.go

@@ -1533,12 +1533,11 @@ func BackupDir(fdst fs.Fs, fsrc fs.Fs, srcFileName string) (backupDir fs.Fs, err
 				}
 			}
 		}
-	} else {
+	} else if fs.Config.Suffix != "" {
-		if srcFileName == "" {
-			return nil, fserrors.FatalError(errors.New("--suffix must be used with a file or with --backup-dir"))
-		}
 		// --backup-dir is not set but --suffix is - use the destination as the backupDir
 		backupDir = fdst
+	} else {
+		return nil, fserrors.FatalError(errors.New("internal error: BackupDir called when --backup-dir and --suffix both empty"))
 	}
 	if !CanServerSideMove(backupDir) {
 		return nil, fserrors.FatalError(errors.New("can't use --backup-dir on a remote which doesn't support server side move or copy"))

fs/sync/sync_test.go

@@ -1590,7 +1590,7 @@ func TestSyncCopyDest(t *testing.T) {
 }
 
 // Test with BackupDir set
-func testSyncBackupDir(t *testing.T, suffix string, suffixKeepExtension bool) {
+func testSyncBackupDir(t *testing.T, backupDir string, suffix string, suffixKeepExtension bool) {
 	r := fstest.NewRun(t)
 	defer r.Finalise()
 
@@ -1599,7 +1599,23 @@ func testSyncBackupDir(t *testing.T, suffix string, suffixKeepExtension bool) {
 	}
 	r.Mkdir(context.Background(), r.Fremote)
 
-	fs.Config.BackupDir = r.FremoteName + "/backup"
+	if backupDir != "" {
+		fs.Config.BackupDir = r.FremoteName + "/" + backupDir
+		backupDir += "/"
+	} else {
+		fs.Config.BackupDir = ""
+		backupDir = "dst/"
+		// Exclude the suffix from the sync otherwise the sync
+		// deletes the old backup files
+		flt, err := filter.NewFilter(nil)
+		require.NoError(t, err)
+		require.NoError(t, flt.AddRule("- *"+suffix))
+		oldFlt := filter.Active
+		filter.Active = flt
+		defer func() {
+			filter.Active = oldFlt
+		}()
+	}
 	fs.Config.Suffix = suffix
 	fs.Config.SuffixKeepExtension = suffixKeepExtension
 	defer func() {
@@ -1627,14 +1643,14 @@ func testSyncBackupDir(t *testing.T, suffix string, suffixKeepExtension bool) {
 	require.NoError(t, err)
 
 	// one should be moved to the backup dir and the new one installed
-	file1.Path = "backup/one" + suffix
+	file1.Path = backupDir + "one" + suffix
 	file1a.Path = "dst/one"
 	// two should be unchanged
 	// three should be moved to the backup dir
 	if suffixKeepExtension {
-		file3.Path = "backup/three" + suffix + ".txt"
+		file3.Path = backupDir + "three" + suffix + ".txt"
 	} else {
-		file3.Path = "backup/three.txt" + suffix
+		file3.Path = backupDir + "three.txt" + suffix
 	}
 
 	fstest.CheckItems(t, r.Fremote, file1, file2, file3, file1a)
@@ -1652,22 +1668,29 @@ func testSyncBackupDir(t *testing.T, suffix string, suffixKeepExtension bool) {
 	require.NoError(t, err)
 
 	// one should be moved to the backup dir and the new one installed
-	file1a.Path = "backup/one" + suffix
+	file1a.Path = backupDir + "one" + suffix
 	file1b.Path = "dst/one"
 	// two should be unchanged
 	// three should be moved to the backup dir
 	if suffixKeepExtension {
-		file3a.Path = "backup/three" + suffix + ".txt"
+		file3a.Path = backupDir + "three" + suffix + ".txt"
 	} else {
-		file3a.Path = "backup/three.txt" + suffix
+		file3a.Path = backupDir + "three.txt" + suffix
 	}
 
 	fstest.CheckItems(t, r.Fremote, file1b, file2, file3a, file1a)
 }
-func TestSyncBackupDir(t *testing.T)           { testSyncBackupDir(t, "", false) }
+func TestSyncBackupDir(t *testing.T) {
-func TestSyncBackupDirWithSuffix(t *testing.T) { testSyncBackupDir(t, ".bak", false) }
+	testSyncBackupDir(t, "backup", "", false)
+}
+func TestSyncBackupDirWithSuffix(t *testing.T) {
+	testSyncBackupDir(t, "backup", ".bak", false)
+}
 func TestSyncBackupDirWithSuffixKeepExtension(t *testing.T) {
-	testSyncBackupDir(t, "-2019-01-01", true)
+	testSyncBackupDir(t, "backup", "-2019-01-01", true)
+}
+func TestSyncBackupDirSuffixOnly(t *testing.T) {
+	testSyncBackupDir(t, "", ".bak", false)
 }
 
 // Test with Suffix set