8000 docker cp: report both content size and transferred size by 4RH1T3CT0R7 · Pull Request #6800 · docker/cli · GitHub
[go: up one dir, main page]
Skip to content

docker cp: report both content size and transferred size#6800

Open
4RH1T3CT0R7 wants to merge 1 commit intodocker:masterfrom
4RH1T3CT0R7:master
Open

docker cp: report both content size and transferred size#6800
4RH1T3CT0R7 wants to merge 1 commit intodocker:masterfrom
4RH1T3CT0R7:master

Conversation

@4RH1T3CT0R7
Copy link
@4RH1T3CT0R7 4RH1T3CT0R7 commented Feb 13, 2026 •
edited
Loading

Summary

  • Report the actual file/content size in the docker cp success message, with the transferred (tar stream) size shown in parentheses
  • Fix copyToContainer() using localContentSize() — fast stat-only metadata lookups on local files (zero goroutines, zero io.Pipes)
  • Fix copyFromContainer() using cpRes.Stat.Size from the container API for regular files, falling back to tar stream size for directories
  • Add 5 tests covering: file from-container, file to-container, empty file, directory to-container, directory from-container

Fixes #5777

Example output

Successfully copied 0B (transferred 1.54kB) to my-container:/empty
Successfully copied 5B (transferred 2.05kB) to my-container:/file
Successfully copied 2.01MB (transferred 2.53MB) to my-container:/dir

How it works

copyToContainer: localContentSize() walks the local filesystem using os.Lstat / filepath.WalkDir (stat-only, no data reads). This is a separate, fast metadata pass that completes in single-digit milliseconds even for 1000+ files. No goroutines, no io.Pipe, no tar parsing overhead.

copyFromContainer: For regular files, uses the exact size from cpRes.Stat.Size (already returned by the Docker API, zero cost). For directories, falls back to the tar stream size as the best available approximation.

Test plan

  • TestCopyFromContainerReportsFileSize — 5-byte file reports "Successfully copied 5B"
  • TestCopyToContainerReportsFileSize — 5-byte file reports "Successfully copied 5B"
  • TestCopyToContainerReportsEmptyFileSize — empty file reports "Successfully copied 0B"
  • TestCopyToContainerReportsDirectorySize — directory with 6 bytes total reports "Successfully copied 6B"
  • TestCopyFromContainerReportsDirectorySize — directory from container uses transferred size
docker cp: report both content size and transferred size

@4RH1T3CT0R7 4RH1T3CT0R7 force-pushed the master branch 2 times, most recently from 4e6606a to ce1c937 Compare February 13, 2026 21:57
@thaJeztah thaJeztah mentioned this pull request Feb 14, 2026
Draft
thaJeztah
thaJeztah reviewed Feb 14, 2026
View reviewed changes
@4RH1T3CT0R7 4RH1T3CT0R7 changed the title Fix docker cp success message to report actual content size instead of tar stream size docker cp: report both content size and transferred size Feb 14, 2026
@4RH1T3CT0R7
Copy link
Author
4RH1T3CT0R7 commented Feb 14, 2026 •
edited
Loading

@thaJeztah Thanks for the review and the alternative approach in #6802! I looked at it carefully and updated this PR based on your feedback. Here are my thoughts:

Adopted your format idea: Agreed that showing both content and transferred size is useful - updated the message to "Successfully copied X (transferred Y) to path".

On "double traversal": localContentSize() performs only stat() system calls (inode metadata lookups, served from the FS cache in microseconds). TarResource → TarWithOptions → Tarballer.Do() performs completely different I/O: it calls filepath.WalkDir (archive.go:693), then addTarFile calls os.Lstat again (archive.go:297), then open()/read() to stream file contents, then writes tar headers and data to the pipe. The existing code already double-stats every file before this change; our localContentSize adds one fast metadata pass. For 1000 files, it completes in single-digit milliseconds vs hundreds of milliseconds for tar creation. The overhead is negligible.

On "race condition": This race exists identically in TarResource itself. Tarballer.Do() runs filepath.WalkDir in a goroutine, then addTarFile calls os.Lstat again and opens each file. If a file changes between the walk and the open, the tar and the stat disagree. calcTARContentSize reports tar-header sizes which are based on the same os.Lstat values - both approaches report a stat-derived size from a point in time, neither is immune to concurrent modification.

Comparing the two approaches:

Aspect This PR (#6800) #6802
Directions fixed Both (from + to container) Only copyToContainer
Goroutines added 0 1 (3rd in the pipeline: TarWithOptions pipe → RebaseArchiveEntries pipe → calcTARContentSize pipe)
io.Pipe layers added 0 1
Tests 5 0
Code complexity ~30 lines (localContentSize - stat walk) ~50 lines (calcTARContentSize - io.Pipe + io.TeeReader + goroutine + tar.Reader + io.Copy(Discard))

Happy to make any further adjustments!

@codecov-commenter
Copy link
codecov-commenter commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 73.33333% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cli/command/container/cp.go 73.33% 8 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

@4RH1T3CT0R7
docker cp: report both content size and transferred size
2bc66ec 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah Awaiting requested review from thaJeztah

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

kind/enhancement status/2-code-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Docker cp success message shows wrong size

3 participants

@4RH1T3CT0R7 @codecov-commenter @thaJeztah
0