Merge pull request #6900 from thaJeztah/cli_stream_cleanups

cli/streams: assorted cleanups

Author: thaJeztah

cli/streams/in.go

@@ -3,17 +3,28 @@ package streams
 import (
 	"errors"
 	"io"
-	"os"
-	"runtime"
 
 	"github.com/moby/term"
 )
 
 // In is an input stream to read user input. It implements [io.ReadCloser]
 // with additional utilities, such as putting the terminal in raw mode.
 type In struct {
-	commonStream
 	in io.ReadCloser
+	cs commonStream
+}
+
+// NewIn returns a new [In] from an [io.ReadCloser].
+func NewIn(in io.ReadCloser) *In {
+	return &In{
+		in: in,
+		cs: newCommonStream(in),
+	}
+}
+
+// FD returns the file descriptor number for this stream.
+func (i *In) FD() uintptr {
+	return i.cs.fd
 }
 
 // Read implements the [io.Reader] interface.
@@ -26,36 +37,37 @@ func (i *In) Close() error {
 	return i.in.Close()
 }
 
+// IsTerminal returns whether this stream is connected to a terminal.
+func (i *In) IsTerminal() bool {
+	return i.cs.isTerminal()
+}
+
 // SetRawTerminal sets raw mode on the input terminal. It is a no-op if In
 // is not a TTY, or if the "NORAW" environment variable is set to a non-empty
 // value.
