From 31887ea9aaaf7a8a4766d48e972ebfd0d50d20dd Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 12 Jul 2024 22:24:35 +0200 Subject: [PATCH 01/11] restore: fix hang on command cancelation --- internal/restorer/filerestorer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index 56059cb16..d2a4ba068 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -192,6 +192,7 @@ func (r *fileRestorer) restoreFiles(ctx context.Context) error { // the main restore loop wg.Go(func() error { + defer close(downloadCh) for _, id := range packOrder { pack := packs[id] // allow garbage collection of packInfo @@ -203,7 +204,6 @@ func (r *fileRestorer) restoreFiles(ctx context.Context) error { debug.Log("Scheduled download pack %s", pack.id.Str()) } } - close(downloadCh) return nil }) From 40e5163114f2b164286bff0272de1aa1847c3172 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 12 Jul 2024 22:26:22 +0200 Subject: [PATCH 02/11] restore: properly cancel file verification --- internal/restorer/restorer.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index a47fd5ef6..29401e5e9 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -158,6 +158,10 @@ func (res *Restorer) traverseTreeInner(ctx context.Context, target, location str filenames = make([]string, 0, len(tree.Nodes)) } for i, node := range tree.Nodes { + if ctx.Err() != nil { + return nil, hasRestored, ctx.Err() + } + // allow GC of tree node tree.Nodes[i] = nil if res.opts.Delete { @@ -394,7 +398,7 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { idx.Add(node.Inode, node.DeviceID, location) } - buf, err = res.withOverwriteCheck(node, target, location, false, buf, func(updateMetadataOnly bool, matches *fileState) error { + buf, err = res.withOverwriteCheck(ctx, node, target, location, false, buf, func(updateMetadataOnly bool, matches *fileState) error { if updateMetadataOnly { res.opts.Progress.AddSkippedFile(location, node.Size) } else { @@ -434,14 +438,14 @@ 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" { - _, err := res.withOverwriteCheck(node, target, location, false, nil, func(_ bool, _ *fileState) error { + _, err := res.withOverwriteCheck(ctx, node, target, location, false, nil, func(_ bool, _ *fileState) error { return res.restoreNodeTo(ctx, node, target, location) }) return err } if idx.Has(node.Inode, node.DeviceID) && idx.Value(node.Inode, node.DeviceID) != location { - _, err := res.withOverwriteCheck(node, target, location, true, nil, func(_ bool, _ *fileState) error { + _, err := res.withOverwriteCheck(ctx, node, target, location, true, nil, func(_ bool, _ *fileState) error { return res.restoreHardlinkAt(node, filerestorer.targetPath(idx.Value(node.Inode, node.DeviceID)), target, location) }) return err @@ -528,7 +532,7 @@ func (res *Restorer) hasRestoredFile(location string) (metadataOnly bool, ok boo return metadataOnly, ok } -func (res *Restorer) withOverwriteCheck(node *restic.Node, target, location string, isHardlink bool, buf []byte, cb func(updateMetadataOnly bool, matches *fileState) error) ([]byte, error) { +func (res *Restorer) withOverwriteCheck(ctx context.Context, node *restic.Node, target, location string, isHardlink bool, buf []byte, cb func(updateMetadataOnly bool, matches *fileState) error) ([]byte, error) { overwrite, err := shouldOverwrite(res.opts.Overwrite, node, target) if err != nil { return buf, err @@ -545,7 +549,7 @@ func (res *Restorer) withOverwriteCheck(node *restic.Node, target, location stri updateMetadataOnly := false if node.Type == "file" && !isHardlink { // if a file fails to verify, then matches is nil which results in restoring from scratch - matches, buf, _ = res.verifyFile(target, node, false, res.opts.Overwrite == OverwriteIfChanged, buf) + matches, buf, _ = res.verifyFile(ctx, target, node, false, res.opts.Overwrite == OverwriteIfChanged, buf) // skip files that are already correct completely updateMetadataOnly = !matches.NeedsRestore() } @@ -628,7 +632,7 @@ func (res *Restorer) VerifyFiles(ctx context.Context, dst string) (int, error) { g.Go(func() (err error) { var buf []byte for job := range work { - _, buf, err = res.verifyFile(job.path, job.node, true, false, buf) + _, buf, err = res.verifyFile(ctx, job.path, job.node, true, false, buf) if err != nil { err = res.Error(job.path, err) } @@ -676,7 +680,7 @@ func (s *fileState) HasMatchingBlob(i int) bool { // buf and the first return value are scratch space, passed around for reuse. // Reusing buffers prevents the verifier goroutines allocating all of RAM and // flushing the filesystem cache (at least on Linux). -func (res *Restorer) verifyFile(target string, node *restic.Node, failFast bool, trustMtime bool, buf []byte) (*fileState, []byte, error) { +func (res *Restorer) verifyFile(ctx context.Context, target string, node *restic.Node, failFast bool, trustMtime bool, buf []byte) (*fileState, []byte, error) { f, err := fs.OpenFile(target, fs.O_RDONLY|fs.O_NOFOLLOW, 0) if err != nil { return nil, buf, err @@ -707,6 +711,9 @@ func (res *Restorer) verifyFile(target string, node *restic.Node, failFast bool, matches := make([]bool, len(node.Content)) var offset int64 for i, blobID := range node.Content { + if ctx.Err() != nil { + return nil, buf, ctx.Err() + } length, found := res.repo.LookupBlobSize(restic.DataBlob, blobID) if !found { return nil, buf, errors.Errorf("Unable to fetch blob %s", blobID) From 2971a769daa5b899151b0e6e78751aa3d5af89dc Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 12 Jul 2024 22:27:00 +0200 Subject: [PATCH 03/11] restore: fix corrupt restore of partially up to date files --- internal/restorer/filerestorer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index d2a4ba068..f0983e003 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -136,8 +136,8 @@ func (r *fileRestorer) restoreFiles(ctx context.Context) error { err := r.forEachBlob(fileBlobs, func(packID restic.ID, blob restic.Blob, idx int) { if largeFile && !file.state.HasMatchingBlob(idx) { packsMap[packID] = append(packsMap[packID], fileBlobInfo{id: blob.ID, offset: fileOffset}) - fileOffset += int64(blob.DataLength()) } + fileOffset += int64(blob.DataLength()) pack, ok := packs[packID] if !ok { pack = &packInfo{ From f64191da9c98feff632d665e6993104f6470cbd7 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 12 Jul 2024 22:27:32 +0200 Subject: [PATCH 04/11] restore: improve reporting of cancelation errors --- internal/restorer/filerestorer.go | 9 ++++-- internal/restorer/restorer.go | 51 ++++++++++++------------------- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index f0983e003..fec8c8780 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -273,10 +273,13 @@ func (r *fileRestorer) downloadPack(ctx context.Context, pack *packInfo) error { } func (r *fileRestorer) sanitizeError(file *fileInfo, err error) error { - if err != nil { - err = r.Error(file.location, err) + switch err { + case nil, context.Canceled, context.DeadlineExceeded: + // Context errors are permanent. + return err + default: + return r.Error(file.location, err) } - return err } func (r *fileRestorer) reportError(blobs blobToFileOffsetsMapping, processedBlobs restic.BlobSet, err error) error { diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 29401e5e9..9d5a5a114 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -115,22 +115,23 @@ type treeVisitor struct { leaveDir func(node *restic.Node, target, location string, entries []string) error } +func (res *Restorer) sanitizeError(location string, err error) error { + switch err { + case nil, context.Canceled, context.DeadlineExceeded: + // Context errors are permanent. + return err + default: + return res.Error(location, err) + } +} + // traverseTree traverses a tree from the repo and calls treeVisitor. // target is the path in the file system, location within the snapshot. func (res *Restorer) traverseTree(ctx context.Context, target string, treeID restic.ID, visitor treeVisitor) error { location := string(filepath.Separator) - sanitizeError := func(err error) error { - switch err { - case nil, context.Canceled, context.DeadlineExceeded: - // Context errors are permanent. - return err - default: - return res.Error(location, err) - } - } if visitor.enterDir != nil { - err := sanitizeError(visitor.enterDir(nil, target, location)) + err := res.sanitizeError(location, visitor.enterDir(nil, target, location)) if err != nil { return err } @@ -140,7 +141,7 @@ func (res *Restorer) traverseTree(ctx context.Context, target string, treeID res return err } if hasRestored && visitor.leaveDir != nil { - err = sanitizeError(visitor.leaveDir(nil, target, location, childFilenames)) + err = res.sanitizeError(location, visitor.leaveDir(nil, target, location, childFilenames)) } return err @@ -151,7 +152,7 @@ func (res *Restorer) traverseTreeInner(ctx context.Context, target, location str tree, err := restic.LoadTree(ctx, res.repo, treeID) if err != nil { debug.Log("error loading tree %v: %v", treeID, err) - return nil, hasRestored, res.Error(location, err) + return nil, hasRestored, res.sanitizeError(location, err) } if res.opts.Delete { @@ -175,7 +176,7 @@ func (res *Restorer) traverseTreeInner(ctx context.Context, target, location str nodeName := filepath.Base(filepath.Join(string(filepath.Separator), node.Name)) if nodeName != node.Name { debug.Log("node %q has invalid name %q", node.Name, nodeName) - err := res.Error(location, errors.Errorf("invalid child node name %s", node.Name)) + err := res.sanitizeError(location, errors.Errorf("invalid child node name %s", node.Name)) if err != nil { return nil, hasRestored, err } @@ -190,7 +191,7 @@ func (res *Restorer) traverseTreeInner(ctx context.Context, target, location str if target == nodeTarget || !fs.HasPathPrefix(target, nodeTarget) { debug.Log("target: %v %v", target, nodeTarget) debug.Log("node %q has invalid target path %q", node.Name, nodeTarget) - err := res.Error(nodeLocation, errors.New("node has invalid path")) + err := res.sanitizeError(nodeLocation, errors.New("node has invalid path")) if err != nil { return nil, hasRestored, err } @@ -211,23 +212,13 @@ func (res *Restorer) traverseTreeInner(ctx context.Context, target, location str hasRestored = true } - sanitizeError := func(err error) error { - switch err { - case nil, context.Canceled, context.DeadlineExceeded: - // Context errors are permanent. - return err - default: - return res.Error(nodeLocation, err) - } - } - if node.Type == "dir" { if node.Subtree == nil { return nil, hasRestored, errors.Errorf("Dir without subtree in tree %v", treeID.Str()) } if selectedForRestore && visitor.enterDir != nil { - err = sanitizeError(visitor.enterDir(node, nodeTarget, nodeLocation)) + err = res.sanitizeError(nodeLocation, visitor.enterDir(node, nodeTarget, nodeLocation)) if err != nil { return nil, hasRestored, err } @@ -240,7 +231,7 @@ func (res *Restorer) traverseTreeInner(ctx context.Context, target, location str if childMayBeSelected { childFilenames, childHasRestored, err = res.traverseTreeInner(ctx, nodeTarget, nodeLocation, *node.Subtree, visitor) - err = sanitizeError(err) + err = res.sanitizeError(nodeLocation, err) if err != nil { return nil, hasRestored, err } @@ -253,7 +244,7 @@ func (res *Restorer) traverseTreeInner(ctx context.Context, target, location str // metadata need to be restore when leaving the directory in both cases // selected for restore or any child of any subtree have been restored if (selectedForRestore || childHasRestored) && visitor.leaveDir != nil { - err = sanitizeError(visitor.leaveDir(node, nodeTarget, nodeLocation, childFilenames)) + err = res.sanitizeError(nodeLocation, visitor.leaveDir(node, nodeTarget, nodeLocation, childFilenames)) if err != nil { return nil, hasRestored, err } @@ -263,7 +254,7 @@ func (res *Restorer) traverseTreeInner(ctx context.Context, target, location str } if selectedForRestore { - err = sanitizeError(visitor.visitNode(node, nodeTarget, nodeLocation)) + err = res.sanitizeError(nodeLocation, visitor.visitNode(node, nodeTarget, nodeLocation)) if err != nil { return nil, hasRestored, err } @@ -633,9 +624,7 @@ func (res *Restorer) VerifyFiles(ctx context.Context, dst string) (int, error) { var buf []byte for job := range work { _, buf, err = res.verifyFile(ctx, job.path, job.node, true, false, buf) - if err != nil { - err = res.Error(job.path, err) - } + err = res.sanitizeError(job.path, err) if err != nil || ctx.Err() != nil { break } From c77b2d5ca20a7b49c7302d62a5970a720a7696bf Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 12 Jul 2024 22:29:56 +0200 Subject: [PATCH 05/11] restore: avoid long cancelation delay for frequently used blobs --- internal/restorer/filerestorer.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index fec8c8780..01d3e43f7 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -327,6 +327,11 @@ func (r *fileRestorer) downloadBlobs(ctx context.Context, packID restic.ID, } for file, offsets := range blob.files { for _, offset := range offsets { + // avoid long cancelation delays for frequently used blobs + if ctx.Err() != nil { + return ctx.Err() + } + writeToFile := func() error { // this looks overly complicated and needs explanation // two competing requirements: From 2833b2f6995d3a17b9c91638519ba028bd39f04a Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 12 Jul 2024 22:59:01 +0200 Subject: [PATCH 06/11] restore: fix progress bar for partially up to date files --- internal/restorer/filerestorer.go | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index 01d3e43f7..648633361 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -134,8 +134,12 @@ func (r *fileRestorer) restoreFiles(ctx context.Context) error { } fileOffset := int64(0) err := r.forEachBlob(fileBlobs, func(packID restic.ID, blob restic.Blob, idx int) { - if largeFile && !file.state.HasMatchingBlob(idx) { - packsMap[packID] = append(packsMap[packID], fileBlobInfo{id: blob.ID, offset: fileOffset}) + if largeFile { + if !file.state.HasMatchingBlob(idx) { + packsMap[packID] = append(packsMap[packID], fileBlobInfo{id: blob.ID, offset: fileOffset}) + } else { + r.reportBlobProgress(file, uint64(blob.DataLength())) + } } fileOffset += int64(blob.DataLength()) pack, ok := packs[packID] @@ -244,8 +248,12 @@ func (r *fileRestorer) downloadPack(ctx context.Context, pack *packInfo) error { if fileBlobs, ok := file.blobs.(restic.IDs); ok { fileOffset := int64(0) err := r.forEachBlob(fileBlobs, func(packID restic.ID, blob restic.Blob, idx int) { - if packID.Equal(pack.id) && !file.state.HasMatchingBlob(idx) { - addBlob(blob, fileOffset) + if packID.Equal(pack.id) { + if !file.state.HasMatchingBlob(idx) { + addBlob(blob, fileOffset) + } else { + r.reportBlobProgress(file, uint64(blob.DataLength())) + } } fileOffset += int64(blob.DataLength()) }) @@ -349,11 +357,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) - action := restore.ActionFileUpdated - if file.state == nil { - action = restore.ActionFileRestored - } - r.progress.AddProgress(file.location, action, uint64(len(blobData)), uint64(file.size)) + r.reportBlobProgress(file, uint64(len(blobData))) return writeErr } err := r.sanitizeError(file, writeToFile()) @@ -365,3 +369,11 @@ func (r *fileRestorer) downloadBlobs(ctx context.Context, packID restic.ID, return nil }) } + +func (r *fileRestorer) reportBlobProgress(file *fileInfo, blobSize uint64) { + action := restore.ActionFileUpdated + if file.state == nil { + action = restore.ActionFileRestored + } + r.progress.AddProgress(file.location, action, uint64(blobSize), uint64(file.size)) +} From 26aa65e0d40baa2f8344597d0119d6cf9f6dba0d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 12 Jul 2024 23:14:03 +0200 Subject: [PATCH 07/11] restore: add regression test for corrupt in-place restore of large file --- internal/restorer/restorer_test.go | 49 ++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index 2f28265cc..cd2e954e5 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "io" "math" "os" @@ -32,6 +33,7 @@ type Snapshot struct { type File struct { Data string + DataParts []string Links uint64 Inode uint64 Mode os.FileMode @@ -59,11 +61,11 @@ type FileAttributes struct { Encrypted bool } -func saveFile(t testing.TB, repo restic.BlobSaver, node File) restic.ID { +func saveFile(t testing.TB, repo restic.BlobSaver, data string) restic.ID { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - id, _, _, err := repo.SaveBlob(ctx, restic.DataBlob, []byte(node.Data), restic.ID{}, false) + id, _, _, err := repo.SaveBlob(ctx, restic.DataBlob, []byte(data), restic.ID{}, false) if err != nil { t.Fatal(err) } @@ -80,17 +82,24 @@ func saveDir(t testing.TB, repo restic.BlobSaver, nodes map[string]Node, inode u inode++ switch node := n.(type) { case File: - fi := n.(File).Inode + fi := node.Inode if fi == 0 { fi = inode } - lc := n.(File).Links + lc := node.Links if lc == 0 { lc = 1 } fc := []restic.ID{} - if len(n.(File).Data) > 0 { - fc = append(fc, saveFile(t, repo, node)) + size := 0 + if len(node.Data) > 0 { + size = len(node.Data) + fc = append(fc, saveFile(t, repo, node.Data)) + } else if len(node.DataParts) > 0 { + for _, part := range node.DataParts { + fc = append(fc, saveFile(t, repo, part)) + size += len(part) + } } mode := node.Mode if mode == 0 { @@ -104,22 +113,21 @@ func saveDir(t testing.TB, repo restic.BlobSaver, nodes map[string]Node, inode u UID: uint32(os.Getuid()), GID: uint32(os.Getgid()), Content: fc, - Size: uint64(len(n.(File).Data)), + Size: uint64(size), Inode: fi, Links: lc, GenericAttributes: getGenericAttributes(node.attributes, false), }) rtest.OK(t, err) case Symlink: - symlink := n.(Symlink) err := tree.Insert(&restic.Node{ Type: "symlink", Mode: os.ModeSymlink | 0o777, - ModTime: symlink.ModTime, + ModTime: node.ModTime, Name: name, UID: uint32(os.Getuid()), GID: uint32(os.Getgid()), - LinkTarget: symlink.Target, + LinkTarget: node.Target, Inode: inode, Links: 1, }) @@ -1050,6 +1058,27 @@ func TestRestorerOverwriteBehavior(t *testing.T) { } } +func TestRestorerOverwriteLarge(t *testing.T) { + parts := make([]string, 100) + for i := 0; i < len(parts); i++ { + parts[i] = fmt.Sprint(i) + } + + baseTime := time.Now() + baseSnapshot := Snapshot{ + Nodes: map[string]Node{ + "foo": File{DataParts: parts[0:5], ModTime: baseTime}, + }, + } + overwriteSnapshot := Snapshot{ + Nodes: map[string]Node{ + "foo": File{DataParts: parts, ModTime: baseTime}, + }, + } + + saveSnapshotsAndOverwrite(t, baseSnapshot, overwriteSnapshot, Options{Overwrite: OverwriteAlways}) +} + func TestRestorerOverwriteSpecial(t *testing.T) { baseTime := time.Now() baseSnapshot := Snapshot{ From 98cfb2c4c8abd199a6e4b6a17c4396af25cef627 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 12 Jul 2024 23:26:18 +0200 Subject: [PATCH 08/11] restore: test progress reporting for partially up to date files --- internal/restorer/restorer_test.go | 81 ++++++++++++++++++++++--- internal/restorer/restorer_unix_test.go | 12 ---- 2 files changed, 74 insertions(+), 19 deletions(-) diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index cd2e954e5..c809c9e20 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -22,6 +22,7 @@ import ( "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" + restoreui "github.com/restic/restic/internal/ui/restore" "golang.org/x/sync/errgroup" ) @@ -940,7 +941,7 @@ func TestRestorerSparseFiles(t *testing.T) { len(zeros), blocks, 100*sparsity) } -func saveSnapshotsAndOverwrite(t *testing.T, baseSnapshot Snapshot, overwriteSnapshot Snapshot, options Options) string { +func saveSnapshotsAndOverwrite(t *testing.T, baseSnapshot Snapshot, overwriteSnapshot Snapshot, baseOptions, overwriteOptions Options) string { repo := repository.TestRepository(t) tempdir := filepath.Join(rtest.TempDir(t), "target") ctx, cancel := context.WithCancel(context.Background()) @@ -950,13 +951,13 @@ func saveSnapshotsAndOverwrite(t *testing.T, baseSnapshot Snapshot, overwriteSna sn, id := saveSnapshot(t, repo, baseSnapshot, noopGetGenericAttributes) t.Logf("base snapshot saved as %v", id.Str()) - res := NewRestorer(repo, sn, options) + res := NewRestorer(repo, sn, baseOptions) 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) + res = NewRestorer(repo, sn, overwriteOptions) rtest.OK(t, res.RestoreTo(ctx, tempdir)) _, err := res.VerifyFiles(ctx, tempdir) @@ -978,7 +979,20 @@ func TestRestorerSparseOverwrite(t *testing.T) { }, } - saveSnapshotsAndOverwrite(t, baseSnapshot, sparseSnapshot, Options{Sparse: true, Overwrite: OverwriteAlways}) + opts := Options{Sparse: true, Overwrite: OverwriteAlways} + saveSnapshotsAndOverwrite(t, baseSnapshot, sparseSnapshot, opts, opts) +} + +type printerMock struct { + s restoreui.State +} + +func (p *printerMock) Update(_ restoreui.State, _ time.Duration) { +} +func (p *printerMock) CompleteItem(action restoreui.ItemAction, item string, size uint64) { +} +func (p *printerMock) Finish(s restoreui.State, _ time.Duration) { + p.s = s } func TestRestorerOverwriteBehavior(t *testing.T) { @@ -1008,6 +1022,7 @@ func TestRestorerOverwriteBehavior(t *testing.T) { var tests = []struct { Overwrite OverwriteBehavior Files map[string]string + Progress restoreui.State }{ { Overwrite: OverwriteAlways, @@ -1015,6 +1030,14 @@ func TestRestorerOverwriteBehavior(t *testing.T) { "foo": "content: new\n", "dirtest/file": "content: file2\n", }, + Progress: restoreui.State{ + FilesFinished: 3, + FilesTotal: 3, + FilesSkipped: 0, + AllBytesWritten: 28, + AllBytesTotal: 28, + AllBytesSkipped: 0, + }, }, { Overwrite: OverwriteIfChanged, @@ -1022,6 +1045,14 @@ func TestRestorerOverwriteBehavior(t *testing.T) { "foo": "content: new\n", "dirtest/file": "content: file2\n", }, + Progress: restoreui.State{ + FilesFinished: 3, + FilesTotal: 3, + FilesSkipped: 0, + AllBytesWritten: 28, + AllBytesTotal: 28, + AllBytesSkipped: 0, + }, }, { Overwrite: OverwriteIfNewer, @@ -1029,6 +1060,14 @@ func TestRestorerOverwriteBehavior(t *testing.T) { "foo": "content: new\n", "dirtest/file": "content: file\n", }, + Progress: restoreui.State{ + FilesFinished: 2, + FilesTotal: 2, + FilesSkipped: 1, + AllBytesWritten: 13, + AllBytesTotal: 13, + AllBytesSkipped: 15, + }, }, { Overwrite: OverwriteNever, @@ -1036,12 +1075,22 @@ func TestRestorerOverwriteBehavior(t *testing.T) { "foo": "content: foo\n", "dirtest/file": "content: file\n", }, + Progress: restoreui.State{ + FilesFinished: 1, + FilesTotal: 1, + FilesSkipped: 2, + AllBytesWritten: 0, + AllBytesTotal: 0, + AllBytesSkipped: 28, + }, }, } for _, test := range tests { t.Run("", func(t *testing.T) { - tempdir := saveSnapshotsAndOverwrite(t, baseSnapshot, overwriteSnapshot, Options{Overwrite: test.Overwrite}) + mock := &printerMock{} + progress := restoreui.NewProgress(mock, 0) + tempdir := saveSnapshotsAndOverwrite(t, baseSnapshot, overwriteSnapshot, Options{}, Options{Overwrite: test.Overwrite, Progress: progress}) for filename, content := range test.Files { data, err := os.ReadFile(filepath.Join(tempdir, filepath.FromSlash(filename))) @@ -1054,14 +1103,19 @@ func TestRestorerOverwriteBehavior(t *testing.T) { t.Errorf("file %v has wrong content: want %q, got %q", filename, content, data) } } + + progress.Finish() + rtest.Equals(t, test.Progress, mock.s) }) } } func TestRestorerOverwriteLarge(t *testing.T) { parts := make([]string, 100) + size := 0 for i := 0; i < len(parts); i++ { parts[i] = fmt.Sprint(i) + size += len(parts[i]) } baseTime := time.Now() @@ -1076,7 +1130,18 @@ func TestRestorerOverwriteLarge(t *testing.T) { }, } - saveSnapshotsAndOverwrite(t, baseSnapshot, overwriteSnapshot, Options{Overwrite: OverwriteAlways}) + mock := &printerMock{} + progress := restoreui.NewProgress(mock, 0) + saveSnapshotsAndOverwrite(t, baseSnapshot, overwriteSnapshot, Options{}, Options{Overwrite: OverwriteAlways, Progress: progress}) + progress.Finish() + rtest.Equals(t, restoreui.State{ + FilesFinished: 1, + FilesTotal: 1, + FilesSkipped: 0, + AllBytesWritten: uint64(size), + AllBytesTotal: uint64(size), + AllBytesSkipped: 0, + }, mock.s) } func TestRestorerOverwriteSpecial(t *testing.T) { @@ -1109,7 +1174,8 @@ func TestRestorerOverwriteSpecial(t *testing.T) { "file": "foo2", } - tempdir := saveSnapshotsAndOverwrite(t, baseSnapshot, overwriteSnapshot, Options{Overwrite: OverwriteAlways}) + opts := Options{Overwrite: OverwriteAlways} + tempdir := saveSnapshotsAndOverwrite(t, baseSnapshot, overwriteSnapshot, opts, opts) for filename, content := range files { data, err := os.ReadFile(filepath.Join(tempdir, filepath.FromSlash(filename))) @@ -1286,6 +1352,7 @@ func TestRestoreOverwriteDirectory(t *testing.T) { "dir": File{Data: "content: file\n"}, }, }, + Options{}, Options{Delete: true}, ) } diff --git a/internal/restorer/restorer_unix_test.go b/internal/restorer/restorer_unix_test.go index febd43ace..27d990af4 100644 --- a/internal/restorer/restorer_unix_test.go +++ b/internal/restorer/restorer_unix_test.go @@ -65,18 +65,6 @@ func getBlockCount(t *testing.T, filename string) int64 { return st.Blocks } -type printerMock struct { - s restoreui.State -} - -func (p *printerMock) Update(_ restoreui.State, _ time.Duration) { -} -func (p *printerMock) CompleteItem(action restoreui.ItemAction, item string, size uint64) { -} -func (p *printerMock) Finish(s restoreui.State, _ time.Duration) { - p.s = s -} - func TestRestorerProgressBar(t *testing.T) { testRestorerProgressBar(t, false) } From 4a9536b829bdb66dc24cebb90fcd1180b7b453a0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 12 Jul 2024 23:31:54 +0200 Subject: [PATCH 09/11] amend restore overwrite changelog --- changelog/unreleased/issue-4817 | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog/unreleased/issue-4817 b/changelog/unreleased/issue-4817 index e9c2d01a5..c1d5f658d 100644 --- a/changelog/unreleased/issue-4817 +++ b/changelog/unreleased/issue-4817 @@ -21,3 +21,4 @@ https://github.com/restic/restic/issues/2662 https://github.com/restic/restic/pull/4837 https://github.com/restic/restic/pull/4838 https://github.com/restic/restic/pull/4864 +https://github.com/restic/restic/pull/4921 From 44e3610b3279ffd984eac367c76d73f4d427575c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 13 Jul 2024 00:02:17 +0200 Subject: [PATCH 10/11] restore: progress bar total on windows --- internal/restorer/restorer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 9d5a5a114..3f41b79a6 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -363,7 +363,7 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { err = res.traverseTree(ctx, dst, *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 location != "/" { + if location != string(filepath.Separator) { res.opts.Progress.AddFile(0) } return res.ensureDir(target) From dcfffd77799f5ac027fd814d030811bf8e0390aa Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 14 Jul 2024 11:30:41 +0200 Subject: [PATCH 11/11] restore: extend overwrite test for small files --- internal/restorer/restorer_test.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index c809c9e20..f8f6f92c0 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -1110,23 +1110,33 @@ func TestRestorerOverwriteBehavior(t *testing.T) { } } -func TestRestorerOverwriteLarge(t *testing.T) { +func TestRestorerOverwritePartial(t *testing.T) { parts := make([]string, 100) size := 0 for i := 0; i < len(parts); i++ { parts[i] = fmt.Sprint(i) size += len(parts[i]) + if i < 8 { + // small file + size += len(parts[i]) + } } + // the data of both snapshots is stored in different pack files + // thus both small an foo in the overwriteSnapshot contain blobs from + // two different pack files. This tests basic handling of blobs from + // different pack files. baseTime := time.Now() baseSnapshot := Snapshot{ Nodes: map[string]Node{ - "foo": File{DataParts: parts[0:5], ModTime: baseTime}, + "foo": File{DataParts: parts[0:5], ModTime: baseTime}, + "small": File{DataParts: parts[0:5], ModTime: baseTime}, }, } overwriteSnapshot := Snapshot{ Nodes: map[string]Node{ - "foo": File{DataParts: parts, ModTime: baseTime}, + "foo": File{DataParts: parts, ModTime: baseTime}, + "small": File{DataParts: parts[0:8], ModTime: baseTime}, }, } @@ -1135,8 +1145,8 @@ func TestRestorerOverwriteLarge(t *testing.T) { saveSnapshotsAndOverwrite(t, baseSnapshot, overwriteSnapshot, Options{}, Options{Overwrite: OverwriteAlways, Progress: progress}) progress.Finish() rtest.Equals(t, restoreui.State{ - FilesFinished: 1, - FilesTotal: 1, + FilesFinished: 2, + FilesTotal: 2, FilesSkipped: 0, AllBytesWritten: uint64(size), AllBytesTotal: uint64(size),