Skip to content

Commit

Permalink
Improve ignore logic. (#496)
Browse files Browse the repository at this point in the history 
* Improve ignore logic.

- Add test cases for absolute patterns
- Support globs for absolute patterns

* Test exclude-include (not supported yet).
  • Loading branch information
@darkdragon-001
darkdragon-001 committed Nov 9, 2020
1 parent 6cf6e4d commit ade7975
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 7 deletions.
98 changes: 98 additions & 0 deletions fs/ignorefs/ignorefs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,104 @@ var cases = []struct {
"./src/some-src/f1",
},
},
{
desc: "absolut match",
setup: func(root *mockfs.Directory) {
root.Subdir("src").AddFileLines(".extraignore", []string{
"/sub/*.foo",
}, 0)
root.Subdir("src").AddDir("sub", 0)
root.Subdir("src").Subdir("sub").AddFile("a.foo", dummyFileContents, 0) // ignored by .extraignore
root.Subdir("src").Subdir("sub").AddFile("b.fooX", dummyFileContents, 0)
root.Subdir("src").Subdir("sub").AddFile("foo", dummyFileContents, 0)
root.Subdir("src").AddFile("c.foo", dummyFileContents, 0) // not ignored, at parent level
},
policyTree: policy.BuildTree(map[string]*policy.Policy{
"./src": {
FilesPolicy: policy.FilesPolicy{
DotIgnoreFiles: []string{
".extraignore",
},
},
},
}, policy.DefaultPolicy),
addedFiles: []string{
"./src/.extraignore",
"./src/sub/",
"./src/sub/b.fooX",
"./src/sub/foo",
"./src/c.foo",
},
ignoredFiles: []string{
"./src/sub/a.foo",
},
},
// Requeres major refactoring of ignore logic: https://github.com/kopia/kopia/pull/496#issuecomment-678790009
// {
// desc: "exclude include",
// setup: func(root *mockfs.Directory) {
// root.Subdir("src").AddFileLines(".extraignore", []string{
// "/sub/*.foo",
// "!/sub/special.foo",
// }, 0)
// root.Subdir("src").AddDir("sub", 0)
// root.Subdir("src").Subdir("sub").AddFile("ignore.foo", dummyFileContents, 0) // ignored by wildcard rule
// root.Subdir("src").Subdir("sub").AddFile("special.foo", dummyFileContents, 0) // explicitly included
// },
// policyTree: policy.BuildTree(map[string]*policy.Policy{
// "./src": {
// FilesPolicy: policy.FilesPolicy{
// DotIgnoreFiles: []string{
// ".extraignore",
// },
// },
// },
// }, policy.DefaultPolicy),
// addedFiles: []string{
// "./src/.extraignore",
// "./src/sub/",
// "./src/sub/special.foo",
// },
// ignoredFiles: []string{
// "./src/sub/ignore.foo",
// },
// },
// Not supported according to spec: https://git-scm.com/docs/gitignore#_pattern_format
// {
// desc: "exclude include wildcard",
// setup: func(root *mockfs.Directory) {
// root.Subdir("src").AddFileLines(".extraignore", []string{
// ".config/",
// "!.config/App/**/special/",
// }, 0)
// root.Subdir("src").AddDir(".config", 0)
// root.Subdir("src").Subdir(".config").AddDir("App", 0)
// root.Subdir("src").Subdir(".config").Subdir("App").AddDir("some", 0)
// root.Subdir("src").Subdir(".config").Subdir("App").Subdir("some").AddDir("thing", 0)
// root.Subdir("src").Subdir(".config").Subdir("App").Subdir("some").Subdir("thing").AddFile("ignored_file.txt", dummyFileContents, 0)
// root.Subdir("src").Subdir(".config").Subdir("App").Subdir("some").Subdir("thing").AddDir("special", 0)
// root.Subdir("src").Subdir(".config").Subdir("App").Subdir("some").Subdir("thing").Subdir("special").AddFile("included_file.txt", dummyFileContents, 0)
// },
// policyTree: policy.BuildTree(map[string]*policy.Policy{
// "./src": {
// FilesPolicy: policy.FilesPolicy{
// DotIgnoreFiles: []string{
// ".extraignore",
// },
// },
// },
// }, policy.DefaultPolicy),
// addedFiles: []string{
// "./src/.config/App/",
// "./src/.config/App/some/",
// "./src/.config/App/some/thing/",
// "./src/.config/App/some/thing/special/",
// "./src/.config/App/some/thing/special/included_file.txt",
// },
// ignoredFiles: []string{
// "./src/.config/App/some/thing/ignored_file.txt",
// },
// },
}

