From e10e2bb50f5e869d9125c663a3599019fbf8f372 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Thu, 3 Oct 2024 21:17:22 +0200 Subject: [PATCH 1/3] fs: Include filename in mknod errors --- internal/fs/mknod_unix.go | 14 +++++++++++--- internal/fs/node_freebsd.go | 13 ++++++++++--- internal/fs/node_unix_test.go | 11 +++++++++++ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/internal/fs/mknod_unix.go b/internal/fs/mknod_unix.go index 6127599f7..024c4d502 100644 --- a/internal/fs/mknod_unix.go +++ b/internal/fs/mknod_unix.go @@ -3,8 +3,16 @@ package fs -import "golang.org/x/sys/unix" +import ( + "os" -func mknod(path string, mode uint32, dev uint64) (err error) { - return unix.Mknod(path, mode, int(dev)) + "golang.org/x/sys/unix" +) + +func mknod(path string, mode uint32, dev uint64) error { + err := unix.Mknod(path, mode, int(dev)) + if err != nil { + err = &os.PathError{Op: "mknod", Path: path, Err: err} + } + return err } diff --git a/internal/fs/node_freebsd.go b/internal/fs/node_freebsd.go index 1b2f2fc7e..0cbe876f1 100644 --- a/internal/fs/node_freebsd.go +++ b/internal/fs/node_freebsd.go @@ -3,12 +3,19 @@ package fs -import "syscall" +import ( + "os" + "syscall" +) func nodeRestoreSymlinkTimestamps(path string, utimes [2]syscall.Timespec) error { return nil } -func mknod(path string, mode uint32, dev uint64) (err error) { - return syscall.Mknod(path, mode, dev) +func mknod(path string, mode uint32, dev uint64) error { + err := syscall.Mknod(path, mode, dev) + if err != nil { + err = &os.PathError{Op: "mknod", Path: path, Err: err} + } + return err } diff --git a/internal/fs/node_unix_test.go b/internal/fs/node_unix_test.go index 4d01b6cc5..f38762fc7 100644 --- a/internal/fs/node_unix_test.go +++ b/internal/fs/node_unix_test.go @@ -8,9 +8,11 @@ import ( "os" "path/filepath" "runtime" + "strings" "syscall" "testing" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) @@ -134,3 +136,12 @@ func TestNodeFromFileInfo(t *testing.T) { }) } } + +func TestMknodError(t *testing.T) { + d := t.TempDir() + // Call mkfifo, which calls mknod, as mknod may give + // "operation not permitted" on Mac. + err := mkfifo(d, 0) + rtest.Assert(t, errors.Is(err, os.ErrExist), "want ErrExist, got %q", err) + rtest.Assert(t, strings.Contains(err.Error(), d), "filename not in %q", err) +} From 19653f9e06ffeede08ffd9387b39c19eabcc9ef8 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Thu, 3 Oct 2024 21:36:48 +0200 Subject: [PATCH 2/3] fs: Simplify NodeCreateAt --- internal/fs/node.go | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/internal/fs/node.go b/internal/fs/node.go index 280e290c2..a5ca1654a 100644 --- a/internal/fs/node.go +++ b/internal/fs/node.go @@ -163,41 +163,29 @@ func lookupGroup(gid uint32) string { } // NodeCreateAt creates the node at the given path but does NOT restore node meta data. -func NodeCreateAt(node *restic.Node, path string) error { +func NodeCreateAt(node *restic.Node, path string) (err error) { debug.Log("create node %v at %v", node.Name, path) switch node.Type { case restic.NodeTypeDir: - if err := nodeCreateDirAt(node, path); err != nil { - return err - } + err = nodeCreateDirAt(node, path) case restic.NodeTypeFile: - if err := nodeCreateFileAt(path); err != nil { - return err - } + err = nodeCreateFileAt(path) case restic.NodeTypeSymlink: - if err := nodeCreateSymlinkAt(node, path); err != nil { - return err - } + err = nodeCreateSymlinkAt(node, path) case restic.NodeTypeDev: - if err := nodeCreateDevAt(node, path); err != nil { - return err - } + err = nodeCreateDevAt(node, path) case restic.NodeTypeCharDev: - if err := nodeCreateCharDevAt(node, path); err != nil { - return err - } + err = nodeCreateCharDevAt(node, path) case restic.NodeTypeFifo: - if err := nodeCreateFifoAt(path); err != nil { - return err - } + err = nodeCreateFifoAt(path) case restic.NodeTypeSocket: - return nil + err = nil default: - return errors.Errorf("filetype %q not implemented", node.Type) + err = errors.Errorf("filetype %q not implemented", node.Type) } - return nil + return err } func nodeCreateDirAt(node *restic.Node, path string) error { From 2b609d3e77bbaacbde8d1a5cfc3bcc688bd98dc3 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Thu, 3 Oct 2024 21:39:35 +0200 Subject: [PATCH 3/3] errors, fs: Replace CombineErrors with stdlib Join This does not produce exactly the same messages, as it inserts newlines instead of "; ". But given how long our error messages can be, that might be a good thing. --- internal/errors/errors.go | 29 ++--------------------------- internal/fs/node.go | 2 +- internal/fs/node_windows.go | 2 +- 3 files changed, 4 insertions(+), 29 deletions(-) diff --git a/internal/errors/errors.go b/internal/errors/errors.go index ca36611eb..96e5b82bb 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -2,7 +2,6 @@ package errors import ( stderrors "errors" - "fmt" "github.com/pkg/errors" ) @@ -36,35 +35,11 @@ func As(err error, tgt interface{}) bool { return stderrors.As(err, tgt) } // Is reports whether any error in err's tree matches target. func Is(x, y error) bool { return stderrors.Is(x, y) } +func Join(errs ...error) error { return stderrors.Join(errs...) } + // Unwrap returns the result of calling the Unwrap method on err, if err's type contains // an Unwrap method returning error. Otherwise, Unwrap returns nil. // // Unwrap only calls a method of the form "Unwrap() error". In particular Unwrap does not // unwrap errors returned by [Join]. func Unwrap(err error) error { return stderrors.Unwrap(err) } - -// CombineErrors combines multiple errors into a single error after filtering out any nil values. -// If no errors are passed, it returns nil. -// If one error is passed, it simply returns that same error. -func CombineErrors(errors ...error) (err error) { - var combinedErrorMsg string - var multipleErrors bool - for _, errVal := range errors { - if errVal != nil { - if combinedErrorMsg != "" { - combinedErrorMsg += "; " // Separate error messages with a delimiter - multipleErrors = true - } else { - // Set the first error - err = errVal - } - combinedErrorMsg += errVal.Error() - } - } - if combinedErrorMsg == "" { - return nil // If no errors, return nil - } else if !multipleErrors { - return err // If only one error, return that first error - } - return fmt.Errorf("multiple errors occurred: [%s]", combinedErrorMsg) -} diff --git a/internal/fs/node.go b/internal/fs/node.go index a5ca1654a..f6bf18087 100644 --- a/internal/fs/node.go +++ b/internal/fs/node.go @@ -103,7 +103,7 @@ func nodeFillExtra(node *restic.Node, path string, fi os.FileInfo, ignoreXattrLi allowExtended, err := nodeFillGenericAttributes(node, path, &stat) if allowExtended { // Skip processing ExtendedAttributes if allowExtended is false. - err = errors.CombineErrors(err, nodeFillExtendedAttributes(node, path, ignoreXattrListError)) + err = errors.Join(err, nodeFillExtendedAttributes(node, path, ignoreXattrListError)) } return err } diff --git a/internal/fs/node_windows.go b/internal/fs/node_windows.go index 9d46143cc..0c1bf0365 100644 --- a/internal/fs/node_windows.go +++ b/internal/fs/node_windows.go @@ -189,7 +189,7 @@ func nodeRestoreGenericAttributes(node *restic.Node, path string, warn func(msg } restic.HandleUnknownGenericAttributesFound(unknownAttribs, warn) - return errors.CombineErrors(errs...) + return errors.Join(errs...) } // genericAttributesToWindowsAttrs converts the generic attributes map to a WindowsAttributes and also returns a string of unknown attributes that it could not convert.