move: add --delete-empty-src-dirs flag - fixes #1854

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)
 		})
 	},
 }

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)
 		})

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

fs/sync.go

@@ -16,11 +16,12 @@ var oldSyncMethod = BoolP("old-sync-method", "", false, "Deprecated - use --fast
 
 type syncCopyMove struct {
 	// parameters
-	fdst       Fs
+	fdst               Fs
-	fsrc       Fs
+	fsrc               Fs
-	deleteMode DeleteMode // how we are doing deletions
+	deleteMode         DeleteMode // how we are doing deletions
-	DoMove     bool
+	DoMove             bool
-	dir        string
+	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,
+		fdst:               fdst,
-		fsrc:           fsrc,
+		fsrc:               fsrc,
-		deleteMode:     deleteMode,
+		deleteMode:         deleteMode,
-		DoMove:         DoMove,
+		DoMove:             DoMove,
-		dir:            "",
+		deleteEmptySrcDirs: deleteEmptySrcDirs,
-		srcFilesChan:   make(chan Object, Config.Checkers+Config.Transfers),
+		dir:                "",
-		srcFilesResult: make(chan error, 1),
+		srcFilesChan:       make(chan Object, Config.Checkers+Config.Transfers),
-		dstFilesResult: make(chan error, 1),
+		srcFilesResult:     make(chan error, 1),
-		noTraverse:     Config.NoTraverse,
+		dstFilesResult:     make(chan error, 1),
-		toBeChecked:    make(ObjectPairChan, Config.Transfers),
+		noTraverse:         Config.NoTraverse,
-		toBeUploaded:   make(ObjectPairChan, Config.Transfers),
+		toBeChecked:        make(ObjectPairChan, Config.Transfers),
-		deleteFilesCh:  make(chan Object, Config.Checkers),
+		toBeUploaded:       make(ObjectPairChan, Config.Transfers),
-		trackRenames:   Config.TrackRenames,
+		deleteFilesCh:      make(chan Object, Config.Checkers),
-		commonHash:     fsrc.Hashes().Overlap(fdst.Hashes()).GetOne(),
+		trackRenames:       Config.TrackRenames,
-		toBeRenamed:    make(ObjectPairChan, Config.Transfers),
+		commonHash:         fsrc.Hashes().Overlap(fdst.Hashes()).GetOne(),
-		trackRenamesCh: make(chan Object, Config.Checkers),
+		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
+	// Delete empty fsrc subdirectories
-	if s.DoMove {
+	// 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 {
+func moveDir(fdst, fsrc Fs, deleteEmptySrcDirs bool) error {
-	return runSyncCopyMove(fdst, fsrc, DeleteModeOff, true)
+	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)
 }

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())
 }