func TestIgnoreFS(t *testing.T) {
Expand Down
22 changes: 15 additions & 7 deletions internal/ignore/ignore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ func ParseGitIgnore(baseDir, pattern string) (Matcher, error) {

var m nameMatcher
if !strings.Contains(pattern, "/") {
m = parseGlobPattern(pattern)
m = parseFilenamePattern(pattern)
} else {
var err error
m, err = parseNonGlobPattern(pattern)
m, err = parsePathPattern(pattern)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -96,7 +96,7 @@ func maybeMatchDirOnly(m nameMatcher, dirOnly bool) Matcher {
}
}

func parseGlobPattern(pattern string) nameMatcher {
func parseFilenamePattern(pattern string) nameMatcher {
return func(path string) bool {
last := path[strings.LastIndex(path, "/")+1:]
ok, _ := filepath.Match(pattern, last)
Expand All @@ -105,11 +105,17 @@ func parseGlobPattern(pattern string) nameMatcher {
}
}

func parseNonGlobPattern(pattern string) (nameMatcher, error) {
func parsePathPattern(pattern string) (nameMatcher, error) {
// Absolute paths are relative to baseDir
if strings.HasPrefix(pattern, "/") {
pattern = strings.TrimPrefix(pattern, "/")
}

// No double-star pattern
if !strings.Contains(pattern, "**") {
return func(path string) bool {
return path == pattern
ok, _ := filepath.Match(pattern, path)
return ok
}, nil
}

Expand All @@ -122,7 +128,8 @@ func parseNonGlobPattern(pattern string) (nameMatcher, error) {
suffixWithSlash := strings.TrimPrefix(pattern, "**")

return func(path string) bool {
return path == suffix || strings.HasSuffix(path, suffixWithSlash)
ok, _ := filepath.Match(suffix, path)
return ok || strings.HasSuffix(path, suffixWithSlash)
}, nil
}

Expand All @@ -133,7 +140,8 @@ func parseNonGlobPattern(pattern string) (nameMatcher, error) {
prefixWithSlash := strings.TrimSuffix(pattern, "**")

return func(path string) bool {
return path == prefix || strings.HasPrefix(path, prefixWithSlash)
ok, _ := filepath.Match(prefix, path)
return ok || strings.HasPrefix(path, prefixWithSlash)
}, nil
}

Expand Down
20 changes: 20 additions & 0 deletions internal/ignore/ignore_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ func TestIgnore(t *testing.T) {
// negated match
{"!foo", "/base/dir", "/base/dir/foo", false, false},
{"!foo", "/base/dir", "/base/dir/foo2", false, true},
{"!foo/bar", "/base/dir", "/base/dir/foo/bar", false, false},
{"!foo/bar*", "/base/dir", "/base/dir/foo/bar.txt", false, false},
{"!foo/s*b/bar", "/base/dir", "/base/dir/foo/sub/bar", false, false},
{"!foo/**/bar", "/base/dir", "/base/dir/foo/a/b/bar", false, false},
{"!foo/**/bar/", "/base/dir", "/base/dir/foo/a/b/bar", true, false},

// escaped !
{"\\!important.txt", "/base/dir", "/base/dir/!important.txt", false, true},
Expand All @@ -41,6 +46,21 @@ func TestIgnore(t *testing.T) {

{"*.foo", "/", "/a.foo", true, true},

// absolute match (relative to baseDir)
// pattern must be relative to baseDir "If there is a separator at the beginning or middle (or both) of the pattern"

{"sub/foo", "/base/dir", "/base/dir/sub/foo", false, true},
{"sub/bar/", "/base/dir", "/base/dir/sub/bar", true, true},

{"/sub/foo", "/base/dir", "/base/dir/sub/foo", false, true},
{"/sub/bar/", "/base/dir", "/base/dir/sub/bar", true, true},

{"bar/*.foo", "/base/dir", "/base/dir/bar/a.foo", false, true},
{"bar/*.foo", "/base/dir", "/base/dir/sub/bar/a.foo", false, false},

{"/bar/*.foo", "/base/dir", "/base/dir/bar/a.foo", false, true},
{"/bar/*.foo", "/base/dir", "/base/dir/sub/bar/a.foo", false, false},

// no match outside of base directory
{"foo", "/base/dir", "/base/other-dir/foo", false, false},

Expand Down

0 comments on commit ade7975

Please sign in to comment.