From 0256f95994fabc670f57e6b4dfa4ea8a1c9bf417 Mon Sep 17 00:00:00 2001 From: DRON-666 <64691982+DRON-666@users.noreply.github.com> Date: Tue, 10 Nov 2020 01:22:27 +0300 Subject: [PATCH 1/8] dump: Split tar and walk logic --- cmd/restic/cmd_dump.go | 17 ++++--- internal/dump/common.go | 104 ++++++++++++++++++++++++++++++++++++++++ internal/dump/tar.go | 101 ++++++-------------------------------- 3 files changed, 127 insertions(+), 95 deletions(-) create mode 100644 internal/dump/common.go diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index d3ca87923..7875baa89 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -65,8 +65,7 @@ func splitPath(p string) []string { return append(s, f) } -func printFromTree(ctx context.Context, tree *restic.Tree, repo restic.Repository, prefix string, pathComponents []string) error { - +func printFromTree(ctx context.Context, tree *restic.Tree, repo restic.Repository, prefix string, pathComponents []string, writeDump dump.WriteDump) error { if tree == nil { return fmt.Errorf("called with a nil tree") } @@ -81,10 +80,10 @@ func printFromTree(ctx context.Context, tree *restic.Tree, repo restic.Repositor // If we print / we need to assume that there are multiple nodes at that // level in the tree. if pathComponents[0] == "" { - if err := checkStdoutTar(); err != nil { + if err := checkStdoutArchive(); err != nil { return err } - return dump.WriteTar(ctx, repo, tree, "/", os.Stdout) + return writeDump(ctx, repo, tree, "/", os.Stdout) } item := filepath.Join(prefix, pathComponents[0]) @@ -100,16 +99,16 @@ func printFromTree(ctx context.Context, tree *restic.Tree, repo restic.Repositor if err != nil { return errors.Wrapf(err, "cannot load subtree for %q", item) } - return printFromTree(ctx, subtree, repo, item, pathComponents[1:]) + return printFromTree(ctx, subtree, repo, item, pathComponents[1:], writeDump) case dump.IsDir(node): - if err := checkStdoutTar(); err != nil { + if err := checkStdoutArchive(); err != nil { return err } subtree, err := repo.LoadTree(ctx, *node.Subtree) if err != nil { return err } - return dump.WriteTar(ctx, repo, subtree, item, os.Stdout) + return writeDump(ctx, repo, subtree, item, os.Stdout) case l > 1: return fmt.Errorf("%q should be a dir, but is a %q", item, node.Type) case !dump.IsFile(node): @@ -176,7 +175,7 @@ func runDump(opts DumpOptions, gopts GlobalOptions, args []string) error { Exitf(2, "loading tree for snapshot %q failed: %v", snapshotIDString, err) } - err = printFromTree(ctx, tree, repo, "/", splittedPath) + err = printFromTree(ctx, tree, repo, "/", splittedPath, dump.WriteTar) if err != nil { Exitf(2, "cannot dump file: %v", err) } @@ -184,7 +183,7 @@ func runDump(opts DumpOptions, gopts GlobalOptions, args []string) error { return nil } -func checkStdoutTar() error { +func checkStdoutArchive() error { if stdoutIsTerminal() { return fmt.Errorf("stdout is the terminal, please redirect output") } diff --git a/internal/dump/common.go b/internal/dump/common.go new file mode 100644 index 000000000..5b3d82068 --- /dev/null +++ b/internal/dump/common.go @@ -0,0 +1,104 @@ +package dump + +import ( + "context" + "io" + "path" + + "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/restic" + "github.com/restic/restic/internal/walker" +) + +// dumper implements saving node data. +type dumper interface { + dumpNode(ctx context.Context, node *restic.Node, repo restic.Repository) error +} + +// WriteDump will write the contents of the given tree to the given destination. +// It will loop over all nodes in the tree and dump them recursively. +type WriteDump func(ctx context.Context, repo restic.Repository, tree *restic.Tree, rootPath string, dst io.Writer) error + +func writeDump(ctx context.Context, repo restic.Repository, tree *restic.Tree, rootPath string, dmp dumper, dst io.Writer) error { + for _, rootNode := range tree.Nodes { + rootNode.Path = rootPath + err := dumpTree(ctx, repo, rootNode, rootPath, dmp) + if err != nil { + return err + } + } + + return nil +} + +func dumpTree(ctx context.Context, repo restic.Repository, rootNode *restic.Node, rootPath string, dmp dumper) error { + rootNode.Path = path.Join(rootNode.Path, rootNode.Name) + rootPath = rootNode.Path + + if err := dmp.dumpNode(ctx, rootNode, repo); err != nil { + return err + } + + // If this is no directory we are finished + if !IsDir(rootNode) { + return nil + } + + err := walker.Walk(ctx, repo, *rootNode.Subtree, nil, func(_ restic.ID, nodepath string, node *restic.Node, err error) (bool, error) { + if err != nil { + return false, err + } + if node == nil { + return false, nil + } + + node.Path = path.Join(rootPath, nodepath) + + if IsFile(node) || IsLink(node) || IsDir(node) { + err := dmp.dumpNode(ctx, node, repo) + if err != nil { + return false, err + } + } + + return false, nil + }) + + return err +} + +// GetNodeData will write the contents of the node to the given output. +func GetNodeData(ctx context.Context, output io.Writer, repo restic.Repository, node *restic.Node) error { + var ( + buf []byte + err error + ) + for _, id := range node.Content { + buf, err = repo.LoadBlob(ctx, restic.DataBlob, id, buf) + if err != nil { + return err + } + + _, err = output.Write(buf) + if err != nil { + return errors.Wrap(err, "Write") + } + } + + return nil +} + +// IsDir checks if the given node is a directory. +func IsDir(node *restic.Node) bool { + return node.Type == "dir" +} + +// IsLink checks if the given node as a link. +func IsLink(node *restic.Node) bool { + return node.Type == "symlink" +} + +// IsFile checks if the given node is a file. +func IsFile(node *restic.Node) bool { + return node.Type == "file" +} diff --git a/internal/dump/tar.go b/internal/dump/tar.go index 816a0ffcf..02fa8e185 100644 --- a/internal/dump/tar.go +++ b/internal/dump/tar.go @@ -5,65 +5,31 @@ import ( "context" "io" "os" - "path" "path/filepath" "strings" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" - "github.com/restic/restic/internal/walker" ) -// WriteTar will write the contents of the given tree, encoded as a tar to the given destination. -// It will loop over all nodes in the tree and dump them recursively. -func WriteTar(ctx context.Context, repo restic.Repository, tree *restic.Tree, rootPath string, dst io.Writer) error { - tw := tar.NewWriter(dst) - - for _, rootNode := range tree.Nodes { - rootNode.Path = rootPath - err := tarTree(ctx, repo, rootNode, rootPath, tw) - if err != nil { - _ = tw.Close() - return err - } - } - return tw.Close() +type tarDumper struct { + w *tar.Writer } -func tarTree(ctx context.Context, repo restic.Repository, rootNode *restic.Node, rootPath string, tw *tar.Writer) error { - rootNode.Path = path.Join(rootNode.Path, rootNode.Name) - rootPath = rootNode.Path +// Statically ensure that tarDumper implements dumper. +var _ dumper = tarDumper{} - if err := tarNode(ctx, tw, rootNode, repo); err != nil { +// WriteTar will write the contents of the given tree, encoded as a tar to the given destination. +func WriteTar(ctx context.Context, repo restic.Repository, tree *restic.Tree, rootPath string, dst io.Writer) error { + dmp := tarDumper{w: tar.NewWriter(dst)} + + err := writeDump(ctx, repo, tree, rootPath, dmp, dst) + if err != nil { + dmp.w.Close() return err } - // If this is no directory we are finished - if !IsDir(rootNode) { - return nil - } - - err := walker.Walk(ctx, repo, *rootNode.Subtree, nil, func(_ restic.ID, nodepath string, node *restic.Node, err error) (bool, error) { - if err != nil { - return false, err - } - if node == nil { - return false, nil - } - - node.Path = path.Join(rootPath, nodepath) - - if IsFile(node) || IsLink(node) || IsDir(node) { - err := tarNode(ctx, tw, node, repo) - if err != nil { - return false, err - } - } - - return false, nil - }) - - return err + return dmp.w.Close() } // copied from archive/tar.FileInfoHeader @@ -75,7 +41,7 @@ const ( c_ISVTX = 01000 // Save text (sticky bit) ) -func tarNode(ctx context.Context, tw *tar.Writer, node *restic.Node, repo restic.Repository) error { +func (dmp tarDumper) dumpNode(ctx context.Context, node *restic.Node, repo restic.Repository) error { relPath, err := filepath.Rel("/", node.Path) if err != nil { return err @@ -120,13 +86,13 @@ func tarNode(ctx context.Context, tw *tar.Writer, node *restic.Node, repo restic header.Name += "/" } - err = tw.WriteHeader(header) + err = dmp.w.WriteHeader(header) if err != nil { return errors.Wrap(err, "TarHeader ") } - return GetNodeData(ctx, tw, repo, node) + return GetNodeData(ctx, dmp.w, repo, node) } func parseXattrs(xattrs []restic.ExtendedAttribute) map[string]string { @@ -146,7 +112,6 @@ func parseXattrs(xattrs []restic.ExtendedAttribute) map[string]string { tmpMap["SCHILY.acl.default"] = na.String() } } - } else { tmpMap["SCHILY.xattr."+attr.Name] = attrString } @@ -154,39 +119,3 @@ func parseXattrs(xattrs []restic.ExtendedAttribute) map[string]string { return tmpMap } - -// GetNodeData will write the contents of the node to the given output -func GetNodeData(ctx context.Context, output io.Writer, repo restic.Repository, node *restic.Node) error { - var ( - buf []byte - err error - ) - for _, id := range node.Content { - buf, err = repo.LoadBlob(ctx, restic.DataBlob, id, buf) - if err != nil { - return err - } - - _, err = output.Write(buf) - if err != nil { - return errors.Wrap(err, "Write") - } - - } - return nil -} - -// IsDir checks if the given node is a directory -func IsDir(node *restic.Node) bool { - return node.Type == "dir" -} - -// IsLink checks if the given node as a link -func IsLink(node *restic.Node) bool { - return node.Type == "symlink" -} - -// IsFile checks if the given node is a file -func IsFile(node *restic.Node) bool { - return node.Type == "file" -} From 89ab6d557e01985035d299d411b571614591a68b Mon Sep 17 00:00:00 2001 From: DRON-666 <64691982+DRON-666@users.noreply.github.com> Date: Tue, 10 Nov 2020 03:44:03 +0300 Subject: [PATCH 2/8] dump: Add zip dumper --- internal/dump/zip.go | 64 +++++++++++++ internal/dump/zip_test.go | 195 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 259 insertions(+) create mode 100644 internal/dump/zip.go create mode 100644 internal/dump/zip_test.go diff --git a/internal/dump/zip.go b/internal/dump/zip.go new file mode 100644 index 000000000..1c1035b08 --- /dev/null +++ b/internal/dump/zip.go @@ -0,0 +1,64 @@ +package dump + +import ( + "archive/zip" + "context" + "io" + "path/filepath" + + "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/restic" +) + +type zipDumper struct { + w *zip.Writer +} + +// Statically ensure that zipDumper implements dumper. +var _ dumper = tarDumper{} + +// WriteZip will write the contents of the given tree, encoded as a zip to the given destination. +func WriteZip(ctx context.Context, repo restic.Repository, tree *restic.Tree, rootPath string, dst io.Writer) error { + dmp := zipDumper{w: zip.NewWriter(dst)} + + err := writeDump(ctx, repo, tree, rootPath, dmp, dst) + if err != nil { + dmp.w.Close() + return err + } + + return dmp.w.Close() +} + +func (dmp zipDumper) dumpNode(ctx context.Context, node *restic.Node, repo restic.Repository) error { + relPath, err := filepath.Rel("/", node.Path) + if err != nil { + return err + } + + header := &zip.FileHeader{ + Name: filepath.ToSlash(relPath), + UncompressedSize64: node.Size, + Modified: node.ModTime, + } + header.SetMode(node.Mode) + + if IsDir(node) { + header.Name += "/" + } + + w, err := dmp.w.CreateHeader(header) + if err != nil { + return errors.Wrap(err, "ZipHeader ") + } + + if IsLink(node) { + if _, err = w.Write([]byte(node.LinkTarget)); err != nil { + return errors.Wrap(err, "Write") + } + + return nil + } + + return GetNodeData(ctx, w, repo, node) +} diff --git a/internal/dump/zip_test.go b/internal/dump/zip_test.go new file mode 100644 index 000000000..70ad00275 --- /dev/null +++ b/internal/dump/zip_test.go @@ -0,0 +1,195 @@ +package dump + +import ( + "archive/zip" + "bytes" + "context" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/restic/restic/internal/archiver" + "github.com/restic/restic/internal/fs" + rtest "github.com/restic/restic/internal/test" +) + +func TestWriteZip(t *testing.T) { + tests := []struct { + name string + args archiver.TestDir + target string + }{ + { + name: "single file in root", + args: archiver.TestDir{ + "file": archiver.TestFile{Content: "string"}, + }, + target: "/", + }, + { + name: "multiple files in root", + args: archiver.TestDir{ + "file1": archiver.TestFile{Content: "string"}, + "file2": archiver.TestFile{Content: "string"}, + }, + target: "/", + }, + { + name: "multiple files and folders in root", + args: archiver.TestDir{ + "file1": archiver.TestFile{Content: "string"}, + "file2": archiver.TestFile{Content: "string"}, + "firstDir": archiver.TestDir{ + "another": archiver.TestFile{Content: "string"}, + }, + "secondDir": archiver.TestDir{ + "another2": archiver.TestFile{Content: "string"}, + }, + }, + target: "/", + }, + { + name: "file and symlink in root", + args: archiver.TestDir{ + "file1": archiver.TestFile{Content: "string"}, + "file2": archiver.TestSymlink{Target: "file1"}, + }, + target: "/", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + tmpdir, repo, cleanup := prepareTempdirRepoSrc(t, tt.args) + defer cleanup() + + arch := archiver.New(repo, fs.Track{FS: fs.Local{}}, archiver.Options{}) + + back := rtest.Chdir(t, tmpdir) + defer back() + + sn, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{}) + rtest.OK(t, err) + + tree, err := repo.LoadTree(ctx, *sn.Tree) + rtest.OK(t, err) + + dst := &bytes.Buffer{} + if err := WriteZip(ctx, repo, tree, tt.target, dst); err != nil { + t.Fatalf("WriteZip() error = %v", err) + } + if err := checkZip(t, tmpdir, dst); err != nil { + t.Errorf("WriteZip() = zip does not match: %v", err) + } + }) + } +} + +func readZipFile(f *zip.File) ([]byte, error) { + rc, err := f.Open() + if err != nil { + return nil, err + } + defer rc.Close() + + b := &bytes.Buffer{} + _, err = b.ReadFrom(rc) + if err != nil { + return nil, err + } + + return b.Bytes(), nil +} + +func checkZip(t *testing.T, testDir string, srcZip *bytes.Buffer) error { + z, err := zip.NewReader(bytes.NewReader(srcZip.Bytes()), int64(srcZip.Len())) + if err != nil { + return err + } + + fileNumber := 0 + zipFiles := len(z.File) + + err = filepath.Walk(testDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.Name() != filepath.Base(testDir) { + fileNumber++ + } + return nil + }) + if err != nil { + return err + } + + for _, f := range z.File { + matchPath := filepath.Join(testDir, f.Name) + match, err := os.Lstat(matchPath) + if err != nil { + return err + } + + // check metadata, zip header contains time rounded to seconds + fileTime := match.ModTime().Truncate(time.Second) + zipTime := f.Modified + if !fileTime.Equal(zipTime) { + return fmt.Errorf("modTime does not match, got: %s, want: %s", zipTime, fileTime) + } + if f.Mode() != match.Mode() { + return fmt.Errorf("mode does not match, got: %v [%08x], want: %v [%08x]", + f.Mode(), uint32(f.Mode()), match.Mode(), uint32(match.Mode())) + } + t.Logf("Mode is %v [%08x] for %s", f.Mode(), uint32(f.Mode()), f.Name) + + switch { + case f.FileInfo().IsDir(): + filebase := filepath.ToSlash(match.Name()) + if filepath.Base(f.Name) != filebase { + return fmt.Errorf("foldernames don't match got %v want %v", filepath.Base(f.Name), filebase) + } + if !strings.HasSuffix(f.Name, "/") { + return fmt.Errorf("foldernames must end with separator got %v", f.Name) + } + case f.Mode()&os.ModeSymlink != 0: + target, err := fs.Readlink(matchPath) + if err != nil { + return err + } + linkName, err := readZipFile(f) + if err != nil { + t.Fatal(err) + } + if target != string(linkName) { + return fmt.Errorf("symlink target does not match, got %s want %s", string(linkName), target) + } + default: + if uint64(match.Size()) != f.UncompressedSize64 { + return fmt.Errorf("size does not match got %v want %v", f.UncompressedSize64, match.Size()) + } + contentsFile, err := ioutil.ReadFile(matchPath) + if err != nil { + t.Fatal(err) + } + contentsZip, err := readZipFile(f) + if err != nil { + t.Fatal(err) + } + if string(contentsZip) != string(contentsFile) { + return fmt.Errorf("contents does not match, got %s want %s", contentsZip, contentsFile) + } + } + } + + if zipFiles != fileNumber { + return fmt.Errorf("not the same amount of files got %v want %v", zipFiles, fileNumber) + } + + return nil +} From 2ca76afc2b9946d29cd7af899beb5b48319028bf Mon Sep 17 00:00:00 2001 From: DRON-666 <64691982+DRON-666@users.noreply.github.com> Date: Tue, 10 Nov 2020 04:24:09 +0300 Subject: [PATCH 3/8] dump: Add new option --archive --- cmd/restic/cmd_dump.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index 7875baa89..667dc3875 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -21,8 +21,8 @@ var cmdDump = &cobra.Command{ Long: ` The "dump" command extracts files from a snapshot from the repository. If a single file is selected, it prints its contents to stdout. Folders are output -as a tar file containing the contents of the specified folder. Pass "/" as -file name to dump the whole snapshot as a tar file. +as a tar (default) or zip file containing the contents of the specified folder. +Pass "/" as file name to dump the whole snapshot as an archive file. The special snapshot "latest" can be used to use the latest snapshot in the repository. @@ -40,9 +40,10 @@ Exit status is 0 if the command was successful, and non-zero if there was any er // DumpOptions collects all options for the dump command. type DumpOptions struct { - Hosts []string - Paths []string - Tags restic.TagLists + Hosts []string + Paths []string + Tags restic.TagLists + Archive string } var dumpOptions DumpOptions @@ -54,6 +55,7 @@ func init() { flags.StringArrayVarP(&dumpOptions.Hosts, "host", "H", nil, `only consider snapshots for this host when the snapshot ID is "latest" (can be specified multiple times)`) flags.Var(&dumpOptions.Tags, "tag", "only consider snapshots which include this `taglist` for snapshot ID \"latest\"") flags.StringArrayVar(&dumpOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path` for snapshot ID \"latest\"") + flags.StringVarP(&dumpOptions.Archive, "archive", "a", "tar", "set archive `format` as \"tar\" or \"zip\"") } func splitPath(p string) []string { @@ -126,6 +128,16 @@ func runDump(opts DumpOptions, gopts GlobalOptions, args []string) error { return errors.Fatal("no file and no snapshot ID specified") } + var wd dump.WriteDump + switch opts.Archive { + case "tar": + wd = dump.WriteTar + case "zip": + wd = dump.WriteZip + default: + return fmt.Errorf("unknown archive format %q", opts.Archive) + } + snapshotIDString := args[0] pathToPrint := args[1] @@ -175,7 +187,7 @@ func runDump(opts DumpOptions, gopts GlobalOptions, args []string) error { Exitf(2, "loading tree for snapshot %q failed: %v", snapshotIDString, err) } - err = printFromTree(ctx, tree, repo, "/", splittedPath, dump.WriteTar) + err = printFromTree(ctx, tree, repo, "/", splittedPath, wd) if err != nil { Exitf(2, "cannot dump file: %v", err) } From ef1aeb8724c04a37935abda155c7cdb185fc8775 Mon Sep 17 00:00:00 2001 From: DRON-666 <64691982+DRON-666@users.noreply.github.com> Date: Tue, 10 Nov 2020 04:56:21 +0300 Subject: [PATCH 4/8] dump: Update docs and changelog --- changelog/unreleased/pull-3081 | 8 ++++++++ doc/050_restore.rst | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/pull-3081 diff --git a/changelog/unreleased/pull-3081 b/changelog/unreleased/pull-3081 new file mode 100644 index 000000000..89de8a728 --- /dev/null +++ b/changelog/unreleased/pull-3081 @@ -0,0 +1,8 @@ +Enhancement: Allow whole folder dump in ZIP format + +Previously, restic can dump the contents of a whole folder structure only +in the tar format. The `dump` command now have a new flag to change output +format to zip. Just pass `--archive zip` as an option to `restic dump`. + +https://github.com/restic/restic/pull/2433 +https://github.com/restic/restic/pull/3081 diff --git a/doc/050_restore.rst b/doc/050_restore.rst index 1f5996ccc..2a636c317 100644 --- a/doc/050_restore.rst +++ b/doc/050_restore.rst @@ -128,10 +128,13 @@ e.g.: It is also possible to ``dump`` the contents of a whole folder structure to stdout. To retain the information about the files and folders Restic will -output the contents in the tar format: +output the contents in the tar (default) or zip formats: .. code-block:: console $ restic -r /srv/restic-repo dump latest /home/other/work > restore.tar + +.. code-block:: console + $ restic -r /srv/restic-repo dump -a zip latest /home/other/work > restore.zip From da9053b184de2bd553f4ae3965c6c53b0e519e9a Mon Sep 17 00:00:00 2001 From: DRON-666 <64691982+DRON-666@users.noreply.github.com> Date: Sat, 19 Dec 2020 01:16:15 +0300 Subject: [PATCH 5/8] Some gramma fixes in documentation --- changelog/unreleased/pull-3081 | 6 +++--- doc/050_restore.rst | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/changelog/unreleased/pull-3081 b/changelog/unreleased/pull-3081 index 89de8a728..34b0dd556 100644 --- a/changelog/unreleased/pull-3081 +++ b/changelog/unreleased/pull-3081 @@ -1,7 +1,7 @@ -Enhancement: Allow whole folder dump in ZIP format +Enhancement: Add zip format support to dump -Previously, restic can dump the contents of a whole folder structure only -in the tar format. The `dump` command now have a new flag to change output +Previously, restic could dump the contents of a whole folder structure only +in the tar format. The `dump` command now has a new flag to change output format to zip. Just pass `--archive zip` as an option to `restic dump`. https://github.com/restic/restic/pull/2433 diff --git a/doc/050_restore.rst b/doc/050_restore.rst index 2a636c317..27568d203 100644 --- a/doc/050_restore.rst +++ b/doc/050_restore.rst @@ -128,7 +128,7 @@ e.g.: It is also possible to ``dump`` the contents of a whole folder structure to stdout. To retain the information about the files and folders Restic will -output the contents in the tar (default) or zip formats: +output the contents in the tar (default) or zip format: .. code-block:: console From 33adb58817cf24740acf599b762e4fb597766884 Mon Sep 17 00:00:00 2001 From: DRON-666 <64691982+DRON-666@users.noreply.github.com> Date: Sat, 19 Dec 2020 02:04:17 +0300 Subject: [PATCH 6/8] Minor fixes and linter suggestions --- internal/dump/tar.go | 17 +++++++++-------- internal/dump/zip.go | 5 +++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/internal/dump/tar.go b/internal/dump/tar.go index 02fa8e185..c1cd9343f 100644 --- a/internal/dump/tar.go +++ b/internal/dump/tar.go @@ -26,6 +26,7 @@ func WriteTar(ctx context.Context, repo restic.Repository, tree *restic.Tree, ro err := writeDump(ctx, repo, tree, rootPath, dmp, dst) if err != nil { dmp.w.Close() + return err } @@ -36,9 +37,9 @@ func WriteTar(ctx context.Context, repo restic.Repository, tree *restic.Tree, ro const ( // Mode constants from the USTAR spec: // See http://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html#tag_20_92_13_06 - c_ISUID = 04000 // Set uid - c_ISGID = 02000 // Set gid - c_ISVTX = 01000 // Save text (sticky bit) + cISUID = 0o4000 // Set uid + cISGID = 0o2000 // Set gid + cISVTX = 0o1000 // Save text (sticky bit) ) func (dmp tarDumper) dumpNode(ctx context.Context, node *restic.Node, repo restic.Repository) error { @@ -50,7 +51,7 @@ func (dmp tarDumper) dumpNode(ctx context.Context, node *restic.Node, repo resti header := &tar.Header{ Name: filepath.ToSlash(relPath), Size: int64(node.Size), - Mode: int64(node.Mode.Perm()), // c_IS* constants are added later + Mode: int64(node.Mode.Perm()), // cIS* constants are added later Uid: int(node.UID), Gid: int(node.GID), Uname: node.User, @@ -63,13 +64,13 @@ func (dmp tarDumper) dumpNode(ctx context.Context, node *restic.Node, repo resti // adapted from archive/tar.FileInfoHeader if node.Mode&os.ModeSetuid != 0 { - header.Mode |= c_ISUID + header.Mode |= cISUID } if node.Mode&os.ModeSetgid != 0 { - header.Mode |= c_ISGID + header.Mode |= cISGID } if node.Mode&os.ModeSticky != 0 { - header.Mode |= c_ISVTX + header.Mode |= cISVTX } if IsFile(node) { @@ -89,7 +90,7 @@ func (dmp tarDumper) dumpNode(ctx context.Context, node *restic.Node, repo resti err = dmp.w.WriteHeader(header) if err != nil { - return errors.Wrap(err, "TarHeader ") + return errors.Wrap(err, "TarHeader") } return GetNodeData(ctx, dmp.w, repo, node) diff --git a/internal/dump/zip.go b/internal/dump/zip.go index 1c1035b08..f98afa4a0 100644 --- a/internal/dump/zip.go +++ b/internal/dump/zip.go @@ -15,7 +15,7 @@ type zipDumper struct { } // Statically ensure that zipDumper implements dumper. -var _ dumper = tarDumper{} +var _ dumper = zipDumper{} // WriteZip will write the contents of the given tree, encoded as a zip to the given destination. func WriteZip(ctx context.Context, repo restic.Repository, tree *restic.Tree, rootPath string, dst io.Writer) error { @@ -24,6 +24,7 @@ func WriteZip(ctx context.Context, repo restic.Repository, tree *restic.Tree, ro err := writeDump(ctx, repo, tree, rootPath, dmp, dst) if err != nil { dmp.w.Close() + return err } @@ -49,7 +50,7 @@ func (dmp zipDumper) dumpNode(ctx context.Context, node *restic.Node, repo resti w, err := dmp.w.CreateHeader(header) if err != nil { - return errors.Wrap(err, "ZipHeader ") + return errors.Wrap(err, "ZipHeader") } if IsLink(node) { From e136dd86967c7776592f22eccbdcf0fd8acf67eb Mon Sep 17 00:00:00 2001 From: DRON-666 <64691982+DRON-666@users.noreply.github.com> Date: Sat, 19 Dec 2020 02:06:54 +0300 Subject: [PATCH 7/8] Deduplicate test code --- internal/dump/common_test.go | 103 +++++++++++++++++++++++++++++++++++ internal/dump/tar_test.go | 91 +------------------------------ internal/dump/zip_test.go | 76 +------------------------- 3 files changed, 106 insertions(+), 164 deletions(-) create mode 100644 internal/dump/common_test.go diff --git a/internal/dump/common_test.go b/internal/dump/common_test.go new file mode 100644 index 000000000..e15659701 --- /dev/null +++ b/internal/dump/common_test.go @@ -0,0 +1,103 @@ +package dump + +import ( + "bytes" + "context" + "testing" + + "github.com/restic/restic/internal/archiver" + "github.com/restic/restic/internal/fs" + "github.com/restic/restic/internal/repository" + "github.com/restic/restic/internal/restic" + rtest "github.com/restic/restic/internal/test" +) + +func prepareTempdirRepoSrc(t testing.TB, src archiver.TestDir) (tempdir string, repo restic.Repository, cleanup func()) { + tempdir, removeTempdir := rtest.TempDir(t) + repo, removeRepository := repository.TestRepository(t) + + archiver.TestCreateFiles(t, tempdir, src) + + cleanup = func() { + removeRepository() + removeTempdir() + } + + return tempdir, repo, cleanup +} + +type CheckDump func(t *testing.T, testDir string, testDump *bytes.Buffer) error + +func WriteTest(t *testing.T, wd WriteDump, cd CheckDump) { + tests := []struct { + name string + args archiver.TestDir + target string + }{ + { + name: "single file in root", + args: archiver.TestDir{ + "file": archiver.TestFile{Content: "string"}, + }, + target: "/", + }, + { + name: "multiple files in root", + args: archiver.TestDir{ + "file1": archiver.TestFile{Content: "string"}, + "file2": archiver.TestFile{Content: "string"}, + }, + target: "/", + }, + { + name: "multiple files and folders in root", + args: archiver.TestDir{ + "file1": archiver.TestFile{Content: "string"}, + "file2": archiver.TestFile{Content: "string"}, + "firstDir": archiver.TestDir{ + "another": archiver.TestFile{Content: "string"}, + }, + "secondDir": archiver.TestDir{ + "another2": archiver.TestFile{Content: "string"}, + }, + }, + target: "/", + }, + { + name: "file and symlink in root", + args: archiver.TestDir{ + "file1": archiver.TestFile{Content: "string"}, + "file2": archiver.TestSymlink{Target: "file1"}, + }, + target: "/", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + tmpdir, repo, cleanup := prepareTempdirRepoSrc(t, tt.args) + defer cleanup() + + arch := archiver.New(repo, fs.Track{FS: fs.Local{}}, archiver.Options{}) + + back := rtest.Chdir(t, tmpdir) + defer back() + + sn, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{}) + rtest.OK(t, err) + + tree, err := repo.LoadTree(ctx, *sn.Tree) + rtest.OK(t, err) + + dst := &bytes.Buffer{} + if err := wd(ctx, repo, tree, tt.target, dst); err != nil { + t.Fatalf("WriteDump() error = %v", err) + } + if err := cd(t, tmpdir, dst); err != nil { + t.Errorf("WriteDump() = does not match: %v", err) + } + }) + } +} diff --git a/internal/dump/tar_test.go b/internal/dump/tar_test.go index 2ef98ffe0..ecf9869ae 100644 --- a/internal/dump/tar_test.go +++ b/internal/dump/tar_test.go @@ -3,7 +3,6 @@ package dump import ( "archive/tar" "bytes" - "context" "fmt" "io" "io/ioutil" @@ -13,99 +12,11 @@ import ( "testing" "time" - "github.com/restic/restic/internal/archiver" "github.com/restic/restic/internal/fs" - "github.com/restic/restic/internal/repository" - "github.com/restic/restic/internal/restic" - rtest "github.com/restic/restic/internal/test" ) -func prepareTempdirRepoSrc(t testing.TB, src archiver.TestDir) (tempdir string, repo restic.Repository, cleanup func()) { - tempdir, removeTempdir := rtest.TempDir(t) - repo, removeRepository := repository.TestRepository(t) - - archiver.TestCreateFiles(t, tempdir, src) - - cleanup = func() { - removeRepository() - removeTempdir() - } - - return tempdir, repo, cleanup -} - func TestWriteTar(t *testing.T) { - tests := []struct { - name string - args archiver.TestDir - target string - }{ - { - name: "single file in root", - args: archiver.TestDir{ - "file": archiver.TestFile{Content: "string"}, - }, - target: "/", - }, - { - name: "multiple files in root", - args: archiver.TestDir{ - "file1": archiver.TestFile{Content: "string"}, - "file2": archiver.TestFile{Content: "string"}, - }, - target: "/", - }, - { - name: "multiple files and folders in root", - args: archiver.TestDir{ - "file1": archiver.TestFile{Content: "string"}, - "file2": archiver.TestFile{Content: "string"}, - "firstDir": archiver.TestDir{ - "another": archiver.TestFile{Content: "string"}, - }, - "secondDir": archiver.TestDir{ - "another2": archiver.TestFile{Content: "string"}, - }, - }, - target: "/", - }, - { - name: "file and symlink in root", - args: archiver.TestDir{ - "file1": archiver.TestFile{Content: "string"}, - "file2": archiver.TestSymlink{Target: "file1"}, - }, - target: "/", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - tmpdir, repo, cleanup := prepareTempdirRepoSrc(t, tt.args) - defer cleanup() - - arch := archiver.New(repo, fs.Track{FS: fs.Local{}}, archiver.Options{}) - - back := rtest.Chdir(t, tmpdir) - defer back() - - sn, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{}) - rtest.OK(t, err) - - tree, err := repo.LoadTree(ctx, *sn.Tree) - rtest.OK(t, err) - - dst := &bytes.Buffer{} - if err := WriteTar(ctx, repo, tree, tt.target, dst); err != nil { - t.Fatalf("WriteTar() error = %v", err) - } - if err := checkTar(t, tmpdir, dst); err != nil { - t.Errorf("WriteTar() = tar does not match: %v", err) - } - }) - } + WriteTest(t, WriteTar, checkTar) } func checkTar(t *testing.T, testDir string, srcTar *bytes.Buffer) error { diff --git a/internal/dump/zip_test.go b/internal/dump/zip_test.go index 70ad00275..84e0d0487 100644 --- a/internal/dump/zip_test.go +++ b/internal/dump/zip_test.go @@ -3,7 +3,6 @@ package dump import ( "archive/zip" "bytes" - "context" "fmt" "io/ioutil" "os" @@ -12,83 +11,11 @@ import ( "testing" "time" - "github.com/restic/restic/internal/archiver" "github.com/restic/restic/internal/fs" - rtest "github.com/restic/restic/internal/test" ) func TestWriteZip(t *testing.T) { - tests := []struct { - name string - args archiver.TestDir - target string - }{ - { - name: "single file in root", - args: archiver.TestDir{ - "file": archiver.TestFile{Content: "string"}, - }, - target: "/", - }, - { - name: "multiple files in root", - args: archiver.TestDir{ - "file1": archiver.TestFile{Content: "string"}, - "file2": archiver.TestFile{Content: "string"}, - }, - target: "/", - }, - { - name: "multiple files and folders in root", - args: archiver.TestDir{ - "file1": archiver.TestFile{Content: "string"}, - "file2": archiver.TestFile{Content: "string"}, - "firstDir": archiver.TestDir{ - "another": archiver.TestFile{Content: "string"}, - }, - "secondDir": archiver.TestDir{ - "another2": archiver.TestFile{Content: "string"}, - }, - }, - target: "/", - }, - { - name: "file and symlink in root", - args: archiver.TestDir{ - "file1": archiver.TestFile{Content: "string"}, - "file2": archiver.TestSymlink{Target: "file1"}, - }, - target: "/", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - tmpdir, repo, cleanup := prepareTempdirRepoSrc(t, tt.args) - defer cleanup() - - arch := archiver.New(repo, fs.Track{FS: fs.Local{}}, archiver.Options{}) - - back := rtest.Chdir(t, tmpdir) - defer back() - - sn, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{}) - rtest.OK(t, err) - - tree, err := repo.LoadTree(ctx, *sn.Tree) - rtest.OK(t, err) - - dst := &bytes.Buffer{} - if err := WriteZip(ctx, repo, tree, tt.target, dst); err != nil { - t.Fatalf("WriteZip() error = %v", err) - } - if err := checkZip(t, tmpdir, dst); err != nil { - t.Errorf("WriteZip() = zip does not match: %v", err) - } - }) - } + WriteTest(t, WriteZip, checkZip) } func readZipFile(f *zip.File) ([]byte, error) { @@ -123,6 +50,7 @@ func checkZip(t *testing.T, testDir string, srcZip *bytes.Buffer) error { if info.Name() != filepath.Base(testDir) { fileNumber++ } + return nil }) if err != nil { From 83b10dbb12d1c0da639b152d2356d3176c3ca1da Mon Sep 17 00:00:00 2001 From: DRON-666 <64691982+DRON-666@users.noreply.github.com> Date: Sat, 19 Dec 2020 02:42:46 +0300 Subject: [PATCH 8/8] Deduplicate dumper closing logic --- internal/dump/common.go | 5 ++++- internal/dump/tar.go | 9 +++------ internal/dump/zip.go | 9 +++------ 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/internal/dump/common.go b/internal/dump/common.go index 5b3d82068..fd74a4a07 100644 --- a/internal/dump/common.go +++ b/internal/dump/common.go @@ -12,6 +12,7 @@ import ( // dumper implements saving node data. type dumper interface { + io.Closer dumpNode(ctx context.Context, node *restic.Node, repo restic.Repository) error } @@ -24,11 +25,13 @@ func writeDump(ctx context.Context, repo restic.Repository, tree *restic.Tree, r rootNode.Path = rootPath err := dumpTree(ctx, repo, rootNode, rootPath, dmp) if err != nil { + dmp.Close() + return err } } - return nil + return dmp.Close() } func dumpTree(ctx context.Context, repo restic.Repository, rootNode *restic.Node, rootPath string, dmp dumper) error { diff --git a/internal/dump/tar.go b/internal/dump/tar.go index c1cd9343f..f9716d152 100644 --- a/internal/dump/tar.go +++ b/internal/dump/tar.go @@ -23,13 +23,10 @@ var _ dumper = tarDumper{} func WriteTar(ctx context.Context, repo restic.Repository, tree *restic.Tree, rootPath string, dst io.Writer) error { dmp := tarDumper{w: tar.NewWriter(dst)} - err := writeDump(ctx, repo, tree, rootPath, dmp, dst) - if err != nil { - dmp.w.Close() - - return err - } + return writeDump(ctx, repo, tree, rootPath, dmp, dst) +} +func (dmp tarDumper) Close() error { return dmp.w.Close() } diff --git a/internal/dump/zip.go b/internal/dump/zip.go index f98afa4a0..f1e779d8e 100644 --- a/internal/dump/zip.go +++ b/internal/dump/zip.go @@ -21,13 +21,10 @@ var _ dumper = zipDumper{} func WriteZip(ctx context.Context, repo restic.Repository, tree *restic.Tree, rootPath string, dst io.Writer) error { dmp := zipDumper{w: zip.NewWriter(dst)} - err := writeDump(ctx, repo, tree, rootPath, dmp, dst) - if err != nil { - dmp.w.Close() - - return err - } + return writeDump(ctx, repo, tree, rootPath, dmp, dst) +} +func (dmp zipDumper) Close() error { return dmp.w.Close() }