Print a failed command's error once on stderr (closes #433)

A command that failed after startup wrote the error twice on stderr:
as an ERROR log record and as the final ERROR: line. The failure
handlers now log the failure at info level, so the record appears only
under --verbose or --debug, next to the records that led up to it.

A failed blob upload is logged at info level for the same reason. A
restore that stops on a file it cannot restore no longer logs that
file's error, which the final line carries; under --skip-errors it is
still logged.

Model: opus-5-5

configs/config.example.yml

@@ -295,7 +295,6 @@ storage_url: "rclone://myremote/path/to/backups"
 
 # Path to local SQLite index database
 # This database tracks file state for incremental backups
-# It must be an absolute path; ~ is expanded
 # Default: the platform data directory, e.g.
 #   macOS: ~/Library/Application Support/vaultik/index.sqlite
 #   Linux: ~/.local/share/vaultik/index.sqlite

internal/cli/config.go

@@ -49,14 +49,8 @@ var (
 	errWouldNotLoad     = errors.New(
 		"vaultik would not load the config after this change, " +
 			"so the file was not changed")
-	errSetS3KeysFirst = errors.New(
-		"set s3.access_key_id and s3.secret_access_key first, " +
-			"then storage_url")
 )
 
-// storageURLKey is the config key that selects the storage backend.
-const storageURLKey = "storage_url"
-
 // macOS is the value of runtime.GOOS on macOS.
 const macOS = "darwin"
 
@@ -282,8 +276,7 @@ storage_url: ""
 # Hostname used in snapshot IDs. Default: system hostname.
 # hostname: myserver
 
-# Path to the local SQLite index database. It must be an absolute path;
+# Path to the local SQLite index database.
-# ~ is expanded.
 # Default: the platform data directory, e.g.
 #   macOS: ~/Library/Application Support/vaultik/index.sqlite
 #   Linux: ~/.local/share/vaultik/index.sqlite
@@ -519,7 +512,7 @@ func writeConfigSet(out *ui.Writer, path, key, value string) error {
 
 	err = checkConfigChange(oldData, data, settingData)
 	if err != nil {
-		return wouldNotLoadError(key, err)
+		return fmt.Errorf("%w: %w", errWouldNotLoad, err)
 	}
 
 	err = os.WriteFile(path, data, configFileMode)
@@ -543,21 +536,6 @@ func writeConfigSet(out *ui.Writer, path, key, value string) error {
 	return nil
 }
 
-// wouldNotLoadError returns the refusal of a change to key that vaultik
-// would not load for reason. When storage_url moves to s3:// before the S3
-// credentials are set, it says to set them first: they can be set while
-// the old storage_url is still in place.
-func wouldNotLoadError(key string, reason error) error {
-	if key == storageURLKey &&
-		(errors.Is(reason, config.ErrS3KeyIDRequired) ||
-			errors.Is(reason, config.ErrS3SecretRequired)) {
-		return fmt.Errorf("%w: %w; %w", errWouldNotLoad, reason,
-			errSetS3KeysFirst)
-	}
-
-	return fmt.Errorf("%w: %w", errWouldNotLoad, reason)
-}
-
 // checkConfigChange returns the reason vaultik would not load newData, the
 // config after a change, if the change brings that failure in. If oldData,
 // the config before it, loads, the new one must load too. A config that

internal/cli/config_test.go

@@ -2,7 +2,6 @@ package cli //nolint:testpackage // exercises unexported yamlPathGet/yamlPathSet
 
 import (
 	"bytes"
-	"errors"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -153,21 +152,6 @@ snapshots:
       - "~"
 `
 
-// testS3URL is an S3 storage_url, which needs credentials to load.
-const testS3URL = "s3://bucket/prefix?endpoint=https://s3.example.com"
-
-// configWithS3Keys loads: its storage_url is testS3URL and both S3 keys are
-// set.
-const configWithS3Keys = storageURLKey + ": " + testS3URL + `
-s3:
-  access_key_id: AKIAEXAMPLE
-  secret_access_key: example-secret
-snapshots:
-  home:
-    paths:
-      - "~"
-`
-
 // configWithUnsetReference loads only where VAULTIK_TEST_UNSET_SECRET is
 // set, as a backup job's environment might set it. No test sets it.
 const configWithUnsetReference = configWithoutRecipients + `s3:
@@ -268,20 +252,12 @@ func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
 		{configWithoutRecipients, "snapshots.home.paths", "/srv",
 			"into []string"},
 		{configWithoutRecipients, "chunk_size", "fast",
-			`chunk_size: "fast" is not a size`},
+			"invalid size format"},
 		// Values of the right type that fail validation.
 		{configWithoutRecipients, "compression_level", "99",
 			"compression_level must be between 1 and 19"},
 		{configWithoutRecipients, "chunk_size", "1KB",
 			"chunk_size must be at least 1MB"},
-		// An s3:// URL before its credentials names them.
-		{configWithoutRecipients, storageURLKey, testS3URL,
-			"s3.access_key_id is required for s3:// URLs; " +
-				"set s3.access_key_id and s3.secret_access_key first"},
-		{configWithoutRecipients + "s3:\n  access_key_id: AKIAEXAMPLE\n",
-			storageURLKey, testS3URL,
-			"s3.secret_access_key is required for s3:// URLs; " +
-				"set s3.access_key_id and s3.secret_access_key first"},
 		// With its reference filled in, the config loads.
 		{levelReference, "blob_size_limit", "1MB",
 			"blob_size_limit must be at least"},
@@ -367,76 +343,6 @@ func TestConfigSetRefusesOnlyItsOwnFailure(t *testing.T) {
 	}
 }
 
