Refuse an exclude pattern that does not compile when the config loads (closes #455)

An exclude pattern that did not compile was skipped with a log warning during the backup, which then stored what it meant to leave out. Config.Validate now refuses such a pattern, global or per snapshot, naming the setting and quoting the pattern. The compile step moves to a new package, internal/exclude, which both the config and the scanner call.

Model: opus-5-5

internal/config/exclude.go

@@ -1,79 +1,11 @@
 package config
 
 import (
-	"errors"
 	"fmt"
-	"strings"
 
-	"github.com/gobwas/glob"
+	"sneak.berlin/go/vaultik/internal/exclude"
 )
 
-var errBadExcludePattern = errors.New("is not a valid exclude pattern")
-
-// CompileExcludePattern compiles one exclude pattern into the globs that
-// match it. anchored reports whether the pattern matches only from the root
-// of the source directory. An empty pattern yields no globs. Config.Validate
-// and the scanner both compile patterns with it, so a pattern that loads is
-// one the scanner can use.
-func CompileExcludePattern(pattern string) ([]glob.Glob, bool, error) {
-	if pattern == "" {
-		return nil, false, nil
-	}
-
-	// A leading "/" anchors the pattern to the source directory.
-	trimmed, anchored := strings.CutPrefix(pattern, "/")
-
-	// A trailing "/" marks a directory and is not matched.
-	trimmed = strings.TrimSuffix(trimmed, "/")
-
-	// A leading "**/" demands a "/" before the name and so misses an
-	// entry directly under the source path. It means "at every depth",
-	// which is what an unanchored pattern already does, so drop the
-	// prefix and match unanchored: as in gitignore, "**/x" and
-	// "/**/x" then also match a top-level "x".
-	if rest, found := strings.CutPrefix(trimmed, "**/"); found {
-		trimmed = rest
-		anchored = false
-	}
-
-	// Likewise "/**/" inside a pattern demands two slashes, so
-	// "a/**/*.log" misses "a/x.log". As in gitignore it stands for
-	// zero or more directories, so also compile the pattern with it
-	// made a single "/". The glob library's {a,b} is no help: with a
-	// "**" inside it in mid-pattern, it misses paths it should match.
-	var globs []glob.Glob
-
-	for _, variant := range doubleStarVariants(trimmed) {
-		g, err := glob.Compile(variant, '/')
-		if err != nil {
-			return nil, false, fmt.Errorf("%w: %w", errBadExcludePattern, err)
-		}
-
-		globs = append(globs, g)
-	}
-
-	return globs, anchored, nil
-}
-
-// doubleStarVariants returns pattern once for every way of keeping each
-// "/**/" in it or making it a single "/".
-func doubleStarVariants(pattern string) []string {
-	const doubleStar = "/**/"
-
-	before, after, found := strings.Cut(pattern, doubleStar)
-	if !found {
-		return []string{pattern}
-	}
-
-	var variants []string
-
-	for _, rest := range doubleStarVariants(after) {
-		variants = append(variants, before+doubleStar+rest, before+"/"+rest)
-	}
-
-	return variants
-}
-
 // validateExcludes refuses an exclude pattern, global or in a snapshot,
 // that does not compile, naming the setting and quoting the pattern.
 func (c *Config) validateExcludes() error {
@@ -97,9 +29,9 @@ func (c *Config) validateExcludes() error {
 // compile. setting is the name of the list, as in "exclude".
 func validateExcludeList(setting string, patterns []string) error {
 	for i, pattern := range patterns {
-		_, _, err := CompileExcludePattern(pattern)
+		_, _, err := exclude.Compile(pattern)
 		if err != nil {
-			return fmt.Errorf("%s[%d]: %q %w", setting, i, pattern, err)
+			return fmt.Errorf("%s[%d]: %w", setting, i, err)
 		}
 	}
 

internal/exclude/exclude.go

@@ -0,0 +1,77 @@
+// Package exclude compiles exclude patterns into globs.
+package exclude
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/gobwas/glob"
+)
+
+var errBadExcludePattern = errors.New("is not a valid exclude pattern")
+
+// Compile compiles one exclude pattern into the globs that match it.
+// anchored reports whether the pattern matches only from the root of the
+// source directory. An empty pattern yields no globs. Config.Validate and
+// the scanner both compile patterns with it, so a pattern that loads is one
+// the scanner can use.
+func Compile(pattern string) ([]glob.Glob, bool, error) {
+	if pattern == "" {
+		return nil, false, nil
+	}
+
+	// A leading "/" anchors the pattern to the source directory.
+	trimmed, anchored := strings.CutPrefix(pattern, "/")
+
+	// A trailing "/" marks a directory and is not matched.
+	trimmed = strings.TrimSuffix(trimmed, "/")
+
+	// A leading "**/" demands a "/" before the name and so misses an
+	// entry directly under the source path. It means "at every depth",
+	// which is what an unanchored pattern already does, so drop the
+	// prefix and match unanchored: as in gitignore, "**/x" and
+	// "/**/x" then also match a top-level "x".
+	if rest, found := strings.CutPrefix(trimmed, "**/"); found {
+		trimmed = rest
+		anchored = false
+	}
+
+	// Likewise "/**/" inside a pattern demands two slashes, so
+	// "a/**/*.log" misses "a/x.log". As in gitignore it stands for
+	// zero or more directories, so also compile the pattern with it
+	// made a single "/". The glob library's {a,b} is no help: with a
+	// "**" inside it in mid-pattern, it misses paths it should match.
+	var globs []glob.Glob
+
+	for _, variant := range doubleStarVariants(trimmed) {
+		g, err := glob.Compile(variant, '/')
+		if err != nil {
+			return nil, false, fmt.Errorf("%q %w: %w",
+				pattern, errBadExcludePattern, err)
+		}
+
+		globs = append(globs, g)
+	}
+
+	return globs, anchored, nil
+}
+
+// doubleStarVariants returns pattern once for every way of keeping each
+// "/**/" in it or making it a single "/".
+func doubleStarVariants(pattern string) []string {
+	const doubleStar = "/**/"
+
+	before, after, found := strings.Cut(pattern, doubleStar)
+	if !found {
+		return []string{pattern}
+	}
+
+	var variants []string
+
+	for _, rest := range doubleStarVariants(after) {
+		variants = append(variants, before+doubleStar+rest, before+"/"+rest)
+	}
+
+	return variants
+}

internal/exclude/exclude_test.go

@@ -0,0 +1,73 @@
+package exclude_test
+
+import (
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/gobwas/glob"
+	"sneak.berlin/go/vaultik/internal/exclude"
+)
+
+// TestCompile checks the error, the anchoring and the globs that Compile
+// returns for one exclude pattern.
+func TestCompile(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name         string
+		pattern      string
+		wantErr      string
+		wantAnchored bool
+		// wantMatches lists paths that one of the globs must match. When
+		// it is empty, Compile must return no globs.
+		wantMatches []string
+	}{
+		{name: "does not compile", pattern: "build[",
+			wantErr: `"build[" is not a valid exclude pattern`},
+		{name: "empty", pattern: ""},
+		{name: "leading slash anchors", pattern: "/build",
+			wantAnchored: true, wantMatches: []string{"build"}},
+		{name: "double star after the slash clears anchoring",
+			pattern: "/**/build", wantMatches: []string{"build"}},
+		{name: "double star in the middle", pattern: "a/**/*.log",
+			wantMatches: []string{"a/x.log", "a/b/x.log"}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			globs, anchored, err := exclude.Compile(tt.pattern)
+			if tt.wantErr == "" && err != nil {
+				t.Fatalf("Compile(%q) error = %v, want none", tt.pattern, err)
+			}
+
+			if tt.wantErr != "" &&
+				(err == nil || !strings.HasPrefix(err.Error(), tt.wantErr)) {
+				t.Fatalf("Compile(%q) error = %v, want one starting with %q",
+					tt.pattern, err, tt.wantErr)
+			}
+
+			if anchored != tt.wantAnchored {
+				t.Errorf("Compile(%q) anchored = %t, want %t",
+					tt.pattern, anchored, tt.wantAnchored)
+			}
+
+			if len(tt.wantMatches) == 0 && len(globs) != 0 {
+				t.Errorf("Compile(%q) returned %d globs, want none",
+					tt.pattern, len(globs))
+			}
+
+			for _, path := range tt.wantMatches {
+				matched := slices.ContainsFunc(globs, func(g glob.Glob) bool {
+					return g.Match(path)
+				})
+				if !matched {
+					t.Errorf("Compile(%q) globs do not match %q",
+						tt.pattern, path)
+				}
+			}
+		})
+	}
+}

internal/snapshot/scanner.go

@@ -19,8 +19,8 @@ import (
 	"github.com/spf13/afero"
 	"sneak.berlin/go/vaultik/internal/blob"
 	"sneak.berlin/go/vaultik/internal/chunker"
-	"sneak.berlin/go/vaultik/internal/config"
 	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/exclude"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/storage"
 	"sneak.berlin/go/vaultik/internal/types"
@@ -2329,9 +2329,9 @@ func compileExcludePatterns(patterns []string) ([]compiledPattern, error) {
 	var compiled []compiledPattern
 
 	for _, p := range patterns {
-		globs, anchored, err := config.CompileExcludePattern(p)
+		globs, anchored, err := exclude.Compile(p)
 		if err != nil {
-			return nil, fmt.Errorf("%q %w", p, err)
+			return nil, err
 		}
 
 		for _, g := range globs {