From aab8051f503dd8dcb2948922c199feeb5a2119e1 Mon Sep 17 00:00:00 2001 From: ishuah Date: Mon, 27 Nov 2017 14:42:02 +0300 Subject: [PATCH] move: add --delete-empty-src-dirs flag - fixes #1854 --- cmd/move/move.go | 11 ++++- cmd/moveto/moveto.go | 2 +- docs/content/commands/rclone_move.md | 5 +- fs/sync.go | 69 +++++++++++++++------------- fs/sync_test.go | 40 +++++++++++++--- 5 files changed, 84 insertions(+), 43 deletions(-) diff --git a/cmd/move/move.go b/cmd/move/move.go index 082142d9b..936538d72 100644 --- a/cmd/move/move.go +++ b/cmd/move/move.go @@ -6,8 +6,14 @@ import ( "github.com/spf13/cobra" ) +// Globals +var ( + deleteEmptySrcDirs = false +) + func init() { cmd.Root.AddCommand(commandDefintion) + commandDefintion.Flags().BoolVarP(&deleteEmptySrcDirs, "delete-empty-src-dirs", "", deleteEmptySrcDirs, "Delete empty source dirs after move") } var commandDefintion = &cobra.Command{ @@ -28,6 +34,8 @@ move will be used, otherwise it will copy it (server side if possible) into ` + "`dest:path`" + ` then delete the original (if no errors on copy) in ` + "`source:path`" + `. +If you want to delete empty source directories after move, use the --delete-empty-src-dirs flag. + **Important**: Since this can cause data loss, test first with the --dry-run flag. `, @@ -35,7 +43,8 @@ into ` + "`dest:path`" + ` then delete the original (if no errors on copy) in cmd.CheckArgs(2, 2, command, args) fsrc, fdst := cmd.NewFsSrcDst(args) cmd.Run(true, true, command, func() error { - return fs.MoveDir(fdst, fsrc) + + return fs.MoveDir(fdst, fsrc, deleteEmptySrcDirs) }) }, } diff --git a/cmd/moveto/moveto.go b/cmd/moveto/moveto.go index 3c00964de..8e36cd734 100644 --- a/cmd/moveto/moveto.go +++ b/cmd/moveto/moveto.go @@ -49,7 +49,7 @@ transfer. cmd.Run(true, true, command, func() error { if srcFileName == "" { - return fs.MoveDir(fdst, fsrc) + return fs.MoveDir(fdst, fsrc, false) } return fs.MoveFile(fdst, fsrc, dstFileName, srcFileName) }) diff --git a/docs/content/commands/rclone_move.md b/docs/content/commands/rclone_move.md index 7ff9de586..7f6a26d5b 100644 --- a/docs/content/commands/rclone_move.md +++ b/docs/content/commands/rclone_move.md @@ -26,6 +26,8 @@ move will be used, otherwise it will copy it (server side if possible) into `dest:path` then delete the original (if no errors on copy) in `source:path`. +If you want to delete empty source directories after move, use the --delete-empty-source-dirs flag. + **Important**: Since this can cause data loss, test first with the --dry-run flag. @@ -37,7 +39,8 @@ rclone move source:path dest:path [flags] ### Options ``` - -h, --help help for move + --delete-empty-src-dirs Delete empty dirs after move + -h, --help help for move ``` ### Options inherited from parent commands diff --git a/fs/sync.go b/fs/sync.go index b4482c84e..c93785f2f 100644 --- a/fs/sync.go +++ b/fs/sync.go @@ -16,11 +16,12 @@ var oldSyncMethod = BoolP("old-sync-method", "", false, "Deprecated - use --fast type syncCopyMove struct { // parameters - fdst Fs - fsrc Fs - deleteMode DeleteMode // how we are doing deletions - DoMove bool - dir string + fdst Fs + fsrc Fs + deleteMode DeleteMode // how we are doing deletions + DoMove bool + deleteEmptySrcDirs bool + dir string // internal state ctx context.Context // internal context for controlling go-routines cancel func() // cancel the context @@ -58,24 +59,25 @@ type syncCopyMove struct { suffix string // suffix to add to files placed in backupDir } -func newSyncCopyMove(fdst, fsrc Fs, deleteMode DeleteMode, DoMove bool) (*syncCopyMove, error) { +func newSyncCopyMove(fdst, fsrc Fs, deleteMode DeleteMode, DoMove bool, deleteEmptySrcDirs bool) (*syncCopyMove, error) { s := &syncCopyMove{ - fdst: fdst, - fsrc: fsrc, - deleteMode: deleteMode, - DoMove: DoMove, - dir: "", - srcFilesChan: make(chan Object, Config.Checkers+Config.Transfers), - srcFilesResult: make(chan error, 1), - dstFilesResult: make(chan error, 1), - noTraverse: Config.NoTraverse, - toBeChecked: make(ObjectPairChan, Config.Transfers), - toBeUploaded: make(ObjectPairChan, Config.Transfers), - deleteFilesCh: make(chan Object, Config.Checkers), - trackRenames: Config.TrackRenames, - commonHash: fsrc.Hashes().Overlap(fdst.Hashes()).GetOne(), - toBeRenamed: make(ObjectPairChan, Config.Transfers), - trackRenamesCh: make(chan Object, Config.Checkers), + fdst: fdst, + fsrc: fsrc, + deleteMode: deleteMode, + DoMove: DoMove, + deleteEmptySrcDirs: deleteEmptySrcDirs, + dir: "", + srcFilesChan: make(chan Object, Config.Checkers+Config.Transfers), + srcFilesResult: make(chan error, 1), + dstFilesResult: make(chan error, 1), + noTraverse: Config.NoTraverse, + toBeChecked: make(ObjectPairChan, Config.Transfers), + toBeUploaded: make(ObjectPairChan, Config.Transfers), + deleteFilesCh: make(chan Object, Config.Checkers), + trackRenames: Config.TrackRenames, + commonHash: fsrc.Hashes().Overlap(fdst.Hashes()).GetOne(), + toBeRenamed: make(ObjectPairChan, Config.Transfers), + trackRenamesCh: make(chan Object, Config.Checkers), } s.ctx, s.cancel = context.WithCancel(context.Background()) if s.noTraverse && s.deleteMode != DeleteModeOff { @@ -697,8 +699,9 @@ func (s *syncCopyMove) run() error { } } - // if DoMove, delete empty fsrc subdirectories after - if s.DoMove { + // Delete empty fsrc subdirectories + // if DoMove and --delete-empty-src-dirs flag is set + if s.DoMove && s.deleteEmptySrcDirs { //delete empty subdirectories that were part of the move s.processError(deleteEmptyDirectories(s.fsrc, s.srcEmptyDirs)) } @@ -809,7 +812,7 @@ func (s *syncCopyMove) Match(dst, src DirEntry) (recurse bool) { // If DoMove is true then files will be moved instead of copied // // dir is the start directory, "" for root -func runSyncCopyMove(fdst, fsrc Fs, deleteMode DeleteMode, DoMove bool) error { +func runSyncCopyMove(fdst, fsrc Fs, deleteMode DeleteMode, DoMove bool, deleteEmptySrcDirs bool) error { if *oldSyncMethod { return FatalError(errors.New("--old-sync-method is deprecated use --fast-list instead")) } @@ -822,7 +825,7 @@ func runSyncCopyMove(fdst, fsrc Fs, deleteMode DeleteMode, DoMove bool) error { return FatalError(errors.New("can't use --delete-before with --track-renames")) } // only delete stuff during in this pass - do, err := newSyncCopyMove(fdst, fsrc, DeleteModeOnly, false) + do, err := newSyncCopyMove(fdst, fsrc, DeleteModeOnly, false, deleteEmptySrcDirs) if err != nil { return err } @@ -833,7 +836,7 @@ func runSyncCopyMove(fdst, fsrc Fs, deleteMode DeleteMode, DoMove bool) error { // Next pass does a copy only deleteMode = DeleteModeOff } - do, err := newSyncCopyMove(fdst, fsrc, deleteMode, DoMove) + do, err := newSyncCopyMove(fdst, fsrc, deleteMode, DoMove, deleteEmptySrcDirs) if err != nil { return err } @@ -842,21 +845,21 @@ func runSyncCopyMove(fdst, fsrc Fs, deleteMode DeleteMode, DoMove bool) error { // Sync fsrc into fdst func Sync(fdst, fsrc Fs) error { - return runSyncCopyMove(fdst, fsrc, Config.DeleteMode, false) + return runSyncCopyMove(fdst, fsrc, Config.DeleteMode, false, false) } // CopyDir copies fsrc into fdst func CopyDir(fdst, fsrc Fs) error { - return runSyncCopyMove(fdst, fsrc, DeleteModeOff, false) + return runSyncCopyMove(fdst, fsrc, DeleteModeOff, false, false) } // moveDir moves fsrc into fdst -func moveDir(fdst, fsrc Fs) error { - return runSyncCopyMove(fdst, fsrc, DeleteModeOff, true) +func moveDir(fdst, fsrc Fs, deleteEmptySrcDirs bool) error { + return runSyncCopyMove(fdst, fsrc, DeleteModeOff, true, deleteEmptySrcDirs) } // MoveDir moves fsrc into fdst -func MoveDir(fdst, fsrc Fs) error { +func MoveDir(fdst, fsrc Fs, deleteEmptySrcDirs bool) error { if Same(fdst, fsrc) { Errorf(fdst, "Nothing to do as source and destination are the same") return nil @@ -891,5 +894,5 @@ func MoveDir(fdst, fsrc Fs) error { } // Otherwise move the files one by one - return moveDir(fdst, fsrc) + return moveDir(fdst, fsrc, deleteEmptySrcDirs) } diff --git a/fs/sync_test.go b/fs/sync_test.go index 74f9ae14e..710831ba5 100644 --- a/fs/sync_test.go +++ b/fs/sync_test.go @@ -798,7 +798,7 @@ func TestSyncWithTrackRenames(t *testing.T) { } // Test a server side move if possible, or the backup path if not -func testServerSideMove(t *testing.T, r *fstest.Run, withFilter bool) { +func testServerSideMove(t *testing.T, r *fstest.Run, withFilter, testDeleteEmptyDirs bool) { FremoteMove, _, finaliseMove, err := fstest.RandomRemote(*fstest.RemoteName, *fstest.SubDir) require.NoError(t, err) defer finaliseMove() @@ -807,6 +807,11 @@ func testServerSideMove(t *testing.T, r *fstest.Run, withFilter bool) { file2 := r.WriteBoth("empty space", "", t2) file3u := r.WriteBoth("potato3", "------------------------------------------------------------ UPDATED", t2) + if testDeleteEmptyDirs { + err := fs.Mkdir(r.Fremote, "tomatoDir") + require.NoError(t, err) + } + fstest.CheckItems(t, r.Fremote, file2, file1, file3u) t.Logf("Server side move (if possible) %v -> %v", r.Fremote, FremoteMove) @@ -818,7 +823,7 @@ func testServerSideMove(t *testing.T, r *fstest.Run, withFilter bool) { // Do server side move fs.Stats.ResetCounters() - err = fs.MoveDir(FremoteMove, r.Fremote) + err = fs.MoveDir(FremoteMove, r.Fremote, testDeleteEmptyDirs) require.NoError(t, err) if withFilter { @@ -826,6 +831,11 @@ func testServerSideMove(t *testing.T, r *fstest.Run, withFilter bool) { } else { fstest.CheckItems(t, r.Fremote) } + + if testDeleteEmptyDirs { + fstest.CheckListingWithPrecision(t, r.Fremote, nil, []string{}, fs.Config.ModifyWindow) + } + fstest.CheckItems(t, FremoteMove, file2, file1, file3u) // Create a new empty remote for stuff to be moved into @@ -833,9 +843,14 @@ func testServerSideMove(t *testing.T, r *fstest.Run, withFilter bool) { require.NoError(t, err) defer finaliseMove2() + if testDeleteEmptyDirs { + err := fs.Mkdir(FremoteMove, "tomatoDir") + require.NoError(t, err) + } + // Move it back to a new empty remote, dst does not exist this time fs.Stats.ResetCounters() - err = fs.MoveDir(FremoteMove2, FremoteMove) + err = fs.MoveDir(FremoteMove2, FremoteMove, testDeleteEmptyDirs) require.NoError(t, err) if withFilter { @@ -845,13 +860,17 @@ func testServerSideMove(t *testing.T, r *fstest.Run, withFilter bool) { fstest.CheckItems(t, FremoteMove2, file2, file1, file3u) fstest.CheckItems(t, FremoteMove) } + + if testDeleteEmptyDirs { + fstest.CheckListingWithPrecision(t, FremoteMove, nil, []string{}, fs.Config.ModifyWindow) + } } // Test a server side move if possible, or the backup path if not func TestServerSideMove(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise() - testServerSideMove(t, r, false) + testServerSideMove(t, r, false, false) } // Test a server side move if possible, or the backup path if not @@ -864,7 +883,14 @@ func TestServerSideMoveWithFilter(t *testing.T) { fs.Config.Filter.MinSize = -1 }() - testServerSideMove(t, r, true) + testServerSideMove(t, r, true, false) +} + +// Test a server side move if possible +func TestServerSideMoveDeleteEmptySourceDirs(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + testServerSideMove(t, r, false, true) } // Test a server side move with overlap @@ -884,7 +910,7 @@ func TestServerSideMoveOverlap(t *testing.T) { fstest.CheckItems(t, r.Fremote, file1) // Subdir move with no filters should return ErrorCantMoveOverlapping - err = fs.MoveDir(FremoteMove, r.Fremote) + err = fs.MoveDir(FremoteMove, r.Fremote, false) assert.EqualError(t, err, fs.ErrorCantMoveOverlapping.Error()) // Now try with a filter which should also fail with ErrorCantMoveOverlapping @@ -892,7 +918,7 @@ func TestServerSideMoveOverlap(t *testing.T) { defer func() { fs.Config.Filter.MinSize = -1 }() - err = fs.MoveDir(FremoteMove, r.Fremote) + err = fs.MoveDir(FremoteMove, r.Fremote, false) assert.EqualError(t, err, fs.ErrorCantMoveOverlapping.Error()) }