-
Notifications
You must be signed in to change notification settings - Fork 80
darwin: ignore fchown failure on darwin #64
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
Conversation
8e1877a to
71d217e
Compare
Codecov Report
@@ Coverage Diff @@
## master #64 +/- ##
==========================================
- Coverage 12.11% 12.06% -0.06%
==========================================
Files 7 7
Lines 652 655 +3
==========================================
Hits 79 79
- Misses 551 554 +3
Partials 22 22
Continue to review full report at Codecov.
|
mikebrow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment
io_unix.go
Outdated
| pipes = append(pipes, stdin) | ||
| if err = unix.Fchown(int(stdin.r.Fd()), uid, gid); err != nil { | ||
| return nil, errors.Wrap(err, "failed to chown stdin") | ||
| // XXX: can't chown to pipe on darwin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest using this skip with a warning or debug, and wrap the err: containerd/containerd@55d9550#diff-b3363345815845245ac7ce3ebbab476bR52-R57
for the comment suggest: TODO: revert with proper darwin solution, skipping for now as darwin chown is returning EINVAL on anonymous pipe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated with your suggestion. and also updated go.mod as it now uses logrus package.
io_unix.go
Outdated
| pipes = append(pipes, stdout) | ||
| if err = unix.Fchown(int(stdout.w.Fd()), uid, gid); err != nil { | ||
| return nil, errors.Wrap(err, "failed to chown stdout") | ||
| // XXX: can't chown to pipe on darwin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
fchown(2) on darwin returns EINVAL on anonymous pipe, thus this commit ignores this checks. Signed-off-by: Hajime Tazaki <[email protected]>
71d217e to
52ac244
Compare
mikebrow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fchown(2) on darwin returns EINVAL on anonymous pipe, thus this commit
ignores this checks.
This is related to the PR of containerd (containerd/containerd#4526).