Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix container cp test to separate source and destination #5715

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

dmcgowan
Copy link
Contributor

Currently the copy will tar from the same directory it will untar into simultaneously. There is a race between reading the file and truncating the file for write, however, the race will not show up with a large enough buffer on the tar side if buffered before the copy begins.

Also removes the unnecessary deferred removal, the removal is handled by cleanup and respects the no cleanup env.

Currently the cp will tar from the same directory it will untar into
simultaneously. There is a race between reading the file and truncating
the file for write, however, the race will not show up with a large
enough buffer on the tar side if buffered before the copy begins.

Also removes the unnecessary deferred removal, the removal is handled by
cleanup and respects the no cleanup env.

Signed-off-by: Derek McGowan <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Dec 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.52%. Comparing base (3b49deb) to head (8c0cb30).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5715      +/-   ##
==========================================
- Coverage   59.54%   59.52%   -0.03%     
==========================================
  Files         346      346              
  Lines       29381    29381              
==========================================
- Hits        17496    17488       -8     
- Misses      10914    10923       +9     
+ Partials      971      970       -1     

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Nice find, thank you!

LGTM

@thaJeztah thaJeztah merged commit 07aca45 into docker:master Dec 27, 2024
104 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants