Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid using syscall for errors on unix #201

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion copy/copy_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ import (
"os"

"github.com/pkg/errors"
"github.com/tonistiigi/fsutil"
"golang.org/x/sys/unix"
)

func copyFile(source, target string) error {
if err := unix.Clonefileat(unix.AT_FDCWD, source, unix.AT_FDCWD, target, unix.CLONE_NOFOLLOW); err != nil {
if err != unix.EINVAL && err != unix.EXDEV {
if err != fsutil.EINVAL && err != fsutil.EXDEV {
return err
}
} else {
Expand Down
3 changes: 2 additions & 1 deletion copy/copy_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"syscall"

"github.com/pkg/errors"
"github.com/tonistiigi/fsutil"
"golang.org/x/sys/unix"
)

Expand Down Expand Up @@ -105,7 +106,7 @@ func copyFileContent(dst, src *os.File) error {
n, err := unix.CopyFileRange(int(src.Fd()), nil, int(dst.Fd()), nil, desired, 0)
if err != nil {
// matches go/src/internal/poll/copy_file_range_linux.go
if (err != unix.ENOSYS && err != unix.EXDEV && err != unix.EPERM && err != syscall.EIO && err != unix.EOPNOTSUPP && err != syscall.EINVAL) || !first {
if (err != fsutil.ENOSYS && err != fsutil.EXDEV && err != fsutil.EPERM && err != fsutil.EIO && err != fsutil.EOPNOTSUPP && err != fsutil.EINVAL) || !first {
return errors.Wrap(err, "copy file range failed")
}

Expand Down
5 changes: 3 additions & 2 deletions copy/mkdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package fs

import (
"os"
"syscall"
"time"

"github.com/tonistiigi/fsutil"
)

// MkdirAll is forked os.MkdirAll
Expand All @@ -14,7 +15,7 @@ func MkdirAll(path string, perm os.FileMode, user Chowner, tm *time.Time) ([]str
if dir.IsDir() {
return nil, nil
}
return nil, &os.PathError{Op: "mkdir", Path: path, Err: syscall.ENOTDIR}
return nil, &os.PathError{Op: "mkdir", Path: path, Err: fsutil.ENOTDIR}
}

// Slow path: make sure parent exists and then call Mkdir for path.
Expand Down
5 changes: 2 additions & 3 deletions diskwriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"path/filepath"
"strconv"
"sync"
"syscall"
"time"

"github.com/opencontainers/go-digest"
Expand Down Expand Up @@ -123,7 +122,7 @@ func (dw *DiskWriter) HandleChange(kind ChangeKind, p string, fi os.FileInfo, er

stat, ok := fi.Sys().(*types.Stat)
if !ok {
return errors.WithStack(&os.PathError{Path: p, Err: syscall.EBADMSG, Op: "change without stat info"})
return errors.WithStack(&os.PathError{Path: p, Err: EBADMSG, Op: "change without stat info"})
}

statCopy := stat.Clone()
Expand Down Expand Up @@ -164,7 +163,7 @@ func (dw *DiskWriter) HandleChange(kind ChangeKind, p string, fi os.FileInfo, er
switch {
case fi.IsDir():
if err := os.Mkdir(newPath, fi.Mode()); err != nil {
if errors.Is(err, syscall.EEXIST) {
if errors.Is(err, EEXIST) {
// we saw a race to create this directory, so try again
return dw.HandleChange(kind, p, fi, nil)
}
Expand Down
15 changes: 15 additions & 0 deletions errors_nounix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//go:build !unix
// +build !unix

package fsutil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if these should live in an internal package, as (I think) we don't want consumers to be using these to match against (they're only do prevent having to split out the implementation further, correct?)

Copy link
Owner

@tonistiigi tonistiigi Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of returning typed errors if we don't want consumers to match against them?

I don't really see an issue with old code as well, but if we define these separately, then we should export them as well to let consumers know what they can match against.


import "syscall"

const (
EBADMSG = syscall.EBADMSG
EEXIST = syscall.EEXIST
EINVAL = syscall.EINVAL
EISDIR = syscall.EISDIR
ENOENT = syscall.ENOENT
ENOTDIR = syscall.ENOTDIR
)
21 changes: 21 additions & 0 deletions errors_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//go:build unix
// +build unix

package fsutil

import "golang.org/x/sys/unix"

const (
EBADMSG = unix.EBADMSG
EEXIST = unix.EEXIST
EINVAL = unix.EINVAL
EIO = unix.EIO
EISDIR = unix.EISDIR
ENOENT = unix.ENOENT
ENOSYS = unix.ENOSYS
ENOTDIR = unix.ENOTDIR
ENOTSUP = unix.ENOTSUP
EOPNOTSUPP = unix.EOPNOTSUPP
EPERM = unix.EPERM
EXDEV = unix.EXDEV
)
7 changes: 3 additions & 4 deletions filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"os"
"path/filepath"
"strings"
"syscall"

"github.com/moby/patternmatcher"
"github.com/pkg/errors"
Expand Down Expand Up @@ -318,7 +317,7 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc
}
stat, ok := fi.Sys().(*types.Stat)
if !ok {
return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"})
return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"})
}

select {
Expand Down Expand Up @@ -346,7 +345,7 @@ func (fs *filterFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc
}
parentStat, ok := parentFi.Sys().(*types.Stat)
if !ok {
return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"})
return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"})
}

select {
Expand Down Expand Up @@ -425,5 +424,5 @@ func patternWithoutTrailingGlob(p *patternmatcher.Pattern) string {
}

func isNotExist(err error) bool {
return errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR)
return errors.Is(err, os.ErrNotExist) || errors.Is(err, ENOTDIR)
}
5 changes: 2 additions & 3 deletions followlinks.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"runtime"
"sort"
strings "strings"
"syscall"

"github.com/pkg/errors"
"github.com/tonistiigi/fsutil/types"
Expand Down Expand Up @@ -122,7 +121,7 @@ func (r *symlinkResolver) readSymlink(p string, allowWildcard bool) ([]string, e
}
stat, ok := fi.Sys().(*types.Stat)
if !ok {
return nil, errors.WithStack(&os.PathError{Path: p, Err: syscall.EBADMSG, Op: "fileinfo without stat info"})
return nil, errors.WithStack(&os.PathError{Path: p, Err: EBADMSG, Op: "fileinfo without stat info"})
}

link := filepath.Clean(stat.Linkname)
Expand Down Expand Up @@ -180,7 +179,7 @@ func readDir(fs FS, root string) ([]os.DirEntry, error) {
}
if p == root {
if !entry.IsDir() {
return errors.WithStack(&os.PathError{Op: "walk", Path: root, Err: syscall.ENOTDIR})
return errors.WithStack(&os.PathError{Op: "walk", Path: root, Err: ENOTDIR})
}
out = make([]gofs.DirEntry, 0)
return nil
Expand Down
13 changes: 6 additions & 7 deletions fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"path/filepath"
"sort"
"strings"
"syscall"
"time"

"github.com/pkg/errors"
Expand All @@ -32,7 +31,7 @@ func NewFS(root string) (FS, error) {
return nil, err
}
if !fi.IsDir() {
return nil, errors.WithStack(&os.PathError{Op: "stat", Path: root, Err: syscall.ENOTDIR})
return nil, errors.WithStack(&os.PathError{Op: "stat", Path: root, Err: ENOTDIR})
}

return &fs{
Expand Down Expand Up @@ -102,10 +101,10 @@ func SubDirFS(dirs []Dir) (FS, error) {
m := map[string]Dir{}
for _, d := range dirs {
if path.Base(d.Stat.Path) != d.Stat.Path {
return nil, errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: syscall.EISDIR, Op: "invalid path"})
return nil, errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: EISDIR, Op: "invalid path"})
}
if _, ok := m[d.Stat.Path]; ok {
return nil, errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: syscall.EEXIST, Op: "duplicate path"})
return nil, errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: EEXIST, Op: "duplicate path"})
}
m[d.Stat.Path] = d
}
Expand All @@ -127,7 +126,7 @@ func (fs *subDirFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc

fi := &StatInfo{d.Stat.Clone()}
if !fi.IsDir() {
return errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: syscall.ENOTDIR, Op: "walk subdir"})
return errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: ENOTDIR, Op: "walk subdir"})
}
dStat := d.Stat.Clone()
if err := fn(d.Stat.Path, &DirEntryInfo{Stat: dStat}, nil); err != nil {
Expand All @@ -144,7 +143,7 @@ func (fs *subDirFS) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc
}
stat, ok := fi.Sys().(*types.Stat)
if !ok {
return errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"})
return errors.WithStack(&os.PathError{Path: d.Stat.Path, Err: EBADMSG, Op: "fileinfo without stat info"})
}
stat.Path = path.Join(d.Stat.Path, stat.Path)
if stat.Linkname != "" {
Expand All @@ -171,7 +170,7 @@ func (fs *subDirFS) Open(p string) (io.ReadCloser, error) {
}
d, ok := fs.m[parts[0]]
if !ok {
return nil, errors.WithStack(&os.PathError{Path: parts[0], Err: syscall.ENOENT, Op: "open"})
return nil, errors.WithStack(&os.PathError{Path: parts[0], Err: ENOENT, Op: "open"})
}
return d.FS.Open(parts[1])
}
Expand Down
5 changes: 2 additions & 3 deletions hardlinks.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"io"
gofs "io/fs"
"os"
"syscall"

"github.com/pkg/errors"
"github.com/tonistiigi/fsutil/types"
Expand All @@ -32,7 +31,7 @@ func (v *Hardlinks) HandleChange(kind ChangeKind, p string, fi os.FileInfo, err

stat, ok := fi.Sys().(*types.Stat)
if !ok {
return errors.WithStack(&os.PathError{Path: p, Err: syscall.EBADMSG, Op: "change without stat info"})
return errors.WithStack(&os.PathError{Path: p, Err: EBADMSG, Op: "change without stat info"})
}

if fi.IsDir() || fi.Mode()&os.ModeSymlink != 0 {
Expand Down Expand Up @@ -80,7 +79,7 @@ func (r *hardlinkFilter) Walk(ctx context.Context, target string, fn gofs.WalkDi

stat, ok := fi.Sys().(*types.Stat)
if !ok {
return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"})
return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"})
}

if stat.Linkname != "" {
Expand Down
3 changes: 1 addition & 2 deletions receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
"os"
"path/filepath"
"sync"
"syscall"

"github.com/pkg/errors"
"github.com/tonistiigi/fsutil/types"
Expand Down Expand Up @@ -221,7 +220,7 @@ func (r *receiver) run(ctx context.Context) error {
path := filepath.FromSlash(p.Stat.Path)
if filepath.ToSlash(path) != p.Stat.Path {
// e.g. a linux path foo/bar\baz cannot be represented on windows
return errors.WithStack(&os.PathError{Path: p.Stat.Path, Err: syscall.EINVAL, Op: "unrepresentable path"})
return errors.WithStack(&os.PathError{Path: p.Stat.Path, Err: EINVAL, Op: "unrepresentable path"})
}
p.Stat.Path = path
p.Stat.Linkname = filepath.FromSlash(p.Stat.Linkname)
Expand Down
3 changes: 1 addition & 2 deletions send.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"os"
"path/filepath"
"sync"
"syscall"

"github.com/pkg/errors"
"github.com/tonistiigi/fsutil/types"
Expand Down Expand Up @@ -158,7 +157,7 @@ func (s *sender) walk(ctx context.Context) error {
}
stat, ok := fi.Sys().(*types.Stat)
if !ok {
return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"})
return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"})
}
stat.Path = filepath.ToSlash(stat.Path)
stat.Linkname = filepath.ToSlash(stat.Linkname)
Expand Down
2 changes: 1 addition & 1 deletion stat_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
func loadXattr(origpath string, stat *types.Stat) error {
xattrs, err := sysx.LListxattr(origpath)
if err != nil {
if errors.Is(err, syscall.ENOTSUP) {
if errors.Is(err, ENOTSUP) {
return nil
}
return errors.Wrapf(err, "failed to xattr %s", origpath)
Expand Down
3 changes: 1 addition & 2 deletions tarwriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"os"
"path/filepath"
"strings"
"syscall"

"github.com/pkg/errors"
"github.com/tonistiigi/fsutil/types"
Expand All @@ -26,7 +25,7 @@ func WriteTar(ctx context.Context, fs FS, w io.Writer) error {
}
stat, ok := fi.Sys().(*types.Stat)
if !ok {
return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"})
return errors.WithStack(&os.PathError{Path: path, Err: EBADMSG, Op: "fileinfo without stat info"})
}
hdr, err := tar.FileInfoHeader(fi, stat.Linkname)
if err != nil {
Expand Down
7 changes: 3 additions & 4 deletions validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"path/filepath"
"sort"
"strings"
"syscall"

"github.com/pkg/errors"
)
Expand All @@ -28,18 +27,18 @@ func (v *Validator) HandleChange(kind ChangeKind, p string, fi os.FileInfo, err
v.parentDirs = make([]parent, 1, 10)
}
if p != filepath.Clean(p) {
return errors.WithStack(&os.PathError{Path: p, Err: syscall.EINVAL, Op: "unclean path"})
return errors.WithStack(&os.PathError{Path: p, Err: EINVAL, Op: "unclean path"})
}
if filepath.IsAbs(p) {
return errors.WithStack(&os.PathError{Path: p, Err: syscall.EINVAL, Op: "absolute path"})
return errors.WithStack(&os.PathError{Path: p, Err: EINVAL, Op: "absolute path"})
}
dir := filepath.Dir(p)
base := filepath.Base(p)
if dir == "." {
dir = ""
}
if dir == ".." || strings.HasPrefix(p, filepath.FromSlash("../")) {
return errors.WithStack(&os.PathError{Path: p, Err: syscall.EINVAL, Op: "escape check"})
return errors.WithStack(&os.PathError{Path: p, Err: EINVAL, Op: "escape check"})
}

// find a parent dir from saved records
Expand Down