Compare commits

...
Sign in to create a new pull request.

12 commits

Author SHA1 Message Date
Nick Craig-Wood
3450d049b5 log: update windows redirection code to use x/sys/windows and include dup
This modernises the Windows redirect code to use x/sys/windows and to
dup os.Stderr for use in the terminal.
2022-10-07 16:14:23 +01:00
Nick Craig-Wood
16b383e18f fix go fmt 2022-10-07 16:10:15 +01:00
Carl Edquist
d59909fb8c log: rename passPromptFd to more generic termFd
as this duped copy now gets used for progress output, too.
2022-10-06 16:56:30 -05:00
Carl Edquist
cec47699d3 terminal, log: rename TerminalOutput to RawOut
it was confusing to have a terminal.Out along with a TerminalOutput
2022-10-06 16:42:22 -05:00
Carl Edquist
0687a263f8 config, terminal: use Out rather than TerminalOut where possible
We're going to try to avoid TerminalOut except for redirectStderr,
which overrides that var with a duped copy of stderr.
2022-10-06 16:41:26 -05:00
Carl Edquist
3192e00f4f progress: output final newline to terminal.Out
fmt.Println writes to stdout, which may be redirected and wrongly
included with the main program output (like the cat command)
instead of written to the terminal output.

Note the rest of the progress output goes to terminal.Out in
printProgress via terminal.Write()
2022-10-06 06:10:05 -05:00
Carl Edquist
14534c573a terminal: WriteTerminalTitle to terminal not stdout
The terminal title output should go to the same place as the rest of the
terminal output.  It's important _not_ to send it to stdout, where it
would get included with the main program output when redirected to a
file - for instance with the rclone cat command.
2022-10-06 02:45:42 -05:00
Carl Edquist
408d0c729b terminal: rename PasswordPromptOutput -> TerminalOutput
Give it a more general name as it gets used for other terminal things
like progress output
2022-10-06 02:45:35 -05:00
Carl Edquist
a716dc2533 config, terminal: move PasswordPromptOutput from config to terminal
First, move this variable in order to avoid circular imports between
config and terminal.  (config already depends on terminal.)

But also, it seems PasswordPromptOutput conceptually fits well in
terminal, as the idea is for it to store the output for the terminal.
2022-10-06 02:45:35 -05:00
Carl Edquist
28f0c08a98 terminal: use stdout on windows to preserve old behavior
--progress and --log-file should be usable together, but windows
does not currently preserve the original stderr in
config.PasswordPromptOutput; so use stdout explicitly to match the
old behavior.
2022-10-06 02:40:07 -05:00
Carl Edquist
458c477ad8 terminal: reuse PasswordPromptOutput for progress output
PasswordPromptOutput is a dup'ed copy of the stderr that the program
started with - in particular, before any redirection for --log-file
was applied.
2022-10-06 02:39:47 -05:00
Carl Edquist
7130a6d2e4 terminal: use stderr for terminal progress output, etc
Avoid mixing rclone command output with progress output by sending
terminal output to stderr.

Discussion:

The --progress option produces output designed for the terminal, and the
terminal library sent all its output to stdout.

This is a problem for any command that produces output for further
processing, when combined with --progress, because the main command
output and the progress output are then sent together to stdout.

This is most obviously a problem for the rclone 'cat' command.  Say you
want to retrieve a large file but output to a pipe for further
processing, rather than write it to a file.  But you also want rclone to
display its progress output as you wait for the transfer.  Because both
the 'cat' output and the progress output go to stdout, this leaves the
progress output as garbage data within the streamed cat output, and also
prevents the progress output from being displayed.

Notably, other rclone commands like 'ls', 'lsjson', and even 'm5dsum',
produce meaningful progress output with the --progress option, but when
mixed with the regular command output, it makes both the progress output
and the command output unusable.

The simple solution here is to send output intended for the terminal
(including progress output) to stderr instead of stdout.  This way the
rclone command output can be redirected as desired from stdout, and the
progress output will still go to the terminal attached to stderr.

If for some reason the user wants to capture/redirect the
terminal/progress output for some other purpose, stderr can be
redirected instead.
2022-09-30 15:13:36 -05:00
7 changed files with 45 additions and 28 deletions

View file

@ -62,7 +62,7 @@ func startProgress() func() {
printProgress("")
fs.LogPrint = oldLogPrint
operations.SyncPrintf = oldSyncPrint
fmt.Println("")
fmt.Fprintln(terminal.Out, "")
return
}
}

View file

