From e584e68e0c4e640dba07fe60b98db6caaa32c9ca Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 1 Aug 2023 12:24:41 -0700 Subject: [PATCH 1/3] More gitignore-like implementation of negative ignore rules Signed-off-by: Evan Anderson --- internal/ignore/doc.go | 2 +- internal/ignore/rules.go | 20 ++++++-------------- internal/ignore/rules_test.go | 10 ++++++---- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/internal/ignore/doc.go b/internal/ignore/doc.go index a1f0fcfc8..ace0e8ef4 100644 --- a/internal/ignore/doc.go +++ b/internal/ignore/doc.go @@ -63,6 +63,6 @@ Notable differences from .gitignore: - The globbing library is Go's 'filepath.Match', not fnmatch(3) - Trailing spaces are always ignored (there is no supported escape sequence) - The evaluation of escape sequences has not been tested for compatibility - - There is no support for '\!' as a special leading sequence. + - There is no support for '\\!' as a special leading sequence for files that begin with `!` */ package ignore // import "helm.sh/helm/v3/internal/ignore" diff --git a/internal/ignore/rules.go b/internal/ignore/rules.go index a80923baf..731c429ec 100644 --- a/internal/ignore/rules.go +++ b/internal/ignore/rules.go @@ -102,30 +102,22 @@ func (r *Rules) Ignore(path string, fi os.FileInfo) bool { } for _, p := range r.patterns { if p.match == nil { - log.Printf("ignore: no matcher supplied for %q", p.raw) return false } - // For negative rules, we need to capture and return non-matches, - // and continue for matches. - if p.negate { - if p.mustDir && !fi.IsDir() { - return true - } - if !p.match(path, fi) { - return true - } - continue - } - // If the rule is looking for directories, and this is not a directory, // skip it. if p.mustDir && !fi.IsDir() { continue } + + // For negative rules, we need to capture and return non-matches, + // and continue for matches. if p.match(path, fi) { - return true + // if negate, then return false to "should I ignore" + return !p.negate } + } return false } diff --git a/internal/ignore/rules_test.go b/internal/ignore/rules_test.go index 9581cf09f..b1f071fed 100644 --- a/internal/ignore/rules_test.go +++ b/internal/ignore/rules_test.go @@ -113,10 +113,12 @@ func TestIgnore(t *testing.T) { {`helm.txt/`, "helm.txt", false}, // Negation tests - {`!helm.txt`, "helm.txt", false}, - {`!helm.txt`, "tiller.txt", true}, - {`!*.txt`, "cargo", true}, - {`!cargo/`, "mast/", true}, + {"!helm.txt\n*", "helm.txt", false}, + {"!helm.txt", "tiller.txt", false}, // Don't ignore files that match zero patterns + {"!helm.txt\n*", "tiller.txt", true}, + {"!*.txt\n*", "cargo", true}, + {"!cargo/\n*", "cargo", false}, + {"!cargo/\n*", "cargo/a.txt", true}, // Absolute path tests {`/a.txt`, "a.txt", true}, From 9d6cde33379395908734500b34e8e9851d462f93 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Fri, 13 Sep 2024 15:54:04 -0700 Subject: [PATCH 2/3] Update pkg/ignore/doc.go Co-authored-by: Eddy Moulton Signed-off-by: Evan Anderson --- pkg/ignore/doc.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/ignore/doc.go b/pkg/ignore/doc.go index b21a661d3..e576e9cee 100644 --- a/pkg/ignore/doc.go +++ b/pkg/ignore/doc.go @@ -64,5 +64,18 @@ Notable differences from .gitignore: - Trailing spaces are always ignored (there is no supported escape sequence) - The evaluation of escape sequences has not been tested for compatibility - There is no support for '\\!' as a special leading sequence for files that begin with `!` + - The first filename match will determine if it is included or excluded, subsequent lines are ignored + +Example: + + # This is valid .gitignore syntax to only include Chart.yaml and values.yaml, but invalid for .helmignore + * + !Chart.yaml + !values.yaml + + # Instead, this syntax will work as .helmignore takes the first match per file + !Chart.yaml + !values.yaml + * */ package ignore // import "helm.sh/helm/v3/pkg/ignore" From a5c7c0b63ae72211ed4f33712387b96f69b194e6 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Wed, 1 Oct 2025 13:36:07 -0700 Subject: [PATCH 3/3] Switch to last-rule-wins, .gitignore style. Address comments. Signed-off-by: Evan Anderson --- pkg/ignore/doc.go | 18 ++++-------------- pkg/ignore/rules.go | 17 +++++++++-------- pkg/ignore/rules_test.go | 10 +++++----- 3 files changed, 18 insertions(+), 27 deletions(-) diff --git a/pkg/ignore/doc.go b/pkg/ignore/doc.go index 7c5708a92..49ea78168 100644 --- a/pkg/ignore/doc.go +++ b/pkg/ignore/doc.go @@ -37,6 +37,8 @@ The formatting rules are as follows: - If a pattern contains no slashes, file basenames are tested (not paths) - The pattern sequence "**", while legal in a glob, will cause an error here (to indicate incompatibility with .gitignore). + - The last matching rule will determine whether a file is included or excluded. + It is recommended to write rules from most-general to most-specific to match this pattern. Example: @@ -49,8 +51,9 @@ Example: # Match only directories named mydir mydir/ - # Match only text files in the top-level directory + # Match text files in the top-level directory, except license.txt /*.txt + !license.txt # Match only the file foo.txt in the top-level directory /foo.txt @@ -64,18 +67,5 @@ Notable differences from .gitignore: - Trailing spaces are always ignored (there is no supported escape sequence) - The evaluation of escape sequences has not been tested for compatibility - There is no support for '\\!' as a special leading sequence for files that begin with `!` - - The first filename match will determine if it is included or excluded, subsequent lines are ignored - -Example: - - # This is valid .gitignore syntax to only include Chart.yaml and values.yaml, but invalid for .helmignore - * - !Chart.yaml - !values.yaml - - # Instead, this syntax will work as .helmignore takes the first match per file - !Chart.yaml - !values.yaml - * */ package ignore // import "helm.sh/helm/v4/pkg/ignore" diff --git a/pkg/ignore/rules.go b/pkg/ignore/rules.go index 88eec19e6..608f7757e 100644 --- a/pkg/ignore/rules.go +++ b/pkg/ignore/rules.go @@ -85,8 +85,9 @@ func Parse(file io.Reader) (*Rules, error) { // Ignore evaluates the file at the given path, and returns true if it should be ignored. // -// Ignore evaluates path against the rules in order. Evaluation stops when a match -// is found. Matching a negative rule will stop evaluation. +// Ignore evaluates path against the rules in order, with the last matching rule dictating +// whether the file is ignored. This follows the pattern of `.gitignore` -- note that +// "true" means to ignore the file, and "false" means to _keep_ the file. func (r *Rules) Ignore(path string, fi os.FileInfo) bool { // Don't match on empty dirs. if path == "" { @@ -99,10 +100,12 @@ func (r *Rules) Ignore(path string, fi os.FileInfo) bool { if path == "." || path == "./" { return false } + ignore := false for _, p := range r.patterns { if p.match == nil { + // This is a logic error; p.match should always be set in parseRule. slog.Info("this will be ignored no matcher supplied", "patterns", p.raw) - return false + continue } // If the rule is looking for directories, and this is not a directory, @@ -111,15 +114,13 @@ func (r *Rules) Ignore(path string, fi os.FileInfo) bool { continue } - // For negative rules, we need to capture and return non-matches, - // and continue for matches. + // `.gitignore` semantics are last match wins, so we can't early-return on a match. if p.match(path, fi) { - // if negate, then return false to "should I ignore" - return !p.negate + ignore = !p.negate } } - return false + return ignore } // parseRule parses a rule string and creates a pattern, which is then stored in the Rules object. diff --git a/pkg/ignore/rules_test.go b/pkg/ignore/rules_test.go index b1f071fed..3b401df86 100644 --- a/pkg/ignore/rules_test.go +++ b/pkg/ignore/rules_test.go @@ -113,12 +113,12 @@ func TestIgnore(t *testing.T) { {`helm.txt/`, "helm.txt", false}, // Negation tests - {"!helm.txt\n*", "helm.txt", false}, + {"*\n!helm.txt", "helm.txt", false}, {"!helm.txt", "tiller.txt", false}, // Don't ignore files that match zero patterns - {"!helm.txt\n*", "tiller.txt", true}, - {"!*.txt\n*", "cargo", true}, - {"!cargo/\n*", "cargo", false}, - {"!cargo/\n*", "cargo/a.txt", true}, + {"*\n!helm.txt", "tiller.txt", true}, + {"*\n!*.txt", "cargo", true}, + {"*\n!cargo/", "cargo", false}, + {"*\n!cargo/", "cargo/a.txt", true}, // Absolute path tests {`/a.txt`, "a.txt", true},