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

proposal: exec: add (*Cmd).ExtraInPipe and ExtraOutPipe #37094

Closed
wader opened this issue Feb 6, 2020 · 4 comments
Closed

proposal: exec: add (*Cmd).ExtraInPipe and ExtraOutPipe #37094

wader opened this issue Feb 6, 2020 · 4 comments

Comments

@wader
Copy link

wader commented Feb 6, 2020

Add convenience methods for using Cmd.ExtraFiles that returns io.ReadCloser and io.WriteCloser and also deal with file descriptor numbers in the child process. Similar to existing StdinPipe and StdoutPipe methods.

An implementation in the exec package could look something like this:

// ExtraInPipe returns a pipe and fd number that will be connected to a
// readable fd in the child process.
//
// See StdinPipe for how to handle close and exit.
func (c *Cmd) ExtraInPipe() (io.WriteCloser, uintptr, error) {
	pr, pw, err := os.Pipe()
	if err != nil {
		return nil, 0, err
	}
	c.ExtraFiles = append(c.ExtraFiles, pr)
	c.closeAfterStart = append(c.closeAfterStart, pr)
	wc := &closeOnce{File: pw}
	c.closeAfterWait = append(c.closeAfterWait, wc)
	return wc, uintptr(len(c.ExtraFiles)) + 2, nil
}

// ExtraOutPipe returns a pipe and a fd number that will be connected to a
// writable fd in the child process.
//
// See StdoutPipe for how to handle close and exit.
func (c *Cmd) ExtraOutPipe() (io.ReadCloser, uintptr, error) {
	pr, pw, err := os.Pipe()
	if err != nil {
		return nil, 0, err
	}
	c.ExtraFiles = append(c.ExtraFiles, pw)
	c.closeAfterStart = append(c.closeAfterStart, pw)
	c.closeAfterWait = append(c.closeAfterWait, pr)
	return pr, uintptr(len(c.ExtraFiles)) + 2, nil
}

Both are very similar to how StdinPipe and StdoutPipe works but also returns the fd number that will be available in the forked child process.

Some design note and issues:

If you want to use exec.Command() and pass an extra pipe fd number as argument you will need to modify Cmd.Args field afterwards.

What happens if ExtraInPipe or ExtraOutPipe is used but Cmd.Start() is never called? enough with os.Pipe() newFile close finalizer for cleanup? user should close them? same issue for StdinPipe and StdoutPipe i guess?

Might be unintuitive that the public ExtraFiles field gets modified.

Uses uintptr for fd number. Same as File.Fd().

Will not work on Windows. Same as for Cmd.ExtraFiles.

Related to #15460

@gopherbot gopherbot added this to the Proposal milestone Feb 6, 2020
@wader wader changed the title proposal: exec: add (*Cmd).ExtraOutPipe and ExtraInPipe proposal: exec: add (*Cmd).ExtraInPipe and ExtraOutPipe Feb 7, 2020
@bradfitz
Copy link
Contributor

I'm not sure there would be enough users/uses of this to warrant complicating the API.

@wader
Copy link
Author

wader commented Feb 23, 2020

@bradfitz Ok thanks. Close or wait for more comments or alternatives?

I did a "execextra" package with a Cmd type that embeds an exec.Cmd and wraps some of the methods. Seems to work quite well. Might add a link to it here if i feel happy with it.

@rsc
Copy link
Contributor

rsc commented Feb 26, 2020

Based on the discussion above, this seems like a likely decline.

@rsc
Copy link
Contributor

rsc commented Mar 4, 2020

No change in consensus, so declined.

@rsc rsc closed this as completed Mar 4, 2020
@rsc rsc moved this from Likely Decline to Declined in Proposals (old) Mar 4, 2020
@golang golang locked and limited conversation to collaborators Mar 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

4 participants