Remove direct dependency on github.com/pkg/errors #185
Conversation
|
I suggest releasing v0.2.0 after merging this |
AkihiroSuda
changed the title
go fmt with Go 1.17 + Remove direct dependency on github.com/pkg/errors
AkihiroSuda
changed the title
go fmt with Go 1.17 + Remove direct dependency on github.com/pkg/errorsgo fmt with Go 1.17 + Remove direct dependency on github.com/pkg/errors
AkihiroSuda
force-pushed
the
remove-pkg-errors
branch
from
c3a4b92
to
ef7dd55
Compare
AkihiroSuda
changed the title
go fmt with Go 1.17 + Remove direct dependency on github.com/pkg/errors
AkihiroSuda
force-pushed
the
remove-pkg-errors
branch
from
ef7dd55
to
858f9c5
Compare
|
oops; generated a conflict from the other PR; needs rebase |
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
AkihiroSuda
force-pushed
the
remove-pkg-errors
branch
from
858f9c5
to
849cf06
Compare
|
rebased |
|
tagged as v0.2.0 |
| } | ||
| default: | ||
| // TODO: Support pipes and sockets | ||
| return errors.Wrapf(err, "unsupported mode %s", fi.Mode()) | ||
| return fmt.Errorf("unsupported mode %s: %w", fi.Mode(), err) |
There was a problem hiding this comment.
This has broken the native snapshotter moby/buildkit#2747
While the new code that passes %w as nil is definitely wrong, there is a behavior change here as well. The previous code(that also was wrong) called Wrap(nil) in here so returned out nil in the middle of a directory, leaving the snapshot is some undefined state. The new code returns an incorrectly formatted error.
I think the correct and backward compatible change is to skip the item, maybe print a log. (edit: creating a stub file instead of skipping might be even better).
Also, changes to these errors in continuity and later in containerd itself have completely broken the buildkit's and docker's stacktrace feature for any errors propagating from containerd, without any meaningful benefit afaics. The error reporting and debugging experience is very poor now. Go stdlib has not implemented a replacement for pkg/errors.
Eg. with this change the report for this error becomes.
error: failed to solve: failed to compute cache key: copying of parent failed: unsupported mode prw-------: %!w(<nil>)
1 v0.9.0-162-g7b171f6b buildkitd --oci-worker-snapshotter=native
github.com/containerd/containerd/snapshots/native.(*snapshotter).createSnapshot
/src/vendor/github.com/containerd/containerd/snapshots/native/native.go:284
github.com/containerd/containerd/snapshots/native.(*snapshotter).Prepare
/src/vendor/github.com/containerd/containerd/snapshots/native/native.go:121
github.com/containerd/containerd/metadata.(*snapshotter).createSnapshot
/src/vendor/github.com/containerd/containerd/metadata/snapshot.go:357
github.com/containerd/containerd/metadata.(*snapshotter).Prepare
/src/vendor/github.com/containerd/containerd/metadata/snapshot.go:277
github.com/moby/buildkit/snapshot/containerd.(*nsSnapshotter).Prepare
/src/snapshot/containerd/snapshotter.go:47
github.com/moby/buildkit/snapshot.(*fromContainerd).Prepare
/src/snapshot/snapshotter.go:59
github.com/moby/buildkit/cache.(*immutableRef).extract.func1
/src/cache/refs.go:734
github.com/moby/buildkit/util/flightcontrol.(*call).run
/src/util/flightcontrol/flightcontrol.go:121
sync.(*Once).doSlow
/usr/local/go/src/sync/once.go:68
sync.(*Once).Do
/usr/local/go/src/sync/once.go:59
runtime.goexit
/usr/local/go/src/runtime/asm_arm64.s:1133
1 v0.9.0-162-g7b171f6b buildkitd --oci-worker-snapshotter=native
github.com/moby/buildkit/solver.(*edge).createInputRequests.func1.1
/src/solver/edge.go:834
github.com/moby/buildkit/solver/internal/pipe.NewWithFunction.func2
/src/solver/internal/pipe/pipe.go:82
runtime.goexit
/usr/local/go/src/runtime/asm_arm64.s:1133
native/native.go:284 is the call to CopyDir() so everything inside continuity is skipped.
Now when similar change in containerd is included as well we don't get any information for the real error location at all.
1 v0.10.0 buildkitd --oci-worker-snapshotter=native
github.com/moby/buildkit/solver.(*edge).createInputRequests.func1.1
/src/solver/edge.go:837
github.com/moby/buildkit/solver/internal/pipe.NewWithFunction.func2
/src/solver/internal/pipe/pipe.go:82
runtime.goexit
/usr/local/go/src/runtime/asm_arm64.s:1133
I'd suggest reverting all of these changes.
There was a problem hiding this comment.
Workaround to fix the nil wrapping: #196
I think the correct and backward compatible change is to skip the item, maybe print a log. (edit: creating a stub file instead of skipping might be even better).
I guess this can be discussed later in a separate issue/PR
I'd suggest reverting all of these changes.
This is hard, as https://github.com/pkg/errors has been already archived.
There was a problem hiding this comment.
creating a stub file instead of skipping might be even better
done:
No description provided.