From c5ac96e9e7afd2e11b69233ed3671e8ad05d24a9 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Fri, 19 Oct 2018 17:41:14 +0100 Subject: [PATCH] Make --files-from only read the objects specified and don't scan directories Before this change using --files-from would scan all the directories that the files could possibly be in causing rclone to do more work that was necessary. After this change, rclone constructs an in memory tree using the --fast-list mechanism but from all of the files in the --files-from list and without scanning any directories. Any objects that are not found in the --files-from list are ignored silently. This mechanism is used for sync/copy/move (march) and all of the listing commands ls/lsf/md5sum/etc (walk). --- docs/content/filtering.md | 4 ++ fs/filter/filter.go | 28 ++++++++++++ fs/filter/filter_test.go | 78 ++++++++++++++++++++++++++++++++ fs/march/march.go | 2 +- fs/operations/operations_test.go | 27 +++++++++++ fs/sync/sync_test.go | 29 ++++++++++++ fs/walk/walk.go | 12 +++++ 7 files changed, 179 insertions(+), 1 deletion(-) diff --git a/docs/content/filtering.md b/docs/content/filtering.md index b9965501b..5dd497834 100644 --- a/docs/content/filtering.md +++ b/docs/content/filtering.md @@ -293,6 +293,10 @@ 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. + 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/filter/filter.go b/fs/filter/filter.go index c63ce9ade..fbcda022c 100644 --- a/fs/filter/filter.go +++ b/fs/filter/filter.go @@ -496,3 +496,31 @@ func (f *Filter) DumpFilters() string { } return strings.Join(rules, "\n") } + +// HaveFilesFrom returns true if --files-from has been supplied +func (f *Filter) HaveFilesFrom() bool { + return f.files != nil +} + +var errFilesFromNotSet = errors.New("--files-from not set so can't use Filter.ListR") + +// MakeListR makes function to return all the files set using --files-from +func (f *Filter) MakeListR(NewObject func(remote string) (fs.Object, error)) fs.ListRFn { + return func(dir string, callback fs.ListRCallback) error { + if !f.HaveFilesFrom() { + return errFilesFromNotSet + } + var entries fs.DirEntries + for remote := range f.files { + entry, err := NewObject(remote) + if err == fs.ErrorObjectNotFound { + // Skip files that are not found + } else if err != nil { + return err + } else { + entries = append(entries, entry) + } + } + return callback(entries) + } +} diff --git a/fs/filter/filter_test.go b/fs/filter/filter_test.go index 1ff52e1db..7f028c118 100644 --- a/fs/filter/filter_test.go +++ b/fs/filter/filter_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/ncw/rclone/fs" + "github.com/ncw/rclone/fstest/mockobject" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -183,6 +184,83 @@ func TestNewFilterIncludeFilesDirs(t *testing.T) { }) } +func TestNewFilterHaveFilesFrom(t *testing.T) { + f, err := NewFilter(nil) + require.NoError(t, err) + + assert.Equal(t, false, f.HaveFilesFrom()) + + require.NoError(t, f.AddFile("file")) + + assert.Equal(t, true, f.HaveFilesFrom()) +} + +func TestNewFilterMakeListR(t *testing.T) { + f, err := NewFilter(nil) + require.NoError(t, err) + + // Check error if no files + listR := f.MakeListR(nil) + err = listR("", nil) + assert.EqualError(t, err, errFilesFromNotSet.Error()) + + // Add some files + for _, path := range []string{ + "path/to/dir/file1.png", + "/path/to/dir/file2.png", + "/path/to/file3.png", + "/path/to/dir2/file4.png", + "notfound", + } { + err = f.AddFile(path) + require.NoError(t, err) + } + + assert.Equal(t, 5, len(f.files)) + + // NewObject function for MakeListR + newObjects := FilesMap{} + NewObject := func(remote string) (fs.Object, error) { + if remote == "notfound" { + return nil, fs.ErrorObjectNotFound + } else if remote == "error" { + return nil, assert.AnError + } + newObjects[remote] = struct{}{} + return mockobject.New(remote), nil + + } + + // Callback for ListRFn + listRObjects := FilesMap{} + listRcallback := func(entries fs.DirEntries) error { + for _, entry := range entries { + listRObjects[entry.Remote()] = struct{}{} + } + return nil + } + + // Make the listR and call it + listR = f.MakeListR(NewObject) + err = listR("", listRcallback) + require.NoError(t, err) + + // Check that the correct objects were created and listed + want := FilesMap{ + "path/to/dir/file1.png": {}, + "path/to/dir/file2.png": {}, + "path/to/file3.png": {}, + "path/to/dir2/file4.png": {}, + } + assert.Equal(t, want, newObjects) + assert.Equal(t, want, listRObjects) + + // Now check an error is returned from NewObject + require.NoError(t, f.AddFile("error")) + err = listR("", listRcallback) + require.EqualError(t, err, assert.AnError.Error()) +} + func TestNewFilterMinSize(t *testing.T) { f, err := NewFilter(nil) require.NoError(t, err) diff --git a/fs/march/march.go b/fs/march/march.go index 7f938b67d..aacef6082 100644 --- a/fs/march/march.go +++ b/fs/march/march.go @@ -71,7 +71,7 @@ type listDirFn func(dir string) (entries fs.DirEntries, err error) // makeListDir makes a listing function for the given fs and includeAll flags func (m *March) makeListDir(f fs.Fs, includeAll bool) listDirFn { - if !fs.Config.UseListR || f.Features().ListR == nil { + if (!fs.Config.UseListR || f.Features().ListR == nil) && !filter.Active.HaveFilesFrom() { return func(dir string) (entries fs.DirEntries, err error) { return list.DirSorted(f, includeAll, dir) } diff --git a/fs/operations/operations_test.go b/fs/operations/operations_test.go index a4870d79e..bb5cb7221 100644 --- a/fs/operations/operations_test.go +++ b/fs/operations/operations_test.go @@ -96,6 +96,33 @@ func TestLs(t *testing.T) { assert.Contains(t, res, " 60 potato2\n") } +func TestLsWithFilesFrom(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + file1 := r.WriteBoth("potato2", "------------------------------------------------------------", t1) + file2 := r.WriteBoth("empty space", "", t2) + + fstest.CheckItems(t, r.Fremote, file1, file2) + + // Set the --files-from equivalent + f, err := filter.NewFilter(nil) + require.NoError(t, err) + require.NoError(t, f.AddFile("potato2")) + require.NoError(t, f.AddFile("notfound")) + + // Monkey patch the active filter + oldFilter := filter.Active + filter.Active = f + defer func() { + filter.Active = oldFilter + }() + + var buf bytes.Buffer + err = operations.List(r.Fremote, &buf) + require.NoError(t, err) + assert.Equal(t, " 60 potato2\n", buf.String()) +} + func TestLsLong(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise() diff --git a/fs/sync/sync_test.go b/fs/sync/sync_test.go index 20c3a6fb8..019b171ca 100644 --- a/fs/sync/sync_test.go +++ b/fs/sync/sync_test.go @@ -79,6 +79,35 @@ func TestCopyWithDepth(t *testing.T) { fstest.CheckItems(t, r.Fremote, file2) } +// Test copy with files from +func TestCopyWithFilesFrom(t *testing.T) { + r := fstest.NewRun(t) + defer r.Finalise() + file1 := r.WriteFile("potato2", "hello world", t1) + file2 := r.WriteFile("hello world2", "hello world2", t2) + + // Set the --files-from equivalent + f, err := filter.NewFilter(nil) + require.NoError(t, err) + require.NoError(t, f.AddFile("potato2")) + require.NoError(t, f.AddFile("notfound")) + + // Monkey patch the active filter + oldFilter := filter.Active + filter.Active = f + unpatch := func() { + filter.Active = oldFilter + } + defer unpatch() + + err = CopyDir(r.Fremote, r.Flocal) + require.NoError(t, err) + unpatch() + + fstest.CheckItems(t, r.Flocal, file1, file2) + fstest.CheckItems(t, r.Fremote, file1) +} + // Test copy empty directories func TestCopyEmptyDirectories(t *testing.T) { r := fstest.NewRun(t) diff --git a/fs/walk/walk.go b/fs/walk/walk.go index d92a21e9f..23534690f 100644 --- a/fs/walk/walk.go +++ b/fs/walk/walk.go @@ -54,8 +54,14 @@ type Func func(path string, entries fs.DirEntries, err error) error // This is implemented by WalkR if Config.UseRecursiveListing 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 +// // 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() { + return walkR(f, path, includeAll, maxLevel, fn, filter.Active.MakeListR(f.NewObject)) + } if (maxLevel < 0 || maxLevel > 1) && fs.Config.UseListR && f.Features().ListR != nil { return walkListR(f, path, includeAll, maxLevel, fn) } @@ -452,8 +458,14 @@ func walkNDirTree(f fs.Fs, path string, includeAll bool, maxLevel int, listDir l // This is implemented by WalkR if Config.UseRecursiveListing 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. +// // 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() { + return walkRDirTree(f, path, includeAll, maxLevel, filter.Active.MakeListR(f.NewObject)) + } if ListR := f.Features().ListR; (maxLevel < 0 || maxLevel > 1) && fs.Config.UseListR && ListR != nil { return walkRDirTree(f, path, includeAll, maxLevel, ListR) }