Revert "filter: Add BoundedRecursion method"

This reverts commit 047f00a411.

It turns out that BoundedRecursion is the wrong thing to measure.
This commit is contained in:
Nick Craig-Wood 2019-08-08 13:35:13 +01:00
parent 6c38bddf3e
commit 99b3154abd
4 changed files with 8 additions and 124 deletions

View File

@ -22,9 +22,8 @@ var Active = mustNewFilter(nil)
// rule is one filter rule // rule is one filter rule
type rule struct { type rule struct {
Include bool Include bool
Regexp *regexp.Regexp Regexp *regexp.Regexp
boundedRecursion bool
} }
// Match returns true if rule matches path // Match returns true if rule matches path
@ -48,14 +47,13 @@ type rules struct {
} }
// add adds a rule if it doesn't exist already // 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 { if rs.existing == nil {
rs.existing = make(map[string]struct{}) rs.existing = make(map[string]struct{})
} }
newRule := rule{ newRule := rule{
Include: Include, Include: Include,
Regexp: re, Regexp: re,
boundedRecursion: boundedRecursion,
} }
newRuleString := newRule.String() newRuleString := newRule.String()
if _, ok := rs.existing[newRuleString]; ok { if _, ok := rs.existing[newRuleString]; ok {
@ -76,23 +74,6 @@ func (rs *rules) len() int {
return len(rs.rules) 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 // FilesMap describes the map of files to transfer
type FilesMap map[string]struct{} type FilesMap map[string]struct{}
@ -252,8 +233,7 @@ func (f *Filter) addDirGlobs(Include bool, glob string) error {
if err != nil { if err != nil {
return err return err
} }
boundedRecursion := globBoundedRecursion(dirGlob) f.dirRules.add(Include, dirRe)
f.dirRules.add(Include, dirRe, boundedRecursion)
} }
return nil return nil
} }
@ -269,9 +249,8 @@ func (f *Filter) Add(Include bool, glob string) error {
if err != nil { if err != nil {
return err return err
} }
boundedRecursion := globBoundedRecursion(glob)
if isFileRule { 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 include rule work out what directories are needed to scan
// if exclude rule, we can't rule anything out // if exclude rule, we can't rule anything out
// Unless it is `*` which matches everything // Unless it is `*` which matches everything
@ -284,7 +263,7 @@ func (f *Filter) Add(Include bool, glob string) error {
} }
} }
if isDirRule { if isDirRule {
f.dirRules.add(Include, re, boundedRecursion) f.dirRules.add(Include, re)
} }
return nil return nil
} }
@ -365,12 +344,6 @@ func (f *Filter) InActive() bool {
len(f.Opt.ExcludeFile) == 0) 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. // includeRemote returns whether this remote passes the filter rules.
func (f *Filter) includeRemote(remote string) bool { func (f *Filter) includeRemote(remote string) bool {
for _, rule := range f.fileRules.rules { for _, rule := range f.fileRules.rules {

View File

@ -26,7 +26,6 @@ func TestNewFilterDefault(t *testing.T) {
assert.Len(t, f.dirRules.rules, 0) assert.Len(t, f.dirRules.rules, 0)
assert.Nil(t, f.files) assert.Nil(t, f.files)
assert.True(t, f.InActive()) assert.True(t, f.InActive())
assert.False(t, f.BoundedRecursion())
} }
// testFile creates a temp file with the contents // 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.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 { type includeTest struct {
@ -185,7 +152,6 @@ func TestNewFilterIncludeFiles(t *testing.T) {
{"file3.jpg", 3, 0, false}, {"file3.jpg", 3, 0, false},
}) })
assert.False(t, f.InActive()) assert.False(t, f.InActive())
assert.False(t, f.BoundedRecursion())
} }
func TestNewFilterIncludeFilesDirs(t *testing.T) { func TestNewFilterIncludeFilesDirs(t *testing.T) {
@ -313,7 +279,6 @@ func TestNewFilterMinSize(t *testing.T) {
{"potato/file2.jpg", 99, 0, false}, {"potato/file2.jpg", 99, 0, false},
}) })
assert.False(t, f.InActive()) assert.False(t, f.InActive())
assert.False(t, f.BoundedRecursion())
} }
func TestNewFilterMaxSize(t *testing.T) { func TestNewFilterMaxSize(t *testing.T) {

View File

@ -167,15 +167,3 @@ func globToDirGlobs(glob string) (out []string) {
return out 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
}

View File

@ -108,45 +108,3 @@ func TestGlobToDirGlobs(t *testing.T) {
assert.Equal(t, test.want, got, test.in) 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)
}
}