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: os/v2: Stdin, Stdout and Stderr should be interfaces #13473

Open
robpike opened this issue Dec 3, 2015 · 21 comments
Open

proposal: os/v2: Stdin, Stdout and Stderr should be interfaces #13473

robpike opened this issue Dec 3, 2015 · 21 comments
Labels
Proposal v2 An incompatible library change
Milestone

Comments

@robpike
Copy link
Contributor

robpike commented Dec 3, 2015

The three variables os.Stdin, os.Stdout, and os.Stderr are all *Files, for historical reasons (they predate the io.Writer interface definition).

They should be of type io.Reader and io.Writer, respectively. This would make it easier to do interesting things with special input or output processors. For instance, one could say

os.Stdout = bufio.NewWriter(os.Stdout)
os.Stderr = bufio.NewWriter(os.Stderr)

and all output, including from log.Printf and fmt.Printf, would be buffered. Much more imaginative things would also be possible.

Also, *File is the root of a large dependency tree that simple programs would be able to avoid.

Can't change it now, but we could in Go 2.

@robpike robpike added the v2 An incompatible library change label Dec 3, 2015
@extemporalgenome
Copy link
Contributor

It's not unheard-of to legitimately close stdio streams ("I'm done with you"). Would it be worth making these io.ReadCloser and io.WriteCloser, respectively (if we had a convenient writer equivalent to io.NopCloser)?

An aside: since stdio streams are just the first 3 file descriptors (and are technically files), there's nothing to prevent your process from receiving a seekable regular file as stdin, or a file opened in rw mode for stdout, etc. This is a fairly common occurrence in practice (redirecting input from a file), but it's rare for applications to notice or care that, e.g. stdin might be able to do more than read bytes. However, the functionality needed to handle these very rare cases could be regained with a call to os.NewFile, passing in 0, 1, or 2 as the fd.

@extemporalgenome
Copy link
Contributor

An example of what I mean by the aside:

// stdin-stat.go

package main

import (
    "fmt"
    "log"
    "os"
)

func main() {
    info, err := os.Stdin.Stat()
    if err != nil {
        log.Fatalln(err)
    }
    fmt.Println("name:", info.Name())
    fmt.Println("mode:", info.Mode())
    fmt.Println("size:", info.Size())
}

If we compile this as stdin-stat, we can run:

$ ./stdin-stat 
name: stdin
mode: Dcrw--w----
size: 0
$ ./stdin-stat < stdin-stat.go 
name: stdin
mode: -rw-r--r--
size: 236
$ yes | ./stdin-stat 
name: stdin
mode: prw-rw----
size: 49152

(above results correspond to darwin 14.5.0)

@bronze1man
Copy link
Contributor

I think we can support interfaced Stdin, Stdout and Stderr in go1.x
Just leave the os.Stdin os.Stdout os.Stderr there and Obsolete, but old api works without touch new api.
And add

var StdinV2 io.ReadCloser = xxx 
var StdoutV2 io.WriteCloser = xxx
var StderrV2 io.WriteCloser = xxx

and redirect panic to StdoutV2 and redirect fmt.Println to StderrV2,etc..

I am writing a ios app with PacketTunnelExtension with golang which can see NSLog, but can not see the stdout and stderr .I can only rediect stdout and stderr into a temp file ,and read the content with next startup.It will be good to have a callback to rediect the panic content into NSLog.

@cznic
Copy link
Contributor

cznic commented Feb 17, 2016

The *Vn thing looks too ugly to me.

@minux
Copy link
Member

minux commented Feb 17, 2016 via email

@rsc
Copy link
Contributor

rsc commented Feb 17, 2016

This is a long-term issue that we are not actively working on.

@gertcuykens
Copy link
Contributor

Can this be implemented as unsafe.os.Stdout maybe? A Go2 label is the worst label there is. It means great idea but never going to happen :)

@jcw
Copy link

jcw commented May 21, 2017

What is the current thinking on this?

I currently have to set up an os.Pipe to be able to override Stdout (and StdErr) - our use case is that we need to insert a CR before each LF when the readline package is active, since that sets the terminal to raw mode.

See https://github.com/jeelabs/folie/blob/master/console.go#L17-L19

@robpike
Copy link
Contributor Author

robpike commented May 21, 2017

As @rsc says, this is a long-term issue that we are not actively working on.

@rsc rsc changed the title os: Stdin, Stdout and Stderr should be interfaces proposal: os: Stdin, Stdout and Stderr should be interfaces Jun 17, 2017
@robpike
Copy link
Contributor Author