-func (i *In) SetRawTerminal() (err error) {
-	if !i.isTerminal || os.Getenv("NORAW") != "" {
-		return nil
-	}
-	i.state, err = term.SetRawTerminal(i.fd)
-	return err
+func (i *In) SetRawTerminal() error {
+	return i.cs.setRawTerminal(term.SetRawTerminal)
+}
+
+// RestoreTerminal restores the terminal state if SetRawTerminal succeeded earlier.
+func (i *In) RestoreTerminal() {
+	i.cs.restoreTerminal()
 }
 
-// CheckTty checks if we are trying to attach to a container TTY
-// from a non-TTY client input stream, and if so, returns an error.
+// CheckTty reports an error when stdin is requested for a TTY-enabled
+// container, but the client stdin is not itself a terminal (for example,
+// when input is piped or redirected).
 func (i *In) CheckTty(attachStdin, ttyMode bool) error {
-	// In order to attach to a container tty, input stream for the client must
-	// be a tty itself: redirecting or piping the client standard input is
-	// incompatible with `docker run -t`, `docker exec -t` or `docker attach`.
-	if ttyMode && attachStdin && !i.isTerminal {
-		const eText = "the input device is not a TTY"
-		if runtime.GOOS == "windows" {
-			return errors.New(eText + ".  If you are using mintty, try prefixing the command with 'winpty'")
-		}
-		return errors.New(eText)
+	// TODO(thaJeztah): consider inlining this code and deprecating the method.
+	if !ttyMode || !attachStdin || i.cs.isTerminal() {
+		return nil
 	}
-	return nil
+	return errors.New("cannot attach stdin to a TTY-enabled container because stdin is not a terminal")
 }
 
-// NewIn returns a new [In] from an [io.ReadCloser].
-func NewIn(in io.ReadCloser) *In {
-	i := &In{in: in}
-	i.fd, i.isTerminal = term.GetFdInfo(in)
-	return i
+// SetIsTerminal overrides whether a terminal is connected. It is used to
+// override this property in unit-tests, and should not be depended on for
+// other purposes.
+func (i *In) SetIsTerminal(isTerminal bool) {
+	i.cs.setIsTerminal(isTerminal)
 }

cli/streams/out.go

@@ -2,57 +2,65 @@ package streams
 
 import (
 	"io"
-	"os"
 
 	"github.com/moby/term"
-	"github.com/sirupsen/logrus"
 )
 
 // Out is an output stream to write normal program output. It implements
 // an [io.Writer], with additional utilities for detecting whether a terminal
 // is connected, getting the TTY size, and putting the terminal in raw mode.
 type Out struct {
-	commonStream
 	out io.Writer
+	cs  commonStream
 }
 
+// NewOut returns a new [Out] from an [io.Writer].
+func NewOut(out io.Writer) *Out {
+	return &Out{
+		out: out,
+		cs:  newCommonStream(out),
+	}
+}
+
+// FD returns the file descriptor number for this stream.
+func (o *Out) FD() uintptr {
+	return o.cs.FD()
+}
+
+// Write writes to the output stream.
 func (o *Out) Write(p []byte) (int, error) {
 	return o.out.Write(p)
 }
 
+// IsTerminal returns whether this stream is connected to a terminal.
+func (o *Out) IsTerminal() bool {
+	return o.cs.isTerminal()
+}
+
 // SetRawTerminal puts the output of the terminal connected to the stream
 // into raw mode.
 //
 // On UNIX, this does nothing. On Windows, it disables LF -> CRLF/ translation.
 // It is a no-op if Out is not a TTY, or if the "NORAW" environment variable is
 // set to a non-empty value.
-func (o *Out) SetRawTerminal() (err error) {
-	if !o.isTerminal || os.Getenv("NORAW") != "" {
-		return nil
-	}
-	o.state, err = term.SetRawTerminalOutput(o.fd)
-	return err
+func (o *Out) SetRawTerminal() error {
+	return o.cs.setRawTerminal(term.SetRawTerminalOutput)
+}
+
+// RestoreTerminal restores the terminal state if SetRawTerminal succeeded earlier.
+func (o *Out) RestoreTerminal() {
+	o.cs.restoreTerminal()
 }
 
 // GetTtySize returns the height and width in characters of the TTY, or
 // zero for both if no TTY is connected.
 func (o *Out) GetTtySize() (height uint, width uint) {
-	if !o.isTerminal {
-		return 0, 0
-	}
-	ws, err := term.GetWinsize(o.fd)
-	if err != nil {
-		logrus.WithError(err).Debug("Error getting TTY size")
-		if ws == nil {
-			return 0, 0
-		}
-	}
-	return uint(ws.Height), uint(ws.Width)
+	return o.cs.terminalSize()
 }
 
-// NewOut returns a new [Out] from an [io.Writer].
-func NewOut(out io.Writer) *Out {
-	o := &Out{out: out}
-	o.fd, o.isTerminal = term.GetFdInfo(out)
-	return o
+// SetIsTerminal overrides whether a terminal is connected. It is used to
+// override this property in unit-tests, and should not be depended on for
+// other purposes.
+func (o *Out) SetIsTerminal(isTerminal bool) {
+	o.cs.setIsTerminal(isTerminal)
 }

cli/streams/stream.go

@@ -1,35 +1,67 @@
+// FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16:
+//go:build go1.25
+
 package streams
 
 import (
+	"os"
+
 	"github.com/moby/term"
+	"github.com/sirupsen/logrus"
 )
 
+func newCommonStream(stream any) commonStream {
+	fd, tty := term.GetFdInfo(stream)
+	return commonStream{
+		fd:  fd,
+		tty: tty,
+	}
+}
+
 type commonStream struct {
-	fd         uintptr
-	isTerminal bool
-	state      *term.State
+	fd    uintptr
+	tty   bool
+	state *term.State
 }
 
 // FD returns the file descriptor number for this stream.
-func (s *commonStream) FD() uintptr {
-	return s.fd
-}
+func (s *commonStream) FD() uintptr { return s.fd }
 
-// IsTerminal returns true if this stream is connected to a terminal.
-func (s *commonStream) IsTerminal() bool {
-	return s.isTerminal
-}
+// isTerminal returns whether this stream is connected to a terminal.
+func (s *commonStream) isTerminal() bool { return s.tty }
 
-// RestoreTerminal restores normal mode to the terminal.
-func (s *commonStream) RestoreTerminal() {
+// setIsTerminal overrides whether a terminal is connected for testing.
+func (s *commonStream) setIsTerminal(isTerminal bool) { s.tty = isTerminal }
+
+// restoreTerminal restores the terminal state if SetRawTerminal succeeded earlier.
+func (s *commonStream) restoreTerminal() {
 	if s.state != nil {
 		_ = term.RestoreTerminal(s.fd, s.state)
 	}
 }
 
-// SetIsTerminal overrides whether a terminal is connected. It is used to
-// override this property in unit-tests, and should not be depended on for
-// other purposes.
-func (s *commonStream) SetIsTerminal(isTerminal bool) {
-	s.isTerminal = isTerminal
+func (s *commonStream) setRawTerminal(setter func(uintptr) (*term.State, error)) error {
+	if !s.tty || os.Getenv("NORAW") != "" {
+		return nil
+	}
+	state, err := setter(s.fd)
+	if err != nil {
+		return err
+	}
+	s.state = state
+	return nil
+}
+
+func (s *commonStream) terminalSize() (height uint, width uint) {
+	if !s.tty {
+		return 0, 0
+	}
+	ws, err := term.GetWinsize(s.fd)
+	if err != nil {
+		logrus.WithError(err).Debug("Error getting TTY size")
+		if ws == nil {
+			return 0, 0
+		}
+	}
+	return uint(ws.Height), uint(ws.Width)
 }