From de14378734dff98497839529f664e2312511680d Mon Sep 17 00:00:00 2001 From: yparitcher Date: Sat, 22 Jun 2019 23:52:09 -0400 Subject: [PATCH] Implement --suffix without --backup-dir for current dir Fixes #2801 --- docs/content/docs.md | 20 +++++-- fs/config/configflags/configflags.go | 6 +- fs/operations/operations.go | 79 ++++++++++++++++++------- fs/sync/sync.go | 2 +- fs/sync/sync_test.go | 88 ++++++++++++++++++++++++++++ 5 files changed, 163 insertions(+), 32 deletions(-) diff --git a/docs/content/docs.md b/docs/content/docs.md index 79e782192..2e37b07cc 100644 --- a/docs/content/docs.md +++ b/docs/content/docs.md @@ -881,11 +881,23 @@ 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. +When using `sync`, `copy` or `move` any files which would have been +overwritten or deleted will have the suffix added to them. If there +is a file with the same path (after the suffix has been added), then +it will be overwritten. -See `--backup-dir` for more info. +The remote in use must support server side move or copy and you must +use the same remote as the destination of the sync. + +This is for use with files to add the suffix in the current directory +or with `--backup-dir`. See `--backup-dir` for more info. + +For example + + rclone sync /path/to/local/file remote:current --suffix .bak + +will sync `/path/to/local` to `remote:current`, but for any files +which would have been updated or deleted have .bak added. ### --suffix-keep-extension ### diff --git a/fs/config/configflags/configflags.go b/fs/config/configflags/configflags.go index f585e283d..97e5cbe11 100644 --- a/fs/config/configflags/configflags.go +++ b/fs/config/configflags/configflags.go @@ -68,7 +68,7 @@ func AddFlags(flagSet *pflag.FlagSet) { flags.BoolVarP(flagSet, &fs.Config.NoTraverse, "no-traverse", "", fs.Config.NoTraverse, "Don't traverse destination file system on copy.") 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.StringVarP(flagSet, &fs.Config.Suffix, "suffix", "", fs.Config.Suffix, "Suffix to add to changed files.") 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.") @@ -152,10 +152,6 @@ func SetFlags() { log.Fatalf(`Can't use --size-only and --ignore-size together.`) } - if fs.Config.Suffix != "" && fs.Config.BackupDir == "" { - log.Fatalf(`Can only use --suffix with --backup-dir.`) - } - switch { case len(fs.Config.StatsOneLineDateFormat) > 0: fs.Config.StatsOneLineDate = true diff --git a/fs/operations/operations.go b/fs/operations/operations.go index 0a70e3152..1fc63561b 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -595,25 +595,41 @@ func Same(fdst, fsrc fs.Info) bool { return SameConfig(fdst, fsrc) && strings.Trim(fdst.Root(), "/") == strings.Trim(fsrc.Root(), "/") } +// fixRoot returns the Root with a trailing / if not empty. It is +// aware of case insensitive filesystems. +func fixRoot(f fs.Info) string { + s := strings.Trim(filepath.ToSlash(f.Root()), "/") + if s != "" { + s += "/" + } + if f.Features().CaseInsensitive { + s = strings.ToLower(s) + } + return s +} + // Overlapping returns true if fdst and fsrc point to the same // underlying Fs and they overlap. func Overlapping(fdst, fsrc fs.Info) bool { if !SameConfig(fdst, fsrc) { return false } - // Return the Root with a trailing / if not empty - fixedRoot := func(f fs.Info) string { - s := strings.Trim(filepath.ToSlash(f.Root()), "/") - if s != "" { - s += "/" - } - return s - } - fdstRoot := fixedRoot(fdst) - fsrcRoot := fixedRoot(fsrc) + fdstRoot := fixRoot(fdst) + fsrcRoot := fixRoot(fsrc) return strings.HasPrefix(fdstRoot, fsrcRoot) || strings.HasPrefix(fsrcRoot, fdstRoot) } +// SameDir returns true if fdst and fsrc point to the same +// underlying Fs and they are the same directory. +func SameDir(fdst, fsrc fs.Info) bool { + if !SameConfig(fdst, fsrc) { + return false + } + fdstRoot := fixRoot(fdst) + fsrcRoot := fixRoot(fsrc) + return fdstRoot == fsrcRoot +} + // checkIdentical checks to see if dst and src are identical // // it returns true if differences were found @@ -1461,18 +1477,37 @@ func CopyURL(ctx context.Context, fdst fs.Fs, dstFileName string, url string) (d // 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 fs.Config.BackupDir != "" { + 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 srcFileName == "" { + 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")) + } + } else { + if fs.Config.Suffix == "" { + if SameDir(fdst, backupDir) { + return nil, fserrors.FatalError(errors.New("destination and parameter to --backup-dir mustn't be the same")) + } + if SameDir(fsrc, backupDir) { + return nil, fserrors.FatalError(errors.New("source and parameter to --backup-dir mustn't be the same")) + } + } + } + } else { + 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 } 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")) diff --git a/fs/sync/sync.go b/fs/sync/sync.go index 840365b62..7e610712e 100644 --- a/fs/sync/sync.go +++ b/fs/sync/sync.go @@ -120,7 +120,7 @@ func newSyncCopyMove(ctx context.Context, fdst, fsrc fs.Fs, deleteMode fs.Delete } } // Make Fs for --backup-dir if required - if fs.Config.BackupDir != "" { + if fs.Config.BackupDir != "" || fs.Config.Suffix != "" { var err error s.backupDir, err = operations.BackupDir(fdst, fsrc, "") if err != nil { diff --git a/fs/sync/sync_test.go b/fs/sync/sync_test.go index 231c2d24a..66fb62b14 100644 --- a/fs/sync/sync_test.go +++ b/fs/sync/sync_test.go @@ -1281,6 +1281,94 @@ func TestSyncBackupDir(t *testing.T) { testSyncBackupDir( func TestSyncBackupDirWithSuffix(t *testing.T) { testSyncBackupDir(t, ".bak", false) } func TestSyncBackupDirWithSuffixKeepExtension(t *testing.T) { testSyncBackupDir(t, "-2019-01-01", true) } +// Test with Suffix set +func testSyncSuffix(t *testing.T, suffix string, suffixKeepExtension bool) { + r := fstest.NewRun(t) + defer r.Finalise() + + if !operations.CanServerSideMove(r.Fremote) { + t.Skip("Skipping test as remote does not support server side move") + } + r.Mkdir(context.Background(), r.Fremote) + + 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(context.Background(), "dst/one", "one", t1) + file2 := r.WriteObject(context.Background(), "dst/two", "two", t1) + file3 := r.WriteObject(context.Background(), "dst/three.txt", "three", t1) + file2a := r.WriteFile("two", "two", t1) + file1a := r.WriteFile("one", "oneA", t2) + file3a := r.WriteFile("three.txt", "threeA", t1) + + fstest.CheckItems(t, r.Fremote, file1, file2, file3) + fstest.CheckItems(t, r.Flocal, file1a, file2a, file3a) + + fdst, err := fs.NewFs(r.FremoteName + "/dst") + require.NoError(t, err) + + accounting.Stats.ResetCounters() + err = operations.CopyFile(context.Background(), fdst, r.Flocal, "one", "one") + require.NoError(t, err) + err = operations.CopyFile(context.Background(), fdst, r.Flocal, "two", "two") + require.NoError(t, err) + err = operations.CopyFile(context.Background(), fdst, r.Flocal, "three.txt", "three.txt") + require.NoError(t, err) + + // one should be moved to the backup dir and the new one installed + file1.Path = "dst/one" + suffix + file1a.Path = "dst/one" + // two should be unchanged + // three should be moved to the backup dir + if suffixKeepExtension { + file3.Path = "dst/three" + suffix + ".txt" + } else { + file3.Path = "dst/three.txt" + suffix + } + file3a.Path = "dst/three.txt" + + fstest.CheckItems(t, r.Fremote, file1, file2, file3, file1a, file3a) + + // Now check what happens if we do it again + // Restore a different three and update one in the source + file3b := r.WriteFile("three.txt", "threeB", t3) + 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 + accounting.Stats.ResetCounters() + err = operations.CopyFile(context.Background(), fdst, r.Flocal, "one", "one") + require.NoError(t, err) + err = operations.CopyFile(context.Background(), fdst, r.Flocal, "two", "two") + require.NoError(t, err) + err = operations.CopyFile(context.Background(), fdst, r.Flocal, "three.txt", "three.txt") + require.NoError(t, err) + + // one should be moved to the backup dir and the new one installed + file1a.Path = "dst/one" + suffix + file1b.Path = "dst/one" + // two should be unchanged + // three should be moved to the backup dir + if suffixKeepExtension { + file3a.Path = "dst/three" + suffix + ".txt" + } else { + file3a.Path = "dst/three.txt" + suffix + } + file3b.Path = "dst/three.txt" + + fstest.CheckItems(t, r.Fremote, file1b, file3b, file2, file3a, file1a) +} +func TestSyncSuffix(t *testing.T) { testSyncSuffix(t, ".bak", false) } +func TestSyncSuffixKeepExtension(t *testing.T) { testSyncSuffix(t, "-2019-01-01", true) } + // Check we can sync two files with differing UTF-8 representations func TestSyncUTFNorm(t *testing.T) { if runtime.GOOS == "darwin" {