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