-// TestConfigSetS3KeysBeforeStorageURL checks that the order the refusal
-// gives works on a config that loads with a file:// storage_url.
-func TestConfigSetS3KeysBeforeStorageURL(t *testing.T) {
-	t.Parallel()
-
-	path := filepath.Join(t.TempDir(), "config.yml")
-
-	err := os.WriteFile(path, []byte(configWithoutRecipients), configFileMode)
-	if err != nil {
-		t.Fatalf("write config: %v", err)
-	}
-
-	for _, set := range [][2]string{
-		{"s3.access_key_id", "AKIAEXAMPLE"},
-		{"s3.secret_access_key", "example-secret"},
-		{storageURLKey, testS3URL},
-	} {
-		err = writeConfigSet(ui.NewWithColor(&bytes.Buffer{}, false),
-			path, set[0], set[1])
-		if err != nil {
-			t.Fatalf("config set %s: %v", set[0], err)
-		}
-	}
-
-	data, err := os.ReadFile(path) //nolint:gosec // G304: test-controlled path
-	if err != nil {
-		t.Fatalf("read config: %v", err)
-	}
-
-	if !strings.Contains(string(data), storageURLKey+": "+testS3URL) {
-		t.Errorf("config set storage_url wrote:\n%s", data)
-	}
-}
-
-// TestConfigSetRefusesClearingS3Key checks that clearing s3.access_key_id on
-// a config with an s3:// storage_url is refused without saying to set the
-// S3 keys first. That advice is only for a change to storage_url.
-func TestConfigSetRefusesClearingS3Key(t *testing.T) {
-	t.Parallel()
-
-	const key = "s3.access_key_id"
-
-	path := filepath.Join(t.TempDir(), "config.yml")
-
-	err := os.WriteFile(path, []byte(configWithS3Keys), configFileMode)
-	if err != nil {
-		t.Fatalf("write config: %v", err)
-	}
-
-	err = writeConfigSet(ui.NewWithColor(&bytes.Buffer{}, false), path, key, "")
-	if !errors.Is(err, errWouldNotLoad) {
-		t.Errorf("clearing %s gives error %v, want one saying %q",
-			key, err, errWouldNotLoad)
-	}
-
-	if errors.Is(err, errSetS3KeysFirst) {
-		t.Errorf("clearing %s gives error %v, want one not saying %q",
-			key, err, errSetS3KeysFirst)
-	}
-
-	data, err := os.ReadFile(path) //nolint:gosec // G304: test-controlled path
-	if err != nil {
-		t.Fatalf("read config: %v", err)
-	}
-
-	if string(data) != configWithS3Keys {
-		t.Errorf("clearing %s changed the file to:\n%s", key, data)
-	}
-}
-
 // TestConfigSetWritesEnvSection checks that config set writes a key under
 // the top-level env section, whose keys are not settings, on a config that
 // loads.

internal/cli/entry_snapshot_create_prune_test.go

