From b23cf58a41f408c200dd1986725776a0d1922f9e Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Fri, 5 Jun 2020 16:13:10 +0100 Subject: [PATCH] operations: Add skip all, do all, quit operations to --interactive - fixes #3886 This also adds SkipDestructive into all the remaing places --dry-run was used and adds documentation. --- docs/content/docs.md | 43 ++++++++++++++- fs/operations/dedupe.go | 4 +- fs/operations/operations.go | 105 ++++++++++++++++++++++++++---------- fs/sync/sync.go | 3 +- 4 files changed, 122 insertions(+), 33 deletions(-) diff --git a/docs/content/docs.md b/docs/content/docs.md index e728cbb98..fae9578fa 100644 --- a/docs/content/docs.md +++ b/docs/content/docs.md @@ -73,6 +73,9 @@ storage system in the config file then the sub path, eg You can define as many storage paths as you like in the config file. +Please use the [`-i` / `--interactive`](#interactive) flag while +learning rclone to avoid accidental data loss. + Subcommands ----------- @@ -686,7 +689,45 @@ This can be useful as an additional layer of protection for immutable or append-only data sets (notably backup archives), where modification implies corruption and should not be propagated. -## --leave-root ### +### -i / --interactive {#interactive} + +This flag can be used to tell rclone that you wish a manual +confirmation before destructive operations. + +It is **recommended** that you use this flag while learning rclone +especially with `rclone sync`. + +For example + +``` +$ rclone delete -i /tmp/dir +rclone: delete "important-file.txt"? +y) Yes, this is OK (default) +n) No, skip this +s) Skip all delete operations with no more questions +!) Do all delete operations with no more questions +q) Exit rclone now. +y/n/s/!/q> n +``` + +The options mean + +- `y`: **Yes**, this operation should go ahead. You can also press Return + for this to happen. You'll be asked every time unless you choose `s` + or `!`. +- `n`: **No**, do not do this operation. You'll be asked every time unless + you choose `s` or `!`. +- `s`: **Skip** all the following operations of this type with no more + questions. This takes effect until rclone exits. If there are any + different kind of operations you'll be prompted for them. +- `!`: **Do all** the following operations with no more + questions. Useful if you've decided that you don't mind rclone doing + that kind of operation. This takes effect until rclone exits . If + there are any different kind of operations you'll be prompted for + them. +- `q`: **Quit** rclone now, just in case! + +### --leave-root #### During rmdirs it will not remove root directory, even if it's empty. diff --git a/fs/operations/dedupe.go b/fs/operations/dedupe.go index ba2c7e447..963155376 100644 --- a/fs/operations/dedupe.go +++ b/fs/operations/dedupe.go @@ -44,7 +44,7 @@ outer: newName = fmt.Sprintf("%s-%d%s", base, i+suffix, ext) _, err = f.NewObject(ctx, newName) } - if !skipDestructive(ctx, o, "rename") { + if !SkipDestructive(ctx, o, "rename") { newObj, err := doMove(ctx, o, newName) if err != nil { err = fs.CountError(err) @@ -253,7 +253,7 @@ func dedupeMergeDuplicateDirs(ctx context.Context, f fs.Fs, duplicateDirs [][]fs return errors.Errorf("%v: can't flush dir cache", f) } for _, dirs := range duplicateDirs { - if !skipDestructive(ctx, dirs[0], "merge duplicate directories") { + if !SkipDestructive(ctx, dirs[0], "merge duplicate directories") { fs.Infof(dirs[0], "Merging contents of duplicate directories") err := mergeDirs(ctx, dirs) if err != nil { diff --git a/fs/operations/operations.go b/fs/operations/operations.go index 1605ab105..413745874 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -11,6 +11,7 @@ import ( "io" "io/ioutil" "net/http" + "os" "path" "path/filepath" "sort" @@ -31,6 +32,7 @@ import ( "github.com/rclone/rclone/fs/march" "github.com/rclone/rclone/fs/object" "github.com/rclone/rclone/fs/walk" + "github.com/rclone/rclone/lib/atexit" "github.com/rclone/rclone/lib/random" "github.com/rclone/rclone/lib/readers" "golang.org/x/sync/errgroup" @@ -212,7 +214,7 @@ func equal(ctx context.Context, src fs.ObjectInfo, dst fs.Object, opt equalOpt) // mod time differs but hash is the same to reset mod time if required if opt.updateModTime { - if !skipDestructive(ctx, src, "update modification time") { + if !SkipDestructive(ctx, src, "update modification time") { // Size and hash the same but mtime different // Error if objects are treated as immutable if fs.Config.Immutable { @@ -347,7 +349,7 @@ func Copy(ctx context.Context, f fs.Fs, dst fs.Object, remote string, src fs.Obj tr.Done(err) }() newDst = dst - if skipDestructive(ctx, src, "copy") { + if SkipDestructive(ctx, src, "copy") { return newDst, nil } maxTries := fs.Config.LowLevelRetries @@ -525,7 +527,7 @@ func Move(ctx context.Context, fdst fs.Fs, dst fs.Object, remote string, src fs. tr.Done(err) }() newDst = dst - if skipDestructive(ctx, src, "move") { + if SkipDestructive(ctx, src, "move") { return newDst, nil } // See if we have Move available @@ -604,7 +606,8 @@ func DeleteFileWithBackupDir(ctx context.Context, dst fs.Object, backupDir fs.Fs if backupDir != nil { action, actioned = "move into backup dir", "Moved into backup dir" } - if skipDestructive(ctx, dst, action) { + skip := SkipDestructive(ctx, dst, action) + if skip { // do nothing } else if backupDir != nil { err = MoveBackupDir(ctx, backupDir, dst) @@ -614,7 +617,7 @@ func DeleteFileWithBackupDir(ctx context.Context, dst fs.Object, backupDir fs.Fs if err != nil { fs.Errorf(dst, "Couldn't %s: %v", action, err) err = fs.CountError(err) - } else if !fs.Config.DryRun { + } else if !skip { fs.Infof(dst, actioned) } return err @@ -1135,8 +1138,7 @@ func ListDir(ctx context.Context, f fs.Fs, w io.Writer) error { // Mkdir makes a destination directory or container func Mkdir(ctx context.Context, f fs.Fs, dir string) error { - if fs.Config.DryRun { - fs.Logf(fs.LogDirName(f, dir), "Not making directory as dry run is set") + if SkipDestructive(ctx, fs.LogDirName(f, dir), "make directory") { return nil } fs.Debugf(fs.LogDirName(f, dir), "Making directory") @@ -1151,7 +1153,7 @@ func Mkdir(ctx context.Context, f fs.Fs, dir string) error { // TryRmdir removes a container but not if not empty. It doesn't // count errors but may return one. func TryRmdir(ctx context.Context, f fs.Fs, dir string) error { - if skipDestructive(ctx, fs.LogDirName(f, dir), "remove directory") { + if SkipDestructive(ctx, fs.LogDirName(f, dir), "remove directory") { return nil } fs.Debugf(fs.LogDirName(f, dir), "Removing directory") @@ -1176,13 +1178,12 @@ func Purge(ctx context.Context, f fs.Fs, dir string) error { // FIXME change the Purge interface so it takes a dir - see #1891 if doPurge := f.Features().Purge; doPurge != nil { doFallbackPurge = false - if skipDestructive(ctx, fs.LogDirName(f, dir), "purge directory") { + if SkipDestructive(ctx, fs.LogDirName(f, dir), "purge directory") { return nil - } else { - err = doPurge(ctx) - if err == fs.ErrorCantPurge { - doFallbackPurge = true - } + } + err = doPurge(ctx) + if err == fs.ErrorCantPurge { + doFallbackPurge = true } } } @@ -1251,7 +1252,7 @@ func CleanUp(ctx context.Context, f fs.Fs) error { if doCleanUp == nil { return errors.Errorf("%v doesn't support cleanup", f) } - if skipDestructive(ctx, f, "clean up old files") { + if SkipDestructive(ctx, f, "clean up old files") { return nil } return doCleanUp(ctx) @@ -1389,7 +1390,7 @@ func Rcat(ctx context.Context, fdst fs.Fs, dstFileName string, in io.ReadCloser, fStreamTo = tmpLocalFs } - if skipDestructive(ctx, dstFileName, "upload from pipe") { + if SkipDestructive(ctx, dstFileName, "upload from pipe") { // prevents "broken pipe" errors _, err = io.Copy(ioutil.Discard, in) return nil, err @@ -1671,7 +1672,7 @@ func RcatSize(ctx context.Context, fdst fs.Fs, dstFileName string, in io.ReadClo body := ioutil.NopCloser(in) // we let the server close the body in := tr.Account(body) // account the transfer (no buffering) - if skipDestructive(ctx, dstFileName, "upload from pipe") { + if SkipDestructive(ctx, dstFileName, "upload from pipe") { // prevents "broken pipe" errors _, err = io.Copy(ioutil.Discard, in) return nil, err @@ -2187,25 +2188,73 @@ func GetFsInfo(f fs.Fs) *FsInfo { return info } -var interactiveMutex sync.Mutex +var ( + interactiveMu sync.Mutex + skipped = map[string]bool{} +) -func skipDestructive(ctx context.Context, subject interface{}, action string) bool { - var ( - flag string - skip bool - ) +// skipDestructiveChoose asks the user which action to take +// +// Call with interactiveMu held +func skipDestructiveChoose(ctx context.Context, subject interface{}, action string) (skip bool) { + fmt.Printf("rclone: %s \"%v\"?\n", action, subject) + switch i := config.CommandDefault([]string{ + "yYes, this is OK", + "nNo, skip this", + fmt.Sprintf("sSkip all %s operations with no more questions", action), + fmt.Sprintf("!Do all %s operations with no more questions", action), + "qExit rclone now.", + }, 0); i { + case 'y': + skip = false + case 'n': + skip = true + case 's': + skip = true + skipped[action] = true + fs.Logf(nil, "Skipping all %s operations from now on without asking", action) + case '!': + skip = false + skipped[action] = false + fs.Logf(nil, "Doing all %s operations from now on without asking", action) + case 'q': + fs.Logf(nil, "Quitting rclone now") + atexit.Run() + os.Exit(0) + default: + skip = true + fs.Errorf(nil, "Bad choice %c", i) + } + return skip +} + +// SkipDestructive should be called whenever rclone is about to do an destructive operation. +// +// It will check the --dry-run flag and it will ask the user if the --interactive flag is set. +// +// subject should be the object or directory in use +// +// action should be a descriptive word or short phrase +// +// Together they should make sense in this sentence: "Rclone is about +// to action subject". +func SkipDestructive(ctx context.Context, subject interface{}, action string) (skip bool) { + var flag string switch { case fs.Config.DryRun: flag = "--dry-run" skip = true case fs.Config.Interactive: flag = "--interactive" - interactiveMutex.Lock() - defer interactiveMutex.Unlock() - fmt.Printf("rclone: %s \"%v\"?\n", action, subject) - skip = !config.Confirm(true) + interactiveMu.Lock() + defer interactiveMu.Unlock() + var found bool + skip, found = skipped[action] + if !found { + skip = skipDestructiveChoose(ctx, subject, action) + } default: - skip = false + return false } if skip { fs.Logf(subject, "Skipped %s as %s is set", action, flag) diff --git a/fs/sync/sync.go b/fs/sync/sync.go index 5d3eb0cd1..fddbe48eb 100644 --- a/fs/sync/sync.go +++ b/fs/sync/sync.go @@ -1063,8 +1063,7 @@ func MoveDir(ctx context.Context, fdst, fsrc fs.Fs, deleteEmptySrcDirs bool, cop // First attempt to use DirMover if exists, same Fs and no filters are active if fdstDirMove := fdst.Features().DirMove; fdstDirMove != nil && operations.SameConfig(fsrc, fdst) && filter.Active.InActive() { - if fs.Config.DryRun { - fs.Logf(fdst, "Not doing server side directory move as --dry-run") + if operations.SkipDestructive(ctx, fdst, "server side directory move") { return nil } fs.Debugf(fdst, "Using server side directory move")