robpike commented Dec 1, 2017

It might be possible to do this almost cleanly without breaking anything. The variables os.Stdin etc. are of type *os.File, and the current definition of os.File is

type File struct {
	*file // os specific
}

while file type varies but is always simple, as in this Unix variant:

type file struct {
	pfd         poll.FD
	name        string
	dirinfo     *dirInfo // nil unless directory being read
	nonblock    bool     // whether we set nonblocking mode
	stdoutOrErr bool     // whether this is stdout or stderr
}

Because File is already accessible only through methods, it would be possible to hack the way file works so that one can simulate a non-File implementation that does ReadCloser etc.

Then one could add functions to package os such as os.SetStdin(io.ReadCloser) that would change the variable to be simulated file rather than a real one.

For programs that only use os.Stdin etc. but don't change it, which is the vast majority of existing code, everything would work as before. For new programs that want to capture standard I/O into Readers and Writers, this would be possible and existing libraries would work as desired.

The main difference between this and perfection is that one can't do

os.Stdin = reader

but must instead call a function. That might be a viable compromise.

Another approach would be to provide os.StdinReader etc. and connect the file type to this; that would be an embellishment of the ideas above.

But I might be missing something.

Remember that this is for Go 2, so slavish compatibility might be set aside for the transition.

@gertcuykens
Copy link
Contributor

gertcuykens commented Dec 1, 2017

Go for it please :) It will be ok, aim for perfection, the community owes you a second chance to achieve it. Can't be that hard to transpile Go1.x to Go2.x code anyway.

@bcmills
Copy link
Contributor

bcmills commented Feb 5, 2018

Packages that attempt to change the behavior of os.Stderr and the like by assigning to them are the source of numerous concurrency bugs. (What happens if a package tries to log a warning to stderr from a goroutine started during package-init?)

If we want to make these outputs more configurable, we should add a proper API for configuring them rather than encouraging users to monkey-patch through global variables.

@bronze1man
Copy link
Contributor

How about redirect the output of panic , println or the runtime.WriteErr? Only change the behavior of os.Stderr or os.Stdout do not affect it.

@bcmills
Copy link
Contributor

bcmills commented Apr 10, 2018

I've been thinking about this some more.

There may be a role for reassignable default inputs and outputs, but the os package is not the right place for them. os.Stdout should be the standard output defined by the operating system, invariantly.

If we instead mean “an arbitrary default io.Writer”, that belongs in the io package.

If we mean “the destination of fmt.Print functions”, that belongs in the fmt package, and should be an interface with exactly (and only) the methods required by that package.

@bradfitz
Copy link
Contributor

I have a counterproposal in the two comments starting at #14106 (comment)

@KevinBuchs
Copy link

Would be helpful in testing programs which read/write to std*. Nice to capture panic generated output.

@tv42
Copy link

tv42 commented Aug 19, 2019

@KevinBuchs You want to avoid globals for the system-under-test anyway. Once you extract that part of the program into a more testable func foo(r io.Reader, w io.Writer) error (or such), your tests will become much nicer, won't accidentally pollute each other, will be parallelizable, etc.

@KevinBuchs
Copy link

Yes, agreed. But my tests can't handle panic(). I cannot overwrite os.Stderr just before I panic to direct the output to a io.Writer.

@bcmills
Copy link
Contributor

bcmills commented Aug 19, 2019

@KevinBuchs, if you really intend to test the exact text of your program's panic messages, you can always run the program as a separate process and capture its stderr output using a pipe.

That said, in my experience it is almost always a mistake to rely on the exact text of a program's panic messages: it is too sensitive to the implementation details of the runtime and compiler, the process's environment, and the general behavior of the program in case of erroneous inputs — if the behavior of the program is defined concretely enough to test, it should usually be defined as a programmer-controlled error message, not a panic.

@KevinBuchs
Copy link

Yes, thanks for the suggestions. I was hoping to be able to capture and output some diagnostic information if there was a panic. I don't care about it for test pass/fail, but I care about it because I want to determine the issue so I can solve it.

@bcmills
Copy link
Contributor

bcmills commented Oct 11, 2019

Here's another reason we should avoid making os.Std* general configuration points: making them replaceable sets a general precedent that leads users to try to replace other global variables derived from standard aspects of the operating system, such as time.Local (#34814).

@ianlancetaylor ianlancetaylor changed the title proposal: os: Stdin, Stdout and Stderr should be interfaces proposal: os/v2: Stdin, Stdout and Stderr should be interfaces Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests