Switch to last-rule-wins, .gitignore style. Address comments.

Signed-off-by: Evan Anderson <evan.k.anderson@gmail.com>

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"

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.

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},