From ce83687829f429403e51b7f9dee06ed1492891f7 Mon Sep 17 00:00:00 2001 From: nielash Date: Mon, 18 Dec 2023 11:09:02 -0500 Subject: [PATCH] operations: --no-block-rmdir for disposable files blocking removal of otherwise empty directory Before this change, rclone commands that remove empty dirs (including rmdir, rmdirs, sync, and bisync) would silently fail to remove dirs if those dirs contained files that were excluded by filters. This can pose a problem for some automatically created system files, such as `.DS_Store` on macOS, which users often prefer to exclude from syncs -- including them makes for noisy logs, but excluding them can make it impossible to remove otherwise empty directories. After this change, a new `--no-block-rmdir` flag allows specifying a comma-separated list of such files which rclone should consider "disposable" if they block removal of otherwise empty directories. When set, if rclone fails to remove a directory, it will check the contents of the directory (ignoring any filters.) If the directory contains ONLY files on the disposable list, rclone will delete the files and then remove the directory. See also: partially-related #6437 --- docs/content/docs.md | 22 ++++++++++++++++++++++ fs/config.go | 1 + fs/operations/operations.go | 27 ++++++++++++++++++++++++++- fs/operations/operations_test.go | 29 +++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 1 deletion(-) diff --git a/docs/content/docs.md b/docs/content/docs.md index eed34ed21..58fd75178 100644 --- a/docs/content/docs.md +++ b/docs/content/docs.md @@ -1786,6 +1786,28 @@ If the backend has a `--backend-upload-concurrency` setting (eg number of transfers instead if it is larger than the value of `--multi-thread-streams` or `--multi-thread-streams` isn't set. +### --no-block-rmdir CommaSepList ### + +Normally, rclone commands that remove empty dirs (including +[`rmdir`](/commands/rclone_rmdir/), [`rmdirs`](/commands/rclone_rmdirs/), +[`sync`](/commands/rclone_sync/), and [`bisync`](https://rclone.org/bisync/)) +will silently fail to remove dirs if those dirs contain files that are excluded +by filters. This can pose a problem for some automatically created system +files, such as `.DS_Store` on macOS, which users often prefer to exclude from +syncs -- including them makes for noisy logs, but excluding them can make it +impossible to remove otherwise empty directories. The `--no-block-rmdir` flag +allows specifying a comma-separated list of such files which rclone should +consider "disposable" if they block removal of otherwise empty directories. +When set, if rclone fails to remove a directory, it will check the contents of +the directory (ignoring any filters.) If the directory contains ONLY files on +the disposable list, rclone will delete the files and then remove the directory. + +Note that `--no-block-rmdir` does not automatically filter such files, nor is it +limited to excluded files. If you want these files excluded, you still have to +apply a filter rule, as usual. + +Example: `--no-block-rmdir ".DS_Store,deleteme.tmp"` + ### --no-check-dest ### The `--no-check-dest` can be used with `move` or `copy` and it causes diff --git a/fs/config.go b/fs/config.go index caf242fdf..1cdbf77e3 100644 --- a/fs/config.go +++ b/fs/config.go @@ -635,6 +635,7 @@ type ConfigInfo struct { Inplace bool `config:"inplace"` // Download directly to destination file instead of atomic download to temp/rename PartialSuffix string `config:"partial_suffix"` MetadataMapper SpaceSepList `config:"metadata_mapper"` + NoBlockRmdir CommaSepList `config:"no_block_rmdir"` } func init() { diff --git a/fs/operations/operations.go b/fs/operations/operations.go index f14ecb61e..72492f106 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -32,6 +32,7 @@ import ( "github.com/rclone/rclone/fs/fserrors" "github.com/rclone/rclone/fs/fshttp" "github.com/rclone/rclone/fs/hash" + "github.com/rclone/rclone/fs/list" "github.com/rclone/rclone/fs/object" "github.com/rclone/rclone/fs/walk" "github.com/rclone/rclone/lib/atexit" @@ -39,6 +40,7 @@ import ( "github.com/rclone/rclone/lib/pacer" "github.com/rclone/rclone/lib/random" "github.com/rclone/rclone/lib/readers" + "golang.org/x/exp/slices" // replace with slices after go1.21 is the minimum version "golang.org/x/sync/errgroup" "golang.org/x/text/unicode/norm" ) @@ -1126,7 +1128,26 @@ func TryRmdir(ctx context.Context, f fs.Fs, dir string) error { return nil } fs.Infof(fs.LogDirName(f, dir), "Removing directory") - return f.Rmdir(ctx, dir) + err := f.Rmdir(ctx, dir) + ci := fs.GetConfig(ctx) + if len(ci.NoBlockRmdir) == 0 || err == nil { + return err + } + entries, err := list.DirSorted(ctx, f, true, dir) // includeAll to ignore filters here + if err != nil { + return err + } + for _, entry := range entries { + obj, ok := entry.(fs.Object) + if ok { + basename := path.Base(obj.Remote()) + if !slices.Contains(ci.NoBlockRmdir, basename) { + return fmt.Errorf("%s: Cannot remove directory due to un-ignorable file: %s", dir, basename) + } + } + } + // directory contains nothing but deletable files + return Purge(ctx, f, dir) } // Rmdir removes a container but not if not empty @@ -1464,6 +1485,10 @@ func Rmdirs(ctx context.Context, f fs.Fs, dir string, leaveRoot bool) error { dirEmpty[dir] = true } case fs.Object: + if len(ci.NoBlockRmdir) > 0 && slices.Contains(ci.NoBlockRmdir, path.Base(x.Remote())) { + fs.Debugf(x.Remote(), "found in --no-block-rmdir list, ignoring") + continue + } // mark the parents of the file as being non-empty dir := x.Remote() for dir != "" { diff --git a/fs/operations/operations_test.go b/fs/operations/operations_test.go index 96ce5704a..50d2553cf 100644 --- a/fs/operations/operations_test.go +++ b/fs/operations/operations_test.go @@ -811,6 +811,35 @@ func TestRmdirsWithFilter(t *testing.T) { ) } +func TestNoBlockRmdir(t *testing.T) { + ctx := context.Background() + r := fstest.NewRun(t) + r.Mkdir(ctx, r.Fremote) + + r.ForceMkdir(ctx, r.Fremote) + + file1 := r.WriteObject(ctx, "A1/B1/C1/.DS_Store", "delete me", t2) + file2 := r.WriteObject(ctx, "A2/B2/C2/Important-File", "keep me", t2) + + r.CheckRemoteItems(t, file1, file2) + + ctx, fi := filter.AddConfig(ctx) + require.NoError(t, fi.AddRule("- .DS_Store")) + ci := fs.GetConfig(ctx) + ci.NoBlockRmdir = []string{".DS_Store"} + + require.NoError(t, operations.Rmdir(ctx, r.Fremote, "A1/B1/C1")) + require.Error(t, operations.Rmdir(ctx, r.Fremote, "A2/B2/C2")) + r.CheckRemoteItems(t, file2) + + file1 = r.WriteObject(ctx, "A1/B1/C1/.DS_Store", "delete me", t2) + r.CheckRemoteItems(t, file1, file2) + + require.NoError(t, operations.Rmdirs(ctx, r.Fremote, "A1", false)) + require.NoError(t, operations.Rmdirs(ctx, r.Fremote, "A2", false)) // Rmdirs does not return errors for non-empty + r.CheckRemoteItems(t, file2) +} + func TestCopyURL(t *testing.T) { ctx := context.Background() ctx, ci := fs.AddConfig(ctx)