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

os/exec: support additional io.Reader and io.Writer streams #15460

Closed
docbrown opened this issue Apr 27, 2016 · 2 comments
Closed

os/exec: support additional io.Reader and io.Writer streams #15460

docbrown opened this issue Apr 27, 2016 · 2 comments

Comments

@docbrown
Copy link

docbrown commented Apr 27, 2016

The os/exec package currently only supports mapping io.Reader and io.Writer objects to Stdin, Stdout, and Stderr. It would be nice if Cmd supported adding additional readers and writers for other file descriptors. For example, I would like to have FFmpeg split an Ogg stream from Stdin into two separate io.Writers (one assigned to Stdout and the other on FD 3). I can do this by creating my own pipe and spinning up a goroutine to do the io.Copy, but that functionality is already built into os/exec. It just needs to be exposed in the public API. Perhaps something like:

cmd := exec.Command(...)
f, _ := cmd.WriterDescriptor(w) // or cmd.ReaderDescriptor(r)
cmd.ExtraFiles = append(cmd.ExtraFiles, f)
@bradfitz bradfitz added this to the Unplanned milestone Apr 27, 2016
@wader
Copy link

wader commented Jun 19, 2018

Hello, i've had similar thought (also using ffmpeg actually 😄) and have done some test implementations and after some fiddling settled on this API.

Add two new method to *Cmd:

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
}

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
}

That works the same way as StdinPipe/StdoutPipe but also returns the fd number that will be available in the forked process.

There are some issues with this design:

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

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

Might be unintuitive that the public ExtraFiles field gets modified?

I also tried changing Cmd.ExtraFiles to []interface{} but it felt weird and might not be backwards compatible?

@bradfitz would this be something worth adding to the standard library? some other nicer design possible?

@seankhliao
Copy link
Member

Duplicate of #37094

@seankhliao seankhliao marked this as a duplicate of #37094 Jun 8, 2022
@golang golang locked and limited conversation to collaborators Jun 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants