From c166ad7daf85e5a64b4d2534cf21ce67f71c896b Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 30 May 2024 22:58:44 +0200 Subject: [PATCH 01/14] restore: factor out file creation helper --- internal/restorer/fileswriter.go | 79 ++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 34 deletions(-) diff --git a/internal/restorer/fileswriter.go b/internal/restorer/fileswriter.go index cbe89c30c..5e4931c63 100644 --- a/internal/restorer/fileswriter.go +++ b/internal/restorer/fileswriter.go @@ -39,6 +39,48 @@ func newFilesWriter(count int) *filesWriter { } } +func createFile(path string, createSize int64, sparse bool) (*os.File, error) { + var f *os.File + var err error + if f, err = os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600); err != nil { + if !fs.IsAccessDenied(err) { + return nil, err + } + + // If file is readonly, clear the readonly flag by resetting the + // permissions of the file and try again + // as the metadata will be set again in the second pass and the + // readonly flag will be applied again if needed. + if err = fs.ResetPermissions(path); err != nil { + return nil, err + } + if f, err = os.OpenFile(path, os.O_TRUNC|os.O_WRONLY, 0600); err != nil { + return nil, err + } + } + + if createSize > 0 { + if sparse { + err = truncateSparse(f, createSize) + if err != nil { + _ = f.Close() + return nil, err + } + } else { + err := fs.PreallocateFile(f, createSize) + if err != nil { + // Just log the preallocate error but don't let it cause the restore process to fail. + // Preallocate might return an error if the filesystem (implementation) does not + // support preallocation or our parameters combination to the preallocate call + // This should yield a syscall.ENOTSUP error, but some other errors might also + // show up. + debug.Log("Failed to preallocate %v with size %v: %v", path, createSize, err) + } + } + } + return f, err +} + func (w *filesWriter) writeToFile(path string, blob []byte, offset int64, createSize int64, sparse bool) error { bucket := &w.buckets[uint(xxhash.Sum64String(path))%uint(len(w.buckets))] @@ -53,21 +95,9 @@ func (w *filesWriter) writeToFile(path string, blob []byte, offset int64, create var f *os.File var err error if createSize >= 0 { - if f, err = os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600); err != nil { - if fs.IsAccessDenied(err) { - // If file is readonly, clear the readonly flag by resetting the - // permissions of the file and try again - // as the metadata will be set again in the second pass and the - // readonly flag will be applied again if needed. - if err = fs.ResetPermissions(path); err != nil { - return nil, err - } - if f, err = os.OpenFile(path, os.O_TRUNC|os.O_WRONLY, 0600); err != nil { - return nil, err - } - } else { - return nil, err - } + f, err = createFile(path, createSize, sparse) + if err != nil { + return nil, err } } else if f, err = os.OpenFile(path, os.O_WRONLY, 0600); err != nil { return nil, err @@ -76,25 +106,6 @@ func (w *filesWriter) writeToFile(path string, blob []byte, offset int64, create wr := &partialFile{File: f, users: 1, sparse: sparse} bucket.files[path] = wr - if createSize >= 0 { - if sparse { - err = truncateSparse(f, createSize) - if err != nil { - return nil, err - } - } else { - err := fs.PreallocateFile(wr.File, createSize) - if err != nil { - // Just log the preallocate error but don't let it cause the restore process to fail. - // Preallocate might return an error if the filesystem (implementation) does not - // support preallocation or our parameters combination to the preallocate call - // This should yield a syscall.ENOTSUP error, but some other errors might also - // show up. - debug.Log("Failed to preallocate %v with size %v: %v", path, createSize, err) - } - } - } - return wr, nil } From 30320a249a9379ad1e17d4a4dc5b3ef9aad0343f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 30 May 2024 23:06:15 +0200 Subject: [PATCH 02/14] restore: let filerestorer also handle empty files This get's rid of the corresponding special cases. --- internal/restorer/filerestorer.go | 22 ++++++++++++++ internal/restorer/filerestorer_test.go | 4 +++ internal/restorer/restorer.go | 40 -------------------------- 3 files changed, 26 insertions(+), 40 deletions(-) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index 3551857dd..f3a68c58a 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -120,6 +120,13 @@ func (r *fileRestorer) restoreFiles(ctx context.Context) error { // create packInfo from fileInfo for _, file := range r.files { fileBlobs := file.blobs.(restic.IDs) + if len(fileBlobs) == 0 { + err := r.restoreEmptyFileAt(file.location) + if errFile := r.sanitizeError(file, err); errFile != nil { + return errFile + } + } + largeFile := len(fileBlobs) > largeFileBlobCount var packsMap map[restic.ID][]fileBlobInfo if largeFile { @@ -195,6 +202,21 @@ func (r *fileRestorer) restoreFiles(ctx context.Context) error { return wg.Wait() } +func (r *fileRestorer) restoreEmptyFileAt(location string) error { + f, err := createFile(r.targetPath(location), 0, false) + if err != nil { + return err + } + if err = f.Close(); err != nil { + return err + } + + if r.progress != nil { + r.progress.AddProgress(location, 0, 0) + } + return nil +} + type blobToFileOffsetsMapping map[restic.ID]struct { files map[*fileInfo][]int64 // file -> offsets (plural!) of the blob in the file blob restic.Blob diff --git a/internal/restorer/filerestorer_test.go b/internal/restorer/filerestorer_test.go index 03797e0c8..d29c0dcea 100644 --- a/internal/restorer/filerestorer_test.go +++ b/internal/restorer/filerestorer_test.go @@ -206,6 +206,10 @@ func TestFileRestorerBasic(t *testing.T) { {"data3-1", "pack3-1"}, }, }, + { + name: "empty", + blobs: []TestBlob{}, + }, }, nil, sparse) } } diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index c471800df..f691c4cae 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -203,31 +203,6 @@ func (res *Restorer) restoreHardlinkAt(node *restic.Node, target, path, location return res.restoreNodeMetadataTo(node, path, location) } -func (res *Restorer) restoreEmptyFileAt(node *restic.Node, target, location string) error { - wr, err := os.OpenFile(target, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) - if fs.IsAccessDenied(err) { - // If file is readonly, clear the readonly flag by resetting the - // permissions of the file and try again - // as the metadata will be set again in the second pass and the - // readonly flag will be applied again if needed. - if err = fs.ResetPermissions(target); err != nil { - return err - } - if wr, err = os.OpenFile(target, os.O_TRUNC|os.O_WRONLY, 0600); err != nil { - return err - } - } - if err = wr.Close(); err != nil { - return err - } - - if res.progress != nil { - res.progress.AddProgress(location, 0, 0) - } - - return res.restoreNodeMetadataTo(node, target, location) -} - // RestoreTo creates the directories and files in the snapshot below dst. // Before an item is created, res.Filter is called. func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { @@ -274,13 +249,6 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { return nil } - if node.Size == 0 { - if res.progress != nil { - res.progress.AddFile(node.Size) - } - return nil // deal with empty files later - } - if node.Links > 1 { if idx.Has(node.Inode, node.DeviceID) { if res.progress != nil { @@ -320,14 +288,6 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { return res.restoreNodeTo(ctx, node, target, location) } - // create empty files, but not hardlinks to empty files - if node.Size == 0 && (node.Links < 2 || !idx.Has(node.Inode, node.DeviceID)) { - if node.Links > 1 { - idx.Add(node.Inode, node.DeviceID, location) - } - return res.restoreEmptyFileAt(node, target, location) - } - if idx.Has(node.Inode, node.DeviceID) && idx.Value(node.Inode, node.DeviceID) != location { return res.restoreHardlinkAt(node, filerestorer.targetPath(idx.Value(node.Inode, node.DeviceID)), target, location) } From 607daeed4ff670a3ab61b4fc8638e88f383d1ecc Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 11:07:53 +0200 Subject: [PATCH 03/14] restore: move nil pointer check into restoreui --- internal/restorer/filerestorer.go | 10 ++-------- internal/restorer/restorer.go | 30 ++++++++---------------------- internal/ui/restore/progress.go | 8 ++++++++ 3 files changed, 18 insertions(+), 30 deletions(-) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index f3a68c58a..49f5f7af8 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -211,9 +211,7 @@ func (r *fileRestorer) restoreEmptyFileAt(location string) error { return err } - if r.progress != nil { - r.progress.AddProgress(location, 0, 0) - } + r.progress.AddProgress(location, 0, 0) return nil } @@ -361,11 +359,7 @@ func (r *fileRestorer) downloadBlobs(ctx context.Context, packID restic.ID, createSize = file.size } writeErr := r.filesWriter.writeToFile(r.targetPath(file.location), blobData, offset, createSize, file.sparse) - - if r.progress != nil { - r.progress.AddProgress(file.location, uint64(len(blobData)), uint64(file.size)) - } - + r.progress.AddProgress(file.location, uint64(len(blobData)), uint64(file.size)) return writeErr } err := r.sanitizeError(file, writeToFile()) diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index f691c4cae..12ce84d5c 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -170,10 +170,7 @@ func (res *Restorer) restoreNodeTo(ctx context.Context, node *restic.Node, targe return err } - if res.progress != nil { - res.progress.AddProgress(location, 0, 0) - } - + res.progress.AddProgress(location, 0, 0) return res.restoreNodeMetadataTo(node, target, location) } @@ -195,9 +192,7 @@ func (res *Restorer) restoreHardlinkAt(node *restic.Node, target, path, location return errors.WithStack(err) } - if res.progress != nil { - res.progress.AddProgress(location, 0, 0) - } + res.progress.AddProgress(location, 0, 0) // TODO investigate if hardlinks have separate metadata on any supported system return res.restoreNodeMetadataTo(node, path, location) @@ -225,9 +220,7 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { _, err = res.traverseTree(ctx, dst, string(filepath.Separator), *res.sn.Tree, treeVisitor{ enterDir: func(_ *restic.Node, target, location string) error { debug.Log("first pass, enterDir: mkdir %q, leaveDir should restore metadata", location) - if res.progress != nil { - res.progress.AddFile(0) - } + res.progress.AddFile(0) // create dir with default permissions // #leaveDir restores dir metadata after visiting all children return fs.MkdirAll(target, 0700) @@ -243,27 +236,20 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { } if node.Type != "file" { - if res.progress != nil { - res.progress.AddFile(0) - } + res.progress.AddFile(0) return nil } if node.Links > 1 { if idx.Has(node.Inode, node.DeviceID) { - if res.progress != nil { - // a hardlinked file does not increase the restore size - res.progress.AddFile(0) - } + // a hardlinked file does not increase the restore size + res.progress.AddFile(0) return nil } idx.Add(node.Inode, node.DeviceID, location) } - if res.progress != nil { - res.progress.AddFile(node.Size) - } - + res.progress.AddFile(node.Size) filerestorer.addFile(location, node.Content, int64(node.Size)) return nil @@ -296,7 +282,7 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { }, leaveDir: func(node *restic.Node, target, location string) error { err := res.restoreNodeMetadataTo(node, target, location) - if err == nil && res.progress != nil { + if err == nil { res.progress.AddProgress(location, 0, 0) } return err diff --git a/internal/ui/restore/progress.go b/internal/ui/restore/progress.go index f2bd5d38b..0e120b6a6 100644 --- a/internal/ui/restore/progress.go +++ b/internal/ui/restore/progress.go @@ -59,6 +59,10 @@ func (p *Progress) update(runtime time.Duration, final bool) { // AddFile starts tracking a new file with the given size func (p *Progress) AddFile(size uint64) { + if p == nil { + return + } + p.m.Lock() defer p.m.Unlock() @@ -68,6 +72,10 @@ func (p *Progress) AddFile(size uint64) { // AddProgress accumulates the number of bytes written for a file func (p *Progress) AddProgress(name string, bytesWrittenPortion uint64, bytesTotal uint64) { + if p == nil { + return + } + p.m.Lock() defer p.m.Unlock() From fd2ff464a241f4239acd1d0c61d411b4769eb6dc Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 11:29:16 +0200 Subject: [PATCH 04/14] restorer: remove stale comment --- internal/restorer/filerestorer.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index 49f5f7af8..8fe01c635 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -14,11 +14,6 @@ import ( "github.com/restic/restic/internal/ui/restore" ) -// TODO if a blob is corrupt, there may be good blob copies in other packs -// TODO evaluate if it makes sense to split download and processing workers -// pro: can (slowly) read network and decrypt/write files concurrently -// con: each worker needs to keep one pack in memory - const ( largeFileBlobCount = 25 ) From 0fcd89f89251d8c220cba1a99a53c737ba214ea3 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 11:29:39 +0200 Subject: [PATCH 05/14] restorer: remove special case for blobs with many occurrences Loading blobs by now is no longer prone to timeouts when processing takes a long time. --- internal/restorer/filerestorer.go | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index 8fe01c635..b71e86712 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -255,32 +255,6 @@ func (r *fileRestorer) downloadPack(ctx context.Context, pack *packInfo) error { // track already processed blobs for precise error reporting processedBlobs := restic.NewBlobSet() - for _, entry := range blobs { - occurrences := 0 - for _, offsets := range entry.files { - occurrences += len(offsets) - } - // With a maximum blob size of 8MB, the normal blob streaming has to write - // at most 800MB for a single blob. This should be short enough to avoid - // network connection timeouts. Based on a quick test, a limit of 100 only - // selects a very small number of blobs (the number of references per blob - // - aka. `count` - seem to follow a expontential distribution) - if occurrences > 100 { - // process frequently referenced blobs first as these can take a long time to write - // which can cause backend connections to time out - delete(blobs, entry.blob.ID) - partialBlobs := blobToFileOffsetsMapping{entry.blob.ID: entry} - err := r.downloadBlobs(ctx, pack.id, partialBlobs, processedBlobs) - if err := r.reportError(blobs, processedBlobs, err); err != nil { - return err - } - } - } - - if len(blobs) == 0 { - return nil - } - err := r.downloadBlobs(ctx, pack.id, blobs, processedBlobs) return r.reportError(blobs, processedBlobs, err) } From 2b50c2606c8962538dd8c5b3639f895b22f14581 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 11:42:25 +0200 Subject: [PATCH 06/14] restorer: use options struct --- cmd/restic/cmd_restore.go | 5 ++++- internal/restorer/restorer.go | 19 +++++++++++-------- internal/restorer/restorer_test.go | 12 ++++++------ internal/restorer/restorer_unix_test.go | 4 ++-- internal/restorer/restorer_windows_test.go | 2 +- 5 files changed, 24 insertions(+), 18 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 5161be50d..f86391b20 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -162,7 +162,10 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, } progress := restoreui.NewProgress(printer, calculateProgressInterval(!gopts.Quiet, gopts.JSON)) - res := restorer.NewRestorer(repo, sn, opts.Sparse, progress) + res := restorer.NewRestorer(repo, sn, restorer.Options{ + Sparse: opts.Sparse, + Progress: progress, + }) totalErrors := 0 res.Error = func(location string, err error) error { diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 12ce84d5c..8b39f138f 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -17,10 +17,9 @@ import ( // Restorer is used to restore a snapshot to a directory. type Restorer struct { - repo restic.Repository - sn *restic.Snapshot - sparse bool - + repo restic.Repository + sn *restic.Snapshot + sparse bool progress *restoreui.Progress Error func(location string, err error) error @@ -30,15 +29,19 @@ type Restorer struct { var restorerAbortOnAllErrors = func(_ string, err error) error { return err } +type Options struct { + Sparse bool + Progress *restoreui.Progress +} + // NewRestorer creates a restorer preloaded with the content from the snapshot id. -func NewRestorer(repo restic.Repository, sn *restic.Snapshot, sparse bool, - progress *restoreui.Progress) *Restorer { +func NewRestorer(repo restic.Repository, sn *restic.Snapshot, opts Options) *Restorer { r := &Restorer{ repo: repo, - sparse: sparse, + sparse: opts.Sparse, + progress: opts.Progress, Error: restorerAbortOnAllErrors, SelectFilter: func(string, string, *restic.Node) (bool, bool) { return true, true }, - progress: progress, sn: sn, } diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index 757a317b2..64e10a6e8 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -343,7 +343,7 @@ func TestRestorer(t *testing.T) { sn, id := saveSnapshot(t, repo, test.Snapshot, noopGetGenericAttributes) t.Logf("snapshot saved as %v", id.Str()) - res := NewRestorer(repo, sn, false, nil) + res := NewRestorer(repo, sn, Options{}) tempdir := rtest.TempDir(t) // make sure we're creating a new subdir of the tempdir @@ -460,7 +460,7 @@ func TestRestorerRelative(t *testing.T) { sn, id := saveSnapshot(t, repo, test.Snapshot, noopGetGenericAttributes) t.Logf("snapshot saved as %v", id.Str()) - res := NewRestorer(repo, sn, false, nil) + res := NewRestorer(repo, sn, Options{}) tempdir := rtest.TempDir(t) cleanup := rtest.Chdir(t, tempdir) @@ -689,7 +689,7 @@ func TestRestorerTraverseTree(t *testing.T) { repo := repository.TestRepository(t) sn, _ := saveSnapshot(t, repo, test.Snapshot, noopGetGenericAttributes) - res := NewRestorer(repo, sn, false, nil) + res := NewRestorer(repo, sn, Options{}) res.SelectFilter = test.Select @@ -765,7 +765,7 @@ func TestRestorerConsistentTimestampsAndPermissions(t *testing.T) { }, }, noopGetGenericAttributes) - res := NewRestorer(repo, sn, false, nil) + res := NewRestorer(repo, sn, Options{}) res.SelectFilter = func(item string, dstpath string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { switch filepath.ToSlash(item) { @@ -820,7 +820,7 @@ func TestVerifyCancel(t *testing.T) { repo := repository.TestRepository(t) sn, _ := saveSnapshot(t, repo, snapshot, noopGetGenericAttributes) - res := NewRestorer(repo, sn, false, nil) + res := NewRestorer(repo, sn, Options{}) tempdir := rtest.TempDir(t) ctx, cancel := context.WithCancel(context.Background()) @@ -862,7 +862,7 @@ func TestRestorerSparseFiles(t *testing.T) { archiver.SnapshotOptions{}) rtest.OK(t, err) - res := NewRestorer(repo, sn, true, nil) + res := NewRestorer(repo, sn, Options{Sparse: true}) tempdir := rtest.TempDir(t) ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/restorer/restorer_unix_test.go b/internal/restorer/restorer_unix_test.go index 0cbfefa92..95a83cdf7 100644 --- a/internal/restorer/restorer_unix_test.go +++ b/internal/restorer/restorer_unix_test.go @@ -31,7 +31,7 @@ func TestRestorerRestoreEmptyHardlinkedFileds(t *testing.T) { }, }, noopGetGenericAttributes) - res := NewRestorer(repo, sn, false, nil) + res := NewRestorer(repo, sn, Options{}) res.SelectFilter = func(item string, dstpath string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { return true, true @@ -99,7 +99,7 @@ func TestRestorerProgressBar(t *testing.T) { mock := &printerMock{} progress := restoreui.NewProgress(mock, 0) - res := NewRestorer(repo, sn, false, progress) + res := NewRestorer(repo, sn, Options{Progress: progress}) res.SelectFilter = func(item string, dstpath string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { return true, true } diff --git a/internal/restorer/restorer_windows_test.go b/internal/restorer/restorer_windows_test.go index 684d51ace..90ece474d 100644 --- a/internal/restorer/restorer_windows_test.go +++ b/internal/restorer/restorer_windows_test.go @@ -269,7 +269,7 @@ func setup(t *testing.T, nodesMap map[string]Node) *Restorer { sn, _ := saveSnapshot(t, repo, Snapshot{ Nodes: nodesMap, }, getFileAttributes) - res := NewRestorer(repo, sn, false, nil) + res := NewRestorer(repo, sn, Options{}) return res } From a23cb3a42801cba15cb40b115158938991c489bc Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 15:50:48 +0200 Subject: [PATCH 07/14] restore: reduce memory usage --- internal/restorer/filerestorer.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index b71e86712..600a702b2 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -161,6 +161,8 @@ func (r *fileRestorer) restoreFiles(ctx context.Context) error { file.blobs = packsMap } } + // drop no longer necessary file list + r.files = nil wg, ctx := errgroup.WithContext(ctx) downloadCh := make(chan *packInfo) From 6a4ae9d6b11deeb3595d640c447aedf33c144441 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 11:43:42 +0200 Subject: [PATCH 08/14] restore: configurable overwrite behavior --- cmd/restic/cmd_restore.go | 7 +- internal/restorer/restorer.go | 139 ++++++++++++++++++++++++++++++---- 2 files changed, 129 insertions(+), 17 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index f86391b20..a9de998be 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -51,8 +51,9 @@ type RestoreOptions struct { InsensitiveInclude []string Target string restic.SnapshotFilter - Sparse bool - Verify bool + Sparse bool + Verify bool + Overwrite restorer.OverwriteBehavior } var restoreOptions RestoreOptions @@ -70,6 +71,7 @@ func init() { initSingleSnapshotFilter(flags, &restoreOptions.SnapshotFilter) flags.BoolVar(&restoreOptions.Sparse, "sparse", false, "restore files as sparse") flags.BoolVar(&restoreOptions.Verify, "verify", false, "verify restored files content") + flags.Var(&restoreOptions.Overwrite, "overwrite", "overwrite behavior, one of (always|if-newer|never) (default: always)") } func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, @@ -165,6 +167,7 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, res := restorer.NewRestorer(repo, sn, restorer.Options{ Sparse: opts.Sparse, Progress: progress, + Overwrite: opts.Overwrite, }) totalErrors := 0 diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 8b39f138f..267b2898c 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -2,6 +2,7 @@ package restorer import ( "context" + "fmt" "os" "path/filepath" "sync/atomic" @@ -17,10 +18,13 @@ import ( // Restorer is used to restore a snapshot to a directory. type Restorer struct { - repo restic.Repository - sn *restic.Snapshot - sparse bool - progress *restoreui.Progress + repo restic.Repository + sn *restic.Snapshot + sparse bool + progress *restoreui.Progress + overwrite OverwriteBehavior + + fileList map[string]struct{} Error func(location string, err error) error Warn func(message string) @@ -30,8 +34,53 @@ type Restorer struct { var restorerAbortOnAllErrors = func(_ string, err error) error { return err } type Options struct { - Sparse bool - Progress *restoreui.Progress + Sparse bool + Progress *restoreui.Progress + Overwrite OverwriteBehavior +} + +type OverwriteBehavior int + +// Constants for different overwrite behavior +const ( + OverwriteAlways OverwriteBehavior = 0 + OverwriteIfNewer OverwriteBehavior = 1 + OverwriteNever OverwriteBehavior = 2 + OverwriteInvalid OverwriteBehavior = 3 +) + +// Set implements the method needed for pflag command flag parsing. +func (c *OverwriteBehavior) Set(s string) error { + switch s { + case "always": + *c = OverwriteAlways + case "if-newer": + *c = OverwriteIfNewer + case "never": + *c = OverwriteNever + default: + *c = OverwriteInvalid + return fmt.Errorf("invalid overwrite behavior %q, must be one of (always|if-newer|never)", s) + } + + return nil +} + +func (c *OverwriteBehavior) String() string { + switch *c { + case OverwriteAlways: + return "always" + case OverwriteIfNewer: + return "if-newer" + case OverwriteNever: + return "never" + default: + return "invalid" + } + +} +func (c *OverwriteBehavior) Type() string { + return "behavior" } // NewRestorer creates a restorer preloaded with the content from the snapshot id. @@ -40,6 +89,8 @@ func NewRestorer(repo restic.Repository, sn *restic.Snapshot, opts Options) *Res repo: repo, sparse: opts.Sparse, progress: opts.Progress, + overwrite: opts.Overwrite, + fileList: make(map[string]struct{}), Error: restorerAbortOnAllErrors, SelectFilter: func(string, string, *restic.Node) (bool, bool) { return true, true }, sn: sn, @@ -252,10 +303,12 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { idx.Add(node.Inode, node.DeviceID, location) } - res.progress.AddFile(node.Size) - filerestorer.addFile(location, node.Content, int64(node.Size)) - - return nil + return res.withOverwriteCheck(node, target, location, false, func() error { + res.progress.AddFile(node.Size) + filerestorer.addFile(location, node.Content, int64(node.Size)) + res.trackFile(location) + return nil + }) }, }) if err != nil { @@ -274,14 +327,22 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { visitNode: func(node *restic.Node, target, location string) error { debug.Log("second pass, visitNode: restore node %q", location) if node.Type != "file" { - return res.restoreNodeTo(ctx, node, target, location) + return res.withOverwriteCheck(node, target, location, false, func() error { + return res.restoreNodeTo(ctx, node, target, location) + }) } if idx.Has(node.Inode, node.DeviceID) && idx.Value(node.Inode, node.DeviceID) != location { - return res.restoreHardlinkAt(node, filerestorer.targetPath(idx.Value(node.Inode, node.DeviceID)), target, location) + return res.withOverwriteCheck(node, target, location, true, func() error { + return res.restoreHardlinkAt(node, filerestorer.targetPath(idx.Value(node.Inode, node.DeviceID)), target, location) + }) } - return res.restoreNodeMetadataTo(node, target, location) + if res.hasRestoredFile(location) { + return res.restoreNodeMetadataTo(node, target, location) + } + // don't touch skipped files + return nil }, leaveDir: func(node *restic.Node, target, location string) error { err := res.restoreNodeMetadataTo(node, target, location) @@ -294,6 +355,54 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { return err } +func (res *Restorer) trackFile(location string) { + res.fileList[location] = struct{}{} +} + +func (res *Restorer) hasRestoredFile(location string) bool { + _, ok := res.fileList[location] + return ok +} + +func (res *Restorer) withOverwriteCheck(node *restic.Node, target, location string, isHardlink bool, cb func() error) error { + overwrite, err := shouldOverwrite(res.overwrite, node, target) + if err != nil { + return err + } else if !overwrite { + size := node.Size + if isHardlink { + size = 0 + } + res.progress.AddFile(size) + res.progress.AddProgress(location, size, size) + return nil + } + return cb() +} + +func shouldOverwrite(overwrite OverwriteBehavior, node *restic.Node, destination string) (bool, error) { + if overwrite == OverwriteAlways { + return true, nil + } + + fi, err := fs.Lstat(destination) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return true, nil + } + return false, err + } + + if overwrite == OverwriteIfNewer { + // return if node is newer + return node.ModTime.After(fi.ModTime()), nil + } else if overwrite == OverwriteNever { + // file exists + return false, nil + } + panic("unknown overwrite behavior") +} + // Snapshot returns the snapshot this restorer is configured to use. func (res *Restorer) Snapshot() *restic.Snapshot { return res.sn @@ -324,8 +433,8 @@ func (res *Restorer) VerifyFiles(ctx context.Context, dst string) (int, error) { defer close(work) _, err := res.traverseTree(ctx, dst, string(filepath.Separator), *res.sn.Tree, treeVisitor{ - visitNode: func(node *restic.Node, target, _ string) error { - if node.Type != "file" { + visitNode: func(node *restic.Node, target, location string) error { + if node.Type != "file" || !res.hasRestoredFile(location) { return nil } select { From 64b7b6b9759c42b1484e15d5c88f7b50d9ce8e9f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 13:43:57 +0200 Subject: [PATCH 09/14] restore/ui: refactor for extensibility --- internal/ui/restore/json.go | 24 ++++++++++++------------ internal/ui/restore/json_test.go | 6 +++--- internal/ui/restore/progress.go | 28 ++++++++++++++++------------ internal/ui/restore/progress_test.go | 24 ++++++++++++------------ internal/ui/restore/text.go | 22 +++++++++++----------- internal/ui/restore/text_test.go | 6 +++--- 6 files changed, 57 insertions(+), 53 deletions(-) diff --git a/internal/ui/restore/json.go b/internal/ui/restore/json.go index c1b95b00b..50d4fe0f7 100644 --- a/internal/ui/restore/json.go +++ b/internal/ui/restore/json.go @@ -20,31 +20,31 @@ func (t *jsonPrinter) print(status interface{}) { t.terminal.Print(ui.ToJSONString(status)) } -func (t *jsonPrinter) Update(filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64, duration time.Duration) { +func (t *jsonPrinter) Update(p State, duration time.Duration) { status := statusUpdate{ MessageType: "status", SecondsElapsed: uint64(duration / time.Second), - TotalFiles: filesTotal, - FilesRestored: filesFinished, - TotalBytes: allBytesTotal, - BytesRestored: allBytesWritten, + TotalFiles: p.FilesTotal, + FilesRestored: p.FilesFinished, + TotalBytes: p.AllBytesTotal, + BytesRestored: p.AllBytesWritten, } - if allBytesTotal > 0 { - status.PercentDone = float64(allBytesWritten) / float64(allBytesTotal) + if p.AllBytesTotal > 0 { + status.PercentDone = float64(p.AllBytesWritten) / float64(p.AllBytesTotal) } t.print(status) } -func (t *jsonPrinter) Finish(filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64, duration time.Duration) { +func (t *jsonPrinter) Finish(p State, duration time.Duration) { status := summaryOutput{ MessageType: "summary", SecondsElapsed: uint64(duration / time.Second), - TotalFiles: filesTotal, - FilesRestored: filesFinished, - TotalBytes: allBytesTotal, - BytesRestored: allBytesWritten, + TotalFiles: p.FilesTotal, + FilesRestored: p.FilesFinished, + TotalBytes: p.AllBytesTotal, + BytesRestored: p.AllBytesWritten, } t.print(status) } diff --git a/internal/ui/restore/json_test.go b/internal/ui/restore/json_test.go index 7bcabb4d7..7ce7b58f3 100644 --- a/internal/ui/restore/json_test.go +++ b/internal/ui/restore/json_test.go @@ -10,20 +10,20 @@ import ( func TestJSONPrintUpdate(t *testing.T) { term := &mockTerm{} printer := NewJSONProgress(term) - printer.Update(3, 11, 29, 47, 5*time.Second) + printer.Update(State{3, 11, 29, 47}, 5*time.Second) test.Equals(t, []string{"{\"message_type\":\"status\",\"seconds_elapsed\":5,\"percent_done\":0.6170212765957447,\"total_files\":11,\"files_restored\":3,\"total_bytes\":47,\"bytes_restored\":29}\n"}, term.output) } func TestJSONPrintSummaryOnSuccess(t *testing.T) { term := &mockTerm{} printer := NewJSONProgress(term) - printer.Finish(11, 11, 47, 47, 5*time.Second) + printer.Finish(State{11, 11, 47, 47}, 5*time.Second) test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":11,\"total_bytes\":47,\"bytes_restored\":47}\n"}, term.output) } func TestJSONPrintSummaryOnErrors(t *testing.T) { term := &mockTerm{} printer := NewJSONProgress(term) - printer.Finish(3, 11, 29, 47, 5*time.Second) + printer.Finish(State{3, 11, 29, 47}, 5*time.Second) test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":3,\"total_bytes\":47,\"bytes_restored\":29}\n"}, term.output) } diff --git a/internal/ui/restore/progress.go b/internal/ui/restore/progress.go index 0e120b6a6..5e501c4b3 100644 --- a/internal/ui/restore/progress.go +++ b/internal/ui/restore/progress.go @@ -7,15 +7,19 @@ import ( "github.com/restic/restic/internal/ui/progress" ) +type State struct { + FilesFinished uint64 + FilesTotal uint64 + AllBytesWritten uint64 + AllBytesTotal uint64 +} + type Progress struct { updater progress.Updater m sync.Mutex progressInfoMap map[string]progressInfoEntry - filesFinished uint64 - filesTotal uint64 - allBytesWritten uint64 - allBytesTotal uint64 + s State started time.Time printer ProgressPrinter @@ -32,8 +36,8 @@ type term interface { } type ProgressPrinter interface { - Update(filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64, duration time.Duration) - Finish(filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64, duration time.Duration) + Update(progress State, duration time.Duration) + Finish(progress State, duration time.Duration) } func NewProgress(printer ProgressPrinter, interval time.Duration) *Progress { @@ -51,9 +55,9 @@ func (p *Progress) update(runtime time.Duration, final bool) { defer p.m.Unlock() if !final { - p.printer.Update(p.filesFinished, p.filesTotal, p.allBytesWritten, p.allBytesTotal, runtime) + p.printer.Update(p.s, runtime) } else { - p.printer.Finish(p.filesFinished, p.filesTotal, p.allBytesWritten, p.allBytesTotal, runtime) + p.printer.Finish(p.s, runtime) } } @@ -66,8 +70,8 @@ func (p *Progress) AddFile(size uint64) { p.m.Lock() defer p.m.Unlock() - p.filesTotal++ - p.allBytesTotal += size + p.s.FilesTotal++ + p.s.AllBytesTotal += size } // AddProgress accumulates the number of bytes written for a file @@ -86,10 +90,10 @@ func (p *Progress) AddProgress(name string, bytesWrittenPortion uint64, bytesTot entry.bytesWritten += bytesWrittenPortion p.progressInfoMap[name] = entry - p.allBytesWritten += bytesWrittenPortion + p.s.AllBytesWritten += bytesWrittenPortion if entry.bytesWritten == entry.bytesTotal { delete(p.progressInfoMap, name) - p.filesFinished++ + p.s.FilesFinished++ } } diff --git a/internal/ui/restore/progress_test.go b/internal/ui/restore/progress_test.go index 9e625aa20..728b74350 100644 --- a/internal/ui/restore/progress_test.go +++ b/internal/ui/restore/progress_test.go @@ -8,7 +8,7 @@ import ( ) type printerTraceEntry struct { - filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64 + progress State duration time.Duration isFinished bool @@ -22,11 +22,11 @@ type mockPrinter struct { const mockFinishDuration = 42 * time.Second -func (p *mockPrinter) Update(filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64, duration time.Duration) { - p.trace = append(p.trace, printerTraceEntry{filesFinished, filesTotal, allBytesWritten, allBytesTotal, duration, false}) +func (p *mockPrinter) Update(progress State, duration time.Duration) { + p.trace = append(p.trace, printerTraceEntry{progress, duration, false}) } -func (p *mockPrinter) Finish(filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64, _ time.Duration) { - p.trace = append(p.trace, printerTraceEntry{filesFinished, filesTotal, allBytesWritten, allBytesTotal, mockFinishDuration, true}) +func (p *mockPrinter) Finish(progress State, _ time.Duration) { + p.trace = append(p.trace, printerTraceEntry{progress, mockFinishDuration, true}) } func testProgress(fn func(progress *Progress) bool) printerTrace { @@ -45,7 +45,7 @@ func TestNew(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{0, 0, 0, 0, 0, false}, + printerTraceEntry{State{0, 0, 0, 0}, 0, false}, }, result) } @@ -57,7 +57,7 @@ func TestAddFile(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{0, 1, 0, fileSize, 0, false}, + printerTraceEntry{State{0, 1, 0, fileSize}, 0, false}, }, result) } @@ -71,7 +71,7 @@ func TestFirstProgressOnAFile(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{0, 1, expectedBytesWritten, expectedBytesTotal, 0, false}, + printerTraceEntry{State{0, 1, expectedBytesWritten, expectedBytesTotal}, 0, false}, }, result) } @@ -86,7 +86,7 @@ func TestLastProgressOnAFile(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{1, 1, fileSize, fileSize, 0, false}, + printerTraceEntry{State{1, 1, fileSize, fileSize}, 0, false}, }, result) } @@ -102,7 +102,7 @@ func TestLastProgressOnLastFile(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{2, 2, 50 + fileSize, 50 + fileSize, 0, false}, + printerTraceEntry{State{2, 2, 50 + fileSize, 50 + fileSize}, 0, false}, }, result) } @@ -117,7 +117,7 @@ func TestSummaryOnSuccess(t *testing.T) { return true }) test.Equals(t, printerTrace{ - printerTraceEntry{2, 2, 50 + fileSize, 50 + fileSize, mockFinishDuration, true}, + printerTraceEntry{State{2, 2, 50 + fileSize, 50 + fileSize}, mockFinishDuration, true}, }, result) } @@ -132,6 +132,6 @@ func TestSummaryOnErrors(t *testing.T) { return true }) test.Equals(t, printerTrace{ - printerTraceEntry{1, 2, 50 + fileSize/2, 50 + fileSize, mockFinishDuration, true}, + printerTraceEntry{State{1, 2, 50 + fileSize/2, 50 + fileSize}, mockFinishDuration, true}, }, result) } diff --git a/internal/ui/restore/text.go b/internal/ui/restore/text.go index 2647bb28b..9da388e51 100644 --- a/internal/ui/restore/text.go +++ b/internal/ui/restore/text.go @@ -17,30 +17,30 @@ func NewTextProgress(terminal term) ProgressPrinter { } } -func (t *textPrinter) Update(filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64, duration time.Duration) { +func (t *textPrinter) Update(p State, duration time.Duration) { timeLeft := ui.FormatDuration(duration) - formattedAllBytesWritten := ui.FormatBytes(allBytesWritten) - formattedAllBytesTotal := ui.FormatBytes(allBytesTotal) - allPercent := ui.FormatPercent(allBytesWritten, allBytesTotal) + formattedAllBytesWritten := ui.FormatBytes(p.AllBytesWritten) + formattedAllBytesTotal := ui.FormatBytes(p.AllBytesTotal) + allPercent := ui.FormatPercent(p.AllBytesWritten, p.AllBytesTotal) progress := fmt.Sprintf("[%s] %s %v files/dirs %s, total %v files/dirs %v", - timeLeft, allPercent, filesFinished, formattedAllBytesWritten, filesTotal, formattedAllBytesTotal) + timeLeft, allPercent, p.FilesFinished, formattedAllBytesWritten, p.FilesTotal, formattedAllBytesTotal) t.terminal.SetStatus([]string{progress}) } -func (t *textPrinter) Finish(filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64, duration time.Duration) { +func (t *textPrinter) Finish(p State, duration time.Duration) { t.terminal.SetStatus([]string{}) timeLeft := ui.FormatDuration(duration) - formattedAllBytesTotal := ui.FormatBytes(allBytesTotal) + formattedAllBytesTotal := ui.FormatBytes(p.AllBytesTotal) var summary string - if filesFinished == filesTotal && allBytesWritten == allBytesTotal { - summary = fmt.Sprintf("Summary: Restored %d files/dirs (%s) in %s", filesTotal, formattedAllBytesTotal, timeLeft) + if p.FilesFinished == p.FilesTotal && p.AllBytesWritten == p.AllBytesTotal { + summary = fmt.Sprintf("Summary: Restored %d files/dirs (%s) in %s", p.FilesTotal, formattedAllBytesTotal, timeLeft) } else { - formattedAllBytesWritten := ui.FormatBytes(allBytesWritten) + formattedAllBytesWritten := ui.FormatBytes(p.AllBytesWritten) summary = fmt.Sprintf("Summary: Restored %d / %d files/dirs (%s / %s) in %s", - filesFinished, filesTotal, formattedAllBytesWritten, formattedAllBytesTotal, timeLeft) + p.FilesFinished, p.FilesTotal, formattedAllBytesWritten, formattedAllBytesTotal, timeLeft) } t.terminal.Print(summary) diff --git a/internal/ui/restore/text_test.go b/internal/ui/restore/text_test.go index fc03904ff..2a1723943 100644 --- a/internal/ui/restore/text_test.go +++ b/internal/ui/restore/text_test.go @@ -22,20 +22,20 @@ func (m *mockTerm) SetStatus(lines []string) { func TestPrintUpdate(t *testing.T) { term := &mockTerm{} printer := NewTextProgress(term) - printer.Update(3, 11, 29, 47, 5*time.Second) + printer.Update(State{3, 11, 29, 47}, 5*time.Second) test.Equals(t, []string{"[0:05] 61.70% 3 files/dirs 29 B, total 11 files/dirs 47 B"}, term.output) } func TestPrintSummaryOnSuccess(t *testing.T) { term := &mockTerm{} printer := NewTextProgress(term) - printer.Finish(11, 11, 47, 47, 5*time.Second) + printer.Finish(State{11, 11, 47, 47}, 5*time.Second) test.Equals(t, []string{"Summary: Restored 11 files/dirs (47 B) in 0:05"}, term.output) } func TestPrintSummaryOnErrors(t *testing.T) { term := &mockTerm{} printer := NewTextProgress(term) - printer.Finish(3, 11, 29, 47, 5*time.Second) + printer.Finish(State{3, 11, 29, 47}, 5*time.Second) test.Equals(t, []string{"Summary: Restored 3 / 11 files/dirs (29 B / 47 B) in 0:05"}, term.output) } From e47e08a68803baf8b240a9abbeb540b492695b2d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 14:12:06 +0200 Subject: [PATCH 10/14] restorer: separately track skipped files --- internal/restorer/restorer.go | 3 +-- internal/restorer/restorer_unix_test.go | 27 +++++++++++-------------- internal/ui/restore/json.go | 8 ++++++++ internal/ui/restore/json_test.go | 20 +++++++++++++++--- internal/ui/restore/progress.go | 14 +++++++++++++ internal/ui/restore/progress_test.go | 26 +++++++++++++++++------- internal/ui/restore/text.go | 6 ++++++ internal/ui/restore/text_test.go | 20 +++++++++++++++--- 8 files changed, 94 insertions(+), 30 deletions(-) diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 267b2898c..ae622874b 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -373,8 +373,7 @@ func (res *Restorer) withOverwriteCheck(node *restic.Node, target, location stri if isHardlink { size = 0 } - res.progress.AddFile(size) - res.progress.AddProgress(location, size, size) + res.progress.AddSkippedFile(size) return nil } return cb() diff --git a/internal/restorer/restorer_unix_test.go b/internal/restorer/restorer_unix_test.go index 95a83cdf7..97d2dd07d 100644 --- a/internal/restorer/restorer_unix_test.go +++ b/internal/restorer/restorer_unix_test.go @@ -70,16 +70,13 @@ func getBlockCount(t *testing.T, filename string) int64 { } type printerMock struct { - filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64 + s restoreui.State } -func (p *printerMock) Update(_, _, _, _ uint64, _ time.Duration) { +func (p *printerMock) Update(_ restoreui.State, _ time.Duration) { } -func (p *printerMock) Finish(filesFinished, filesTotal, allBytesWritten, allBytesTotal uint64, _ time.Duration) { - p.filesFinished = filesFinished - p.filesTotal = filesTotal - p.allBytesWritten = allBytesWritten - p.allBytesTotal = allBytesTotal +func (p *printerMock) Finish(s restoreui.State, _ time.Duration) { + p.s = s } func TestRestorerProgressBar(t *testing.T) { @@ -112,12 +109,12 @@ func TestRestorerProgressBar(t *testing.T) { rtest.OK(t, err) progress.Finish() - const filesFinished = 4 - const filesTotal = filesFinished - const allBytesWritten = 10 - const allBytesTotal = allBytesWritten - rtest.Assert(t, mock.filesFinished == filesFinished, "filesFinished: expected %v, got %v", filesFinished, mock.filesFinished) - rtest.Assert(t, mock.filesTotal == filesTotal, "filesTotal: expected %v, got %v", filesTotal, mock.filesTotal) - rtest.Assert(t, mock.allBytesWritten == allBytesWritten, "allBytesWritten: expected %v, got %v", allBytesWritten, mock.allBytesWritten) - rtest.Assert(t, mock.allBytesTotal == allBytesTotal, "allBytesTotal: expected %v, got %v", allBytesTotal, mock.allBytesTotal) + rtest.Equals(t, restoreui.State{ + FilesFinished: 4, + FilesTotal: 4, + FilesSkipped: 0, + AllBytesWritten: 10, + AllBytesTotal: 10, + AllBytesSkipped: 0, + }, mock.s) } diff --git a/internal/ui/restore/json.go b/internal/ui/restore/json.go index 50d4fe0f7..512640a7a 100644 --- a/internal/ui/restore/json.go +++ b/internal/ui/restore/json.go @@ -26,8 +26,10 @@ func (t *jsonPrinter) Update(p State, duration time.Duration) { SecondsElapsed: uint64(duration / time.Second), TotalFiles: p.FilesTotal, FilesRestored: p.FilesFinished, + FilesSkipped: p.FilesSkipped, TotalBytes: p.AllBytesTotal, BytesRestored: p.AllBytesWritten, + BytesSkipped: p.AllBytesSkipped, } if p.AllBytesTotal > 0 { @@ -43,8 +45,10 @@ func (t *jsonPrinter) Finish(p State, duration time.Duration) { SecondsElapsed: uint64(duration / time.Second), TotalFiles: p.FilesTotal, FilesRestored: p.FilesFinished, + FilesSkipped: p.FilesSkipped, TotalBytes: p.AllBytesTotal, BytesRestored: p.AllBytesWritten, + BytesSkipped: p.AllBytesSkipped, } t.print(status) } @@ -55,8 +59,10 @@ type statusUpdate struct { PercentDone float64 `json:"percent_done"` TotalFiles uint64 `json:"total_files,omitempty"` FilesRestored uint64 `json:"files_restored,omitempty"` + FilesSkipped uint64 `json:"files_skipped,omitempty"` TotalBytes uint64 `json:"total_bytes,omitempty"` BytesRestored uint64 `json:"bytes_restored,omitempty"` + BytesSkipped uint64 `json:"bytes_skipped,omitempty"` } type summaryOutput struct { @@ -64,6 +70,8 @@ type summaryOutput struct { SecondsElapsed uint64 `json:"seconds_elapsed,omitempty"` TotalFiles uint64 `json:"total_files,omitempty"` FilesRestored uint64 `json:"files_restored,omitempty"` + FilesSkipped uint64 `json:"files_skipped,omitempty"` TotalBytes uint64 `json:"total_bytes,omitempty"` BytesRestored uint64 `json:"bytes_restored,omitempty"` + BytesSkipped uint64 `json:"bytes_skipped,omitempty"` } diff --git a/internal/ui/restore/json_test.go b/internal/ui/restore/json_test.go index 7ce7b58f3..37983f7d7 100644 --- a/internal/ui/restore/json_test.go +++ b/internal/ui/restore/json_test.go @@ -10,20 +10,34 @@ import ( func TestJSONPrintUpdate(t *testing.T) { term := &mockTerm{} printer := NewJSONProgress(term) - printer.Update(State{3, 11, 29, 47}, 5*time.Second) + printer.Update(State{3, 11, 0, 29, 47, 0}, 5*time.Second) test.Equals(t, []string{"{\"message_type\":\"status\",\"seconds_elapsed\":5,\"percent_done\":0.6170212765957447,\"total_files\":11,\"files_restored\":3,\"total_bytes\":47,\"bytes_restored\":29}\n"}, term.output) } +func TestJSONPrintUpdateWithSkipped(t *testing.T) { + term := &mockTerm{} + printer := NewJSONProgress(term) + printer.Update(State{3, 11, 2, 29, 47, 59}, 5*time.Second) + test.Equals(t, []string{"{\"message_type\":\"status\",\"seconds_elapsed\":5,\"percent_done\":0.6170212765957447,\"total_files\":11,\"files_restored\":3,\"files_skipped\":2,\"total_bytes\":47,\"bytes_restored\":29,\"bytes_skipped\":59}\n"}, term.output) +} + func TestJSONPrintSummaryOnSuccess(t *testing.T) { term := &mockTerm{} printer := NewJSONProgress(term) - printer.Finish(State{11, 11, 47, 47}, 5*time.Second) + printer.Finish(State{11, 11, 0, 47, 47, 0}, 5*time.Second) test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":11,\"total_bytes\":47,\"bytes_restored\":47}\n"}, term.output) } func TestJSONPrintSummaryOnErrors(t *testing.T) { term := &mockTerm{} printer := NewJSONProgress(term) - printer.Finish(State{3, 11, 29, 47}, 5*time.Second) + printer.Finish(State{3, 11, 0, 29, 47, 0}, 5*time.Second) test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":3,\"total_bytes\":47,\"bytes_restored\":29}\n"}, term.output) } + +func TestJSONPrintSummaryOnSuccessWithSkipped(t *testing.T) { + term := &mockTerm{} + printer := NewJSONProgress(term) + printer.Finish(State{11, 11, 2, 47, 47, 59}, 5*time.Second) + test.Equals(t, []string{"{\"message_type\":\"summary\",\"seconds_elapsed\":5,\"total_files\":11,\"files_restored\":11,\"files_skipped\":2,\"total_bytes\":47,\"bytes_restored\":47,\"bytes_skipped\":59}\n"}, term.output) +} diff --git a/internal/ui/restore/progress.go b/internal/ui/restore/progress.go index 5e501c4b3..7e8bcfd25 100644 --- a/internal/ui/restore/progress.go +++ b/internal/ui/restore/progress.go @@ -10,8 +10,10 @@ import ( type State struct { FilesFinished uint64 FilesTotal uint64 + FilesSkipped uint64 AllBytesWritten uint64 AllBytesTotal uint64 + AllBytesSkipped uint64 } type Progress struct { @@ -97,6 +99,18 @@ func (p *Progress) AddProgress(name string, bytesWrittenPortion uint64, bytesTot } } +func (p *Progress) AddSkippedFile(size uint64) { + if p == nil { + return + } + + p.m.Lock() + defer p.m.Unlock() + + p.s.FilesSkipped++ + p.s.AllBytesSkipped += size +} + func (p *Progress) Finish() { p.updater.Done() } diff --git a/internal/ui/restore/progress_test.go b/internal/ui/restore/progress_test.go index 728b74350..56f5f62ce 100644 --- a/internal/ui/restore/progress_test.go +++ b/internal/ui/restore/progress_test.go @@ -45,7 +45,7 @@ func TestNew(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{State{0, 0, 0, 0}, 0, false}, + printerTraceEntry{State{0, 0, 0, 0, 0, 0}, 0, false}, }, result) } @@ -57,7 +57,7 @@ func TestAddFile(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{State{0, 1, 0, fileSize}, 0, false}, + printerTraceEntry{State{0, 1, 0, 0, fileSize, 0}, 0, false}, }, result) } @@ -71,7 +71,7 @@ func TestFirstProgressOnAFile(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{State{0, 1, expectedBytesWritten, expectedBytesTotal}, 0, false}, + printerTraceEntry{State{0, 1, 0, expectedBytesWritten, expectedBytesTotal, 0}, 0, false}, }, result) } @@ -86,7 +86,7 @@ func TestLastProgressOnAFile(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{State{1, 1, fileSize, fileSize}, 0, false}, + printerTraceEntry{State{1, 1, 0, fileSize, fileSize, 0}, 0, false}, }, result) } @@ -102,7 +102,7 @@ func TestLastProgressOnLastFile(t *testing.T) { return false }) test.Equals(t, printerTrace{ - printerTraceEntry{State{2, 2, 50 + fileSize, 50 + fileSize}, 0, false}, + printerTraceEntry{State{2, 2, 0, 50 + fileSize, 50 + fileSize, 0}, 0, false}, }, result) } @@ -117,7 +117,7 @@ func TestSummaryOnSuccess(t *testing.T) { return true }) test.Equals(t, printerTrace{ - printerTraceEntry{State{2, 2, 50 + fileSize, 50 + fileSize}, mockFinishDuration, true}, + printerTraceEntry{State{2, 2, 0, 50 + fileSize, 50 + fileSize, 0}, mockFinishDuration, true}, }, result) } @@ -132,6 +132,18 @@ func TestSummaryOnErrors(t *testing.T) { return true }) test.Equals(t, printerTrace{ - printerTraceEntry{State{1, 2, 50 + fileSize/2, 50 + fileSize}, mockFinishDuration, true}, + printerTraceEntry{State{1, 2, 0, 50 + fileSize/2, 50 + fileSize, 0}, mockFinishDuration, true}, + }, result) +} + +func TestSkipFile(t *testing.T) { + fileSize := uint64(100) + + result := testProgress(func(progress *Progress) bool { + progress.AddSkippedFile(fileSize) + return true + }) + test.Equals(t, printerTrace{ + printerTraceEntry{State{0, 0, 1, 0, 0, fileSize}, mockFinishDuration, true}, }, result) } diff --git a/internal/ui/restore/text.go b/internal/ui/restore/text.go index 9da388e51..28a6eb965 100644 --- a/internal/ui/restore/text.go +++ b/internal/ui/restore/text.go @@ -24,6 +24,9 @@ func (t *textPrinter) Update(p State, duration time.Duration) { allPercent := ui.FormatPercent(p.AllBytesWritten, p.AllBytesTotal) progress := fmt.Sprintf("[%s] %s %v files/dirs %s, total %v files/dirs %v", timeLeft, allPercent, p.FilesFinished, formattedAllBytesWritten, p.FilesTotal, formattedAllBytesTotal) + if p.FilesSkipped > 0 { + progress += fmt.Sprintf(", skipped %v files/dirs %v", p.FilesSkipped, ui.FormatBytes(p.AllBytesSkipped)) + } t.terminal.SetStatus([]string{progress}) } @@ -42,6 +45,9 @@ func (t *textPrinter) Finish(p State, duration time.Duration) { summary = fmt.Sprintf("Summary: Restored %d / %d files/dirs (%s / %s) in %s", p.FilesFinished, p.FilesTotal, formattedAllBytesWritten, formattedAllBytesTotal, timeLeft) } + if p.FilesSkipped > 0 { + summary += fmt.Sprintf(", skipped %v files/dirs %v", p.FilesSkipped, ui.FormatBytes(p.AllBytesSkipped)) + } t.terminal.Print(summary) } diff --git a/internal/ui/restore/text_test.go b/internal/ui/restore/text_test.go index 2a1723943..3b776a7df 100644 --- a/internal/ui/restore/text_test.go +++ b/internal/ui/restore/text_test.go @@ -22,20 +22,34 @@ func (m *mockTerm) SetStatus(lines []string) { func TestPrintUpdate(t *testing.T) { term := &mockTerm{} printer := NewTextProgress(term) - printer.Update(State{3, 11, 29, 47}, 5*time.Second) + printer.Update(State{3, 11, 0, 29, 47, 0}, 5*time.Second) test.Equals(t, []string{"[0:05] 61.70% 3 files/dirs 29 B, total 11 files/dirs 47 B"}, term.output) } +func TestPrintUpdateWithSkipped(t *testing.T) { + term := &mockTerm{} + printer := NewTextProgress(term) + printer.Update(State{3, 11, 2, 29, 47, 59}, 5*time.Second) + test.Equals(t, []string{"[0:05] 61.70% 3 files/dirs 29 B, total 11 files/dirs 47 B, skipped 2 files/dirs 59 B"}, term.output) +} + func TestPrintSummaryOnSuccess(t *testing.T) { term := &mockTerm{} printer := NewTextProgress(term) - printer.Finish(State{11, 11, 47, 47}, 5*time.Second) + printer.Finish(State{11, 11, 0, 47, 47, 0}, 5*time.Second) test.Equals(t, []string{"Summary: Restored 11 files/dirs (47 B) in 0:05"}, term.output) } func TestPrintSummaryOnErrors(t *testing.T) { term := &mockTerm{} printer := NewTextProgress(term) - printer.Finish(State{3, 11, 29, 47}, 5*time.Second) + printer.Finish(State{3, 11, 0, 29, 47, 0}, 5*time.Second) test.Equals(t, []string{"Summary: Restored 3 / 11 files/dirs (29 B / 47 B) in 0:05"}, term.output) } + +func TestPrintSummaryOnSuccessWithSkipped(t *testing.T) { + term := &mockTerm{} + printer := NewTextProgress(term) + printer.Finish(State{11, 11, 2, 47, 47, 59}, 5*time.Second) + test.Equals(t, []string{"Summary: Restored 11 files/dirs (47 B) in 0:05, skipped 2 files/dirs 59 B"}, term.output) +} From ba53a2abb5220a96b53598d77e9c45e960e7ae7e Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 15:30:30 +0200 Subject: [PATCH 11/14] test overwrite behavior --- internal/restorer/restorer_test.go | 89 ++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index 64e10a6e8..635c30ee6 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -893,3 +893,92 @@ func TestRestorerSparseFiles(t *testing.T) { t.Logf("wrote %d zeros as %d blocks, %.1f%% sparse", len(zeros), blocks, 100*sparsity) } + +func TestRestorerOverwriteBehavior(t *testing.T) { + baseTime := time.Now() + baseSnapshot := Snapshot{ + Nodes: map[string]Node{ + "foo": File{Data: "content: foo\n", ModTime: baseTime}, + "dirtest": Dir{ + Nodes: map[string]Node{ + "file": File{Data: "content: file\n", ModTime: baseTime}, + }, + ModTime: baseTime, + }, + }, + } + overwriteSnapshot := Snapshot{ + Nodes: map[string]Node{ + "foo": File{Data: "content: new\n", ModTime: baseTime.Add(time.Second)}, + "dirtest": Dir{ + Nodes: map[string]Node{ + "file": File{Data: "content: file2\n", ModTime: baseTime.Add(-time.Second)}, + }, + }, + }, + } + + var tests = []struct { + Overwrite OverwriteBehavior + Files map[string]string + }{ + { + Overwrite: OverwriteAlways, + Files: map[string]string{ + "foo": "content: new\n", + "dirtest/file": "content: file2\n", + }, + }, + { + Overwrite: OverwriteIfNewer, + Files: map[string]string{ + "foo": "content: new\n", + "dirtest/file": "content: file\n", + }, + }, + { + Overwrite: OverwriteNever, + Files: map[string]string{ + "foo": "content: foo\n", + "dirtest/file": "content: file\n", + }, + }, + } + + for _, test := range tests { + t.Run("", func(t *testing.T) { + repo := repository.TestRepository(t) + tempdir := filepath.Join(rtest.TempDir(t), "target") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // base snapshot + sn, id := saveSnapshot(t, repo, baseSnapshot, noopGetGenericAttributes) + t.Logf("base snapshot saved as %v", id.Str()) + + res := NewRestorer(repo, sn, Options{}) + rtest.OK(t, res.RestoreTo(ctx, tempdir)) + + // overwrite snapshot + sn, id = saveSnapshot(t, repo, overwriteSnapshot, noopGetGenericAttributes) + t.Logf("overwrite snapshot saved as %v", id.Str()) + res = NewRestorer(repo, sn, Options{Overwrite: test.Overwrite}) + rtest.OK(t, res.RestoreTo(ctx, tempdir)) + + _, err := res.VerifyFiles(ctx, tempdir) + rtest.OK(t, err) + + for filename, content := range test.Files { + data, err := os.ReadFile(filepath.Join(tempdir, filepath.FromSlash(filename))) + if err != nil { + t.Errorf("unable to read file %v: %v", filename, err) + continue + } + + if !bytes.Equal(data, []byte(content)) { + t.Errorf("file %v has wrong content: want %q, got %q", filename, content, data) + } + } + }) + } +} From e1ec60c2ee4eb66bcd69f4f745ebc1fa1b98ee38 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 16:11:18 +0200 Subject: [PATCH 12/14] document restore --overwrite --- doc/050_restore.rst | 9 +++++++++ doc/075_scripting.rst | 9 ++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/doc/050_restore.rst b/doc/050_restore.rst index ce17a1cf7..b5eb41349 100644 --- a/doc/050_restore.rst +++ b/doc/050_restore.rst @@ -85,6 +85,15 @@ disk space. Note that the exact location of the holes can differ from those in the original file, as their location is determined while restoring and is not stored explicitly. +Restoring in-place +------------------ + +By default, the ``restore`` command overwrites already existing files in the target +directory. This behavior can be configured via the ``--overwrite`` option. The +default is ``--overwrite always``. To only overwrite existing files if the file in +the snapshot is newer, use ``--overwrite if-newer``. To never overwrite existing files, +use ``--overwrite never``. + Restore using mount =================== diff --git a/doc/075_scripting.rst b/doc/075_scripting.rst index e413e349f..d40f7c976 100644 --- a/doc/075_scripting.rst +++ b/doc/075_scripting.rst @@ -502,11 +502,14 @@ Status +----------------------+------------------------------------------------------------+ |``files_restored`` | Files restored | +----------------------+------------------------------------------------------------+ +|``files_skipped`` | Files skipped due to overwrite setting | ++----------------------+------------------------------------------------------------+ |``total_bytes`` | Total number of bytes in restore set | +----------------------+------------------------------------------------------------+ |``bytes_restored`` | Number of bytes restored | +----------------------+------------------------------------------------------------+ - +|``bytes_skipped`` | Total size of skipped files | ++----------------------+------------------------------------------------------------+ Summary ^^^^^^^ @@ -520,10 +523,14 @@ Summary +----------------------+------------------------------------------------------------+ |``files_restored`` | Files restored | +----------------------+------------------------------------------------------------+ +|``files_skipped`` | Files skipped due to overwrite setting | ++----------------------+------------------------------------------------------------+ |``total_bytes`` | Total number of bytes in restore set | +----------------------+------------------------------------------------------------+ |``bytes_restored`` | Number of bytes restored | +----------------------+------------------------------------------------------------+ +|``bytes_skipped`` | Total size of skipped files | ++----------------------+------------------------------------------------------------+ snapshots From 105261e12e58b95e66f3bce5d5b52cad7f03c63c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 16:25:42 +0200 Subject: [PATCH 13/14] add changelog for restore --overwrite --- changelog/unreleased/issue-4817 | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changelog/unreleased/issue-4817 diff --git a/changelog/unreleased/issue-4817 b/changelog/unreleased/issue-4817 new file mode 100644 index 000000000..b8ffc35ca --- /dev/null +++ b/changelog/unreleased/issue-4817 @@ -0,0 +1,11 @@ +Enhancement: Make overwrite behavior of `restore` customizable + +The `restore` command now supports an `--overwrite` option to configure whether +already existing files are overwritten. The default is `--overwrite always`, +which overwrites existing files. `--overwrite if-newer` only restores files +from the snapshot that are newer than the local state. And `--overwrite never` +does not modify existing files. + +https://github.com/restic/restic/issues/4817 +https://github.com/restic/restic/issues/200 +https://github.com/restic/restic/pull/4837 From 7f7c995977b9dfd420cee3ce843ae40a171e661a Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 31 May 2024 18:30:51 +0200 Subject: [PATCH 14/14] fix linter warnings --- internal/restorer/restorer.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index ae622874b..84b786997 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -303,7 +303,7 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { idx.Add(node.Inode, node.DeviceID, location) } - return res.withOverwriteCheck(node, target, location, false, func() error { + return res.withOverwriteCheck(node, target, false, func() error { res.progress.AddFile(node.Size) filerestorer.addFile(location, node.Content, int64(node.Size)) res.trackFile(location) @@ -327,13 +327,13 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { visitNode: func(node *restic.Node, target, location string) error { debug.Log("second pass, visitNode: restore node %q", location) if node.Type != "file" { - return res.withOverwriteCheck(node, target, location, false, func() error { + return res.withOverwriteCheck(node, target, false, func() error { return res.restoreNodeTo(ctx, node, target, location) }) } if idx.Has(node.Inode, node.DeviceID) && idx.Value(node.Inode, node.DeviceID) != location { - return res.withOverwriteCheck(node, target, location, true, func() error { + return res.withOverwriteCheck(node, target, true, func() error { return res.restoreHardlinkAt(node, filerestorer.targetPath(idx.Value(node.Inode, node.DeviceID)), target, location) }) } @@ -364,7 +364,7 @@ func (res *Restorer) hasRestoredFile(location string) bool { return ok } -func (res *Restorer) withOverwriteCheck(node *restic.Node, target, location string, isHardlink bool, cb func() error) error { +func (res *Restorer) withOverwriteCheck(node *restic.Node, target string, isHardlink bool, cb func() error) error { overwrite, err := shouldOverwrite(res.overwrite, node, target) if err != nil { return err