@@ -61,8 +61,8 @@ func TestEntrySnapshotCreateRejectsKeepNewerThanBeforeBackup(t *testing.T) {
 
 // TestEntrySnapshotCreateReportsPostBackupPruneFailure makes the
 // post-backup prune fail after a good backup, by planting a manifest
-// that cannot be decoded, and checks that the failure is reported once,
+// that cannot be decoded, and checks that the failure is reported as the
-// as the prune's and not as a failed snapshot creation.
+// prune's and not as a failed snapshot creation.
 //
 //nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
 func TestEntrySnapshotCreateReportsPostBackupPruneFailure(t *testing.T) {
@@ -83,9 +83,6 @@ func TestEntrySnapshotCreateReportsPostBackupPruneFailure(t *testing.T) {
 	assert.Contains(t, stderr, "Backup completed, but post-backup prune failed")
 	assert.NotContains(t, stderr, "Snapshot creation failed")
 
-	_, repeats := failureOutput(t, stderr)
-	assert.Zero(t, repeats, "no log record may repeat the error:\n%s", stderr)
-
 	snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
 	require.NoError(t, err)
 	assert.Len(t, snapshots, 2,
@@ -96,8 +93,7 @@ func TestEntrySnapshotCreateReportsPostBackupPruneFailure(t *testing.T) {
 // empties the source directory, as a volume that is not mounted leaves
 // its mount point, and runs `snapshot create --prune`. The command exits
 // non-zero and reports the backup as completed, not as a failed snapshot
-// creation. The error is printed once. See
+// creation. See https://git.eeqj.de/sneak/vaultik/issues/388.
-// https://git.eeqj.de/sneak/vaultik/issues/388.
 //
 //nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
 func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) {
@@ -118,9 +114,6 @@ func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) {
 		"Backup completed, but 1 source path(s) were empty")
 	assert.NotContains(t, stderr, "Snapshot creation failed")
 
-	_, repeats := failureOutput(t, stderr)
-	assert.Zero(t, repeats, "no log record may repeat the error:\n%s", stderr)
-
 	snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
 	require.NoError(t, err)
 	assert.Len(t, snapshots, 2, "the older snapshot is kept")
@@ -131,8 +124,7 @@ func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) {
 // directory as a mount point, and runs `snapshot create --prune`, which
 // does not find it as one. The command exits non-zero, reports the
 // backup as completed, not as a failed snapshot creation, and keeps the
-// older snapshot. The error is printed once. See
+// older snapshot. See https://git.eeqj.de/sneak/vaultik/issues/395.
-// https://git.eeqj.de/sneak/vaultik/issues/395.
 //
 //nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
 func TestEntrySnapshotCreatePruneExitsNonZeroWhenMountPointIsMissing(
@@ -167,9 +159,6 @@ func TestEntrySnapshotCreatePruneExitsNonZeroWhenMountPointIsMissing(
 		"an earlier snapshot recorded were not found")
 	assert.NotContains(t, stderr, "Snapshot creation failed")
 
-	_, repeats := failureOutput(t, stderr)
-	assert.Zero(t, repeats, "no log record may repeat the error:\n%s", stderr)
-
 	snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
 	require.NoError(t, err)
 	assert.Len(t, snapshots, 2, "the older snapshot is kept")

internal/config/config.go

@@ -70,22 +70,14 @@ var (
 	errFileURLNotAbsolute = errors.New(
 		"a file:// storage_url needs an absolute path, " +
 			"as in file:///path/to/backup")
-	errIndexPathNotAbsolute = errors.New(
-		"index_path and VAULTIK_INDEX_PATH need an absolute path, " +
-			"as in ~/.local/share/vaultik/index.sqlite")
 	errNegativePruneGracePeriod = errors.New(
 		"prune_grace_period must not be negative")
 	errStorageNotConfigured = errors.New(
 		"storage not configured; set storage_url or provide s3.endpoint + " +
 			"s3.bucket + credentials")
 	errS3BucketRequired = errors.New("s3.bucket is required (or set storage_url)")
-)
+	errS3KeyIDRequired  = errors.New("s3.access_key_id is required")
-
+	errS3SecretRequired = errors.New("s3.secret_access_key is required")
-// Validate returns these when S3 storage is configured without its
-// credentials.
-var (
-	ErrS3KeyIDRequired  = errors.New("s3.access_key_id is required")
-	ErrS3SecretRequired = errors.New("s3.secret_access_key is required")
 )
 
 // expandTilde expands ~ at the start of a path to the user's home directory.
@@ -295,8 +287,7 @@ func Load(path string) (*Config, error) {
 }
 
 // Parse reads YAML config data as Load does, without validating it. It
-// refuses a key that no setting has and a size setting whose value is not a
+// refuses a key that no setting has, applies default values for optional
-// size, naming the setting. It applies default values for optional
 // fields, fills in ${...} references using smartconfig, and expands ~ in
 // paths.
 func Parse(data []byte) (*Config, error) {
@@ -321,9 +312,8 @@ func Parse(data []byte) (*Config, error) {
 		return nil, fmt.Errorf("failed to load config file: %w", err)
 	}
 
-	// A ${...} reference can bring in keys that are not in the file, and
+	// A ${...} reference can bring in keys that are not in the file.
-	// is the value of a size setting only once it is filled in.
+	err = checkFilledKeys(sc.Data())
-	err = checkFilledConfig(sc.Data())
 	if err != nil {
 		return nil, err
 	}
@@ -396,11 +386,7 @@ func Parse(data []byte) (*Config, error) {
 //     never the value. An empty list is accepted, because only snapshot
 //     create needs a recipient and it checks for one itself
 //   - At least one snapshot must be configured with at least one path
-//   - Every exclude pattern, global or in a snapshot, must compile; errors
-//     name the setting and quote the pattern
 //   - Storage must be configured (either storage_url or s3.* fields)
-//   - The index path must be absolute: a relative one would resolve against
-//     the working directory, so each directory would get its own index
 //   - Chunk size must be at least 1MB
 //   - Blob size limit must be at least the largest chunk the chunker can emit
 //     (chunk_size times chunker.ChunkSizeSpread), so a single-chunk blob never
@@ -418,26 +404,22 @@ func (c *Config) Validate() error {
 		}
 	}
 
-	err := c.validateSnapshots()
+	if len(c.Snapshots) == 0 {
-	if err != nil {
+		return errNoSnapshots
-		return err
 	}
 
-	err = c.validateExcludes()
+	for name, snap := range c.Snapshots {
-	if err != nil {
+		if len(snap.Paths) == 0 {
-		return err
+			return fmt.Errorf("%w: %q", errSnapshotNoPaths, name)
+		}
 	}
 
 	// Validate storage configuration
-	err = c.validateStorage()
+	err := c.validateStorage()
 	if err != nil {
 		return err
 	}
 
-	if !filepath.IsAbs(c.IndexPath) {
-		return fmt.Errorf("%w: %q", errIndexPathNotAbsolute, c.IndexPath)
-	}
-
 	if c.ChunkSize.Int64() < minChunkSize {
 		return errChunkSizeTooSmall
 	}
@@ -468,22 +450,6 @@ func (c *Config) Validate() error {
 	return nil
 }
 
-// validateSnapshots refuses a config with no snapshot or with a snapshot
-// that has no path.
-func (c *Config) validateSnapshots() error {
-	if len(c.Snapshots) == 0 {
-		return errNoSnapshots
-	}
-
-	for name, snap := range c.Snapshots {
-		if len(snap.Paths) == 0 {
-			return fmt.Errorf("%w: %q", errSnapshotNoPaths, name)
-		}
-	}
-
-	return nil
-}
-
 // validateAgeRecipient parses one age_recipients entry with the age library
 // and returns a value-free error on failure. A recipient string can be
 // sensitive (an operator may paste a secret key by mistake), so neither the
@@ -547,11 +513,11 @@ func (c *Config) validateStorage() error {
 	}
 
 	if c.S3.AccessKeyID == "" {
-		return ErrS3KeyIDRequired
+		return errS3KeyIDRequired
 	}
 
 	if c.S3.SecretAccessKey == "" {
-		return ErrS3SecretRequired
+		return errS3SecretRequired
 	}
 
 	return nil
@@ -578,11 +544,11 @@ func (c *Config) validateStorageURL() error {
 	case strings.HasPrefix(c.StorageURL, "s3://"):
 		// S3 storage needs credentials
 		if c.S3.AccessKeyID == "" {
-			return fmt.Errorf("%w for s3:// URLs", ErrS3KeyIDRequired)
+			return fmt.Errorf("%w for s3:// URLs", errS3KeyIDRequired)
 		}
 
 		if c.S3.SecretAccessKey == "" {
-			return fmt.Errorf("%w for s3:// URLs", ErrS3SecretRequired)
+			return fmt.Errorf("%w for s3:// URLs", errS3SecretRequired)
 		}
 
 		return nil

internal/config/config_test.go

@@ -165,7 +165,6 @@ func TestValidateBlobSizeLimit(t *testing.T) {
 			AgeRecipients:    []string{testSneakAgePublicKey},
 			Snapshots:        map[string]SnapshotConfig{"test": {Paths: []string{"/tmp/src"}}},
 			StorageURL:       "file:///tmp/vaultik-test-store",
-			IndexPath:        "/tmp/vaultik-test-index.sqlite",
 			ChunkSize:        chunkSize,
 			BlobSizeLimit:    blobLimit,
 			CompressionLevel: 3,
@@ -511,65 +510,6 @@ func TestLoadRejectsRelativeFileURL(t *testing.T) {
 	}
 }
 
-// TestLoadIndexPath checks that a relative index_path fails at load, naming
-// the value. Accepted, it would resolve against the working directory, so a
-// backup run from another directory would start from an empty index. A path
-// starting with ~ is expanded and loads.
-func TestLoadIndexPath(t *testing.T) {
-	t.Parallel()
-
-	home, err := os.UserHomeDir()
-	if err != nil {
-		t.Fatalf("home directory: %v", err)
-	}
-
-	load := func(t *testing.T, indexPath string) (*Config, error) {
-		t.Helper()
-
-		data := "snapshots:\n" +
-			"  test:\n" +
-			"    paths: [/tmp/vaultik-test-source]\n" +
-			"storage_url: file:///tmp/vaultik-test-storage\n" +
-			"index_path: " + indexPath + "\n"
-
-		path := filepath.Join(t.TempDir(), "config.yml")
-
-		err := os.WriteFile(path, []byte(data), 0o600)
-		if err != nil {
-			t.Fatalf("write config: %v", err)
-		}
-
-		return Load(path)
-	}
-
-	t.Run("relative path is rejected", func(t *testing.T) {
-		t.Parallel()
-
-		_, err := load(t, "relative/index.sqlite")
-		if !errors.Is(err, errIndexPathNotAbsolute) {
-			t.Fatalf("Load() error = %v, want errIndexPathNotAbsolute", err)
-		}
-
-		if !strings.Contains(err.Error(), `"relative/index.sqlite"`) {
-			t.Errorf("error %q does not name the value", err.Error())
-		}
-	})
-
-	t.Run("path starting with ~ is expanded", func(t *testing.T) {
-		t.Parallel()
-
-		cfg, err := load(t, "~/vaultik/index.sqlite")
-		if err != nil {
-			t.Fatalf("Load() unexpected error: %v", err)
-		}
-
-		want := filepath.Join(home, "vaultik", "index.sqlite")
-		if cfg.IndexPath != want {
-			t.Errorf("IndexPath = %q, want %q", cfg.IndexPath, want)
-		}
-	})
-}
-
 // TestValidateAgeRecipients checks that recipients are parsed at config load
 // (a bad entry fails immediately, not mid-backup) and that no invalid entry —
 // least of all a pasted secret key — is echoed in the error. An empty list
@@ -582,7 +522,6 @@ func TestValidateAgeRecipients(t *testing.T) {
 			AgeRecipients:    recipients,
 			Snapshots:        map[string]SnapshotConfig{"test": {Paths: []string{"/tmp/src"}}},
 			StorageURL:       "file:///tmp/vaultik-test-store",
-			IndexPath:        "/tmp/vaultik-test-index.sqlite",
 			ChunkSize:        Size(10 * 1024 * 1024),
 			BlobSizeLimit:    Size(10 * 1024 * 1024 * 1024),
 			CompressionLevel: 3,

internal/config/exclude.go

@@ -1,39 +0,0 @@
-package config
-
-import (
-	"fmt"
-
-	"sneak.berlin/go/vaultik/internal/exclude"
-)
-
-// 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 {
-	err := validateExcludeList("exclude", c.Exclude)
-	if err != nil {
-		return err
-	}
-
-	for _, name := range c.SnapshotNames() {
-		err = validateExcludeList("snapshots."+name+".exclude",
-			c.Snapshots[name].Exclude)
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
-// validateExcludeList refuses the first pattern in patterns that does not
-// compile. setting is the name of the list, as in "exclude".
-func validateExcludeList(setting string, patterns []string) error {
-	for i, pattern := range patterns {
-		_, _, err := exclude.Compile(pattern)
-		if err != nil {
-			return fmt.Errorf("%s[%d]: %w", setting, i, err)
-		}
-	}
-
-	return nil
-}

internal/config/exclude_test.go

@@ -1,65 +0,0 @@
-package config_test
-
-import (
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-
-	"sneak.berlin/go/vaultik/internal/config"
-)
-
-// TestLoadRefusesBadExcludePattern checks that an exclude pattern that does
-// not compile, global or in a snapshot, fails at load with the setting and
-// the pattern named, rather than the backup storing what it meant to leave
-// out.
-func TestLoadRefusesBadExcludePattern(t *testing.T) {
-	t.Parallel()
-
-	const storage = "storage_url: file:///tmp/vaultik-test-storage\n"
-
-	tests := []struct {
-		name    string
-		config  string
-		wantErr string
-	}{
-		{
-			name: "global",
-			config: "exclude: ['*.tmp', '.cache', 'build[']\n" +
-				"snapshots:\n" +
-				"  home:\n" +
-				"    paths: [/tmp/vaultik-test-source]\n" + storage,
-			wantErr: `exclude[2]: "build[" is not a valid exclude pattern`,
-		},
-		{
-			name: "snapshot",
-			config: "snapshots:\n" +
-				"  home:\n" +
-				"    paths: [/tmp/vaultik-test-source]\n" +
-				"    exclude: ['/a/**/x[']\n" + storage,
-			wantErr: `snapshots.home.exclude[0]: "/a/**/x[" is not a valid exclude pattern`,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			path := filepath.Join(t.TempDir(), "config.yml")
-
-			err := os.WriteFile(path, []byte(tt.config), 0o600)
-			if err != nil {
-				t.Fatalf("write config: %v", err)
-			}
-
-			_, err = config.Load(path)
-			if err == nil {
-				t.Fatalf("Load() returned nil, want an error containing %q", tt.wantErr)
-			}
-
-			if !strings.Contains(err.Error(), tt.wantErr) {
-				t.Fatalf("Load() error = %q, want it to contain %q", err, tt.wantErr)
-			}
-		})
-	}
-}

internal/config/keys.go

@@ -129,11 +129,10 @@ func checkKeys(node *yaml.Node, typ reflect.Type, path string,
 	return nil
 }
 
-// checkFilledConfig checks data, the config after smartconfig has filled in
+// checkFilledKeys is checkKeys for data, the config after smartconfig has
-// its ${...} references: its keys with checkKeys, then its size settings
+// filled in its ${...} references. A key that a reference brings in is not
-// with checkSizes. A key that a reference brings in is not in the file, and
+// in the file, and the copy that Encode makes has no line numbers.
-// the copy that Encode makes has no line numbers.
+func checkFilledKeys(data map[string]any) error {
-func checkFilledConfig(data map[string]any) error {
 	var filled yaml.Node
 
 	err := filled.Encode(data)
@@ -141,13 +140,8 @@ func checkFilledConfig(data map[string]any) error {
 		return fmt.Errorf("failed to marshal config data: %w", err)
 	}
 
-	err = checkKeys(&filled, reflect.TypeFor[Config](), "",
+	return checkKeys(&filled, reflect.TypeFor[Config](), "",
 		map[*yaml.Node]bool{}, map[nodeAndType]bool{})
-	if err != nil {
-		return err
-	}
-
-	return checkSizes(&filled, nil)
 }
 
 // checkMappingKeys is checkKeys for a map.

internal/config/size.go

@@ -4,20 +4,13 @@ import (
 	"errors"
 	"fmt"
 	"math"
-	"reflect"
-	"slices"
-	"strconv"
-	"strings"
 
 	"github.com/dustin/go-humanize"
-	"gopkg.in/yaml.v3"
 )
 
 var (
 	errSizeType     = errors.New("size must be a number or string")
 	errSizeTooLarge = errors.New("size exceeds maximum supported value")
-	errNotASize     = errors.New(
-		"is not a size; use a number of bytes or a unit such as 10MB")
 )
 
 // Size represents a byte size that can be specified in configuration files.
@@ -39,15 +32,6 @@ func (s *Size) UnmarshalYAML(unmarshal func(any) error) error {
 		return nil
 	}
 
-	// A number too large for an int64 decodes as a float64. Its text can be
-	// 1e+20, which is not a size string.
-	var floatVal float64
-
-	err = unmarshal(&floatVal)
-	if err == nil && floatVal > math.MaxInt64 {
-		return errSizeTooLarge
-	}
-
 	// Try to unmarshal as string
 	var strVal string
 
@@ -56,66 +40,17 @@ func (s *Size) UnmarshalYAML(unmarshal func(any) error) error {
 		return errSizeType
 	}
 
-	size, err := ParseSize(strVal)
+	// Parse the string using go-humanize
+	bytes, err := humanize.ParseBytes(strVal)
 	if err != nil {
-		return err
+		return fmt.Errorf("invalid size format: %w", err)
 	}
 
-	*s = size
+	if bytes > math.MaxInt64 {
-
+		return fmt.Errorf("%w: %s", errSizeTooLarge, strVal)
-	return nil
 	}
 
-// checkSizes returns an error naming the first size setting in node whose
+	*s = Size(bytes)
-// value is not a size. node is the config, or the part of it at the dotted
-// keys, after smartconfig has filled in its ${...} references.
-func checkSizes(node *yaml.Node, keys []string) error {
-	typ, err := SettingType(keys)
-	if err != nil {
-		// Unknown keys are refused before this check, so keys is in one of
-		// two places. The env section holds no settings. A value of the
-		// wrong shape, such as a map given for hostname or inside exclude,
-		// is reported later by the YAML decoder.
-		return nil //nolint:nilerr // not an error here
-	}
-
-	if typ == reflect.TypeFor[Size]() {
-		var size Size
-
-		err = node.Decode(&size)
-		if err != nil {
-			return fmt.Errorf("%s: %w", strings.Join(keys, "."), err)
-		}
-
-		return nil
-	}
-
-	switch node.Kind {
-	case yaml.DocumentNode:
-		for _, child := range node.Content {
-			err = checkSizes(child, keys)
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.MappingNode:
-		for i := 0; i+1 < len(node.Content); i += 2 {
-			err = checkSizes(node.Content[i+1],
-				slices.Concat(keys, []string{node.Content[i].Value}))
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.SequenceNode:
-		for i, child := range node.Content {
-			err = checkSizes(child, slices.Concat(keys, []string{strconv.Itoa(i)}))
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.ScalarNode, yaml.AliasNode:
-		// A scalar holds no settings, and the filled config has no aliases.
-	}
 
 	return nil
 }
@@ -135,17 +70,16 @@ func (s Size) String() string {
 	return humanize.Bytes(uint64(s))
 }
 
-// ParseSize parses a size string, such as "10MB", into a Size value. An
+// ParseSize parses a size string into a Size value
-// error quotes s.
 func ParseSize(s string) (Size, error) {
-	bytes, err := humanize.ParseBigBytes(s)
+	bytes, err := humanize.ParseBytes(s)
 	if err != nil {
-		return 0, fmt.Errorf("%q %w", s, errNotASize)
+		return 0, fmt.Errorf("invalid size format: %w", err)
 	}
 
-	if !bytes.IsInt64() {
+	if bytes > math.MaxInt64 {
 		return 0, fmt.Errorf("%w: %s", errSizeTooLarge, s)
 	}
 
-	return Size(bytes.Int64()), nil
+	return Size(bytes), nil
 }

internal/config/size_test.go

@@ -1,120 +0,0 @@
-package config //nolint:testpackage // checks errNotASize and errSizeTooLarge
-
-import (
-	"errors"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-)
-
-// TestParseRefusesValueThatIsNotASize checks that a size setting whose
-// value is not a size fails the parse with an error naming the setting and
-// quoting the value, also when a ${...} reference fills the value in.
-func TestParseRefusesValueThatIsNotASize(t *testing.T) {
-	t.Parallel()
-
-	sizePath := filepath.Join(t.TempDir(), "size")
-
-	err := os.WriteFile(sizePath, []byte("lots"), 0o600)
-	if err != nil {
-		t.Fatalf("write size: %v", err)
-	}
-
-	tests := []struct {
-		name string
-		yaml string
-		want string
-	}{
-		{"chunk_size", keysTestConfig + "chunk_size: banana\n",
-			`chunk_size: "banana" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-		{"blob_size_limit", keysTestConfig + "blob_size_limit: 10XB\n",
-			`blob_size_limit: "10XB" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-		{"s3.part_size", strings.Replace(keysTestConfig, "  region:",
-			"  part_size: big\n  region:", 1),
-			`s3.part_size: "big" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-		{"from a reference",
-			keysTestConfig + "chunk_size: ${FILE:" + sizePath + "}\n",
-			`chunk_size: "lots" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := Parse([]byte(tt.yaml))
-			if !errors.Is(err, errNotASize) {
-				t.Fatalf("Parse() error = %v, want errNotASize", err)
-			}
-
-			if err.Error() != tt.want {
-				t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
-			}
-		})
-	}
-}
-
-// TestParseRefusesSizeTooLarge checks that a size setting whose value is
-// too large for a size fails the parse with an error naming the setting.
-// YAML reads the second value as a float.
-func TestParseRefusesSizeTooLarge(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		value string
-		want  string
-	}{
-		{"100EB", "chunk_size: size exceeds maximum supported value: 100EB"},
-		{"99999999999999999999",
-			"chunk_size: size exceeds maximum supported value"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.value, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := Parse([]byte(keysTestConfig + "chunk_size: " + tt.value + "\n"))
-			if !errors.Is(err, errSizeTooLarge) {
-				t.Fatalf("Parse() error = %v, want errSizeTooLarge", err)
-			}
-
-			if err.Error() != tt.want {
-				t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
-			}
-		})
-	}
-}
-
-// TestParseAcceptsSizes checks that size settings given as a number of
-// bytes, as a unit, and through a ${...} reference still parse.
-func TestParseAcceptsSizes(t *testing.T) {
-	t.Parallel()
-
-	sizePath := filepath.Join(t.TempDir(), "size")
-
-	err := os.WriteFile(sizePath, []byte("20MB"), 0o600)
-	if err != nil {
-		t.Fatalf("write size: %v", err)
-	}
-
-	data := strings.Replace(keysTestConfig, "  region:",
-		"  part_size: 6000000\n  region:", 1) +
-		"chunk_size: ${FILE:" + sizePath + "}\n" +
-		"blob_size_limit: 1GB\n"
-
-	cfg, err := Parse([]byte(data))
-	if err != nil {
-		t.Fatalf("Parse() unexpected error: %v", err)
-	}
-
-	if cfg.ChunkSize != 20_000_000 || cfg.BlobSizeLimit != 1_000_000_000 ||
-		cfg.S3.PartSize != 6_000_000 {
-		t.Errorf("chunk_size, blob_size_limit, s3.part_size = %d, %d, %d, "+
-			"want 20000000, 1000000000, 6000000",
-			cfg.ChunkSize, cfg.BlobSizeLimit, cfg.S3.PartSize)
-	}
-}

internal/exclude/exclude.go

@@ -1,77 +0,0 @@
-// 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

@@ -1,73 +0,0 @@
-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

@@ -20,7 +20,6 @@ import (
 	"sneak.berlin/go/vaultik/internal/blob"
 	"sneak.berlin/go/vaultik/internal/chunker"
 	"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"
@@ -222,12 +221,8 @@ func NewScanner(cfg ScannerConfig) *Scanner {
 		progress = NewProgressReporter()
 	}
 
-	compiledExclude, err := compileExcludePatterns(cfg.Exclude)
+	// Compile exclude patterns
-	if err != nil {
+	compiledExclude := compileExcludePatterns(cfg.Exclude)
-		log.Error("Failed to compile exclude patterns", "error", err)
-
-		return nil
-	}
 
 	uiw := cfg.UI
 	if uiw == nil {
@@ -2323,18 +2318,51 @@ func wrapPermissionError(path string, err error) error {
 		"with --skip-errors to continue past unreadable files)", path, err)
 }
 
-// compileExcludePatterns compiles the exclude patterns into glob matchers.
+// compileExcludePatterns compiles the exclude patterns into glob matchers
-// Config.Validate has already refused a pattern that does not compile.
+func compileExcludePatterns(patterns []string) []compiledPattern {
-func compileExcludePatterns(patterns []string) ([]compiledPattern, error) {
 	var compiled []compiledPattern
 
 	for _, p := range patterns {
-		globs, anchored, err := exclude.Compile(p)
+		if p == "" {
+			continue
+		}
+
+		// Check if pattern is anchored (starts with /)
+		anchored := strings.HasPrefix(p, "/")
+
+		pattern := p
+		if anchored {
+			pattern = p[1:] // Remove leading /
+		}
+
+		// Remove trailing slash if present (directory indicator)
+		pattern = strings.TrimSuffix(pattern, "/")
+
+		// 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(pattern, "**/"); found {
+			pattern = 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.
+		for _, variant := range doubleStarVariants(pattern) {
+			// For patterns without path separators, we need to match them as components
+			// e.g., ".git" should match ".git" anywhere in the path
+			g, err := glob.Compile(variant, '/')
 			if err != nil {
-			return nil, err
+				log.Warn("Invalid exclude pattern, skipping", "pattern", p, "error", err)
+
+				break
 			}
 
-		for _, g := range globs {
 			compiled = append(compiled, compiledPattern{
 				pattern:  g,
 				anchored: anchored,
@@ -2343,7 +2371,26 @@ func compileExcludePatterns(patterns []string) ([]compiledPattern, error) {
 		}
 	}
 
-	return compiled, nil
+	return compiled
+}
+
+// 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
 }
 
 // shouldExclude checks if a path should be excluded based on exclude patterns