@ -26,9 +26,6 @@ var (
// When nil, no encryption will be used for saving.
configKey []byte
// PasswordPromptOutput is output of prompt for password
PasswordPromptOutput = os.Stderr
// PassConfigKeyForDaemonization if set to true, the configKey
// is obscured with obscure.Obscure and saved to a temp file
// when it is calculated from the password. The path of that

View file

@ -716,9 +716,9 @@ func checkPassword(password string) (string, error) {
// GetPassword asks the user for a password with the prompt given.
func GetPassword(prompt string) string {
_, _ = fmt.Fprintln(PasswordPromptOutput, prompt)
_, _ = fmt.Fprintln(terminal.Out, prompt)
for {
_, _ = fmt.Fprint(PasswordPromptOutput, "password:")
_, _ = fmt.Fprint(terminal.Out, "password:")
password := ReadPassword()
password, err := checkPassword(password)
if err == nil {

View file

@ -9,17 +9,17 @@ import (
"log"
"os"
"github.com/rclone/rclone/fs/config"
"github.com/rclone/rclone/lib/terminal"
"golang.org/x/sys/unix"
)
// redirectStderr to the file passed in
func redirectStderr(f *os.File) {
passPromptFd, err := unix.Dup(int(os.Stderr.Fd()))
termFd, err := unix.Dup(int(os.Stderr.Fd()))
if err != nil {
log.Fatalf("Failed to duplicate stderr: %v", err)
}
config.PasswordPromptOutput = os.NewFile(uintptr(passPromptFd), "passPrompt")
terminal.RawOut = os.NewFile(uintptr(termFd), "termOut")
err = unix.Dup2(int(f.Fd()), int(os.Stderr.Fd()))
if err != nil {
log.Fatalf("Failed to redirect stderr to file: %v", err)

View file

@ -12,29 +12,43 @@ package log
import (
"log"
"os"
"syscall"
"github.com/rclone/rclone/lib/terminal"
"golang.org/x/sys/windows"
)
var (
kernel32 = syscall.MustLoadDLL("kernel32.dll")
procSetStdHandle = kernel32.MustFindProc("SetStdHandle")
)
func setStdHandle(stdhandle int32, handle syscall.Handle) error {
r0, _, e1 := syscall.Syscall(procSetStdHandle.Addr(), 2, uintptr(stdhandle), uintptr(handle), 0)
if r0 == 0 {
if e1 != 0 {
return error(e1)
}
return syscall.EINVAL
// dup oldfd creating a functional copy as newfd
// conceptually the same as the unix `dup()` function
func dup(oldfd uintptr) (newfd uintptr, err error) {
var (
newfdHandle windows.Handle
processHandle = windows.CurrentProcess()
)
err = windows.DuplicateHandle(
processHandle, // hSourceProcessHandle
windows.Handle(oldfd), // hSourceHandle
processHandle, // hTargetProcessHandle
&newfdHandle, // lpTargetHandle
0, // dwDesiredAccess
true, // bInheritHandle
windows.DUPLICATE_SAME_ACCESS, // dwOptions
)
if err != nil {
return 0, err
}
return nil
return uintptr(newfdHandle), nil
}
// redirectStderr to the file passed in
func redirectStderr(f *os.File) {
err := setStdHandle(syscall.STD_ERROR_HANDLE, syscall.Handle(f.Fd()))
termFd, err := dup(os.Stderr.Fd())
if err != nil {
log.Fatalf("Failed to duplicate stderr: %v", err)
}
terminal.RawOut = os.NewFile(termFd, "termOut")
err = windows.SetStdHandle(windows.STD_ERROR_HANDLE, windows.Handle(f.Fd()))
if err != nil {
log.Fatalf("Failed to redirect stderr to file: %v", err)
}
os.Stderr = f
}

View file

@ -68,17 +68,23 @@ const (
var (
// make sure that start is only called once
once sync.Once
// RawOut is the underlying *os.File intended for terminal output
RawOut = os.Stderr
)
// Start the terminal - must be called before use
func Start() {
once.Do(func() {
f := os.Stdout
f := RawOut
if !IsTerminal(int(f.Fd())) {
// If stdout not a tty then remove escape codes
// If output is not a tty then remove escape codes
Out = colorable.NewNonColorable(f)
} else if runtime.GOOS == "windows" && os.Getenv("TERM") != "" {
// If TERM is set just use stdout
// If TERM is set on Windows then we should just send output
// straight to the terminal for cygwin/git bash environments.
// We don't want to use NewColorable here because it will
// use Windows console calls which cygwin/git bash don't support.
Out = f
} else {
Out = colorable.NewColorable(f)

View file

@ -34,5 +34,5 @@ func ReadPassword(fd int) ([]byte, error) {
// WriteTerminalTitle writes a string to the terminal title
func WriteTerminalTitle(title string) {
fmt.Printf(ChangeTitle + title + BEL)
fmt.Fprintf(Out, ChangeTitle+title+BEL)
}