Skip to content

Fix duplicate directories entries on metadata change#2054

Merged
AkihiroSuda merged 1 commit intocontainerd:masterfrom
dmcgowan:fix-duplicate-tar-file
Jan 25, 2018
Merged

Fix duplicate directories entries on metadata change#2054
AkihiroSuda merged 1 commit intocontainerd:masterfrom
dmcgowan:fix-duplicate-tar-file

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan commented Jan 24, 2018

Currently directory changes are not added to the list of included directories, allowing those directories to receive duplicate entries where there is both a metadata change to the directory and a change to a file under that directory.

Adding this for cherry-pick, this could be considered a regression in 1.0.1 since 1.0 would have had the correct behavior by not including the directory as a parent. Note this only effects created content, the main consumer of which right now is buildkit or other build tools using containerd.

See moby/buildkit#266

Currently directory changes are not added to the list of
included directories, allowing those directories to receive
duplicate entries where there is both a metadata change to the
directory and a change to a file under that directory.

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

codecov-io commented Jan 24, 2018

Codecov Report

Merging #2054 into master will decrease coverage by 4.91%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2054      +/-   ##
==========================================
- Coverage   50.35%   45.44%   -4.92%     
==========================================
  Files          84       96      +12     
  Lines        7393     9432    +2039     
==========================================
+ Hits         3723     4286     +563     
- Misses       2964     4436    +1472     
- Partials      706      710       +4
Flag Coverage Δ
#linux 50.37% <100%> (+0.01%) ⬆️
#windows 40.29% <0%> (?)
Impacted Files Coverage Δ
archive/tar.go 43.41% <66.66%> (-6.74%) ⬇️
fs/path.go 60.69% <0%> (-12.56%) ⬇️
oci/spec.go 40% <0%> (-10%) ⬇️
snapshots/naive/naive.go 43.89% <0%> (-10%) ⬇️
metadata/snapshot.go 45.95% <0%> (-8.96%) ⬇️
archive/compression/compression.go 43.93% <0%> (-8.9%) ⬇️
remotes/docker/fetcher.go 41.42% <0%> (-7.6%) ⬇️
content/local/writer.go 52.63% <0%> (-7.37%) ⬇️
fs/copy.go 34.83% <0%> (-7.2%) ⬇️
... and 62 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d32d9e...dfadd8c. Read the comment docs.

@stevvooe
Copy link
Copy Markdown
Member

LGTM

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for working on this 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants