From dcc74fa404b1882236d991c3e4e03efeb8e352b7 Mon Sep 17 00:00:00 2001 From: ishuah Date: Tue, 17 Jul 2018 08:43:58 +0300 Subject: [PATCH] move: fix delete-empty-src-dirs flag to delete all empty dirs on move - fixes #2372 --- fs/sync/sync.go | 22 +++++++++++++-------- fs/sync/sync_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/fs/sync/sync.go b/fs/sync/sync.go index 6df8791b9..8c0d88baa 100644 --- a/fs/sync/sync.go +++ b/fs/sync/sync.go @@ -522,13 +522,18 @@ func copyEmptyDirectories(f fs.Fs, entries map[string]fs.DirEntry) error { return nil } -func parentDirCheck(entries map[string]fs.DirEntry, entry fs.DirEntry) { +func (s *syncCopyMove) srcParentDirCheck(entry fs.DirEntry) { + // If we are moving files then we don't want to remove directories with files in them + // from the srcEmptyDirs as we are about to move them making the directory empty. + if s.DoMove { + return + } parentDir := path.Dir(entry.Remote()) if parentDir == "." { parentDir = "" } - if _, ok := entries[parentDir]; ok { - delete(entries, parentDir) + if _, ok := s.srcEmptyDirs[parentDir]; ok { + delete(s.srcEmptyDirs, parentDir) } } @@ -771,10 +776,11 @@ func (s *syncCopyMove) SrcOnly(src fs.DirEntry) (recurse bool) { } switch x := src.(type) { case fs.Object: - // Remove parent directory from srcEmptyDirs + // If it's a copy operation, + // remove parent directory from srcEmptyDirs // since it's not really empty s.srcEmptyDirsMu.Lock() - parentDirCheck(s.srcEmptyDirs, src) + s.srcParentDirCheck(src) s.srcEmptyDirsMu.Unlock() if s.trackRenames { @@ -796,7 +802,7 @@ func (s *syncCopyMove) SrcOnly(src fs.DirEntry) (recurse bool) { // Do the same thing to the entire contents of the directory // Record the directory for deletion s.srcEmptyDirsMu.Lock() - parentDirCheck(s.srcEmptyDirs, src) + s.srcParentDirCheck(src) s.srcEmptyDirs[src.Remote()] = src s.srcEmptyDirsMu.Unlock() return true @@ -811,7 +817,7 @@ func (s *syncCopyMove) Match(dst, src fs.DirEntry) (recurse bool) { switch srcX := src.(type) { case fs.Object: s.srcEmptyDirsMu.Lock() - parentDirCheck(s.srcEmptyDirs, src) + s.srcParentDirCheck(src) s.srcEmptyDirsMu.Unlock() if s.deleteMode == fs.DeleteModeOnly { @@ -836,7 +842,7 @@ func (s *syncCopyMove) Match(dst, src fs.DirEntry) (recurse bool) { if ok { // Record the src directory for deletion s.srcEmptyDirsMu.Lock() - parentDirCheck(s.srcEmptyDirs, src) + s.srcParentDirCheck(src) s.srcEmptyDirs[src.Remote()] = src s.srcEmptyDirsMu.Unlock() return true diff --git a/fs/sync/sync_test.go b/fs/sync/sync_test.go index 11204506c..20c3a6fb8 100644 --- a/fs/sync/sync_test.go +++ b/fs/sync/sync_test.go @@ -947,6 +947,52 @@ func testServerSideMove(t *testing.T, r *fstest.Run, withFilter, testDeleteEmpty } } +// Test move +func TestMoveWithDeleteEmptySrcDirs(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + file1 := r.WriteFile("sub dir/hello world", "hello world", t1) + file2 := r.WriteFile("nested/sub dir/file", "nested", t1) + r.Mkdir(r.Fremote) + + // run move with --delete-empty-src-dirs + err := MoveDir(r.Fremote, r.Flocal, true) + require.NoError(t, err) + + fstest.CheckListingWithPrecision( + t, + r.Flocal, + nil, + []string{}, + fs.GetModifyWindow(r.Flocal), + ) + fstest.CheckItems(t, r.Fremote, file1, file2) +} + +func TestMoveWithoutDeleteEmptySrcDirs(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + file1 := r.WriteFile("sub dir/hello world", "hello world", t1) + file2 := r.WriteFile("nested/sub dir/file", "nested", t1) + r.Mkdir(r.Fremote) + + err := MoveDir(r.Fremote, r.Flocal, false) + require.NoError(t, err) + + fstest.CheckListingWithPrecision( + t, + r.Flocal, + nil, + []string{ + "sub dir", + "nested", + "nested/sub dir", + }, + fs.GetModifyWindow(r.Flocal), + ) + fstest.CheckItems(t, r.Fremote, file1, file2) +} + // Test a server side move if possible, or the backup path if not func TestServerSideMove(t *testing.T) { r := fstest.NewRun(t)