From 52e9235641da16b07c105a5e52ef80b3676405f2 Mon Sep 17 00:00:00 2001 From: Paulo Saraiva Date: Mon, 24 Nov 2025 14:46:33 +0100 Subject: [PATCH] --skip-if-unchanged skips backup based on targets Previously, when using the --skip-if-unchanged flag for a backup, restic would only check if the root tree ID remained the same. This would mean if changes were made in unrelated paths, for example in the parent directories, the backup would still be made. If this flag is enabled, restic now goes through each target and checks if the specified base targets have changed. --- changelog/unreleased/issue-5575 | 14 +++++++++ cmd/restic/cmd_backup_integration_test.go | 21 +++++++++++-- doc/040_backup.rst | 12 ++----- internal/archiver/archiver.go | 38 +++++++++++++++++++---- internal/archiver/archiver_test.go | 3 ++ internal/backend/local/local_windows.go | 2 +- internal/data/tree.go | 12 +++++++ 7 files changed, 84 insertions(+), 18 deletions(-) create mode 100644 changelog/unreleased/issue-5575 diff --git a/changelog/unreleased/issue-5575 b/changelog/unreleased/issue-5575 new file mode 100644 index 000000000..c7c8fa154 --- /dev/null +++ b/changelog/unreleased/issue-5575 @@ -0,0 +1,14 @@ +Enhancement: Make --skip-if-unchanged ignore changes in parent directories + +Restic’s `backup` command previously created a new snapshot even when the +contents of the selected backup targets were unchanged. This happened because +`--skip-if-unchanged` also considered metadata changes in ancestor directories +outside the specified backup path. As a result, unrelated filesystem activity +(such as modifying files in parent directories) still triggered new snapshots. + +Restic now compares only the tree nodes corresponding to the actual backup +targets when `--skip-if-unchanged` is used. Snapshot creation is skipped as +long as the backed-up directory tree itself remains unchanged. + +https://github.com/restic/restic/issues/5575 +https://github.com/restic/restic/pull/5623 diff --git a/cmd/restic/cmd_backup_integration_test.go b/cmd/restic/cmd_backup_integration_test.go index 840ef65b9..d74292125 100644 --- a/cmd/restic/cmd_backup_integration_test.go +++ b/cmd/restic/cmd_backup_integration_test.go @@ -440,15 +440,17 @@ func TestIncrementalBackup(t *testing.T) { rtest.OK(t, appendRandomData(testfile, incrementalFirstWrite)) - opts := BackupOptions{} + opts := BackupOptions{SkipIfUnchanged: runtime.GOOS != "windows"} testRunBackup(t, "", []string{datadir}, opts, env.gopts) + testListSnapshots(t, env.gopts, 1) testRunCheck(t, env.gopts) stat1 := dirStats(t, env.repo) rtest.OK(t, appendRandomData(testfile, incrementalSecondWrite)) testRunBackup(t, "", []string{datadir}, opts, env.gopts) + testListSnapshots(t, env.gopts, 2) testRunCheck(t, env.gopts) stat2 := dirStats(t, env.repo) if stat2.size-stat1.size > incrementalFirstWrite { @@ -459,6 +461,7 @@ func TestIncrementalBackup(t *testing.T) { rtest.OK(t, appendRandomData(testfile, incrementalThirdWrite)) testRunBackup(t, "", []string{datadir}, opts, env.gopts) + testListSnapshots(t, env.gopts, 3) testRunCheck(t, env.gopts) stat3 := dirStats(t, env.repo) if stat3.size-stat2.size > incrementalFirstWrite { @@ -727,6 +730,20 @@ func TestBackupSkipIfUnchanged(t *testing.T) { testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts) testListSnapshots(t, env.gopts, 1) } - testRunCheck(t, env.gopts) + + for i := 0; i < 3; i++ { + err := testRunBackupAssumeFailure(t, filepath.Dir(env.testdata), []string{"testdata/0", "testdata/nonexisting"}, opts, env.gopts) + rtest.Assert(t, err != nil && err.Error() == "at least one source file could not be read", "No data error expected") + testListSnapshots(t, env.gopts, 2) + rtest.OK(t, os.Chtimes(env.testdata, time.Now(), time.Now())) + } + + t.Helper() + // when skipping with changed parents, few blobs will be created + output, err := testRunCheckOutput(t, env.gopts, false) + if err != nil { + t.Error(output) + t.Fatalf("unexpected error: %+v", err) + } } diff --git a/doc/040_backup.rst b/doc/040_backup.rst index 3abd2aeb9..334c5f94f 100644 --- a/doc/040_backup.rst +++ b/doc/040_backup.rst @@ -239,23 +239,17 @@ By default, restic always creates a new snapshot even if nothing has changed compared to the parent snapshot. To omit the creation of a new snapshot in this case, specify the ``--skip-if-unchanged`` option. -Note that when using absolute paths to specify the backup source, then also -changes to the parent folders result in a changed snapshot. For example, a backup -of ``/home/user/work`` will create a new snapshot if the metadata of either -``/``, ``/home`` or ``/home/user`` change. To avoid this problem run restic from -the corresponding folder and use relative paths. - .. code-block:: console - $ cd /home/user/work && restic -r /srv/restic-repo backup . --skip-if-unchanged + $ restic -r /srv/restic-repo backup /home/user/work --skip-if-unchanged open repository enter password for repository: repository a14e5863 opened (version 2, compression level auto) load index files using parent snapshot 40dc1520 - start scan on [.] - start backup on [.] + start scan on [/home/user/work] + start backup on [/home/user/work] scan finished in 1.814s: 5307 files, 1.720 GiB Files: 0 new, 0 changed, 5307 unmodified diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index d619ad9b4..b1711b557 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -764,6 +764,35 @@ func (arch *Archiver) dirPathToNode(snPath, target string) (node *data.Node, err return node, err } +// targetsChanged returns true if the targets have changed compared to the +// parent snapshot. +func (arch *Archiver) targetsChanged(ctx context.Context, rootTreeID restic.ID, parent *data.Snapshot, targets []string) bool { + if parent == nil || parent.Tree == nil { + return true + } + if rootTreeID.Equal(*parent.Tree) { + return false + } + + for _, target := range targets { + node, err := data.FindTreeNode(ctx, arch.Repo, &rootTreeID, target) + parentNode, parentErr := data.FindTreeNode(ctx, arch.Repo, parent.Tree, target) + if !errors.Is(err, parentErr) { + return true + } + + if node == nil && parentNode == nil { + continue + } + + if node == nil || parentNode == nil || !node.Equals(*parentNode) { + return true + } + } + + return false +} + // resolveRelativeTargets replaces targets that only contain relative // directories ("." or "../../") with the contents of the directory. Each // element of target is processed with fs.Clean(). @@ -920,12 +949,9 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps return nil, restic.ID{}, nil, err } - if opts.ParentSnapshot != nil && opts.SkipIfUnchanged { - ps := opts.ParentSnapshot - if ps.Tree != nil && rootTreeID.Equal(*ps.Tree) { - arch.summary.BackupEnd = time.Now() - return nil, restic.ID{}, arch.summary, nil - } + if opts.SkipIfUnchanged && !arch.targetsChanged(ctx, rootTreeID, opts.ParentSnapshot, cleanTargets) { + arch.summary.BackupEnd = time.Now() + return nil, restic.ID{}, arch.summary, nil } sn, err := data.NewSnapshot(targets, opts.Tags, opts.Hostname, opts.Time) diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index adc7695cb..1c20c4b76 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -1753,6 +1753,9 @@ func TestArchiverParent(t *testing.T) { }, "targetfile2": TestFile{Content: string(rtest.Random(888, 1235))}, }, + opts: SnapshotOptions{ + SkipIfUnchanged: true, + }, modify: func(path string) { remove(t, filepath.Join(path, "targetDir", "targetfile")) save(t, filepath.Join(path, "targetfile2"), []byte("foobar")) diff --git a/internal/backend/local/local_windows.go b/internal/backend/local/local_windows.go index fa21d8240..b3677b0ef 100644 --- a/internal/backend/local/local_windows.go +++ b/internal/backend/local/local_windows.go @@ -24,7 +24,7 @@ func removeFile(f string) error { // as Windows won't let you delete a read-only file err := os.Chmod(f, 0666) if err != nil && !os.IsPermission(err) { - return errors.WithStack(err) + return errors.WithStack(err) } return os.Remove(f) diff --git a/internal/data/tree.go b/internal/data/tree.go index 9031f3bf5..7eff126d4 100644 --- a/internal/data/tree.go +++ b/internal/data/tree.go @@ -208,3 +208,15 @@ func FindTreeDirectory(ctx context.Context, repo restic.BlobLoader, id *restic.I } return id, nil } + +func FindTreeNode(ctx context.Context, repo restic.BlobLoader, id *restic.ID, nodepath string) (*Node, error) { + dir, err := FindTreeDirectory(ctx, repo, id, path.Dir(nodepath)) + if err != nil { + return nil, err + } + tree, err := LoadTree(ctx, repo, *dir) + if err != nil { + return nil, err + } + return tree.Find(path.Base(nodepath)), nil +}