diff --git a/changelog/unreleased/issue-21866 b/changelog/unreleased/issue-21866 new file mode 100644 index 000000000..815a472c1 --- /dev/null +++ b/changelog/unreleased/issue-21866 @@ -0,0 +1,8 @@ +Bugfix: Hide progress bar for stats command in JSON mode + +Since restic 0.19.0, the stats command shows a progress bar. However, +it was also active when given the `--json` option resulting in text +mixed with JSON. This has been fixed. + +https://github.com/restic/restic/issues/21866 +https://github.com/restic/restic/pull/21871 diff --git a/changelog/unreleased/issue-21869 b/changelog/unreleased/issue-21869 new file mode 100644 index 000000000..774c5e4d1 --- /dev/null +++ b/changelog/unreleased/issue-21869 @@ -0,0 +1,11 @@ +Bugfix: Restore old behavior of `snapshots --latest ` without `--group-by` + +Restic 0.19.0 changed the behavior of `snapshots --latest ` to no longer +group snapshots by default. + +`snapshots --latest ` has been reverted to the old behavior if `--group-by` +is not specified. When specifying `--group-by`, the output is still grouped as +requested like in restic 0.19.0. + +https://github.com/restic/restic/issues/21869 +https://github.com/restic/restic/pull/21875 diff --git a/changelog/unreleased/issue-21895 b/changelog/unreleased/issue-21895 new file mode 100644 index 000000000..a566df2cf --- /dev/null +++ b/changelog/unreleased/issue-21895 @@ -0,0 +1,9 @@ +Bugfix: Remove read-only files via the SFTP backend on Windows servers + +Since restic 0.19.0, repository files on the SFTP backend are marked +read-only after save. On Windows SFTP servers, removing them failed +with a permission error. The SFTP backend now clears the read-only flag +before removing the file. + +https://github.com/restic/restic/issues/21895 +https://github.com/restic/restic/pull/21897 diff --git a/changelog/unreleased/issue-21899 b/changelog/unreleased/issue-21899 new file mode 100644 index 000000000..1eceba1d9 --- /dev/null +++ b/changelog/unreleased/issue-21899 @@ -0,0 +1,9 @@ +Bugfix: Fix excludes of duplicate directory entries during `backup` + +Since restic 0.19.0, creating a backup of a directory containing +duplicate directory entries always resulted in "Warning: at least +one source file could not be read" even if the files in question +were excluded. This has been fixed. + +https://github.com/restic/restic/issues/21899 +https://github.com/restic/restic/pull/21900 diff --git a/changelog/unreleased/issue-5234 b/changelog/unreleased/issue-5234 new file mode 100644 index 000000000..1374efdfe --- /dev/null +++ b/changelog/unreleased/issue-5234 @@ -0,0 +1,12 @@ +Bugfix: Prevent mounting over the repository directory + +Using a local repository directory as the `mount` target — or a path +that contains it, or that it contains — caused the FUSE server to +read its own backend files through the new mount, deadlocking the +kernel and requiring a long reboot to recover. + +Restic now resolves both paths and refuses any such overlap with a +clear error before mounting. + +https://github.com/restic/restic/issues/5234 +https://github.com/restic/restic/pull/5348 diff --git a/changelog/unreleased/issue-5667 b/changelog/unreleased/issue-5667 new file mode 100644 index 000000000..8dcbc45d0 --- /dev/null +++ b/changelog/unreleased/issue-5667 @@ -0,0 +1,9 @@ +Bugfix: Skip inaccessible `backup` source paths + +The `backup` command only skipped source paths that did not exist. A path that +could not be accessed for another reason, such as a malformed path on Windows, +was kept and produced an empty snapshot. Restic now skips any such path and +aborts if none remain. + +https://github.com/restic/restic/issues/5667 +https://github.com/restic/restic/pull/21852 diff --git a/changelog/unreleased/issue-5722 b/changelog/unreleased/issue-5722 new file mode 100644 index 000000000..ffe9338f2 --- /dev/null +++ b/changelog/unreleased/issue-5722 @@ -0,0 +1,10 @@ +Bugfix: Update `mount` latest symlink after snapshot reload + +When `restic mount` was kept running while new snapshots were +created, the new snapshots appeared in the mountpoint, but the `latest` +symlink could still point to the previously latest snapshot. Restic now +invalidates the cached snapshot directory entries after a snapshot reload so +that `latest` points to the newest snapshot. + +https://github.com/restic/restic/issues/5722 +https://github.com/restic/restic/pull/21873 diff --git a/changelog/unreleased/pull-21876 b/changelog/unreleased/pull-21876 new file mode 100644 index 000000000..ec8e2d8f1 --- /dev/null +++ b/changelog/unreleased/pull-21876 @@ -0,0 +1,8 @@ +Bugfix: Show timezone location in `snapshots` output + +Since restic 0.19.0, the `snapshots` command printed the current timezone when +listing snapshots. However, the timezone can change during the year, for example, +due to daylight saving time. Instead just print the name of the current location. + +https://github.com/restic/restic/pull/21876 +https://forum.restic.net/t/possible-bug-in-timezone-naming/10867 diff --git a/changelog/unreleased/pull-21879 b/changelog/unreleased/pull-21879 new file mode 100644 index 000000000..a64a4900f --- /dev/null +++ b/changelog/unreleased/pull-21879 @@ -0,0 +1,7 @@ +Bugfix: prevent crash in mountpoint validation if mountpoint is inaccessible + +Since restic 0.19.0, the `mount` command validates a mountpoint before loading +the repository. If restic was unable to stat the mountpoint, this would result +in a crash. This has been fixed to correctly return an error instead. + +https://github.com/restic/restic/pull/21879 diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index e0f901738..94348880f 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -171,13 +171,17 @@ var ErrInvalidSourceData = errors.New("at least one source file could not be rea // ErrNoSourceData is used to report that no source data was found var ErrNoSourceData = errors.Fatal("all source directories/files do not exist") -// filterExisting returns a slice of all existing items, or an error if no -// items exist at all. +// filterExisting returns the items that exist and can be accessed. It returns +// ErrNoSourceData if none remain, or ErrInvalidSourceData if some were skipped. func filterExisting(items []string, warnf func(msg string, args ...interface{})) (result []string, err error) { for _, item := range items { _, err := fs.Lstat(item) - if errors.Is(err, os.ErrNotExist) { - warnf("%v does not exist, skipping\n", item) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + warnf("%v does not exist, skipping\n", item) + } else { + warnf("%v cannot be accessed, skipping\n", item) + } continue } diff --git a/cmd/restic/cmd_backup_test.go b/cmd/restic/cmd_backup_test.go index e6f00aa5d..07b62c335 100644 --- a/cmd/restic/cmd_backup_test.go +++ b/cmd/restic/cmd_backup_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/restic/restic/internal/errors" rtest "github.com/restic/restic/internal/test" ) @@ -76,6 +77,28 @@ func TestCollectTargets(t *testing.T) { rtest.Assert(t, err == ErrInvalidSourceData, "expected error when not all targets exist") } +func TestFilterExistingUnreadable(t *testing.T) { + dir := rtest.TempDir(t) + + existing := filepath.Join(dir, "existing") + rtest.OK(t, os.Mkdir(existing, 0755)) + + file := filepath.Join(dir, "file") + rtest.OK(t, os.WriteFile(file, []byte("x"), 0600)) + + // Regression test for #5667. A target whose Lstat fails with an error other + // than ErrNotExist must be skipped (ENOTDIR on unix, NUL byte everywhere). + for _, unreadable := range []string{filepath.Join(file, "child"), "invalid\x00path"} { + result, err := filterExisting([]string{unreadable}, t.Logf) + rtest.Assert(t, errors.Is(err, ErrNoSourceData), "input %q: expected ErrNoSourceData; got %v", unreadable, err) + rtest.Assert(t, len(result) == 0, "input %q: expected no targets; got %v", unreadable, result) + + result, err = filterExisting([]string{existing, unreadable}, t.Logf) + rtest.Assert(t, errors.Is(err, ErrInvalidSourceData), "input %q: expected ErrInvalidSourceData; got %v", unreadable, err) + rtest.Equals(t, []string{existing}, result) + } +} + func TestReadFilenamesRaw(t *testing.T) { // These should all be returned exactly as-is. expected := []string{ diff --git a/cmd/restic/cmd_forget.go b/cmd/restic/cmd_forget.go index 976db4d0d..1472e02b3 100644 --- a/cmd/restic/cmd_forget.go +++ b/cmd/restic/cmd_forget.go @@ -261,7 +261,7 @@ func runForget(ctx context.Context, opts ForgetOptions, pruneOptions PruneOption } var key data.SnapshotGroupKey - if json.Unmarshal([]byte(k), &key) != nil { + if err := json.Unmarshal([]byte(k), &key); err != nil { return err } diff --git a/cmd/restic/cmd_mount.go b/cmd/restic/cmd_mount.go index cf521d643..a47734dbb 100644 --- a/cmd/restic/cmd_mount.go +++ b/cmd/restic/cmd_mount.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "os" + "path/filepath" "strings" "time" @@ -13,6 +14,8 @@ import ( "github.com/spf13/pflag" "golang.org/x/sys/unix" + "github.com/restic/restic/internal/backend/local" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/data" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" @@ -131,22 +134,8 @@ func runMount(ctx context.Context, opts MountOptions, gopts global.Options, args } mountpoint := args[0] - - // Check the existence of the mount point at the earliest stage to - // prevent unnecessary computations while opening the repository. - stat, err := os.Stat(mountpoint) - if errors.Is(err, os.ErrNotExist) { - printer.P("Mountpoint %s doesn't exist", mountpoint) - return errors.Fatal("invalid mountpoint") - } else if !stat.IsDir() { - printer.P("Mountpoint %s is not a directory", mountpoint) - return errors.Fatal("invalid mountpoint") - } - - err = unix.Access(mountpoint, unix.W_OK|unix.X_OK) - if err != nil { - printer.P("Mountpoint %s is not writeable or not executable", mountpoint) - return errors.Fatal("inaccessible mountpoint") + if err := validateMountpoint(mountpoint, gopts); err != nil { + return err } debug.Log("start mount") @@ -230,3 +219,91 @@ func runMount(ctx context.Context, opts MountOptions, gopts global.Options, args return err } + +func validateMountpoint(mountpoint string, gopts global.Options) error { + // Check the existence of the mount point at the earliest stage to + // prevent unnecessary computations while opening the repository. + stat, err := os.Stat(mountpoint) + if errors.Is(err, os.ErrNotExist) { + return errors.Fatal(fmt.Sprintf("mountpoint %s does not exist", mountpoint)) + } else if err != nil { + return errors.Fatal(fmt.Sprintf("mountpoint %s is inaccessible: %v", mountpoint, err)) + } else if !stat.IsDir() { + return errors.Fatal(fmt.Sprintf("mountpoint %s is not a directory", mountpoint)) + } + + err = unix.Access(mountpoint, unix.W_OK|unix.X_OK) + if err != nil { + return errors.Fatal(fmt.Sprintf("mountpoint %s is not writeable or not executable", mountpoint)) + } + + // Refuse to mount onto (or under, or over) the local repository directory. + // Doing so makes the FUSE server read its own backend files through the + // mount it just created, deadlocking the kernel (GH #5234). + loc, err := location.Parse(gopts.Backends, gopts.Repo) + if err != nil { + return err + } + if loc.Scheme == "local" { + if err := checkMountpointOverlap(loc.Config.(*local.Config).Path, mountpoint); err != nil { + return err + } + } + return nil +} + +// checkMountpointOverlap returns an error.Fatal if the local repository at +// repoPath and the mountpoint overlap: equal paths, mountpoint nested inside +// the repo, or the repo nested inside the mountpoint. Any overlap deadlocks +// the FUSE server (GH #5234). +func checkMountpointOverlap(repoPath, mountpoint string) error { + rp, err := resolvePath(repoPath) + if err != nil { + return err + } + mp, err := resolvePath(mountpoint) + if err != nil { + return err + } + + const tail = "; refusing to mount to avoid deadlocking the FUSE server" + switch { + case rp == mp: + return errors.Fatal(fmt.Sprintf("mountpoint %s is the local repository directory%s", mp, tail)) + case isInside(rp, mp): + return errors.Fatal(fmt.Sprintf("mountpoint %s is inside the local repository directory %s%s", mp, rp, tail)) + case isInside(mp, rp): + return errors.Fatal(fmt.Sprintf("local repository directory %s is inside the mountpoint %s%s", rp, mp, tail)) + } + return nil +} + +// resolvePath returns p as an absolute, symlink-resolved path. If EvalSymlinks +// fails (e.g. the path does not fully exist), it falls back to the absolute +// form: overlap detection is best-effort and we'd rather refuse a clear +// overlap than abort on an unrelated stat error. +func resolvePath(p string) (string, error) { + abs, err := filepath.Abs(p) + if err != nil { + return "", err + } + resolved, err := filepath.EvalSymlinks(abs) + if err != nil { + return abs, nil + } + return resolved, nil +} + +// isInside reports whether child is strictly nested inside parent. Both paths +// must already be cleaned and absolute. Equal paths return false; the caller +// handles equality separately so it can produce a distinct error message. +func isInside(parent, child string) bool { + rel, err := filepath.Rel(parent, child) + if err != nil { + return false + } + if rel == "." || rel == ".." { + return false + } + return !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} diff --git a/cmd/restic/cmd_mount_integration_test.go b/cmd/restic/cmd_mount_integration_test.go index a5f4a7aef..6f5557778 100644 --- a/cmd/restic/cmd_mount_integration_test.go +++ b/cmd/restic/cmd_mount_integration_test.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "sync" "testing" "time" @@ -212,6 +213,62 @@ func TestMount(t *testing.T) { checkSnapshots(t, env.gopts, env.mountpoint, snapshotIDs, 4) } +func TestCheckMountpointOverlap(t *testing.T) { + tempdir := t.TempDir() + repo := filepath.Join(tempdir, "repo") + repoSub := filepath.Join(repo, "sub") + sibling := filepath.Join(tempdir, "mnt") + for _, d := range []string{repo, repoSub, sibling} { + rtest.OK(t, os.MkdirAll(d, 0700)) + } + + cases := []struct { + name string + repo string + mount string + wantSub string // substring of expected error; empty means expect nil + }{ + {"equal", repo, repo, "is the local repository directory"}, + {"mount inside repo", repo, repoSub, "is inside the local repository directory"}, + {"repo inside mount", repoSub, repo, "is inside the mountpoint"}, + {"disjoint", repo, sibling, ""}, + {"prefix-not-subpath", repo, repo + "-other", ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rtest.OK(t, os.MkdirAll(tc.mount, 0700)) + err := checkMountpointOverlap(tc.repo, tc.mount) + if tc.wantSub == "" { + rtest.OK(t, err) + return + } + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.wantSub) + } + if !strings.Contains(err.Error(), tc.wantSub) { + t.Fatalf("error %q does not contain %q", err.Error(), tc.wantSub) + } + }) + } +} + +func TestCheckMountpointOverlapSymlink(t *testing.T) { + tempdir := t.TempDir() + repo := filepath.Join(tempdir, "repo") + rtest.OK(t, os.MkdirAll(repo, 0700)) + link := filepath.Join(tempdir, "link-to-repo") + rtest.OK(t, os.Symlink(repo, link)) + + err := checkMountpointOverlap(repo, link) + if err == nil { + t.Fatal("expected overlap error when mountpoint is a symlink to repo, got nil") + } + if !strings.Contains(err.Error(), "is the local repository directory") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestMountSameTimestamps(t *testing.T) { if !rtest.RunFuseTest { t.Skip("Skipping fuse tests") diff --git a/cmd/restic/cmd_snapshots.go b/cmd/restic/cmd_snapshots.go index 5f938fbe8..015b218cf 100644 --- a/cmd/restic/cmd_snapshots.go +++ b/cmd/restic/cmd_snapshots.go @@ -38,6 +38,9 @@ Exit status is 12 if the password is incorrect. `, GroupID: cmdGroupDefault, DisableAutoGenTag: true, + PreRunE: func(_ *cobra.Command, _ []string) error { + return opts.Finalize() + }, RunE: func(cmd *cobra.Command, args []string) error { finalizeSnapshotFilter(&opts.SnapshotFilter) return runSnapshots(cmd.Context(), opts, *globalOptions, args, globalOptions.Term) @@ -52,7 +55,7 @@ Exit status is 12 if the password is incorrect. type SnapshotOptions struct { data.SnapshotFilter Compact bool - Last bool // This option should be removed in favour of Latest. + last bool // Deprecated in favour of Latest. Latest int GroupBy data.SnapshotGroupByOptions } @@ -60,7 +63,7 @@ type SnapshotOptions struct { func (opts *SnapshotOptions) AddFlags(f *pflag.FlagSet) { initMultiSnapshotFilter(f, &opts.SnapshotFilter, true) f.BoolVarP(&opts.Compact, "compact", "c", false, "use compact output format") - f.BoolVar(&opts.Last, "last", false, "only show the last snapshot for each host and path") + f.BoolVar(&opts.last, "last", false, "only show the last snapshot for each host and path") err := f.MarkDeprecated("last", "use --latest 1") if err != nil { // MarkDeprecated only returns an error when the flag is not found @@ -70,6 +73,13 @@ func (opts *SnapshotOptions) AddFlags(f *pflag.FlagSet) { f.VarP(&opts.GroupBy, "group-by", "g", "`group` snapshots by host, paths and/or tags, separated by comma") } +func (opts *SnapshotOptions) Finalize() error { + if opts.last && opts.Latest == 0 { + opts.Latest = 1 + } + return nil +} + func runSnapshots(ctx context.Context, opts SnapshotOptions, gopts global.Options, args []string, term ui.Terminal) error { printer := ui.NewProgressPrinter(gopts.JSON, gopts.Verbosity, term) ctx, repo, unlock, err := openWithReadLock(ctx, gopts, gopts.NoLock, printer) @@ -95,12 +105,12 @@ func runSnapshots(ctx context.Context, opts SnapshotOptions, gopts global.Option return ctx.Err() } - if opts.Last { - // This branch should be removed in the same time - // that --last. - list = filterLatestSnapshotsInGroup(list, 1) - } else if opts.Latest > 0 { - list = filterLatestSnapshotsInGroup(list, opts.Latest) + if opts.Latest > 0 { + if grouped { + list = filterLatestSnapshotsInGroup(list, opts.Latest) + } else { + list = filterLatestSnapshots(list, opts.Latest) + } } sort.Sort(sort.Reverse(list)) snapshotGroups[k] = list @@ -134,6 +144,43 @@ func runSnapshots(ctx context.Context, opts SnapshotOptions, gopts global.Option return nil } +// filterLastSnapshotsKey is used by FilterLastSnapshots. +type filterLastSnapshotsKey struct { + Hostname string + JoinedPaths string +} + +// newFilterLastSnapshotsKey initializes a filterLastSnapshotsKey from a Snapshot +func newFilterLastSnapshotsKey(sn *data.Snapshot) filterLastSnapshotsKey { + // Shallow slice copy + var paths = make([]string, len(sn.Paths)) + copy(paths, sn.Paths) + sort.Strings(paths) + return filterLastSnapshotsKey{sn.Hostname, strings.Join(paths, "|")} +} + +// filterLatestSnapshots filters a list of snapshots to only return +// the limit last entries for each hostname and path. If the snapshot +// contains multiple paths, they will be joined and treated as one +// item. +func filterLatestSnapshots(list data.Snapshots, limit int) data.Snapshots { + // Sort the snapshots so that the newer ones are listed first + sort.SliceStable(list, func(i, j int) bool { + return list[i].Time.After(list[j].Time) + }) + + var results data.Snapshots + seen := make(map[filterLastSnapshotsKey]int) + for _, sn := range list { + key := newFilterLastSnapshotsKey(sn) + if seen[key] < limit { + seen[key]++ + results = append(results, sn) + } + } + return results +} + // filterLatestSnapshotsInGroup filters a list of snapshots to only return // the `limit` last entries. It is assumed that the snapshot list only contains // one group of snapshots. @@ -245,11 +292,8 @@ func PrintSnapshots(stdout io.Writer, list data.Snapshots, reasons []data.KeepRe // Each snapshot can be registered in different timezones, // but we display them all in local timezone on this output. footer := fmt.Sprintf("%d snapshots", len(list)) - zoneName, _ := time.Now().Local().Zone() - if zoneName != "" { - footer = fmt.Sprintf("Timestamps shown in %s timezone\n%s", zoneName, footer) - } - tab.AddFooter(footer) + zoneName := time.Now().Local().Location().String() + tab.AddFooter(fmt.Sprintf("Timestamps shown in %s timezone\n%s", zoneName, footer)) if multiline { // print an additional blank line between snapshots diff --git a/cmd/restic/cmd_snapshots_integration_test.go b/cmd/restic/cmd_snapshots_integration_test.go index 2401544d7..761d50e96 100644 --- a/cmd/restic/cmd_snapshots_integration_test.go +++ b/cmd/restic/cmd_snapshots_integration_test.go @@ -35,10 +35,7 @@ func testRunSnapshots(t testing.TB, gopts global.Options) (newest *Snapshot, sna return } -func TestSnapshotsGroupByAndLatest(t *testing.T) { - env, cleanup := withTestEnvironment(t) - defer cleanup() - +func snapshotsGroupTestData(t *testing.T, env *testEnvironment, keepPath bool) string { testSetupBackupData(t, env) // two backups on the same host but with different paths opts := BackupOptions{Host: "testhost", TimeStamp: time.Now().Format(time.DateTime)} @@ -46,9 +43,22 @@ func TestSnapshotsGroupByAndLatest(t *testing.T) { // Use later timestamp for second backup opts.TimeStamp = time.Now().Add(time.Second).Format(time.DateTime) snapshotsIDs := loadSnapshotMap(t, env.gopts) - testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata/0"}, opts, env.gopts) + + targets := []string{"testdata/0"} + if keepPath { + targets = []string{"testdata"} + } + testRunBackup(t, filepath.Dir(env.testdata), targets, opts, env.gopts) _, secondSnapshotID := lastSnapshot(snapshotsIDs, loadSnapshotMap(t, env.gopts)) + return secondSnapshotID +} + +func TestSnapshotsGroupByAndLatest(t *testing.T) { + env, cleanup := withTestEnvironment(t) + defer cleanup() + + secondSnapshotID := snapshotsGroupTestData(t, env, false) buf, err := withCaptureStdout(t, env.gopts, func(ctx context.Context, gopts global.Options) error { gopts.JSON = true // only group by host but not path @@ -65,3 +75,21 @@ func TestSnapshotsGroupByAndLatest(t *testing.T) { rtest.Assert(t, len(snapshots[0].Snapshots) == 1, "expected only one latest snapshot, got %d", len(snapshots[0].Snapshots)) rtest.Equals(t, snapshots[0].Snapshots[0].ID.String(), secondSnapshotID, "unexpected snapshot ID") } + +func TestSnapshotsLatest(t *testing.T) { + env, cleanup := withTestEnvironment(t) + defer cleanup() + + secondSnapshotID := snapshotsGroupTestData(t, env, true) + + buf, err := withCaptureStdout(t, env.gopts, func(ctx context.Context, gopts global.Options) error { + gopts.JSON = true + opts := SnapshotOptions{Latest: 1} + return runSnapshots(ctx, opts, gopts, []string{}, gopts.Term) + }) + rtest.OK(t, err) + snapshots := []Snapshot{} + rtest.OK(t, json.Unmarshal(buf.Bytes(), &snapshots)) + rtest.Assert(t, len(snapshots) == 1, "expected only one snapshot, got %d", len(snapshots)) + rtest.Equals(t, snapshots[0].ID.String(), secondSnapshotID, "unexpected snapshot ID") +} diff --git a/cmd/restic/cmd_stats.go b/cmd/restic/cmd_stats.go index f2b793bbc..d09f97ecb 100644 --- a/cmd/restic/cmd_stats.go +++ b/cmd/restic/cmd_stats.go @@ -141,7 +141,7 @@ func runStats(ctx context.Context, opts StatsOptions, gopts global.Options, args snapshots = append(snapshots, sn) } - statsProgress := newStatsProgress(term, uint64(len(snapshots))) + statsProgress := newStatsProgress(term, !gopts.JSON, uint64(len(snapshots))) updater := progress.NewUpdater(ui.CalculateProgressInterval(!gopts.Quiet, gopts.JSON, term.CanUpdateStatus()), func(runtime time.Duration, final bool) { statsProgress.printProgress(runtime, final) @@ -375,6 +375,7 @@ type statsProgress struct { term ui.Terminal m sync.Mutex snapshotCount uint64 + show bool processedSnapshotCount uint64 processedFileCount uint64 @@ -382,14 +383,18 @@ type statsProgress struct { processedSize uint64 } -func newStatsProgress(term ui.Terminal, snapshotCount uint64) *statsProgress { +func newStatsProgress(term ui.Terminal, show bool, snapshotCount uint64) *statsProgress { return &statsProgress{ term: term, + show: show, snapshotCount: snapshotCount, } } func (s *statsProgress) printProgress(runtime time.Duration, final bool) { + if !s.show { + return + } s.m.Lock() progressBase := s.processedSnapshotCount diff --git a/cmd/restic/cmd_stats_test.go b/cmd/restic/cmd_stats_test.go index 231bd65ff..a2068de44 100644 --- a/cmd/restic/cmd_stats_test.go +++ b/cmd/restic/cmd_stats_test.go @@ -66,7 +66,7 @@ func TestSizeHistogramString(t *testing.T) { func TestStatsProgress(t *testing.T) { term := &ui.MockTerminal{} - progress := newStatsProgress(term, 2) + progress := newStatsProgress(term, true, 2) progress.printProgress(0*time.Second, false) rtest.Equals(t, []string{"[0:00] 0.00% 0 / 2 snapshots, 0 B"}, term.Output) @@ -88,3 +88,12 @@ func TestStatsProgress(t *testing.T) { progress.printProgress(20*time.Second, true) rtest.Equals(t, []string{"[0:20] 100.00% 2 / 2 snapshots, 4 files, 5 blobs, 6 B"}, term.Output) } + +func TestStatsProgressJSON(t *testing.T) { + term := &ui.MockTerminal{} + + progress := newStatsProgress(term, false, 2) + progress.printProgress(0*time.Second, false) + // JSON output is not available yet, so just make sure to not break normal json output + rtest.Equals(t, nil, term.Output) +} diff --git a/doc/050_restore.rst b/doc/050_restore.rst index ec88d55c0..2ab37e5ba 100644 --- a/doc/050_restore.rst +++ b/doc/050_restore.rst @@ -208,6 +208,12 @@ command needs to be in the ``PATH``. On macOS, you need `FUSE-T `__ or `FUSE for macOS `__. On FreeBSD, you may need to install FUSE and load the kernel module (``kldload fuse``). +.. note:: The mountpoint must not overlap the local repository directory. + Using the repository directory itself, a subdirectory of it, or a parent + of it as the mountpoint causes the FUSE server to read its own backend + files through the new mount and deadlock the kernel. ``restic mount`` + detects this and refuses such mountpoints. + Restic supports storage and preservation of hard links. However, since hard links exist in the scope of a filesystem by definition, restoring hard links from a FUSE mount should be done by a program that preserves diff --git a/go.mod b/go.mod index a4b9584c6..00e1171de 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/go-ole/go-ole v1.3.0 github.com/google/go-cmp v0.7.0 github.com/hashicorp/golang-lru/v2 v2.0.7 - github.com/klauspost/compress v1.18.4 + github.com/klauspost/compress v1.18.6 github.com/minio/minio-go/v7 v7.1.0 github.com/ncw/swift/v2 v2.0.5 github.com/peterbourgon/unixtransport v0.0.7 diff --git a/go.sum b/go.sum index 93a347a04..aa573603e 100644 --- a/go.sum +++ b/go.sum @@ -131,8 +131,8 @@ github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU= github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k= -github.com/klauspost/compress v1.18.4 h1:RPhnKRAQ4Fh8zU2FY/6ZFDwTVTxgJ/EMydqSTzE9a2c= -github.com/klauspost/compress v1.18.4/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4= +github.com/klauspost/compress v1.18.6 h1:2jupLlAwFm95+YDR+NwD2MEfFO9d4z4Prjl1XXDjuao= +github.com/klauspost/compress v1.18.6/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/klauspost/cpuid/v2 v2.0.1/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.2.11 h1:0OwqZRYI2rFrjS4kvkDnqJkKHdHaRnCm68/DY4OxRzU= github.com/klauspost/cpuid/v2 v2.2.11/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= diff --git a/helpers/prepare-release/main.go b/helpers/prepare-release/main.go index 1bb042969..b0a636c29 100644 --- a/helpers/prepare-release/main.go +++ b/helpers/prepare-release/main.go @@ -333,7 +333,7 @@ func updateVersionDev() { die("unable to write version to file: %v", err) } - newVersion := fmt.Sprintf(`var version = "%s-dev (compiled manually)"`, opts.Version) + newVersion := fmt.Sprintf(`const Version = "%s-dev (compiled manually)"`, opts.Version) replace(versionCodeFile, versionPattern, newVersion) msg("committing cmd/restic/global.go with dev version") diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 312119884..eddb84f78 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -320,6 +320,8 @@ func (arch *Archiver) saveDir(ctx context.Context, snPath string, dir string, me finder := data.NewTreeFinder(previous) defer finder.Close() + var lastExcluded string + for _, name := range names { // test if context has been cancelled if ctx.Err() != nil { @@ -327,6 +329,12 @@ func (arch *Archiver) saveDir(ctx context.Context, snPath string, dir string, me return futureNode{}, ctx.Err() } + if name == lastExcluded { + // Skip duplicate directory entry if it was already excluded. + // This avoids printing errors about duplicate directory entries even though the entry in question is ignored. + continue + } + pathname := arch.FS.Join(dir, name) oldNode, err := finder.Find(name) err = arch.error(pathname, err) @@ -348,6 +356,7 @@ func (arch *Archiver) saveDir(ctx context.Context, snPath string, dir string, me } if excluded { + lastExcluded = name continue } diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 104818910..7111688f9 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -894,6 +894,90 @@ func TestArchiverSaveDir(t *testing.T) { } } +type duplicateReaddirFS struct { + fs.FS + dir string + names []string +} + +func (d *duplicateReaddirFS) OpenFile(name string, flag int, metadataOnly bool) (fs.File, error) { + f, err := d.FS.OpenFile(name, flag, metadataOnly) + if err != nil { + return nil, err + } + + if name == d.dir { + return &duplicateReaddirFile{File: f, names: d.names}, nil + } + return f, nil +} + +type duplicateReaddirFile struct { + fs.File + names []string +} + +func (f *duplicateReaddirFile) Readdirnames(int) ([]string, error) { + return append([]string(nil), f.names...), nil +} + +func TestArchiverSaveDirDuplicateExcludedEntry(t *testing.T) { + const targetNodeName = "targetdir" + + src := TestDir{ + "excluded": TestFile{Content: "skip me"}, + "keep": TestFile{Content: "keep me"}, + } + tempdir, repo := prepareTempdirRepoSrc(t, src) + + testFS := fs.Track{FS: &duplicateReaddirFS{ + FS: &fs.Local{}, + dir: ".", + names: []string{"excluded", "excluded", "keep"}, + }} + arch := New(repo, testFS, Options{}) + arch.summary = &Summary{} + arch.Select = func(item string, fi *fs.ExtendedFileInfo, _ fs.FS) bool { + return filepath.Base(item) != "excluded" + } + arch.Error = func(item string, err error) error { + t.Errorf("unexpected archiver error for %v: %v", item, err) + return err + } + + back := rtest.Chdir(t, tempdir) + defer back() + + // duplicate node check in tree finder is only done if the previous tree is not nil + previousTree, err := data.NewTreeNodeIterator(strings.NewReader(`{"nodes":[]}`)) + rtest.OK(t, err) + + var treeID restic.ID + err = repo.WithBlobUploader(context.TODO(), func(ctx context.Context, uploader restic.BlobSaverWithAsync) error { + wg, ctx := errgroup.WithContext(ctx) + arch.runWorkers(ctx, wg, uploader) + meta, err := testFS.OpenFile(".", fs.O_NOFOLLOW, true) + rtest.OK(t, err) + ft, err := arch.saveDir(ctx, "/", ".", meta, previousTree, nil) + rtest.OK(t, err) + rtest.OK(t, meta.Close()) + + fnr := ft.take(ctx) + node := fnr.node + node.Name = targetNodeName + treeID = data.TestSaveNodes(t, ctx, uploader, []*data.Node{node}) + arch.stopWorkers() + return wg.Wait() + }) + rtest.OK(t, err) + + TestEnsureTree(context.TODO(), t, "/", repo, treeID, TestDir{ + "targetdir": TestDir{ + "keep": TestFile{Content: "keep me"}, + }, + }) +} + func TestArchiverSaveDirIncremental(t *testing.T) { tempdir := rtest.TempDir(t) diff --git a/internal/backend/b2/b2.go b/internal/backend/b2/b2.go index 6e751479d..646d28d8e 100644 --- a/internal/backend/b2/b2.go +++ b/internal/backend/b2/b2.go @@ -246,6 +246,7 @@ func (be *b2Backend) Save(ctx context.Context, h backend.Handle, rd backend.Rewi // sanity check if n != rd.Length() { + _ = w.Close() return errors.Errorf("wrote %d bytes instead of the expected %d bytes", n, rd.Length()) } return errors.Wrap(w.Close(), "Close") diff --git a/internal/backend/rest/rest.go b/internal/backend/rest/rest.go index d0158ab58..c3ed1801f 100644 --- a/internal/backend/rest/rest.go +++ b/internal/backend/rest/rest.go @@ -245,6 +245,7 @@ func (b *Backend) openReader(ctx context.Context, h backend.Handle, length int, } if feature.Flag.Enabled(feature.BackendErrorRedesign) && length > 0 && resp.ContentLength != int64(length) { + _ = drainAndClose(resp) return nil, &restError{h, http.StatusRequestedRangeNotSatisfiable, "partial out of bounds read"} } diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index 141b71bac..dcc2ea5a7 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -11,6 +11,7 @@ import ( "os" "os/exec" "path" + "sync/atomic" "syscall" "time" @@ -37,7 +38,8 @@ type SFTP struct { cmd *exec.Cmd result <-chan error - posixRename bool + posixRename bool + chmodBeforeRemove atomic.Bool layout.Layout Config @@ -363,6 +365,7 @@ func (r *SFTP) Save(_ context.Context, h backend.Handle, rd backend.RewindReader if err == nil { err = f.Chmod(r.Modes.File) if err != nil { + _ = f.Close() return errors.Wrapf(err, "Chmod %v", tmpFilename) } } @@ -404,12 +407,11 @@ func (r *SFTP) Save(_ context.Context, h backend.Handle, rd backend.RewindReader } else { err = r.c.Rename(tmpFilename, filename) } - err = setFileReadonly(r.c, filename, r.Modes.File) if err != nil { - return errors.Errorf("sftp setFileReadonly: %v", err) + return errors.Wrapf(err, "Rename %v", tmpFilename) } - - return errors.Wrapf(err, "Rename %v", tmpFilename) + err = setFileReadonly(r.c, filename, r.Modes.File) + return errors.Wrapf(err, "setFileReadonly %v", filename) } // checkNoSpace checks if err was likely caused by lack of available space @@ -507,7 +509,37 @@ func (r *SFTP) Remove(_ context.Context, h backend.Handle) error { return err } - return errors.Wrapf(r.c.Remove(r.Filename(h)), "Remove %v", r.Filename(h)) + path := r.Filename(h) + + if r.chmodBeforeRemove.Load() { + return r.removeWithChmod(path) + } + + // optimistically try to remove the file + err := r.c.Remove(path) + if err == nil { + return nil + } + if !errors.Is(err, os.ErrPermission) { + return errors.Wrapf(err, "Remove %v", path) + } + + // fallback to chmod + remove + // this is necessary on Windows where read-only files cannot be deleted without chmod. + if err := r.removeWithChmod(path); err != nil { + return err + } + r.chmodBeforeRemove.Store(true) + return nil +} + +func (r *SFTP) removeWithChmod(path string) error { + err := r.c.Chmod(path, r.Modes.File) + if err != nil { + return errors.Wrapf(err, "Chmod %v", path) + } + + return errors.Wrapf(r.c.Remove(path), "Remove %v", path) } // List runs fn for each file in the backend which has the type t. When an diff --git a/internal/backend/util/defaults.go b/internal/backend/util/defaults.go index e5b6fc456..1f6c4965f 100644 --- a/internal/backend/util/defaults.go +++ b/internal/backend/util/defaults.go @@ -35,10 +35,15 @@ func DefaultDelete(ctx context.Context, be backend.Backend) error { for _, t := range alltypes { err := be.List(ctx, t, func(fi backend.FileInfo) error { - return be.Remove(ctx, backend.Handle{Type: t, Name: fi.Name}) + err := be.Remove(ctx, backend.Handle{Type: t, Name: fi.Name}) + if err != nil && be.IsNotExist(err) { + // deletion of files created by TestSaveError may happen with a delay for the REST server, so we ignore the error + err = nil + } + return err }) if err != nil { - return nil + return err } } err := be.Remove(ctx, backend.Handle{Type: backend.ConfigFile}) diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index eb53b14c2..3d7cd3c00 100644 --- a/internal/checker/checker_test.go +++ b/internal/checker/checker_test.go @@ -75,8 +75,7 @@ func assertOnlyMixedPackHints(t *testing.T, hints []error) { } func TestCheckRepo(t *testing.T) { - repo, _, cleanup := repository.TestFromFixture(t, checkerTestData) - defer cleanup() + repo, _ := repository.TestFromFixture(t, checkerTestData) chkr := checker.New(repo, false) hints, errs := chkr.LoadIndex(context.TODO(), nil) @@ -93,8 +92,7 @@ func TestCheckRepo(t *testing.T) { } func TestMissingPack(t *testing.T) { - repo, be, cleanup := repository.TestFromFixture(t, checkerTestData) - defer cleanup() + repo, be := repository.TestFromFixture(t, checkerTestData) packID := restic.TestParseID("657f7fb64f6a854fff6fe9279998ee09034901eded4e6db9bcee0e59745bbce6") test.OK(t, be.Remove(context.TODO(), backend.Handle{Type: restic.PackFile, Name: packID.String()})) @@ -119,8 +117,7 @@ func TestMissingPack(t *testing.T) { } func TestUnreferencedPack(t *testing.T) { - repo, be, cleanup := repository.TestFromFixture(t, checkerTestData) - defer cleanup() + repo, be := repository.TestFromFixture(t, checkerTestData) // index 3f1a only references pack 60e0 packID := "60e0438dcb978ec6860cc1f8c43da648170ee9129af8f650f876bad19f8f788e" @@ -147,8 +144,7 @@ func TestUnreferencedPack(t *testing.T) { } func TestUnreferencedBlobs(t *testing.T) { - repo, be, cleanup := repository.TestFromFixture(t, checkerTestData) - defer cleanup() + repo, be := repository.TestFromFixture(t, checkerTestData) snapshotID := restic.TestParseID("51d249d28815200d59e4be7b3f21a157b864dc343353df9d8e498220c2499b02") test.OK(t, be.Remove(context.TODO(), backend.Handle{Type: restic.SnapshotFile, Name: snapshotID.String()})) @@ -182,8 +178,7 @@ func TestUnreferencedBlobs(t *testing.T) { } func TestModifiedIndex(t *testing.T) { - repo, be, cleanup := repository.TestFromFixture(t, checkerTestData) - defer cleanup() + repo, be := repository.TestFromFixture(t, checkerTestData) done := make(chan struct{}) defer close(done) @@ -259,8 +254,7 @@ func TestModifiedIndex(t *testing.T) { var checkerDuplicateIndexTestData = filepath.Join("testdata", "duplicate-packs-in-index-test-repo.tar.gz") func TestDuplicatePacksInIndex(t *testing.T) { - repo, _, cleanup := repository.TestFromFixture(t, checkerDuplicateIndexTestData) - defer cleanup() + repo, _ := repository.TestFromFixture(t, checkerDuplicateIndexTestData) chkr := checker.New(repo, false) hints, errs := chkr.LoadIndex(context.TODO(), nil) @@ -459,8 +453,7 @@ func (r *loadTreesOnceRepository) LoadBlob(ctx context.Context, t restic.BlobTyp } func TestCheckerNoDuplicateTreeDecodes(t *testing.T) { - repo, _, cleanup := repository.TestFromFixture(t, checkerTestData) - defer cleanup() + repo, _ := repository.TestFromFixture(t, checkerTestData) checkRepo := &loadTreesOnceRepository{ Repository: repo, loadedTrees: restic.NewIDSet(), @@ -608,13 +601,12 @@ func TestCheckerBlobTypeConfusion(t *testing.T) { test.Assert(t, delayRepo.Triggered, "delay repository did not trigger") } -func loadBenchRepository(t *testing.B) (*checker.Checker, restic.Repository, func()) { - repo, _, cleanup := repository.TestFromFixture(t, checkerTestData) +func loadBenchRepository(t *testing.B) (*checker.Checker, restic.Repository) { + repo, _ := repository.TestFromFixture(t, checkerTestData) chkr := checker.New(repo, false) hints, errs := chkr.LoadIndex(context.TODO(), nil) if len(errs) > 0 { - defer cleanup() t.Fatalf("expected no errors, got %v: %v", len(errs), errs) } @@ -623,12 +615,11 @@ func loadBenchRepository(t *testing.B) (*checker.Checker, restic.Repository, fun t.Fatalf("expected mixed pack hint, got %v", err) } } - return chkr, repo, cleanup + return chkr, repo } func BenchmarkChecker(t *testing.B) { - chkr, _, cleanup := loadBenchRepository(t) - defer cleanup() + chkr, _ := loadBenchRepository(t) t.ResetTimer() @@ -640,8 +631,7 @@ func BenchmarkChecker(t *testing.B) { } func benchmarkSnapshotScaling(t *testing.B, newSnapshots int) { - chkr, repo, cleanup := loadBenchRepository(t) - defer cleanup() + chkr, repo := loadBenchRepository(t) snID := restic.TestParseID("51d249d28815200d59e4be7b3f21a157b864dc343353df9d8e498220c2499b02") sn2, err := data.LoadSnapshot(context.TODO(), repo, snID) diff --git a/internal/fs/node_xattr.go b/internal/fs/node_xattr.go index d6289c74a..71ecf92c1 100644 --- a/internal/fs/node_xattr.go +++ b/internal/fs/node_xattr.go @@ -109,7 +109,7 @@ func nodeFillExtendedAttributes(node *data.Node, path string, ignoreListError bo for _, attr := range xattrs { attrVal, err := getxattr(path, attr) if err != nil { - warnf("can not obtain extended attribute %v for %v:\n", attr, path) + warnf("can not obtain extended attribute %v for %v: %v\n", attr, path, err) continue } attr := data.ExtendedAttribute{ diff --git a/internal/fuse/dir.go b/internal/fuse/dir.go index df558ac1f..94b18e2aa 100644 --- a/internal/fuse/dir.go +++ b/internal/fuse/dir.go @@ -225,7 +225,7 @@ func (d *dir) Lookup(ctx context.Context, name string) (fs.Node, error) { return nil, err } - return d.cache.lookupOrCreate(name, func(forget forgetFn) (fs.Node, error) { + return d.cache.lookupOrCreate(name, -1, func(forget forgetFn) (fs.Node, error) { node, ok := d.items[name] if !ok { debug.Log(" Lookup(%v) -> not found", name) diff --git a/internal/fuse/fuse_test.go b/internal/fuse/fuse_test.go index 05dc5f91e..ec4ee4eab 100644 --- a/internal/fuse/fuse_test.go +++ b/internal/fuse/fuse_test.go @@ -247,6 +247,39 @@ func TestStableNodeObjects(t *testing.T) { testStableLookup(t, dir, "file-2") } +func TestSnapshotsDirLatestSymlinkUpdatesAfterReload(t *testing.T) { + repo := repository.TestRepository(t) + timeTemplate := "2006-01-02T15:04:05" + + firstTime, err := time.Parse(time.RFC3339, "2017-01-24T10:42:56Z") + rtest.OK(t, err) + secondTime := firstTime.Add(time.Hour) + + data.TestCreateSnapshot(t, repo, firstTime, 0) + root := NewRoot(repo, Config{ + TimeTemplate: timeTemplate, + PathTemplates: []string{"snapshots/%T"}, + }) + + snapshotsDir := testStableLookup(t, root, "snapshots") + rtest.Equals(t, firstTime.Format(timeTemplate), readLatestTarget(t, snapshotsDir)) + + data.TestCreateSnapshot(t, repo, secondTime, 0) + root.SnapshotsDir.dirStruct.lastCheck = time.Now().Add(-2 * minSnapshotsReloadTime) + + rtest.Equals(t, secondTime.Format(timeTemplate), readLatestTarget(t, snapshotsDir)) +} + +func readLatestTarget(t testing.TB, node fs.Node) string { + t.Helper() + + latest, err := node.(fs.NodeStringLookuper).Lookup(context.TODO(), "latest") + rtest.OK(t, err) + target, err := latest.(fs.NodeReadlinker).Readlink(context.TODO(), nil) + rtest.OK(t, err) + return target +} + // Test reporting of fuse.Attr.Blocks in multiples of 512. func TestBlocks(t *testing.T) { root := &Root{} diff --git a/internal/fuse/snapshots_dir.go b/internal/fuse/snapshots_dir.go index 645249417..e9733c2cb 100644 --- a/internal/fuse/snapshots_dir.go +++ b/internal/fuse/snapshots_dir.go @@ -61,7 +61,7 @@ func (d *SnapshotsDir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) { debug.Log("ReadDirAll()") // update snapshots - meta, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) + meta, _, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) if err != nil { return nil, unwrapCtxCanceled(err) } else if meta == nil { @@ -104,14 +104,14 @@ func (d *SnapshotsDir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) { func (d *SnapshotsDir) Lookup(ctx context.Context, name string) (fs.Node, error) { debug.Log("Lookup(%s)", name) - meta, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) + meta, gen, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) if err != nil { return nil, unwrapCtxCanceled(err) } else if meta == nil { return nil, syscall.ENOENT } - return d.cache.lookupOrCreate(name, func(forget forgetFn) (fs.Node, error) { + return d.cache.lookupOrCreate(name, gen, func(forget forgetFn) (fs.Node, error) { entry := meta.names[name] if entry == nil { return nil, syscall.ENOENT diff --git a/internal/fuse/snapshots_dirstruct.go b/internal/fuse/snapshots_dirstruct.go index e9b7e7aa6..a8d126b9f 100644 --- a/internal/fuse/snapshots_dirstruct.go +++ b/internal/fuse/snapshots_dirstruct.go @@ -42,6 +42,10 @@ type SnapshotsDirStructure struct { hash [sha256.Size]byte // Hash at last check. lastCheck time.Time + + // generation is incremented whenever the directory structure is rebuilt. + // It allows treeCache instances to detect stale entries and reset themselves. + generation int64 } // NewSnapshotsDirStructure returns a new directory structure for snapshots. @@ -280,6 +284,7 @@ func (d *SnapshotsDirStructure) makeDirs(snapshots data.Snapshots) { } d.entries = entries + d.generation++ } const minSnapshotsReloadTime = 60 * time.Second @@ -337,13 +342,13 @@ func (d *SnapshotsDirStructure) updateSnapshots(ctx context.Context) error { return nil } -func (d *SnapshotsDirStructure) UpdatePrefix(ctx context.Context, prefix string) (*MetaDirData, error) { +func (d *SnapshotsDirStructure) UpdatePrefix(ctx context.Context, prefix string) (*MetaDirData, int64, error) { err := d.updateSnapshots(ctx) if err != nil { - return nil, err + return nil, 0, err } d.mutex.Lock() defer d.mutex.Unlock() - return d.entries[prefix], nil + return d.entries[prefix], d.generation, nil } diff --git a/internal/fuse/tree_cache.go b/internal/fuse/tree_cache.go index 4b60aeb11..e4fb2fe23 100644 --- a/internal/fuse/tree_cache.go +++ b/internal/fuse/tree_cache.go @@ -5,12 +5,15 @@ package fuse import ( "sync" + "github.com/restic/restic/internal/debug" + "github.com/anacrolix/fuse/fs" ) type treeCache struct { - nodes map[string]fs.Node - m sync.Mutex + nodes map[string]fs.Node + m sync.Mutex + generation int64 } type forgetFn func() @@ -21,19 +24,28 @@ func newTreeCache() *treeCache { } } -func (t *treeCache) lookupOrCreate(name string, create func(forget forgetFn) (fs.Node, error)) (fs.Node, error) { +func (t *treeCache) lookupOrCreate(name string, generation int64, create func(forget forgetFn) (fs.Node, error)) (fs.Node, error) { t.m.Lock() defer t.m.Unlock() + if generation >= 0 && generation != t.generation { + debug.Log("treeCache generation changed %d -> %d, resetting cache", t.generation, generation) + t.nodes = make(map[string]fs.Node) + t.generation = generation + } + if node, ok := t.nodes[name]; ok { return node, nil } + cacheGeneration := t.generation node, err := create(func() { t.m.Lock() defer t.m.Unlock() - delete(t.nodes, name) + if t.generation == cacheGeneration { + delete(t.nodes, name) + } }) if err != nil { return nil, err diff --git a/internal/fuse/tree_cache_test.go b/internal/fuse/tree_cache_test.go new file mode 100644 index 000000000..594b47336 --- /dev/null +++ b/internal/fuse/tree_cache_test.go @@ -0,0 +1,82 @@ +//go:build darwin || freebsd || linux + +package fuse + +import ( + "context" + "testing" + + "github.com/anacrolix/fuse" + "github.com/anacrolix/fuse/fs" + + "github.com/restic/restic/internal/test" +) + +type cacheTestNode struct { + id int +} + +func (n cacheTestNode) Attr(context.Context, *fuse.Attr) error { + return nil +} + +func TestTreeCacheGeneration(t *testing.T) { + cache := newTreeCache() + created := 0 + create := func(forgetFn) (fs.Node, error) { + created++ + return cacheTestNode{id: created}, nil + } + + node1, err := cache.lookupOrCreate("node", 1, create) + test.OK(t, err) + node2, err := cache.lookupOrCreate("node", 1, create) + test.OK(t, err) + test.Assert(t, node1 == node2, "lookup should reuse cached node") + test.Equals(t, 1, created) + + node3, err := cache.lookupOrCreate("node", 2, create) + test.OK(t, err) + test.Assert(t, node1 != node3, "lookup should recreate node after generation change") + test.Equals(t, 2, created) + + node4, err := cache.lookupOrCreate("node", -1, create) + test.OK(t, err) + test.Assert(t, node3 == node4, "negative generation should not reset cache") + test.Equals(t, 2, created) + + node5, err := cache.lookupOrCreate("node", 3, create) + test.OK(t, err) + test.Assert(t, node4 != node5, "lookup should still track later generation changes") + test.Equals(t, 3, created) +} + +func TestTreeCacheForgetOnlyRemovesSameGeneration(t *testing.T) { + cache := newTreeCache() + created := 0 + var forgets []forgetFn + create := func(forget forgetFn) (fs.Node, error) { + created++ + forgets = append(forgets, forget) + return cacheTestNode{id: created}, nil + } + + node1, err := cache.lookupOrCreate("node", 1, create) + test.OK(t, err) + node2, err := cache.lookupOrCreate("node", 2, create) + test.OK(t, err) + test.Assert(t, node1 != node2, "lookup should recreate node after generation change") + test.Equals(t, 2, created) + + forgets[0]() + node3, err := cache.lookupOrCreate("node", 2, create) + test.OK(t, err) + test.Assert(t, node2 == node3, "forget from an old generation must not remove the current node") + test.Equals(t, 2, created) + + forgets[1]() + node4, err := cache.lookupOrCreate("node", 2, create) + test.OK(t, err) + test.Assert(t, node3 != node4, "forget from the current generation should remove the cached node") + test.Equals(t, 3, created) +} diff --git a/internal/repository/checker.go b/internal/repository/checker.go index 691ffe961..6a77074c7 100644 --- a/internal/repository/checker.go +++ b/internal/repository/checker.go @@ -253,7 +253,7 @@ func (c *Checker) ReadPacks(ctx context.Context, filter func(packs map[restic.ID bufRd := bufio.NewReaderSize(nil, maxStreamBufferSize) dec, err := zstd.NewReader(nil) if err != nil { - panic(dec) + panic(err) } defer dec.Close() for { diff --git a/internal/repository/index/index_parallel_test.go b/internal/repository/index/index_parallel_test.go index 96f1c2a6a..030fb9ea0 100644 --- a/internal/repository/index/index_parallel_test.go +++ b/internal/repository/index/index_parallel_test.go @@ -15,8 +15,7 @@ import ( var repoFixture = filepath.Join("..", "testdata", "test-repo.tar.gz") func TestRepositoryForAllIndexes(t *testing.T) { - repo, _, cleanup := repository.TestFromFixture(t, repoFixture) - defer cleanup() + repo, _ := repository.TestFromFixture(t, repoFixture) expectedIndexIDs := restic.NewIDSet() rtest.OK(t, repo.List(context.TODO(), restic.IndexFile, func(id restic.ID, size int64) error { diff --git a/internal/repository/index/master_index.go b/internal/repository/index/master_index.go index 84db0d962..5aeda7e42 100644 --- a/internal/repository/index/master_index.go +++ b/internal/repository/index/master_index.go @@ -280,6 +280,7 @@ func (mi *MasterIndex) MergeFinalIndexes() error { } func (mi *MasterIndex) Load(ctx context.Context, r restic.ListerLoaderUnpacked, p *progress.Counter, cb func(id restic.ID, idx *Index, err error) error) error { + defer p.Done() indexList, err := restic.MemorizeList(ctx, r, restic.IndexFile) if err != nil { return err @@ -302,7 +303,6 @@ func (mi *MasterIndex) Load(ctx context.Context, r restic.ListerLoaderUnpacked, return err } p.SetMax(numIndexFiles) - defer p.Done() } err = ForAllIndexes(ctx, indexList, r, func(id restic.ID, idx *Index, err error) error { diff --git a/internal/repository/prune.go b/internal/repository/prune.go index 393c2e45b..9828d7027 100644 --- a/internal/repository/prune.go +++ b/internal/repository/prune.go @@ -586,6 +586,7 @@ func (plan *PrunePlan) Execute(ctx context.Context, printer progress.Printer) er // unreferenced packs can be safely deleted first if len(plan.removePacksFirst) != 0 { printer.P("deleting unreferenced packs\n") + // ignoring errors is fine here as keeping too many packs cannot damage the repository _ = deleteFiles(ctx, true, &internalRepository{repo}, plan.removePacksFirst, restic.PackFile, printer) // forget unused data plan.removePacksFirst = nil @@ -643,6 +644,7 @@ func (plan *PrunePlan) Execute(ctx context.Context, printer progress.Printer) er if len(plan.removePacks) != 0 { printer.P("removing %d old packs", len(plan.removePacks)) + // ignoring errors is fine here as keeping too many packs cannot damage the repository _ = deleteFiles(ctx, true, &internalRepository{repo}, plan.removePacks, restic.PackFile, printer) } if ctx.Err() != nil { diff --git a/internal/repository/repair_pack.go b/internal/repository/repair_pack.go index 39d057976..7e7261d53 100644 --- a/internal/repository/repair_pack.go +++ b/internal/repository/repair_pack.go @@ -78,6 +78,7 @@ func resolveBlobsForPacks(ctx context.Context, repo *Repository, ids restic.IDSe if ids.Has(id) { blobs, _, err := repo.ListPack(ctx, id, size) if err != nil { + // ignore errors for broken pack files to be able to salvage as much as possible return nil } packToBlobs[id] = blobs diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 86eab8183..3e0ae33c3 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -380,11 +380,13 @@ func (r *Repository) saveAndEncrypt(ctx context.Context, t restic.BlobType, data uncompressedLength := 0 if r.cfg.Version > 1 { - // we have a repo v2, so compression is available. if the user opts to // not compress, we won't compress any data, but everything else is // compressed. - if r.opts.Compression != CompressionOff || t != restic.DataBlob { + // uncompressedLength != 0 is used to indicate compressed data. Thus, a zero-sized blob + // cannot be compressed. This special case is only relevant for tests, normal operation does not + // generate zero-sized blobs. + if len(data) > 0 && (r.opts.Compression != CompressionOff || t != restic.DataBlob) { uncompressedLength = len(data) data = r.getZstdEncoder().EncodeAll(data, nil) } diff --git a/internal/repository/repository_test.go b/internal/repository/repository_test.go index 7bb89d385..5e0e04376 100644 --- a/internal/repository/repository_test.go +++ b/internal/repository/repository_test.go @@ -319,8 +319,7 @@ func benchmarkLoadUnpacked(b *testing.B, version uint) { var repoFixture = filepath.Join("testdata", "test-repo.tar.gz") func TestRepositoryLoadIndex(t *testing.T) { - repo, _, cleanup := repository.TestFromFixture(t, repoFixture) - defer cleanup() + repo, _ := repository.TestFromFixture(t, repoFixture) rtest.OK(t, repo.LoadIndex(context.TODO(), nil)) } @@ -373,8 +372,7 @@ func (be *damageOnceBackend) Load(ctx context.Context, h backend.Handle, length } func TestRepositoryLoadUnpackedRetryBroken(t *testing.T) { - repodir, cleanup := rtest.Env(t, repoFixture) - defer cleanup() + repodir := rtest.Env(t, repoFixture) be, err := local.Open(context.TODO(), local.Config{Path: repodir, Connections: 2}, t.Logf) rtest.OK(t, err) diff --git a/internal/repository/testing.go b/internal/repository/testing.go index 1cdca4110..3f8473f4a 100644 --- a/internal/repository/testing.go +++ b/internal/repository/testing.go @@ -105,11 +105,11 @@ func TestRepositoryWithVersion(t testing.TB, version uint) (*Repository, restic. return repo, &internalRepository{repo}, be } -func TestFromFixture(t testing.TB, repoFixture string) (*Repository, backend.Backend, func()) { - repodir, cleanup := test.Env(t, repoFixture) +func TestFromFixture(t testing.TB, repoFixture string) (*Repository, backend.Backend) { + repodir := test.Env(t, repoFixture) repo, be := TestOpenLocal(t, repodir) - return repo, be, cleanup + return repo, be } // TestOpenLocal opens a local repository. diff --git a/internal/selfupdate/download.go b/internal/selfupdate/download.go index 271383d86..00721af80 100644 --- a/internal/selfupdate/download.go +++ b/internal/selfupdate/download.go @@ -80,9 +80,12 @@ func extractToFile(buf []byte, filename, target string, printf func(string, ...i return err } if err = newFile.Sync(); err != nil { + _ = newFile.Close() + _ = os.Remove(newFile.Name()) return err } if err = newFile.Close(); err != nil { + _ = os.Remove(newFile.Name()) return err } diff --git a/internal/selfupdate/github.go b/internal/selfupdate/github.go index 2ac1ef07f..8cb825b7b 100644 --- a/internal/selfupdate/github.go +++ b/internal/selfupdate/github.go @@ -87,6 +87,7 @@ func GitHubLatestRelease(ctx context.Context, owner, repo string) (Release, erro var msg githubError jerr := json.NewDecoder(res.Body).Decode(&msg) if jerr == nil { + _ = res.Body.Close() return Release{}, fmt.Errorf("unexpected status %v (%v) returned, message:\n %v", res.StatusCode, res.Status, msg.Message) } } @@ -137,6 +138,7 @@ func getGithubData(ctx context.Context, url string) ([]byte, error) { } if res.StatusCode != http.StatusOK { + _ = res.Body.Close() return nil, fmt.Errorf("unexpected status %v (%v) returned", res.StatusCode, res.Status) } diff --git a/internal/test/helpers.go b/internal/test/helpers.go index e3fded66e..035afff0e 100644 --- a/internal/test/helpers.go +++ b/internal/test/helpers.go @@ -137,10 +137,18 @@ func SetupTarTestFixture(t testing.TB, outputDir, tarFile string) { // Env creates a test environment and extracts the repository fixture. // Returned is the repo path and a cleanup function. -func Env(t testing.TB, repoFixture string) (repodir string, cleanup func()) { +func Env(t testing.TB, repoFixture string) string { t.Helper() - tempdir, err := os.MkdirTemp(TestTempDir, "restic-test-env-") - OK(t, err) + + var tempdir string + if TestCleanupTempDirs { + tempdir = t.TempDir() + } else { + var err error + tempdir, err = os.MkdirTemp(TestTempDir, "restic-test-env-") + OK(t, err) + t.Logf("leaving temporary directory %v used for test", tempdir) + } fd, err := os.Open(repoFixture) if err != nil { @@ -150,14 +158,7 @@ func Env(t testing.TB, repoFixture string) (repodir string, cleanup func()) { SetupTarTestFixture(t, tempdir, repoFixture) - return filepath.Join(tempdir, "repo"), func() { - if !TestCleanupTempDirs { - t.Logf("leaving temporary directory %v used for test", tempdir) - return - } - - RemoveAll(t, tempdir) - } + return filepath.Join(tempdir, "repo") } func isFile(fi os.FileInfo) bool {