From a28239f005aeb290528e8373d5b99d2efa34c870 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 13 Feb 2019 17:14:51 +0000 Subject: [PATCH] filter: Make --files-from traverse as before unless --no-traverse is set In c5ac96e9e7 we made --files-from only read the objects specified and don't scan directories. This caused problems with Google drive (very very slow) and B2 (excessive API consumption) so it was decided to make the old behaviour (traversing the directories) the default with --files-from and use the existing --no-traverse flag (which has exactly the right semantics) to enable the new non scanning behaviour. See: https://forum.rclone.org/t/using-files-from-with-drive-hammers-the-api/8726 Fixes #3102 Fixes #3095 --- docs/content/filtering.md | 11 ++++++++--- fs/operations/operations_test.go | 12 ++++++++++++ fs/sync/sync_test.go | 7 ++++++- fs/walk/walk.go | 12 ++++++------ 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/docs/content/filtering.md b/docs/content/filtering.md index 783e97a5f..98a183bf6 100644 --- a/docs/content/filtering.md +++ b/docs/content/filtering.md @@ -305,9 +305,14 @@ This reads a list of file names from the file passed in and **only** these files are transferred. The **filtering rules are ignored** completely if you use this option. -Rclone will not scan any directories if you use `--files-from` it will -just look at the files specified. Rclone will not error if any of the -files are missing from the source. +Rclone will traverse the file system if you use `--files-from`, +effectively using the files in `--files-from` as a set of filters. +Rclone will not error if any of the files are missing. + +If you use `--no-traverse` as well as `--files-from` then rclone will +not traverse the destination file system, it will find each file +individually using approximately 1 API call. This can be more +efficient for small lists of files. This option can be repeated to read from more than one file. These are read in the order that they are placed on the command line. diff --git a/fs/operations/operations_test.go b/fs/operations/operations_test.go index 30aa155a8..01cc9f2d4 100644 --- a/fs/operations/operations_test.go +++ b/fs/operations/operations_test.go @@ -124,6 +124,18 @@ func TestLsWithFilesFrom(t *testing.T) { err = operations.List(r.Fremote, &buf) require.NoError(t, err) assert.Equal(t, " 60 potato2\n", buf.String()) + + // Now try with --no-traverse + oldNoTraverse := fs.Config.NoTraverse + fs.Config.NoTraverse = true + defer func() { + fs.Config.NoTraverse = oldNoTraverse + }() + + buf.Reset() + err = operations.List(r.Fremote, &buf) + require.NoError(t, err) + assert.Equal(t, " 60 potato2\n", buf.String()) } func TestLsLong(t *testing.T) { diff --git a/fs/sync/sync_test.go b/fs/sync/sync_test.go index 0456945ef..ed4c8ee7b 100644 --- a/fs/sync/sync_test.go +++ b/fs/sync/sync_test.go @@ -117,7 +117,7 @@ func TestCopyWithDepth(t *testing.T) { } // Test copy with files from -func TestCopyWithFilesFrom(t *testing.T) { +func testCopyWithFilesFrom(t *testing.T, noTraverse bool) { r := fstest.NewRun(t) defer r.Finalise() file1 := r.WriteFile("potato2", "hello world", t1) @@ -131,9 +131,12 @@ func TestCopyWithFilesFrom(t *testing.T) { // Monkey patch the active filter oldFilter := filter.Active + oldNoTraverse := fs.Config.NoTraverse filter.Active = f + fs.Config.NoTraverse = noTraverse unpatch := func() { filter.Active = oldFilter + fs.Config.NoTraverse = oldNoTraverse } defer unpatch() @@ -144,6 +147,8 @@ func TestCopyWithFilesFrom(t *testing.T) { fstest.CheckItems(t, r.Flocal, file1, file2) fstest.CheckItems(t, r.Fremote, file1) } +func TestCopyWithFilesFrom(t *testing.T) { testCopyWithFilesFrom(t, false) } +func TestCopyWithFilesFromAndNoTraverse(t *testing.T) { testCopyWithFilesFrom(t, true) } // Test copy empty directories func TestCopyEmptyDirectories(t *testing.T) { diff --git a/fs/walk/walk.go b/fs/walk/walk.go index 19023300c..b6b5ddf12 100644 --- a/fs/walk/walk.go +++ b/fs/walk/walk.go @@ -54,12 +54,12 @@ type Func func(path string, entries fs.DirEntries, err error) error // This is implemented by WalkR if Config.UseUseListR is true // and f supports it and level > 1, or WalkN otherwise. // -// If --files-from is set then a DirTree will be constructed with just -// those files in and then walked with WalkR +// If --files-from and --no-traverse is set then a DirTree will be +// constructed with just those files in and then walked with WalkR // // NB (f, path) to be replaced by fs.Dir at some point func Walk(f fs.Fs, path string, includeAll bool, maxLevel int, fn Func) error { - if filter.Active.HaveFilesFrom() { + if fs.Config.NoTraverse && filter.Active.HaveFilesFrom() { return walkR(f, path, includeAll, maxLevel, fn, filter.Active.MakeListR(f.NewObject)) } // FIXME should this just be maxLevel < 0 - why the maxLevel > 1 @@ -711,12 +711,12 @@ func walkNDirTree(f fs.Fs, path string, includeAll bool, maxLevel int, listDir l // This is implemented by WalkR if f supports ListR and level > 1, or // WalkN otherwise. // -// If --files-from is set then a DirTree will be constructed with just -// those files in. +// If --files-from and --no-traverse is set then a DirTree will be +// constructed with just those files in. // // NB (f, path) to be replaced by fs.Dir at some point func NewDirTree(f fs.Fs, path string, includeAll bool, maxLevel int) (DirTree, error) { - if filter.Active.HaveFilesFrom() { + if fs.Config.NoTraverse && filter.Active.HaveFilesFrom() { return walkRDirTree(f, path, includeAll, maxLevel, filter.Active.MakeListR(f.NewObject)) } if ListR := f.Features().ListR; (maxLevel < 0 || maxLevel > 1) && ListR != nil {