diff --git a/changelog/unreleased/issue-3111 b/changelog/unreleased/issue-3111 new file mode 100644 index 000000000..56ccc86b7 --- /dev/null +++ b/changelog/unreleased/issue-3111 @@ -0,0 +1,11 @@ +Bugfix: Fix terminal output redirection for powershell + +When redirecting the output of restic using powershell on Windows, the +output contained terminal escape characters. This has been fixed by +properly detecting the terminal type. + +In addition, the mintty terminal now shows progress output for the backup +command. + +https://github.com/restic/restic/issues/3111 +https://github.com/restic/restic/pull/3325 diff --git a/cmd/restic/global.go b/cmd/restic/global.go index f5255e22b..8d02b228e 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -31,6 +31,7 @@ import ( "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" "github.com/restic/restic/internal/textfile" + "github.com/restic/restic/internal/ui/termstatus" "github.com/restic/restic/internal/errors" @@ -142,7 +143,13 @@ func stdinIsTerminal() bool { } func stdoutIsTerminal() bool { - return terminal.IsTerminal(int(os.Stdout.Fd())) + // mintty on windows can use pipes which behave like a posix terminal, + // but which are not a terminal handle + return terminal.IsTerminal(int(os.Stdout.Fd())) || stdoutCanUpdateStatus() +} + +func stdoutCanUpdateStatus() bool { + return termstatus.CanUpdateStatus(os.Stdout.Fd()) } func stdoutTerminalWidth() int { @@ -159,7 +166,7 @@ func stdoutTerminalWidth() int { // program execution must revert changes to the terminal configuration itself. // The terminal configuration is only restored while reading a password. func restoreTerminal() { - if !stdoutIsTerminal() { + if !terminal.IsTerminal(int(os.Stdout.Fd())) { return } @@ -248,7 +255,7 @@ func PrintProgress(format string, args ...interface{}) { message = fmt.Sprintf(format, args...) if !(strings.HasSuffix(message, "\r") || strings.HasSuffix(message, "\n")) { - if stdoutIsTerminal() { + if stdoutCanUpdateStatus() { carriageControl = "\r" } else { carriageControl = "\n" @@ -256,7 +263,7 @@ func PrintProgress(format string, args ...interface{}) { message = fmt.Sprintf("%s%s", message, carriageControl) } - if stdoutIsTerminal() { + if stdoutCanUpdateStatus() { message = fmt.Sprintf("%s%s", ClearLine(), message) } diff --git a/cmd/restic/progress.go b/cmd/restic/progress.go index 62f2e6396..0c2a24271 100644 --- a/cmd/restic/progress.go +++ b/cmd/restic/progress.go @@ -20,7 +20,7 @@ func calculateProgressInterval(show bool) time.Duration { fps = 60 } interval = time.Duration(float64(time.Second) / fps) - } else if !stdoutIsTerminal() || !show { + } else if !stdoutCanUpdateStatus() || !show { interval = 0 } return interval diff --git a/go.mod b/go.mod index 32c8d19ba..b27b58185 100644 --- a/go.mod +++ b/go.mod @@ -37,7 +37,7 @@ require ( golang.org/x/net v0.0.0-20200904194848-62affa334b73 golang.org/x/oauth2 v0.0.0-20200902213428-5d25da1a8d43 golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 - golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 + golang.org/x/sys v0.0.0-20210305230114-8fe3ee5dd75b golang.org/x/text v0.3.4 google.golang.org/api v0.32.0 gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b // indirect diff --git a/go.sum b/go.sum index 114b206b1..d03c1708d 100644 --- a/go.sum +++ b/go.sum @@ -379,8 +379,9 @@ golang.org/x/sys v0.0.0-20200803210538-64077c9b5642/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200828194041-157a740278f4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200905004654-be1d3432aa8f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201101102859-da207088b7d1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210305230114-8fe3ee5dd75b h1:ggRgirZABFolTmi3sn6Ivd9SipZwLedQ5wR0aAKnFxU= +golang.org/x/sys v0.0.0-20210305230114-8fe3ee5dd75b/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221 h1:/ZHdbVpdR/jk3g30/d4yUL0JU9kksj8+F/bnQUVLGDM= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/internal/ui/termstatus/status.go b/internal/ui/termstatus/status.go index 6b50effbb..e275f5b7d 100644 --- a/internal/ui/termstatus/status.go +++ b/internal/ui/termstatus/status.go @@ -67,7 +67,7 @@ func New(wr io.Writer, errWriter io.Writer, disableStatus bool) *Terminal { return t } - if d, ok := wr.(fder); ok && canUpdateStatus(d.Fd()) { + if d, ok := wr.(fder); ok && CanUpdateStatus(d.Fd()) { // only use the fancy status code when we're running on a real terminal. t.canUpdateStatus = true t.fd = d.Fd() diff --git a/internal/ui/termstatus/terminal_unix.go b/internal/ui/termstatus/terminal_unix.go index 3f0061c01..67ce06b0b 100644 --- a/internal/ui/termstatus/terminal_unix.go +++ b/internal/ui/termstatus/terminal_unix.go @@ -20,9 +20,9 @@ func moveCursorUp(wr io.Writer, fd uintptr) func(io.Writer, uintptr, int) { return posixMoveCursorUp } -// canUpdateStatus returns true if status lines can be printed, the process +// CanUpdateStatus returns true if status lines can be printed, the process // output is not redirected to a file or pipe. -func canUpdateStatus(fd uintptr) bool { +func CanUpdateStatus(fd uintptr) bool { if !terminal.IsTerminal(int(fd)) { return false } diff --git a/internal/ui/termstatus/terminal_windows.go b/internal/ui/termstatus/terminal_windows.go index 723aebdff..f7217e35d 100644 --- a/internal/ui/termstatus/terminal_windows.go +++ b/internal/ui/termstatus/terminal_windows.go @@ -4,6 +4,7 @@ package termstatus import ( "io" + "strings" "syscall" "unsafe" @@ -80,19 +81,47 @@ func isPipe(fd uintptr) bool { return err == nil && typ == windows.FILE_TYPE_PIPE } -// canUpdateStatus returns true if status lines can be printed, the process +func getFileNameByHandle(fd uintptr) (string, error) { + type FILE_NAME_INFO struct { + FileNameLength int32 + FileName [windows.MAX_LONG_PATH]uint16 + } + + var fi FILE_NAME_INFO + err := windows.GetFileInformationByHandleEx(windows.Handle(fd), windows.FileNameInfo, (*byte)(unsafe.Pointer(&fi)), uint32(unsafe.Sizeof(fi))) + if err != nil { + return "", err + } + + filename := syscall.UTF16ToString(fi.FileName[:]) + return filename, nil +} + +// CanUpdateStatus returns true if status lines can be printed, the process // output is not redirected to a file or pipe. -func canUpdateStatus(fd uintptr) bool { +func CanUpdateStatus(fd uintptr) bool { // easy case, the terminal is cmd or psh, without redirection if isWindowsTerminal(fd) { return true } - // check that the output file type is a pipe (0x0003) + // pipes require special handling if !isPipe(fd) { return false } - // assume we're running in mintty/cygwin - return true + fn, err := getFileNameByHandle(fd) + if err != nil { + return false + } + + // inspired by https://github.com/RyanGlScott/mintty/blob/master/src/System/Console/MinTTY/Win32.hsc + // terminal: \msys-dd50a72ab4668b33-pty0-to-master + // pipe to cat: \msys-dd50a72ab4668b33-13244-pipe-0x16 + if (strings.HasPrefix(fn, "\\cygwin-") || strings.HasPrefix(fn, "\\msys-")) && + strings.Contains(fn, "-pty") && strings.HasSuffix(fn, "-master") { + return true + } + + return false } diff --git a/internal/ui/termstatus/terminal_windows_test.go b/internal/ui/termstatus/terminal_windows_test.go new file mode 100644 index 000000000..e6eb39dd5 --- /dev/null +++ b/internal/ui/termstatus/terminal_windows_test.go @@ -0,0 +1,30 @@ +package termstatus + +import ( + "syscall" + "testing" + + "golang.org/x/sys/windows" + + rtest "github.com/restic/restic/internal/test" +) + +func TestIsMinTTY(t *testing.T) { + for _, test := range []struct { + path string + result bool + }{ + {`\\.\pipe\msys-dd50a72ab4668b33-pty0-to-master`, true}, + {`\\.\pipe\msys-dd50a72ab4668b33-13244-pipe-0x16`, false}, + } { + filename, err := syscall.UTF16FromString(test.path) + rtest.OK(t, err) + handle, err := windows.CreateNamedPipe(&filename[0], windows.PIPE_ACCESS_DUPLEX, + windows.PIPE_TYPE_BYTE, 1, 1024, 1024, 0, nil) + rtest.OK(t, err) + defer windows.CloseHandle(handle) + + rtest.Assert(t, CanUpdateStatus(uintptr(handle)) == test.result, + "expected CanUpdateStatus(%v) == %v", test.path, test.result) + } +}