From 99b3154abd0730f373b9ba995ff6819053d79b7f Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Thu, 8 Aug 2019 13:35:13 +0100 Subject: [PATCH] Revert "filter: Add BoundedRecursion method" This reverts commit 047f00a41123735b7953ad52669084448fb672ca. It turns out that BoundedRecursion is the wrong thing to measure. --- fs/filter/filter.go | 43 ++++++++-------------------------------- fs/filter/filter_test.go | 35 -------------------------------- fs/filter/glob.go | 12 ----------- fs/filter/glob_test.go | 42 --------------------------------------- 4 files changed, 8 insertions(+), 124 deletions(-) diff --git a/fs/filter/filter.go b/fs/filter/filter.go index 3944c2df7..b693c2129 100644 --- a/fs/filter/filter.go +++ b/fs/filter/filter.go @@ -22,9 +22,8 @@ var Active = mustNewFilter(nil) // rule is one filter rule type rule struct { - Include bool - Regexp *regexp.Regexp - boundedRecursion bool + Include bool + Regexp *regexp.Regexp } // Match returns true if rule matches path @@ -48,14 +47,13 @@ type rules struct { } // add adds a rule if it doesn't exist already -func (rs *rules) add(Include bool, re *regexp.Regexp, boundedRecursion bool) { +func (rs *rules) add(Include bool, re *regexp.Regexp) { if rs.existing == nil { rs.existing = make(map[string]struct{}) } newRule := rule{ - Include: Include, - Regexp: re, - boundedRecursion: boundedRecursion, + Include: Include, + Regexp: re, } newRuleString := newRule.String() if _, ok := rs.existing[newRuleString]; ok { @@ -76,23 +74,6 @@ func (rs *rules) len() int { return len(rs.rules) } -// boundedRecursion returns true if the set of filters would only -// need bounded recursion to evaluate -func (rs *rules) boundedRecursion() bool { - var ( - excludeAll = false - boundedRecursion = true - ) - for _, rule := range rs.rules { - if rule.Include { - boundedRecursion = boundedRecursion && rule.boundedRecursion - } else if rule.Regexp.String() == `^.*$` { - excludeAll = true - } - } - return excludeAll && boundedRecursion -} - // FilesMap describes the map of files to transfer type FilesMap map[string]struct{} @@ -252,8 +233,7 @@ func (f *Filter) addDirGlobs(Include bool, glob string) error { if err != nil { return err } - boundedRecursion := globBoundedRecursion(dirGlob) - f.dirRules.add(Include, dirRe, boundedRecursion) + f.dirRules.add(Include, dirRe) } return nil } @@ -269,9 +249,8 @@ func (f *Filter) Add(Include bool, glob string) error { if err != nil { return err } - boundedRecursion := globBoundedRecursion(glob) if isFileRule { - f.fileRules.add(Include, re, boundedRecursion) + f.fileRules.add(Include, re) // If include rule work out what directories are needed to scan // if exclude rule, we can't rule anything out // Unless it is `*` which matches everything @@ -284,7 +263,7 @@ func (f *Filter) Add(Include bool, glob string) error { } } if isDirRule { - f.dirRules.add(Include, re, boundedRecursion) + f.dirRules.add(Include, re) } return nil } @@ -365,12 +344,6 @@ func (f *Filter) InActive() bool { len(f.Opt.ExcludeFile) == 0) } -// BoundedRecursion returns true if the filter can be evaluated with -// bounded recursion only. -func (f *Filter) BoundedRecursion() bool { - return f.fileRules.boundedRecursion() -} - // includeRemote returns whether this remote passes the filter rules. func (f *Filter) includeRemote(remote string) bool { for _, rule := range f.fileRules.rules { diff --git a/fs/filter/filter_test.go b/fs/filter/filter_test.go index 276ed83e6..36c425cba 100644 --- a/fs/filter/filter_test.go +++ b/fs/filter/filter_test.go @@ -26,7 +26,6 @@ func TestNewFilterDefault(t *testing.T) { assert.Len(t, f.dirRules.rules, 0) assert.Nil(t, f.files) assert.True(t, f.InActive()) - assert.False(t, f.BoundedRecursion()) } // testFile creates a temp file with the contents @@ -105,38 +104,6 @@ func TestNewFilterFull(t *testing.T) { } } assert.False(t, f.InActive()) - assert.False(t, f.BoundedRecursion()) -} - -func TestFilterBoundedRecursion(t *testing.T) { - for _, test := range []struct { - in string - want bool - }{ - {"", false}, - {"- /**", true}, - {"+ *.jpg", false}, - {"+ *.jpg\n- /**", false}, - {"+ /*.jpg\n- /**", true}, - {"+ *.png\n+ /*.jpg\n- /**", false}, - {"+ /*.png\n+ /*.jpg\n- /**", true}, - {"- *.jpg\n- /**", true}, - {"+ /*.jpg\n- /**", true}, - {"+ /*dir/\n- /**", true}, - {"+ /*dir/\n", false}, - {"+ /*dir/**\n- /**", false}, - {"+ **/pics*/*.jpg\n- /**", false}, - } { - f, err := NewFilter(nil) - require.NoError(t, err) - for _, rule := range strings.Split(test.in, "\n") { - if rule != "" { - require.NoError(t, f.AddRule(rule)) - } - } - got := f.BoundedRecursion() - assert.Equal(t, test.want, got, test.in) - } } type includeTest struct { @@ -185,7 +152,6 @@ func TestNewFilterIncludeFiles(t *testing.T) { {"file3.jpg", 3, 0, false}, }) assert.False(t, f.InActive()) - assert.False(t, f.BoundedRecursion()) } func TestNewFilterIncludeFilesDirs(t *testing.T) { @@ -313,7 +279,6 @@ func TestNewFilterMinSize(t *testing.T) { {"potato/file2.jpg", 99, 0, false}, }) assert.False(t, f.InActive()) - assert.False(t, f.BoundedRecursion()) } func TestNewFilterMaxSize(t *testing.T) { diff --git a/fs/filter/glob.go b/fs/filter/glob.go index 9fff1ca73..96a48d3d4 100644 --- a/fs/filter/glob.go +++ b/fs/filter/glob.go @@ -167,15 +167,3 @@ func globToDirGlobs(glob string) (out []string) { return out } - -// globBoundedRecursion returns true if the glob only needs bounded -// recursion in the file tree to evaluate. -func globBoundedRecursion(glob string) bool { - if strings.Contains(glob, "**") { - return false - } - if strings.HasPrefix(glob, "/") { - return true - } - return false -} diff --git a/fs/filter/glob_test.go b/fs/filter/glob_test.go index 372fa63e3..008d4bfd3 100644 --- a/fs/filter/glob_test.go +++ b/fs/filter/glob_test.go @@ -108,45 +108,3 @@ func TestGlobToDirGlobs(t *testing.T) { assert.Equal(t, test.want, got, test.in) } } - -func TestGlobBoundedRecursion(t *testing.T) { - for _, test := range []struct { - in string - want bool - }{ - {`*`, false}, - {`/*`, true}, - {`/**`, false}, - {`*.jpg`, false}, - {`/*.jpg`, true}, - {`/a/*.jpg`, true}, - {`/a/b/*.jpg`, true}, - {`*/*/*.jpg`, false}, - {`a/b/`, false}, - {`a/b`, false}, - {`a/b/*.{png,gif}`, false}, - {`/a/{jpg,png,gif}/*.{jpg,true,gif}`, true}, - {`a/{a,a*b,a**c}/d/`, false}, - {`/a/{a,a*b,a/c,d}/d/`, true}, - {`**`, false}, - {`a**`, false}, - {`a**b`, false}, - {`a**b**c**d`, false}, - {`a**b/c**d`, false}, - {`/A/a**b/B/c**d/C/`, false}, - {`/var/spool/**/ncw`, false}, - {`var/spool/**/ncw/`, false}, - {"/file1.jpg", true}, - {"/file2.png", true}, - {"/*.jpg", true}, - {"/*.png", true}, - {"/potato", true}, - {"/sausage1", true}, - {"/sausage2*", true}, - {"/sausage3**", false}, - {"/a/*.jpg", true}, - } { - got := globBoundedRecursion(test.in) - assert.Equal(t, test.want, got, test.in) - } -}