From f27927d934bdc1e9e9d82bfd50338a4f5ced4048 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 29 Mar 2023 23:05:54 +0000 Subject: [PATCH 1/6] cp: do not emit progress if stderr is not a term This fixes a case where a non-tty will have control characters + the log line for every single read operation. Signed-off-by: Brian Goff --- cli/command/container/cp.go | 50 +++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index 67e5e598ea..0ac209ea14 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -10,6 +10,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/streams" "github.com/docker/docker/api/types" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/system" @@ -51,19 +52,22 @@ type copyProgressPrinter struct { toContainer bool total *float64 writer io.Writer + isTerm bool } func (pt *copyProgressPrinter) Read(p []byte) (int, error) { n, err := pt.ReadCloser.Read(p) - *pt.total += float64(n) + if n > 0 { + *pt.total += float64(n) - if err == nil { - fmt.Fprint(pt.writer, aec.Restore) - fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.All)) - if pt.toContainer { - fmt.Fprintln(pt.writer, "Copying to container - "+units.HumanSize(*pt.total)) - } else { - fmt.Fprintln(pt.writer, "Copying from container - "+units.HumanSize(*pt.total)) + if pt.isTerm { + fmt.Fprint(pt.writer, aec.Restore) + fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.All)) + if pt.toContainer { + fmt.Fprintln(pt.writer, "Copying to container - "+units.HumanSize(*pt.total)) + } else { + fmt.Fprintln(pt.writer, "Copying from container - "+units.HumanSize(*pt.total)) + } } } @@ -211,6 +215,8 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp RebaseName: rebaseName, } + stderrIsTerm := streams.NewOut(dockerCli.Err()).IsTerminal() + var copiedSize float64 if !copyConfig.quiet { content = ©ProgressPrinter{ @@ -218,6 +224,7 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp toContainer: false, writer: dockerCli.Err(), total: &copiedSize, + isTerm: stderrIsTerm, } } @@ -231,11 +238,15 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp return archive.CopyTo(preArchive, srcInfo, dstPath) } - fmt.Fprint(dockerCli.Err(), aec.Save) - fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") + if stderrIsTerm { + fmt.Fprint(dockerCli.Err(), aec.Save) + fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") + } res := archive.CopyTo(preArchive, srcInfo, dstPath) - fmt.Fprint(dockerCli.Err(), aec.Restore) - fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) + if stderrIsTerm { + fmt.Fprint(dockerCli.Err(), aec.Restore) + fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) + } fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", dstPath) return res @@ -296,6 +307,8 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo copiedSize float64 ) + stderrIsTerm := streams.NewOut(dockerCli.Err()).IsTerminal() + if srcPath == "-" { content = os.Stdin resolvedDstPath = dstInfo.Path @@ -341,6 +354,7 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo toContainer: true, writer: dockerCli.Err(), total: &copiedSize, + isTerm: stderrIsTerm, } } } @@ -354,11 +368,15 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo return client.CopyToContainer(ctx, copyConfig.container, resolvedDstPath, content, options) } - fmt.Fprint(dockerCli.Err(), aec.Save) - fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") + if stderrIsTerm { + fmt.Fprint(dockerCli.Err(), aec.Save) + fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") + } res := client.CopyToContainer(ctx, copyConfig.container, resolvedDstPath, content, options) - fmt.Fprint(dockerCli.Err(), aec.Restore) - fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) + if stderrIsTerm { + fmt.Fprint(dockerCli.Err(), aec.Restore) + fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) + } fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", copyConfig.container+":"+dstInfo.Path) return res From ccae6e9299e9c2d7c9e53bd4a485d43581a795ee Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 29 Mar 2023 23:47:50 +0000 Subject: [PATCH 2/6] cp: Improve tty flashing on progress updates - Instead of rewriting the entire line every time only clear and write the parts that changed. - Hide the cursor while writing progress Both these things make the progress updates significantly easier to read. Signed-off-by: Brian Goff --- cli/command/container/cp.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index 0ac209ea14..42205918c3 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -58,16 +58,24 @@ type copyProgressPrinter struct { func (pt *copyProgressPrinter) Read(p []byte) (int, error) { n, err := pt.ReadCloser.Read(p) if n > 0 { + isFirst := *pt.total == 0 *pt.total += float64(n) if pt.isTerm { - fmt.Fprint(pt.writer, aec.Restore) - fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.All)) + var header string if pt.toContainer { - fmt.Fprintln(pt.writer, "Copying to container - "+units.HumanSize(*pt.total)) + header = "Copying to container - " } else { - fmt.Fprintln(pt.writer, "Copying from container - "+units.HumanSize(*pt.total)) + header = "Copying from container - " } + if isFirst { + fmt.Fprint(pt.writer, aec.Restore) + fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.All)) + fmt.Fprint(pt.writer, header) + } + fmt.Fprint(pt.writer, aec.Column(uint(len(header)+1))) + fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.Tail)) + fmt.Fprint(pt.writer, units.HumanSize(*pt.total)) } } @@ -239,6 +247,7 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp } if stderrIsTerm { + fmt.Fprint(dockerCli.Err(), aec.Hide) fmt.Fprint(dockerCli.Err(), aec.Save) fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") } @@ -246,6 +255,7 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp if stderrIsTerm { fmt.Fprint(dockerCli.Err(), aec.Restore) fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) + fmt.Fprint(dockerCli.Err(), aec.Show) } fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", dstPath) @@ -369,6 +379,7 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo } if stderrIsTerm { + fmt.Fprint(dockerCli.Err(), aec.Hide) fmt.Fprint(dockerCli.Err(), aec.Save) fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") } @@ -376,6 +387,7 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo if stderrIsTerm { fmt.Fprint(dockerCli.Err(), aec.Restore) fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) + fmt.Fprint(dockerCli.Err(), aec.Show) } fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", copyConfig.container+":"+dstInfo.Path) From efd011b793ad3b9440acabd7ea95a109baf3e7fb Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 30 Mar 2023 00:00:03 +0000 Subject: [PATCH 3/6] cp: reduce branching in progress printer This just makes it easier to reason about what is happening. Signed-off-by: Brian Goff --- cli/command/container/cp.go | 68 ++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index 42205918c3..7d88783276 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -49,35 +49,39 @@ type cpConfig struct { // copying files to/from a container. type copyProgressPrinter struct { io.ReadCloser - toContainer bool - total *float64 - writer io.Writer - isTerm bool + total *float64 + writer io.Writer + isTerm bool + header string } +const ( + copyToContainerHeader = "Copying to container - " + copyFromContainerHeader = "Copying from container - " +) + func (pt *copyProgressPrinter) Read(p []byte) (int, error) { n, err := pt.ReadCloser.Read(p) + isFirst := *pt.total == 0 if n > 0 { - isFirst := *pt.total == 0 *pt.total += float64(n) - - if pt.isTerm { - var header string - if pt.toContainer { - header = "Copying to container - " - } else { - header = "Copying from container - " - } - if isFirst { - fmt.Fprint(pt.writer, aec.Restore) - fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.All)) - fmt.Fprint(pt.writer, header) - } - fmt.Fprint(pt.writer, aec.Column(uint(len(header)+1))) - fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.Tail)) - fmt.Fprint(pt.writer, units.HumanSize(*pt.total)) - } } + if err != nil && err != io.EOF { + return n, err + } + + if !pt.isTerm { + return n, err + } + + if isFirst { + fmt.Fprint(pt.writer, aec.Restore) + fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.All)) + fmt.Fprint(pt.writer, pt.header) + } + fmt.Fprint(pt.writer, aec.Column(uint(len(pt.header)+1))) + fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.Tail)) + fmt.Fprint(pt.writer, units.HumanSize(*pt.total)) return n, err } @@ -228,11 +232,11 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp var copiedSize float64 if !copyConfig.quiet { content = ©ProgressPrinter{ - ReadCloser: content, - toContainer: false, - writer: dockerCli.Err(), - total: &copiedSize, - isTerm: stderrIsTerm, + ReadCloser: content, + writer: dockerCli.Err(), + total: &copiedSize, + isTerm: stderrIsTerm, + header: copyFromContainerHeader, } } @@ -360,11 +364,11 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo content = preparedArchive if !copyConfig.quiet { content = ©ProgressPrinter{ - ReadCloser: content, - toContainer: true, - writer: dockerCli.Err(), - total: &copiedSize, - isTerm: stderrIsTerm, + ReadCloser: content, + writer: dockerCli.Err(), + total: &copiedSize, + isTerm: stderrIsTerm, + header: copyToContainerHeader, } } } From 90b7bc36d4ef13c6c79ed518337503d4696398ec Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 30 Mar 2023 00:32:43 +0000 Subject: [PATCH 4/6] cp: Reduce number of progress updates Only show progress updates after a time threshold has elapsed in order to reduce the number of writes to the terminal. This improves readability of the progress. Also moves cursor show/hide into the progress printer to reduce chances if messing up the user's terminal in case of cancellation. Signed-off-by: Brian Goff --- cli/command/container/cp.go | 38 +++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index 7d88783276..ad053522c5 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -49,15 +50,17 @@ type cpConfig struct { // copying files to/from a container. type copyProgressPrinter struct { io.ReadCloser - total *float64 - writer io.Writer - isTerm bool - header string + total *float64 + writer io.Writer + isTerm bool + header string + lastUpdate time.Time } const ( - copyToContainerHeader = "Copying to container - " - copyFromContainerHeader = "Copying from container - " + copyToContainerHeader = "Copying to container - " + copyFromContainerHeader = "Copying from container - " + copyProgressUpdateThreshold = 75 * time.Millisecond ) func (pt *copyProgressPrinter) Read(p []byte) (int, error) { @@ -66,23 +69,30 @@ func (pt *copyProgressPrinter) Read(p []byte) (int, error) { if n > 0 { *pt.total += float64(n) } - if err != nil && err != io.EOF { + if !pt.isTerm { return n, err } - if !pt.isTerm { - return n, err + doUpdate := func() { + fmt.Fprint(pt.writer, aec.Hide) + fmt.Fprint(pt.writer, aec.Column(uint(len(pt.header)+1))) + fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.Tail)) + fmt.Fprint(pt.writer, units.HumanSize(*pt.total)) + fmt.Fprint(pt.writer, aec.Show) + pt.lastUpdate = time.Now() } if isFirst { fmt.Fprint(pt.writer, aec.Restore) fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.All)) fmt.Fprint(pt.writer, pt.header) + doUpdate() + return n, err } - fmt.Fprint(pt.writer, aec.Column(uint(len(pt.header)+1))) - fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.Tail)) - fmt.Fprint(pt.writer, units.HumanSize(*pt.total)) + if err != nil || time.Since(pt.lastUpdate) > copyProgressUpdateThreshold { + doUpdate() + } return n, err } @@ -251,7 +261,6 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp } if stderrIsTerm { - fmt.Fprint(dockerCli.Err(), aec.Hide) fmt.Fprint(dockerCli.Err(), aec.Save) fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") } @@ -259,7 +268,6 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp if stderrIsTerm { fmt.Fprint(dockerCli.Err(), aec.Restore) fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) - fmt.Fprint(dockerCli.Err(), aec.Show) } fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", dstPath) @@ -383,7 +391,6 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo } if stderrIsTerm { - fmt.Fprint(dockerCli.Err(), aec.Hide) fmt.Fprint(dockerCli.Err(), aec.Save) fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") } @@ -391,7 +398,6 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo if stderrIsTerm { fmt.Fprint(dockerCli.Err(), aec.Restore) fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) - fmt.Fprint(dockerCli.Err(), aec.Show) } fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", copyConfig.container+":"+dstInfo.Path) From b9a1b0928af630ee20d91b89784b862655a04bd1 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 30 Mar 2023 01:04:37 +0000 Subject: [PATCH 5/6] cp: Make gocyclo happy Signed-off-by: Brian Goff --- cli/command/container/cp.go | 41 +++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index ad053522c5..aad1460c00 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -237,15 +237,13 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp RebaseName: rebaseName, } - stderrIsTerm := streams.NewOut(dockerCli.Err()).IsTerminal() - var copiedSize float64 if !copyConfig.quiet { content = ©ProgressPrinter{ ReadCloser: content, writer: dockerCli.Err(), total: &copiedSize, - isTerm: stderrIsTerm, + isTerm: streams.NewOut(dockerCli.Err()).IsTerminal(), header: copyFromContainerHeader, } } @@ -260,15 +258,9 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp return archive.CopyTo(preArchive, srcInfo, dstPath) } - if stderrIsTerm { - fmt.Fprint(dockerCli.Err(), aec.Save) - fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") - } + restore := prepareTTYCopyProgress(dockerCli) res := archive.CopyTo(preArchive, srcInfo, dstPath) - if stderrIsTerm { - fmt.Fprint(dockerCli.Err(), aec.Restore) - fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) - } + restore() fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", dstPath) return res @@ -329,8 +321,6 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo copiedSize float64 ) - stderrIsTerm := streams.NewOut(dockerCli.Err()).IsTerminal() - if srcPath == "-" { content = os.Stdin resolvedDstPath = dstInfo.Path @@ -375,7 +365,7 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo ReadCloser: content, writer: dockerCli.Err(), total: &copiedSize, - isTerm: stderrIsTerm, + isTerm: streams.NewOut(dockerCli.Err()).IsTerminal(), header: copyToContainerHeader, } } @@ -390,20 +380,27 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo return client.CopyToContainer(ctx, copyConfig.container, resolvedDstPath, content, options) } - if stderrIsTerm { - fmt.Fprint(dockerCli.Err(), aec.Save) - fmt.Fprintln(dockerCli.Err(), "Preparing to copy...") - } + restore := prepareTTYCopyProgress(dockerCli) res := client.CopyToContainer(ctx, copyConfig.container, resolvedDstPath, content, options) - if stderrIsTerm { - fmt.Fprint(dockerCli.Err(), aec.Restore) - fmt.Fprint(dockerCli.Err(), aec.EraseLine(aec.EraseModes.All)) - } + restore() fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", copyConfig.container+":"+dstInfo.Path) return res } +func prepareTTYCopyProgress(cli command.Cli) func() { + if !streams.NewOut(cli.Err()).IsTerminal() { + return func() {} + } + + fmt.Fprint(cli.Err(), aec.Save) + fmt.Fprintln(cli.Err(), "Preparing to copy...") + return func() { + fmt.Fprint(cli.Err(), aec.Restore) + fmt.Fprint(cli.Err(), aec.EraseLine(aec.EraseModes.All)) + } +} + // We use `:` as a delimiter between CONTAINER and PATH, but `:` could also be // in a valid LOCALPATH, like `file:name.txt`. We can resolve this ambiguity by // requiring a LOCALPATH with a `:` to be made explicit with a relative or From eb392ff4ce56960b75cfa752fa9cdc821974bc42 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 30 Mar 2023 17:22:17 +0000 Subject: [PATCH 6/6] cp: Do not block transfer on writing to terminal This moves all the terminal writing to a goroutine that updates the terminal periodically. In our MITM copier we just use an atomic to add to the total number of bytes read/written, the goroutine reads the total and updates the terminal as needed. Signed-off-by: Brian Goff --- cli/command/container/cp.go | 135 ++++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 58 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index aad1460c00..116d636243 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -1,12 +1,15 @@ package container import ( + "bytes" "context" "fmt" "io" "os" + "os/signal" "path/filepath" "strings" + "sync/atomic" "time" "github.com/docker/cli/cli" @@ -50,11 +53,7 @@ type cpConfig struct { // copying files to/from a container. type copyProgressPrinter struct { io.ReadCloser - total *float64 - writer io.Writer - isTerm bool - header string - lastUpdate time.Time + total *int64 } const ( @@ -65,37 +64,64 @@ const ( func (pt *copyProgressPrinter) Read(p []byte) (int, error) { n, err := pt.ReadCloser.Read(p) - isFirst := *pt.total == 0 - if n > 0 { - *pt.total += float64(n) - } - if !pt.isTerm { - return n, err - } - - doUpdate := func() { - fmt.Fprint(pt.writer, aec.Hide) - fmt.Fprint(pt.writer, aec.Column(uint(len(pt.header)+1))) - fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.Tail)) - fmt.Fprint(pt.writer, units.HumanSize(*pt.total)) - fmt.Fprint(pt.writer, aec.Show) - pt.lastUpdate = time.Now() - } - - if isFirst { - fmt.Fprint(pt.writer, aec.Restore) - fmt.Fprint(pt.writer, aec.EraseLine(aec.EraseModes.All)) - fmt.Fprint(pt.writer, pt.header) - doUpdate() - return n, err - } - - if err != nil || time.Since(pt.lastUpdate) > copyProgressUpdateThreshold { - doUpdate() - } + atomic.AddInt64(pt.total, int64(n)) return n, err } +func copyProgress(ctx context.Context, dst io.Writer, header string, total *int64) (func(), <-chan struct{}) { + done := make(chan struct{}) + if !streams.NewOut(dst).IsTerminal() { + close(done) + return func() {}, done + } + + fmt.Fprint(dst, aec.Save) + fmt.Fprint(dst, "Preparing to copy...") + + restore := func() { + fmt.Fprint(dst, aec.Restore) + fmt.Fprint(dst, aec.EraseLine(aec.EraseModes.All)) + } + + go func() { + defer close(done) + fmt.Fprint(dst, aec.Hide) + defer fmt.Fprint(dst, aec.Show) + + fmt.Fprint(dst, aec.Restore) + fmt.Fprint(dst, aec.EraseLine(aec.EraseModes.All)) + fmt.Fprint(dst, header) + + var last int64 + fmt.Fprint(dst, progressHumanSize(last)) + + buf := bytes.NewBuffer(nil) + ticker := time.NewTicker(copyProgressUpdateThreshold) + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + n := atomic.LoadInt64(total) + if n == last { + // Don't write to the terminal, if we don't need to. + continue + } + + // Write to the buffer first to avoid flickering and context switching + fmt.Fprint(buf, aec.Column(uint(len(header)+1))) + fmt.Fprint(buf, aec.EraseLine(aec.EraseModes.Tail)) + fmt.Fprint(buf, progressHumanSize(n)) + + buf.WriteTo(dst) + buf.Reset() + last += n + } + } + }() + return restore, done +} + // NewCopyCommand creates a new `docker cp` command func NewCopyCommand(dockerCli command.Cli) *cobra.Command { var opts copyOptions @@ -139,6 +165,10 @@ func NewCopyCommand(dockerCli command.Cli) *cobra.Command { return cmd } +func progressHumanSize(n int64) string { + return units.HumanSizeWithPrecision(float64(n), 3) +} + func runCopy(dockerCli command.Cli, opts copyOptions) error { srcContainer, srcPath := splitCpArg(opts.source) destContainer, destPath := splitCpArg(opts.destination) @@ -219,6 +249,9 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp } + ctx, cancel := signal.NotifyContext(ctx, os.Interrupt) + defer cancel() + content, stat, err := client.CopyFromContainer(ctx, copyConfig.container, srcPath) if err != nil { return err @@ -237,14 +270,11 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp RebaseName: rebaseName, } - var copiedSize float64 + var copiedSize int64 if !copyConfig.quiet { content = ©ProgressPrinter{ ReadCloser: content, - writer: dockerCli.Err(), total: &copiedSize, - isTerm: streams.NewOut(dockerCli.Err()).IsTerminal(), - header: copyFromContainerHeader, } } @@ -258,10 +288,12 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp return archive.CopyTo(preArchive, srcInfo, dstPath) } - restore := prepareTTYCopyProgress(dockerCli) + restore, done := copyProgress(ctx, dockerCli.Err(), copyFromContainerHeader, &copiedSize) res := archive.CopyTo(preArchive, srcInfo, dstPath) + cancel() + <-done restore() - fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", dstPath) + fmt.Fprintln(dockerCli.Err(), "Successfully copied", progressHumanSize(copiedSize), "to", dstPath) return res } @@ -318,7 +350,7 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo var ( content io.ReadCloser resolvedDstPath string - copiedSize float64 + copiedSize int64 ) if srcPath == "-" { @@ -363,10 +395,7 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo if !copyConfig.quiet { content = ©ProgressPrinter{ ReadCloser: content, - writer: dockerCli.Err(), total: &copiedSize, - isTerm: streams.NewOut(dockerCli.Err()).IsTerminal(), - header: copyToContainerHeader, } } } @@ -380,27 +409,17 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo return client.CopyToContainer(ctx, copyConfig.container, resolvedDstPath, content, options) } - restore := prepareTTYCopyProgress(dockerCli) + ctx, cancel := signal.NotifyContext(ctx, os.Interrupt) + restore, done := copyProgress(ctx, dockerCli.Err(), copyToContainerHeader, &copiedSize) res := client.CopyToContainer(ctx, copyConfig.container, resolvedDstPath, content, options) + cancel() + <-done restore() - fmt.Fprintln(dockerCli.Err(), "Successfully copied", units.HumanSize(copiedSize), "to", copyConfig.container+":"+dstInfo.Path) + fmt.Fprintln(dockerCli.Err(), "Successfully copied", progressHumanSize(copiedSize), "to", copyConfig.container+":"+dstInfo.Path) return res } -func prepareTTYCopyProgress(cli command.Cli) func() { - if !streams.NewOut(cli.Err()).IsTerminal() { - return func() {} - } - - fmt.Fprint(cli.Err(), aec.Save) - fmt.Fprintln(cli.Err(), "Preparing to copy...") - return func() { - fmt.Fprint(cli.Err(), aec.Restore) - fmt.Fprint(cli.Err(), aec.EraseLine(aec.EraseModes.All)) - } -} - // We use `:` as a delimiter between CONTAINER and PATH, but `:` could also be // in a valid LOCALPATH, like `file:name.txt`. We can resolve this ambiguity by // requiring a LOCALPATH with a `:` to be made explicit with a relative or