From a5c7c0b63ae72211ed4f33712387b96f69b194e6 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Wed, 1 Oct 2025 13:36:07 -0700 Subject: [PATCH] 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},