Skip to content

Break apart NewPipeIO to windows/unix#45

Merged
estesp merged 2 commits intocontainerd:masterfrom
jterry75:io_windows
Aug 20, 2018
Merged

Break apart NewPipeIO to windows/unix#45
estesp merged 2 commits intocontainerd:masterfrom
jterry75:io_windows

Conversation

@jterry75
Copy link
Contributor

@jterry75 jterry75 commented Aug 17, 2018

Moves NewPipeIO to GOOS specific to avoid setting the unix.Fchown flags
that dont apply on windows.

Signed-off-by: Justin Terry (VM) [email protected]

@jterry75
Break apart NewPipeIO to windows/unix
d4cf276 
Moves NewPipeIO to GOOS specific to avoid setting the unix.Fchown flags
that dont apply on windows.

Signed-off-by: Justin Terry (VM) <[email protected]>
@jterry75
Make console.go +build !windows
cc5515f 
Signed-off-by: Justin Terry (VM) <[email protected]>
@crosbymichael
Copy link
Member

crosbymichael commented Aug 17, 2018 •
edited
Loading

Which part of this pkg were you needing to depend on? Just the I/O part?

@jterry75
Copy link
Contributor Author

jterry75 commented Aug 20, 2018

@crosbymichael - I decided to make the equivalent go-runhcs for a few reasons and rather than re-inventing the wheel I am trying to simply reuse the go-runc: Monitor, IO portions since they are already well written to be platform agnostic. This is a super early commit in my testing/impl but take a look at: Microsoft/hcsshim:go_runhcs. Then I can make the shim actually use this package and avoid all command logic in the shim at all.

@crosbymichael
Copy link
Member

crosbymichael commented Aug 20, 2018

LGTM

estesp
estesp approved these changes Aug 20, 2018
View reviewed changes
Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 808e844 into containerd:master Aug 20, 2018
@jterry75 jterry75 deleted the io_windows branch August 20, 2018 14:38
@thaJeztah thaJeztah mentioned this pull request Aug 23, 2018
Merged
@thaJeztah thaJeztah mentioned this pull request Sep 19, 2018
Merged
@kzys kzys mentioned this pull request Jan 13, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jterry75 @crosbymichael @estesp