From f747ba6d69ce27e1b929613382eac3f4d0fc48c3 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Tue, 26 Nov 2019 08:37:41 +0100 Subject: [PATCH] lib/ignore: Keep skipping ignored dirs for rooted patterns (#6151) * lib/ignore: Keep skipping ignored dirs for rooted patterns * review * clarify comment and lint * glob.QuoteMeta * review --- lib/ignore/ignore.go | 191 +++++++++++++++++++++++--------------- lib/ignore/ignore_test.go | 52 +++++++++++ lib/scanner/walk_test.go | 45 +++++++++ 3 files changed, 212 insertions(+), 76 deletions(-) diff --git a/lib/ignore/ignore.go b/lib/ignore/ignore.go index 9ea85515..58d8ed71 100644 --- a/lib/ignore/ignore.go +++ b/lib/ignore/ignore.go @@ -32,6 +32,14 @@ const ( resultFoldCase = 1 << iota ) +var defaultResult Result = resultInclude + +func init() { + if runtime.GOOS == "darwin" || runtime.GOOS == "windows" { + defaultResult |= resultFoldCase + } +} + type Pattern struct { pattern string match glob.Glob @@ -52,6 +60,25 @@ func (p Pattern) String() string { return ret } +func (p Pattern) allowsSkippingIgnoredDirs() bool { + if p.result.IsIgnored() { + return true + } + if p.pattern[0] != '/' { + return false + } + // Double asterisk everywhere in the path except at the end is bad + if strings.Contains(strings.TrimSuffix(p.pattern, "**"), "**") { + return false + } + // Any wildcards anywhere except for the last path component are bad + lastSep := strings.LastIndex(p.pattern, "/") + if lastSep == -1 { + return true + } + return p.pattern[:lastSep] == glob.QuoteMeta(p.pattern[:lastSep]) +} + type Result uint8 func (r Result) IsIgnored() bool { @@ -174,11 +201,20 @@ func (m *Matcher) parseLocked(r io.Reader, file string) error { } m.skipIgnoredDirs = true + var previous string for _, p := range patterns { - if !p.result.IsIgnored() { + // We automatically add patterns with a /** suffix, which normally + // means that we cannot skip directories. However if the same + // pattern without the /** already exists (which is true for + // automatically added patterns) we can skip. + if l := len(p.pattern); l > 3 && p.pattern[:len(p.pattern)-3] == previous { + continue + } + if !p.allowsSkippingIgnoredDirs() { m.skipIgnoredDirs = false break } + previous = p.pattern } m.curHash = newHash @@ -360,87 +396,90 @@ func loadParseIncludeFile(filesystem fs.Filesystem, file string, cd ChangeDetect return patterns, err } +func parseLine(line string) ([]Pattern, error) { + pattern := Pattern{ + result: defaultResult, + } + + // Allow prefixes to be specified in any order, but only once. + var seenPrefix [3]bool + + for { + if strings.HasPrefix(line, "!") && !seenPrefix[0] { + seenPrefix[0] = true + line = line[1:] + pattern.result ^= resultInclude + } else if strings.HasPrefix(line, "(?i)") && !seenPrefix[1] { + seenPrefix[1] = true + pattern.result |= resultFoldCase + line = line[4:] + } else if strings.HasPrefix(line, "(?d)") && !seenPrefix[2] { + seenPrefix[2] = true + pattern.result |= resultDeletable + line = line[4:] + } else { + break + } + } + + if pattern.result.IsCaseFolded() { + line = strings.ToLower(line) + } + + pattern.pattern = line + + var err error + if strings.HasPrefix(line, "/") { + // Pattern is rooted in the current dir only + pattern.match, err = glob.Compile(line[1:], '/') + return []Pattern{pattern}, err + } + patterns := make([]Pattern, 2) + if strings.HasPrefix(line, "**/") { + // Add the pattern as is, and without **/ so it matches in current dir + pattern.match, err = glob.Compile(line, '/') + if err != nil { + return nil, err + } + patterns[0] = pattern + + line = line[3:] + pattern.pattern = line + pattern.match, err = glob.Compile(line, '/') + if err != nil { + return nil, err + } + patterns[1] = pattern + return patterns, nil + } + // Path name or pattern, add it so it matches files both in + // current directory and subdirs. + pattern.match, err = glob.Compile(line, '/') + if err != nil { + return nil, err + } + patterns[0] = pattern + + line = "**/" + line + pattern.pattern = line + pattern.match, err = glob.Compile(line, '/') + if err != nil { + return nil, err + } + patterns[1] = pattern + return patterns, nil +} + func parseIgnoreFile(fs fs.Filesystem, fd io.Reader, currentFile string, cd ChangeDetector, linesSeen map[string]struct{}) ([]string, []Pattern, error) { var lines []string var patterns []Pattern - defaultResult := resultInclude - if runtime.GOOS == "darwin" || runtime.GOOS == "windows" { - defaultResult |= resultFoldCase - } - addPattern := func(line string) error { - pattern := Pattern{ - result: defaultResult, - } - - // Allow prefixes to be specified in any order, but only once. - var seenPrefix [3]bool - - for { - if strings.HasPrefix(line, "!") && !seenPrefix[0] { - seenPrefix[0] = true - line = line[1:] - pattern.result ^= resultInclude - } else if strings.HasPrefix(line, "(?i)") && !seenPrefix[1] { - seenPrefix[1] = true - pattern.result |= resultFoldCase - line = line[4:] - } else if strings.HasPrefix(line, "(?d)") && !seenPrefix[2] { - seenPrefix[2] = true - pattern.result |= resultDeletable - line = line[4:] - } else { - break - } - } - - if pattern.result.IsCaseFolded() { - line = strings.ToLower(line) - } - - pattern.pattern = line - - var err error - if strings.HasPrefix(line, "/") { - // Pattern is rooted in the current dir only - pattern.match, err = glob.Compile(line[1:], '/') - if err != nil { - return errors.Wrapf(err, "invalid pattern %q in ignore file", line) - } - patterns = append(patterns, pattern) - } else if strings.HasPrefix(line, "**/") { - // Add the pattern as is, and without **/ so it matches in current dir - pattern.match, err = glob.Compile(line, '/') - if err != nil { - return errors.Wrapf(err, "invalid pattern %q in ignore file", line) - } - patterns = append(patterns, pattern) - - line = line[3:] - pattern.pattern = line - pattern.match, err = glob.Compile(line, '/') - if err != nil { - return errors.Wrapf(err, "invalid pattern %q in ignore file", line) - } - patterns = append(patterns, pattern) - } else { - // Path name or pattern, add it so it matches files both in - // current directory and subdirs. - pattern.match, err = glob.Compile(line, '/') - if err != nil { - return errors.Wrapf(err, "invalid pattern %q in ignore file", line) - } - patterns = append(patterns, pattern) - - line := "**/" + line - pattern.pattern = line - pattern.match, err = glob.Compile(line, '/') - if err != nil { - return errors.Wrapf(err, "invalid pattern %q in ignore file", line) - } - patterns = append(patterns, pattern) + newPatterns, err := parseLine(line) + if err != nil { + return errors.Wrapf(err, "invalid pattern %q in ignore file", line) } + patterns = append(patterns, newPatterns...) return nil } diff --git a/lib/ignore/ignore_test.go b/lib/ignore/ignore_test.go index 93a72c5d..462f88a3 100644 --- a/lib/ignore/ignore_test.go +++ b/lib/ignore/ignore_test.go @@ -1098,3 +1098,55 @@ func TestPartialIncludeLine(t *testing.T) { } } } + +func TestSkipIgnoredDirs(t *testing.T) { + tcs := []struct { + pattern string + expected bool + }{ + {`!/test`, true}, + {`!/t[eih]t`, true}, + {`!/t*t`, true}, + {`!/t?t`, true}, + {`!/**`, true}, + {`!/parent/test`, true}, + {`!/parent/t[eih]t`, true}, + {`!/parent/t*t`, true}, + {`!/parent/t?t`, true}, + {`!/**.mp3`, false}, + {`!/pa*nt/test`, false}, + {`!/pa[sdf]nt/t[eih]t`, false}, + {`!/lowest/pa[sdf]nt/test`, false}, + {`!/lo*st/parent/test`, false}, + {`/pa*nt/test`, true}, + {`test`, true}, + {`*`, true}, + } + + for _, tc := range tcs { + pats, err := parseLine(tc.pattern) + if err != nil { + t.Error(err) + } + for _, pat := range pats { + if got := pat.allowsSkippingIgnoredDirs(); got != tc.expected { + t.Errorf(`Pattern "%v": got %v, expected %v`, pat, got, tc.expected) + } + } + } + + pats := New(fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata"), WithCache(true)) + + stignore := ` + /foo/ign* + !/f* + !/bar + * + ` + if err := pats.Parse(bytes.NewBufferString(stignore), ".stignore"); err != nil { + t.Fatal(err) + } + if !pats.SkipIgnoredDirs() { + t.Error("SkipIgnoredDirs should be true") + } +} diff --git a/lib/scanner/walk_test.go b/lib/scanner/walk_test.go index b6df67b8..e0a5e297 100644 --- a/lib/scanner/walk_test.go +++ b/lib/scanner/walk_test.go @@ -765,6 +765,51 @@ func TestNotExistingError(t *testing.T) { } } +func TestSkipIgnoredDirs(t *testing.T) { + tmp, err := ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmp) + + fss := fs.NewFilesystem(fs.FilesystemTypeBasic, tmp) + + name := "foo/ignored" + err = fss.MkdirAll(name, 0777) + if err != nil { + t.Fatal(err) + } + + stat, err := fss.Lstat(name) + if err != nil { + t.Fatal(err) + } + + w := &walker{} + + pats := ignore.New(fss, ignore.WithCache(true)) + + stignore := ` + /foo/ign* + !/f* + * + ` + if err := pats.Parse(bytes.NewBufferString(stignore), ".stignore"); err != nil { + t.Fatal(err) + } + if !pats.SkipIgnoredDirs() { + t.Error("SkipIgnoredDirs should be true") + } + + w.Matcher = pats + + fn := w.walkAndHashFiles(context.Background(), nil, nil) + + if err := fn(name, stat, nil); err != fs.SkipDir { + t.Errorf("Expected %v, got %v", fs.SkipDir, err) + } +} + // Verify returns nil or an error describing the mismatch between the block // list and actual reader contents func verify(r io.Reader, blocksize int, blocks []protocol.BlockInfo) error {