diff --git a/cli/command/container/client_test.go b/cli/command/container/client_test.go index 9b66f713cb59..7c38e7bec8d6 100644 --- a/cli/command/container/client_test.go +++ b/cli/command/container/client_test.go @@ -36,6 +36,7 @@ type fakeClient struct { infoFunc func() (client.SystemInfoResult, error) containerStatPathFunc func(containerID, path string) (client.ContainerStatPathResult, error) containerCopyFromFunc func(containerID, srcPath string) (client.CopyFromContainerResult, error) + containerCopyToFunc func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) logFunc func(string, client.ContainerLogsOptions) (client.ContainerLogsResult, error) waitFunc func(string) client.ContainerWaitResult containerListFunc func(client.ContainerListOptions) (client.ContainerListResult, error) @@ -128,6 +129,13 @@ func (f *fakeClient) CopyFromContainer(_ context.Context, containerID string, op return client.CopyFromContainerResult{}, nil } +func (f *fakeClient) CopyToContainer(_ context.Context, containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) { + if f.containerCopyToFunc != nil { + return f.containerCopyToFunc(containerID, options) + } + return client.CopyToContainerResult{}, nil +} + func (f *fakeClient) ContainerLogs(_ context.Context, containerID string, options client.ContainerLogsOptions) (client.ContainerLogsResult, error) { if f.logFunc != nil { return f.logFunc(containerID, options) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index 5121cb88a594..8193e8b04fe2 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -168,6 +168,50 @@ func progressHumanSize(n int64) string { return units.HumanSizeWithPrecision(float64(n), 3) } +// localContentSize returns the total size of regular file content at path. +// For a regular file it returns the file size. For a directory it walks +// the tree and sums sizes of all regular files. +func localContentSize(path string) (int64, error) { + fi, err := os.Lstat(path) + if err != nil { + return -1, err + } + if !fi.IsDir() { + if fi.Mode().IsRegular() { + return fi.Size(), nil + } + return 0, nil + } + var total int64 + err = filepath.WalkDir(path, func(_ string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if d.Type().IsRegular() { + info, err := d.Info() + if err != nil { + return err + } + total += info.Size() + } + return nil + }) + return total, err +} + +// copySummary formats the "Successfully copied ..." message. +// When contentSize differs from transferredSize, both values are shown. +func copySummary(contentSize, transferredSize int64, dest string) string { + if contentSize != transferredSize { + return fmt.Sprintf("Successfully copied %s (transferred %s) to %s\n", + progressHumanSize(contentSize), progressHumanSize(transferredSize), dest, + ) + } + return fmt.Sprintf("Successfully copied %s to %s\n", + progressHumanSize(contentSize), dest, + ) +} + func runCopy(ctx context.Context, dockerCli command.Cli, opts copyOptions) error { srcContainer, srcPath := splitCpArg(opts.source) destContainer, destPath := splitCpArg(opts.destination) @@ -295,7 +339,11 @@ func copyFromContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cp cancel() <-done restore() - _, _ = fmt.Fprintln(dockerCLI.Err(), "Successfully copied", progressHumanSize(copiedSize), "to", dstPath) + reportedSize := copiedSize + if !cpRes.Stat.Mode.IsDir() { + reportedSize = cpRes.Stat.Size + } + _, _ = fmt.Fprint(dockerCLI.Err(), copySummary(reportedSize, copiedSize, dstPath)) return res } @@ -354,11 +402,14 @@ func copyToContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cpCo content io.ReadCloser resolvedDstPath string copiedSize int64 + contentSize int64 + sizeErr error ) if srcPath == "-" { content = os.Stdin resolvedDstPath = dstInfo.Path + sizeErr = errors.New("content size not available for stdin") if !dstInfo.IsDir { return fmt.Errorf(`destination "%s:%s" must be a directory`, copyConfig.container, dstPath) } @@ -369,6 +420,8 @@ func copyToContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cpCo return err } + contentSize, sizeErr = localContentSize(srcInfo.Path) + srcArchive, err := archive.TarResource(srcInfo) if err != nil { return err @@ -421,7 +474,11 @@ func copyToContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cpCo cancel() <-done restore() - _, _ = fmt.Fprintln(dockerCLI.Err(), "Successfully copied", progressHumanSize(copiedSize), "to", copyConfig.container+":"+dstInfo.Path) + reportedSize := copiedSize + if sizeErr == nil { + reportedSize = contentSize + } + _, _ = fmt.Fprint(dockerCLI.Err(), copySummary(reportedSize, copiedSize, copyConfig.container+":"+dstInfo.Path)) return err } diff --git a/cli/command/container/cp_test.go b/cli/command/container/cp_test.go index cb724f9d9823..e193ebbf411b 100644 --- a/cli/command/container/cp_test.go +++ b/cli/command/container/cp_test.go @@ -11,6 +11,7 @@ import ( "github.com/docker/cli/internal/test" "github.com/moby/go-archive" "github.com/moby/go-archive/compression" + "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" @@ -211,3 +212,237 @@ func TestRunCopyFromContainerToFilesystemIrregularDestination(t *testing.T) { expected := `"/dev/random" must be a directory or a regular file` assert.ErrorContains(t, err, expected) } + +func TestCopySummary(t *testing.T) { + tests := []struct { + name string + contentSize int64 + transferredSize int64 + dest string + wantContains string + wantNoContain string + }{ + { + name: "different sizes shows both", + contentSize: 5, + transferredSize: 2048, + dest: "/dst", + wantContains: "(transferred", + }, + { + name: "equal sizes shows single value", + contentSize: 100, + transferredSize: 100, + dest: "/dst", + wantNoContain: "(transferred", + }, + { + name: "both zero", + contentSize: 0, + transferredSize: 0, + dest: "ctr:/dst", + wantContains: "Successfully copied 0B to ctr:/dst", + wantNoContain: "(transferred", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := copySummary(tc.contentSize, tc.transferredSize, tc.dest) + if tc.wantContains != "" { + assert.Check(t, is.Contains(got, tc.wantContains)) + } + if tc.wantNoContain != "" { + assert.Check(t, !strings.Contains(got, tc.wantNoContain), "unexpected substring %q in %q", tc.wantNoContain, got) + } + }) + } +} + +func TestCopyFromContainerReportsFileSize(t *testing.T) { + // The file content is "hello" (5 bytes), but the TAR archive wrapping + // it is much larger due to headers and padding. The success message + // should report the actual file size (5B), not the TAR stream size. + srcDir := fs.NewDir(t, "cp-test-from", + fs.WithFile("file1", "hello")) + + destDir := fs.NewDir(t, "cp-test-from-dest") + + const fileSize int64 = 5 + fakeCli := test.NewFakeCli(&fakeClient{ + containerCopyFromFunc: func(ctr, srcPath string) (client.CopyFromContainerResult, error) { + readCloser, err := archive.Tar(srcDir.Path(), compression.None) + return client.CopyFromContainerResult{ + Content: readCloser, + Stat: container.PathStat{ + Name: "file1", + Size: fileSize, + }, + }, err + }, + }) + err := runCopy(context.TODO(), fakeCli, copyOptions{ + source: "container:/file1", + destination: destDir.Path(), + }) + assert.NilError(t, err) + errOut := fakeCli.ErrBuffer().String() + assert.Check(t, is.Contains(errOut, "Successfully copied 5B")) + assert.Check(t, is.Contains(errOut, "(transferred")) +} + +func TestCopyToContainerReportsFileSize(t *testing.T) { + // Create a temp file with known content ("hello" = 5 bytes). + // The TAR archive sent to the container is larger, but the success + // message should report the actual content size. + srcFile := fs.NewFile(t, "cp-test-to", fs.WithContent("hello")) + + fakeCli := test.NewFakeCli(&fakeClient{ + containerStatPathFunc: func(containerID, path string) (client.ContainerStatPathResult, error) { + return client.ContainerStatPathResult{ + Stat: container.PathStat{ + Name: "tmp", + Mode: os.ModeDir | 0o755, + }, + }, nil + }, + containerCopyToFunc: func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) { + _, _ = io.Copy(io.Discard, options.Content) + return client.CopyToContainerResult{}, nil + }, + }) + err := runCopy(context.TODO(), fakeCli, copyOptions{ + source: srcFile.Path(), + destination: "container:/tmp", + }) + assert.NilError(t, err) + errOut := fakeCli.ErrBuffer().String() + assert.Check(t, is.Contains(errOut, "Successfully copied 5B")) + assert.Check(t, is.Contains(errOut, "(transferred")) +} + +func TestCopyToContainerReportsEmptyFileSize(t *testing.T) { + srcFile := fs.NewFile(t, "cp-test-empty", fs.WithContent("")) + + fakeCli := test.NewFakeCli(&fakeClient{ + containerStatPathFunc: func(containerID, path string) (client.ContainerStatPathResult, error) { + return client.ContainerStatPathResult{ + Stat: container.PathStat{ + Name: "tmp", + Mode: os.ModeDir | 0o755, + }, + }, nil + }, + containerCopyToFunc: func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) { + _, _ = io.Copy(io.Discard, options.Content) + return client.CopyToContainerResult{}, nil + }, + }) + err := runCopy(context.TODO(), fakeCli, copyOptions{ + source: srcFile.Path(), + destination: "container:/tmp", + }) + assert.NilError(t, err) + errOut := fakeCli.ErrBuffer().String() + assert.Check(t, is.Contains(errOut, "Successfully copied 0B")) + assert.Check(t, is.Contains(errOut, "(transferred")) +} + +func TestCopyToContainerReportsDirectorySize(t *testing.T) { + // Create a temp directory with files "aaa" (3 bytes) + "bbb" (3 bytes) = 6 bytes. + // The TAR archive is much larger, but the success message should report 6B. + srcDir := fs.NewDir(t, "cp-test-dir", + fs.WithFile("aaa", "aaa"), + fs.WithFile("bbb", "bbb"), + ) + + fakeCli := test.NewFakeCli(&fakeClient{ + containerStatPathFunc: func(containerID, path string) (client.ContainerStatPathResult, error) { + return client.ContainerStatPathResult{ + Stat: container.PathStat{ + Name: "tmp", + Mode: os.ModeDir | 0o755, + }, + }, nil + }, + containerCopyToFunc: func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) { + _, _ = io.Copy(io.Discard, options.Content) + return client.CopyToContainerResult{}, nil + }, + }) + err := runCopy(context.TODO(), fakeCli, copyOptions{ + source: srcDir.Path() + string(os.PathSeparator), + destination: "container:/tmp", + }) + assert.NilError(t, err) + errOut := fakeCli.ErrBuffer().String() + assert.Check(t, is.Contains(errOut, "Successfully copied 6B")) + assert.Check(t, is.Contains(errOut, "(transferred")) +} + +func TestCopyFromContainerReportsDirectorySize(t *testing.T) { + // When copying a directory from a container, cpRes.Stat.Mode.IsDir() is true, + // so reportedSize falls back to copiedSize (the tar stream bytes). + srcDir := fs.NewDir(t, "cp-test-fromdir", + fs.WithFile("file1", "hello")) + + destDir := fs.NewDir(t, "cp-test-fromdir-dest") + + fakeCli := test.NewFakeCli(&fakeClient{ + containerCopyFromFunc: func(ctr, srcPath string) (client.CopyFromContainerResult, error) { + readCloser, err := archive.Tar(srcDir.Path(), compression.None) + return client.CopyFromContainerResult{ + Content: readCloser, + Stat: container.PathStat{ + Name: "mydir", + Mode: os.ModeDir | 0o755, + }, + }, err + }, + }) + err := runCopy(context.TODO(), fakeCli, copyOptions{ + source: "container:/mydir", + destination: destDir.Path(), + }) + assert.NilError(t, err) + errOut := fakeCli.ErrBuffer().String() + assert.Check(t, is.Contains(errOut, "Successfully copied")) + // For directories from container, content size is unknown so + // reportedSize == copiedSize and "(transferred ...)" is omitted. + assert.Check(t, !strings.Contains(errOut, "(transferred")) +} + +func TestCopyToContainerStdinReportsTransferredSize(t *testing.T) { + // When copying from stdin, content size is unknown. + // The message should report transferred bytes without "(transferred ...)". + r, w, _ := os.Pipe() + _, _ = w.WriteString("some data from stdin") + w.Close() + oldStdin := os.Stdin + os.Stdin = r + t.Cleanup(func() { os.Stdin = oldStdin }) + + fakeCli := test.NewFakeCli(&fakeClient{ + containerStatPathFunc: func(containerID, path string) (client.ContainerStatPathResult, error) { + return client.ContainerStatPathResult{ + Stat: container.PathStat{ + Name: "tmp", + Mode: os.ModeDir | 0o755, + }, + }, nil + }, + containerCopyToFunc: func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) { + _, _ = io.Copy(io.Discard, options.Content) + return client.CopyToContainerResult{}, nil + }, + }) + err := runCopy(context.TODO(), fakeCli, copyOptions{ + source: "-", + destination: "container:/tmp", + }) + assert.NilError(t, err) + errOut := fakeCli.ErrBuffer().String() + assert.Check(t, is.Contains(errOut, "Successfully copied")) + // stdin has no content size, so reportedSize == copiedSize and + // "(transferred ...)" should not appear. + assert.Check(t, !strings.Contains(errOut, "(transferred")) +}