Skip to content

Commit 2bc66ec

Browse files
committed
docker cp: report both content size and transferred size
When copying files with `docker cp`, the success message now shows both the actual content size and the transferred (tar stream) size when they differ, making it easier to understand compression and overhead: Successfully copied 2.01MB (transferred 2.53MB) to ctr:/dir Extract copySummary helper to keep copyToContainer under the gocyclo complexity threshold. Add unit tests for copySummary and stdin path. Signed-off-by: 4RH1T3CT0R7 <iprintercanon@gmail.com>
1 parent 7b93d61 commit 2bc66ec

File tree

3 files changed

+302
-2
lines changed

3 files changed

+302
-2
lines changed

cli/command/container/client_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ type fakeClient struct {
3636
infoFunc func() (client.SystemInfoResult, error)
3737
containerStatPathFunc func(containerID, path string) (client.ContainerStatPathResult, error)
3838
containerCopyFromFunc func(containerID, srcPath string) (client.CopyFromContainerResult, error)
39+
containerCopyToFunc func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error)
3940
logFunc func(string, client.ContainerLogsOptions) (client.ContainerLogsResult, error)
4041
waitFunc func(string) client.ContainerWaitResult
4142
containerListFunc func(client.ContainerListOptions) (client.ContainerListResult, error)
@@ -128,6 +129,13 @@ func (f *fakeClient) CopyFromContainer(_ context.Context, containerID string, op
128129
return client.CopyFromContainerResult{}, nil
129130
}
130131

