From ade79752121bbfabc30ad792a1a5e14d550744d2 Mon Sep 17 00:00:00 2001 From: darkdragon-001 Date: Mon, 9 Nov 2020 02:26:48 +0100 Subject: [PATCH] Improve ignore logic. (#496) * Improve ignore logic. - Add test cases for absolute patterns - Support globs for absolute patterns * Test exclude-include (not supported yet). --- fs/ignorefs/ignorefs_test.go | 98 ++++++++++++++++++++++++++++++++++ internal/ignore/ignore.go | 22 +++++--- internal/ignore/ignore_test.go | 20 +++++++ 3 files changed, 133 insertions(+), 7 deletions(-) diff --git a/fs/ignorefs/ignorefs_test.go b/fs/ignorefs/ignorefs_test.go index 67d1df496..cffb2ea73 100644 --- a/fs/ignorefs/ignorefs_test.go +++ b/fs/ignorefs/ignorefs_test.go @@ -241,6 +241,104 @@ func setupFilesystem() *mockfs.Directory { "./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) { diff --git a/internal/ignore/ignore.go b/internal/ignore/ignore.go index d1138e87b..3111e2db7 100644 --- a/internal/ignore/ignore.go +++ b/internal/ignore/ignore.go @@ -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 } @@ -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) @@ -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 } @@ -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 } @@ -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 } diff --git a/internal/ignore/ignore_test.go b/internal/ignore/ignore_test.go index 2a2913cd5..69015b089 100644 --- a/internal/ignore/ignore_test.go +++ b/internal/ignore/ignore_test.go @@ -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}, @@ -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},