Abstract --Backup-dir checks so can be applied across Sync, Copy, Move

cmd/sync/sync.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/ncw/rclone/cmd"
+	"github.com/ncw/rclone/fs/operations"
 	"github.com/ncw/rclone/fs/sync"
 	"github.com/spf13/cobra"
 )
@@ -44,9 +45,12 @@ go there.
 `,
 	Run: func(command *cobra.Command, args []string) {
 		cmd.CheckArgs(2, 2, command, args)
-		fsrc, fdst := cmd.NewFsSrcDst(args)
+		fsrc, srcFileName, fdst := cmd.NewFsSrcFileDst(args)
 		cmd.Run(true, true, command, func() error {
-			return sync.Sync(context.Background(), fdst, fsrc, createEmptySrcDirs)
+			if srcFileName == "" {
+				return sync.Sync(context.Background(), fdst, fsrc, createEmptySrcDirs)
+			}
+			return operations.CopyFile(context.Background(), fdst, fsrc, srcFileName, srcFileName)
 		})
 	},
 }

fs/operations/operations.go

@@ -513,13 +513,7 @@ func DeleteFileWithBackupDir(ctx context.Context, dst fs.Object, backupDir fs.Fs
 	if fs.Config.DryRun {
 		fs.Logf(dst, "Not %s as --dry-run", actioning)
 	} else if backupDir != nil {
-		if !SameConfig(dst.Fs(), backupDir) {
+		err = MoveBackupDir(ctx, backupDir, dst)
-			err = errors.New("parameter to --backup-dir has to be on the same remote as destination")
-		} else {
-			remoteWithSuffix := SuffixName(dst.Remote())
-			overwritten, _ := backupDir.NewObject(ctx, remoteWithSuffix)
-			_, err = Move(ctx, backupDir, overwritten, remoteWithSuffix, dst)
-		}
 	} else {
 		err = dst.Remove(ctx)
 	}
@@ -1465,6 +1459,35 @@ func CopyURL(ctx context.Context, fdst fs.Fs, dstFileName string, url string) (d
 	return RcatSize(ctx, fdst, dstFileName, resp.Body, resp.ContentLength, time.Now())
 }
 
+// BackupDir returns the correctly configured --backup-dir
+func BackupDir(fdst fs.Fs, fsrc fs.Fs, srcFileName string) (backupDir fs.Fs, err error) {
+	backupDir, err = cache.Get(fs.Config.BackupDir)
+	if err != nil {
+		return nil, fserrors.FatalError(errors.Errorf("Failed to make fs for --backup-dir %q: %v", fs.Config.BackupDir, err))
+	}
+	if !SameConfig(fdst, backupDir) {
+		return nil, fserrors.FatalError(errors.New("parameter to --backup-dir has to be on the same remote as destination"))
+	}
+	if Overlapping(fdst, backupDir) {
+		return nil, fserrors.FatalError(errors.New("destination and parameter to --backup-dir mustn't overlap"))
+	}
+	if Overlapping(fsrc, backupDir) {
+		return nil, fserrors.FatalError(errors.New("source and parameter to --backup-dir mustn't overlap"))
+	}
+	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"))
+	}
+	return backupDir, nil
+}
+
+// MoveBackupDir moves a file to the backup dir
+func MoveBackupDir(ctx context.Context, backupDir fs.Fs, dst fs.Object) (err error) {
+	remoteWithSuffix := SuffixName(dst.Remote())
+	overwritten, _ := backupDir.NewObject(ctx, remoteWithSuffix)
+	_, err = Move(ctx, backupDir, overwritten, remoteWithSuffix, dst)
+	return err
+}
+
 // moveOrCopyFile moves or copies a single file possibly to a new name
 func moveOrCopyFile(ctx context.Context, fdst fs.Fs, fsrc fs.Fs, dstFileName string, srcFileName string, cp bool) (err error) {
 	dstFilePath := path.Join(fdst.Root(), dstFileName)
@@ -1521,14 +1544,12 @@ func moveOrCopyFile(ctx context.Context, fdst fs.Fs, fsrc fs.Fs, dstFileName str
 
 	if NeedTransfer(ctx, dstObj, srcObj) {
 		// If destination already exists, then we must move it into --backup-dir if required
-		if dstObj != nil && fs.Config.BackupDir != "" {
+		if dstObj != nil && (fs.Config.BackupDir != "" || fs.Config.Suffix != "") {
-			backupDir, err := cache.Get(fs.Config.BackupDir)
+			backupDir, err := BackupDir(fdst, fsrc, srcFileName)
 			if err != nil {
 				return errors.Wrap(err, "creating Fs for --backup-dir failed")
 			}
-			remoteWithSuffix := SuffixName(dstObj.Remote())
+			err = MoveBackupDir(ctx, backupDir, dstObj)
-			overwritten, _ := backupDir.NewObject(ctx, remoteWithSuffix)
-			_, err = Move(ctx, backupDir, overwritten, remoteWithSuffix, dstObj)
 			if err != nil {
 				return errors.Wrap(err, "moving to --backup-dir failed")
 			}

fs/sync/sync.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/ncw/rclone/fs"
 	"github.com/ncw/rclone/fs/accounting"
-	"github.com/ncw/rclone/fs/cache"
 	"github.com/ncw/rclone/fs/filter"
 	"github.com/ncw/rclone/fs/fserrors"
 	"github.com/ncw/rclone/fs/hash"
@@ -62,7 +61,6 @@ type syncCopyMove struct {
 	trackRenamesCh chan fs.Object         // objects are pumped in here
 	renameCheck    []fs.Object            // accumulate files to check for rename here
 	backupDir      fs.Fs                  // place to store overwrites/deletes
-	suffix         string                 // suffix to add to files placed in backupDir
 }
 
 func newSyncCopyMove(ctx context.Context, fdst, fsrc fs.Fs, deleteMode fs.DeleteMode, DoMove bool, deleteEmptySrcDirs bool, copyEmptySrcDirs bool) (*syncCopyMove, error) {
@@ -124,23 +122,10 @@ func newSyncCopyMove(ctx context.Context, fdst, fsrc fs.Fs, deleteMode fs.Delete
 	// Make Fs for --backup-dir if required
 	if fs.Config.BackupDir != "" {
 		var err error
-		s.backupDir, err = cache.Get(fs.Config.BackupDir)
+		s.backupDir, err = operations.BackupDir(fdst, fsrc, "")
 		if err != nil {
-			return nil, fserrors.FatalError(errors.Errorf("Failed to make fs for --backup-dir %q: %v", fs.Config.BackupDir, err))
+			return nil, err
 		}
-		if !operations.CanServerSideMove(s.backupDir) {
-			return nil, fserrors.FatalError(errors.New("can't use --backup-dir on a remote which doesn't support server side move or copy"))
-		}
-		if !operations.SameConfig(fdst, s.backupDir) {
-			return nil, fserrors.FatalError(errors.New("parameter to --backup-dir has to be on the same remote as destination"))
-		}
-		if operations.Overlapping(fdst, s.backupDir) {
-			return nil, fserrors.FatalError(errors.New("destination and parameter to --backup-dir mustn't overlap"))
-		}
-		if operations.Overlapping(fsrc, s.backupDir) {
-			return nil, fserrors.FatalError(errors.New("source and parameter to --backup-dir mustn't overlap"))
-		}
-		s.suffix = fs.Config.Suffix
 	}
 	return s, nil
 }
@@ -227,9 +212,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 := operations.SuffixName(pair.Dst.Remote())
+						err := operations.MoveBackupDir(s.ctx, s.backupDir, pair.Dst)
-						overwritten, _ := s.backupDir.NewObject(s.ctx, remoteWithSuffix)
-						_, err := operations.Move(s.ctx, s.backupDir, overwritten, remoteWithSuffix, pair.Dst)
 						if err != nil {
 							s.processError(err)
 						} else {