132+
func (f *fakeClient) CopyToContainer(_ context.Context, containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) {
133+
if f.containerCopyToFunc != nil {
134+
return f.containerCopyToFunc(containerID, options)
135+
}
136+
return client.CopyToContainerResult{}, nil
137+
}
138+
131139
func (f *fakeClient) ContainerLogs(_ context.Context, containerID string, options client.ContainerLogsOptions) (client.ContainerLogsResult, error) {
132140
if f.logFunc != nil {
133141
return f.logFunc(containerID, options)

cli/command/container/cp.go

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,50 @@ func progressHumanSize(n int64) string {
168168
return units.HumanSizeWithPrecision(float64(n), 3)
169169
}
170170

171+
// localContentSize returns the total size of regular file content at path.
172+
// For a regular file it returns the file size. For a directory it walks
173+
// the tree and sums sizes of all regular files.
174+
func localContentSize(path string) (int64, error) {
175+
fi, err := os.Lstat(path)
176+
if err != nil {
177+
return -1, err
178+
}
179+
if !fi.IsDir() {
180+
if fi.Mode().IsRegular() {
181+
return fi.Size(), nil
182+
}
183+
return 0, nil
184+
}
185+
var total int64
186+
err = filepath.WalkDir(path, func(_ string, d os.DirEntry, err error) error {
187+
if err != nil {
188+
return err
189+
}
190+
if d.Type().IsRegular() {
191+
info, err := d.Info()
192+
if err != nil {
193+
return err
194+
}
195+
total += info.Size()
196+
}
197+
return nil
198+
})
199+
return total, err
200+
}
201+
202+
// copySummary formats the "Successfully copied ..." message.
203+
// When contentSize differs from transferredSize, both values are shown.
204+
func copySummary(contentSize, transferredSize int64, dest string) string {
205+
if contentSize != transferredSize {
206+
return fmt.Sprintf("Successfully copied %s (transferred %s) to %s\n",
207+
progressHumanSize(contentSize), progressHumanSize(transferredSize), dest,
208+
)
209+
}
210+
return fmt.Sprintf("Successfully copied %s to %s\n",
211+
progressHumanSize(contentSize), dest,
212+
)
213+
}
214+
171215
func runCopy(ctx context.Context, dockerCli command.Cli, opts copyOptions) error {
172216
srcContainer, srcPath := splitCpArg(opts.source)
173217
destContainer, destPath := splitCpArg(opts.destination)
@@ -295,7 +339,11 @@ func copyFromContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cp
295339
cancel()
296340
<-done
297341
restore()
298-
_, _ = fmt.Fprintln(dockerCLI.Err(), "Successfully copied", progressHumanSize(copiedSize), "to", dstPath)
342+
reportedSize := copiedSize
343+
if !cpRes.Stat.Mode.IsDir() {
344+
reportedSize = cpRes.Stat.Size
345+
}
346+
_, _ = fmt.Fprint(dockerCLI.Err(), copySummary(reportedSize, copiedSize, dstPath))
299347

300348
return res
301349
}
@@ -354,11 +402,14 @@ func copyToContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cpCo
354402
content io.ReadCloser
355403
resolvedDstPath string
356404
copiedSize int64
405+
contentSize int64
406+
sizeErr error
357407
)
358408

359409
if srcPath == "-" {
360410
content = os.Stdin
361411
resolvedDstPath = dstInfo.Path
412+
sizeErr = errors.New("content size not available for stdin")
362413
if !dstInfo.IsDir {
363414
return fmt.Errorf(`destination "%s:%s" must be a directory`, copyConfig.container, dstPath)
364415
}
@@ -369,6 +420,8 @@ func copyToContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cpCo
369420
return err
370421
}
371422

423+
contentSize, sizeErr = localContentSize(srcInfo.Path)
424+
372425
srcArchive, err := archive.TarResource(srcInfo)
373426
if err != nil {
374427
return err
@@ -421,7 +474,11 @@ func copyToContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cpCo
421474
cancel()
422475
<-done
423476
restore()
424-
_, _ = fmt.Fprintln(dockerCLI.Err(), "Successfully copied", progressHumanSize(copiedSize), "to", copyConfig.container+":"+dstInfo.Path)
477+
reportedSize := copiedSize
478+
if sizeErr == nil {
479+
reportedSize = contentSize
480+
}
481+
_, _ = fmt.Fprint(dockerCLI.Err(), copySummary(reportedSize, copiedSize, copyConfig.container+":"+dstInfo.Path))
425482

426483
return err
427484
}

cli/command/container/cp_test.go

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/docker/cli/internal/test"
1212
"github.com/moby/go-archive"
1313
"github.com/moby/go-archive/compression"
14+
"github.com/moby/moby/api/types/container"
1415
"github.com/moby/moby/client"
1516
"gotest.tools/v3/assert"
1617
is "gotest.tools/v3/assert/cmp"
@@ -211,3 +212,237 @@ func TestRunCopyFromContainerToFilesystemIrregularDestination(t *testing.T) {
211212
expected := `"/dev/random" must be a directory or a regular file`
212213
assert.ErrorContains(t, err, expected)
213214
}
215+
216+
func TestCopySummary(t *testing.T) {
217+
tests := []struct {
218+
name string
219+
contentSize int64
220+
transferredSize int64
221+
dest string
222+
wantContains string
223+
wantNoContain string
224+
}{
225+
{
226+
name: "different sizes shows both",
227+
contentSize: 5,
228+
transferredSize: 2048,
229+
dest: "/dst",
230+
wantContains: "(transferred",
231+
},
232+
{
233+
name: "equal sizes shows single value",
234+
contentSize: 100,
235+
transferredSize: 100,
236+
dest: "/dst",
237+
wantNoContain: "(transferred",
238+
},
239+
{
240+
name: "both zero",
241+
contentSize: 0,
242+
transferredSize: 0,
243+
dest: "ctr:/dst",
244+
wantContains: "Successfully copied 0B to ctr:/dst",
245+
wantNoContain: "(transferred",
246+
},
247+
}
248+
for _, tc := range tests {
249+
t.Run(tc.name, func(t *testing.T) {
250+
got := copySummary(tc.contentSize, tc.transferredSize, tc.dest)
251+
if tc.wantContains != "" {
252+
assert.Check(t, is.Contains(got, tc.wantContains))
253+
}
254+
if tc.wantNoContain != "" {
255+
assert.Check(t, !strings.Contains(got, tc.wantNoContain), "unexpected substring %q in %q", tc.wantNoContain, got)
256+
}
257+
})
258+
}
259+
}
260+
261+
func TestCopyFromContainerReportsFileSize(t *testing.T) {
262+
// The file content is "hello" (5 bytes), but the TAR archive wrapping
263+
// it is much larger due to headers and padding. The success message
264+
// should report the actual file size (5B), not the TAR stream size.
265+
srcDir := fs.NewDir(t, "cp-test-from",
266+
fs.WithFile("file1", "hello"))
267+
268+
destDir := fs.NewDir(t, "cp-test-from-dest")
269+
270+
const fileSize int64 = 5
271+
fakeCli := test.NewFakeCli(&fakeClient{
272+
containerCopyFromFunc: func(ctr, srcPath string) (client.CopyFromContainerResult, error) {
273+
readCloser, err := archive.Tar(srcDir.Path(), compression.None)
274+
return client.CopyFromContainerResult{
275+
Content: readCloser,
276+
Stat: container.PathStat{
277+
Name: "file1",
278+
Size: fileSize,
279+
},
280+
}, err
281+
},
282+
})
283+
err := runCopy(context.TODO(), fakeCli, copyOptions{
284+
source: "container:/file1",
285+
destination: destDir.Path(),
286+
})
287+
assert.NilError(t, err)
288+
errOut := fakeCli.ErrBuffer().String()
289+
assert.Check(t, is.Contains(errOut, "Successfully copied 5B"))
290+
assert.Check(t, is.Contains(errOut, "(transferred"))
291+
}
292+
293+
func TestCopyToContainerReportsFileSize(t *testing.T) {
294+
// Create a temp file with known content ("hello" = 5 bytes).
295+
// The TAR archive sent to the container is larger, but the success
296+
// message should report the actual content size.
297+
srcFile := fs.NewFile(t, "cp-test-to", fs.WithContent("hello"))
298+
299+
fakeCli := test.NewFakeCli(&fakeClient{
300+
containerStatPathFunc: func(containerID, path string) (client.ContainerStatPathResult, error) {
301+
return client.ContainerStatPathResult{
302+
Stat: container.PathStat{
303+
Name: "tmp",
304+
Mode: os.ModeDir | 0o755,
305+
},
306+
}, nil
307+
},
308+
containerCopyToFunc: func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) {
309+
_, _ = io.Copy(io.Discard, options.Content)
310+
return client.CopyToContainerResult{}, nil
311+
},
312+
})
313+
err := runCopy(context.TODO(), fakeCli, copyOptions{
314+
source: srcFile.Path(),
315+
destination: "container:/tmp",
316+
})
317+
assert.NilError(t, err)
318+
errOut := fakeCli.ErrBuffer().String()
319+
assert.Check(t, is.Contains(errOut, "Successfully copied 5B"))
320+
assert.Check(t, is.Contains(errOut, "(transferred"))
321+
}
322+
323+
func TestCopyToContainerReportsEmptyFileSize(t *testing.T) {
324+
srcFile := fs.NewFile(t, "cp-test-empty", fs.WithContent(""))
325+
326+
fakeCli := test.NewFakeCli(&fakeClient{
327+
containerStatPathFunc: func(containerID, path string) (client.ContainerStatPathResult, error) {
328+
return client.ContainerStatPathResult{
329+
Stat: container.PathStat{
330+
Name: "tmp",
331+
Mode: os.ModeDir | 0o755,
332+
},
333+
}, nil
334+
},
335+
containerCopyToFunc: func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) {
336+
_, _ = io.Copy(io.Discard, options.Content)
337+
return client.CopyToContainerResult{}, nil
338+
},
339+
})
340+
err := runCopy(context.TODO(), fakeCli, copyOptions{
341+
source: srcFile.Path(),
342+
destination: "container:/tmp",
343+
})
344+
assert.NilError(t, err)
345+
errOut := fakeCli.ErrBuffer().String()
346+
assert.Check(t, is.Contains(errOut, "Successfully copied 0B"))
347+
assert.Check(t, is.Contains(errOut, "(transferred"))
348+
}
349+
350+
func TestCopyToContainerReportsDirectorySize(t *testing.T) {
351+
// Create a temp directory with files "aaa" (3 bytes) + "bbb" (3 bytes) = 6 bytes.
352+
// The TAR archive is much larger, but the success message should report 6B.
353+
srcDir := fs.NewDir(t, "cp-test-dir",
354+
fs.WithFile("aaa", "aaa"),
355+
fs.WithFile("bbb", "bbb"),
356+
)
357+
358+
fakeCli := test.NewFakeCli(&fakeClient{
359+
containerStatPathFunc: func(containerID, path string) (client.ContainerStatPathResult, error) {
360+
return client.ContainerStatPathResult{
361+
Stat: container.PathStat{
362+
Name: "tmp",
363+
Mode: os.ModeDir | 0o755,
364+
},
365+
}, nil
366+
},
367+
containerCopyToFunc: func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) {
368+
_, _ = io.Copy(io.Discard, options.Content)
369+
return client.CopyToContainerResult{}, nil
370+
},
371+
})
372+
err := runCopy(context.TODO(), fakeCli, copyOptions{
373+
source: srcDir.Path() + string(os.PathSeparator),
374+
destination: "container:/tmp",
375+
})
376+
assert.NilError(t, err)
377+
errOut := fakeCli.ErrBuffer().String()
378+
assert.Check(t, is.Contains(errOut, "Successfully copied 6B"))
379+
assert.Check(t, is.Contains(errOut, "(transferred"))
380+
}
381+
382+
func TestCopyFromContainerReportsDirectorySize(t *testing.T) {
383+
// When copying a directory from a container, cpRes.Stat.Mode.IsDir() is true,
384+
// so reportedSize falls back to copiedSize (the tar stream bytes).
385+
srcDir := fs.NewDir(t, "cp-test-fromdir",
386+
fs.WithFile("file1", "hello"))
387+
388+
destDir := fs.NewDir(t, "cp-test-fromdir-dest")
389+
390+
fakeCli := test.NewFakeCli(&fakeClient{
391+
containerCopyFromFunc: func(ctr, srcPath string) (client.CopyFromContainerResult, error) {
392+
readCloser, err := archive.Tar(srcDir.Path(), compression.None)
393+
return client.CopyFromContainerResult{
394+
Content: readCloser,
395+
Stat: container.PathStat{
396+
Name: "mydir",
397+
Mode: os.ModeDir | 0o755,
398+
},
399+
}, err
400+
},
401+
})
402+
err := runCopy(context.TODO(), fakeCli, copyOptions{
403+
source: "container:/mydir",
404+
destination: destDir.Path(),
405+
})
406+
assert.NilError(t, err)
407+
errOut := fakeCli.ErrBuffer().String()
408+
assert.Check(t, is.Contains(errOut, "Successfully copied"))
409+
// For directories from container, content size is unknown so
410+
// reportedSize == copiedSize and "(transferred ...)" is omitted.
411+
assert.Check(t, !strings.Contains(errOut, "(transferred"))
412+
}
413+
414+
func TestCopyToContainerStdinReportsTransferredSize(t *testing.T) {
415+
// When copying from stdin, content size is unknown.
416+
// The message should report transferred bytes without "(transferred ...)".
417+
r, w, _ := os.Pipe()
418+
_, _ = w.WriteString("some data from stdin")
419+
w.Close()
420+
oldStdin := os.Stdin
421+
os.Stdin = r
422+
t.Cleanup(func() { os.Stdin = oldStdin })
423+
424+
fakeCli := test.NewFakeCli(&fakeClient{
425+
containerStatPathFunc: func(containerID, path string) (client.ContainerStatPathResult, error) {
426+
return client.ContainerStatPathResult{
427+
Stat: container.PathStat{
428+
Name: "tmp",
429+
Mode: os.ModeDir | 0o755,
430+
},
431+
}, nil
432+
},
433+
containerCopyToFunc: func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) {
434+
_, _ = io.Copy(io.Discard, options.Content)
435+
return client.CopyToContainerResult{}, nil
436+
},
437+
})
438+
err := runCopy(context.TODO(), fakeCli, copyOptions{
439+
source: "-",
440+
destination: "container:/tmp",
441+
})
442+
assert.NilError(t, err)
443+
errOut := fakeCli.ErrBuffer().String()
444+
assert.Check(t, is.Contains(errOut, "Successfully copied"))
445+
// stdin has no content size, so reportedSize == copiedSize and
446+
// "(transferred ...)" should not appear.
447+
assert.Check(t, !strings.Contains(errOut, "(transferred"))
448+
}

0 commit comments

Comments
 (0)