Tar.create: include entries for all directories #106
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #106 +/- ##
=======================================
Coverage 96.29% 96.30%
=======================================
Files 4 4
Lines 648 649 +1
=======================================
+ Hits 624 625 +1
Misses 24 24
Continue to review full report at Codecov.
|
|
Aside: I'm quite pleased that the test changes required for this are minimal but not non-existent. That means that we are testing whether we record non-empty directories or not, so the format cannot accidentally change without triggering a test failure. But at the same time, the tests are not overly tightly coupled to the exact format. |
Issue #103 points out that omitting directory entries for non-empty directories can confuse some tools that consume tarballs, including docker, which applies overly restrictive permissions to directories which are not explicitly included in a tarball. This commit changes `Tar.create` and `Tar.rewrite` to produce tarballs which include explicit directory entries for all (non-root) directories. This changes Tar.jl's "canonical format", which is, by design one-to-one with git tree hashes. However, it does not seem like anyone currently depends on that exact reproducibility and it seems worth making this change in order to avoid confusing external consumers of our tarballs. Closes #103.
StefanKarpinski
force-pushed
the
sk/directory-entries
branch
from
a423e6a
to
1a40250
Compare
StefanKarpinski
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #103 points out that omitting directory entries for non-empty directories can confuse some tools that consume tarballs, including docker, which applies overly restrictive permissions to directories which are not explicitly included in a tarball.
This commit changes
Tar.createandTar.rewriteto produce tarballs which include explicit directory entries for all (non-root) directories. This changes Tar.jl's "canonical format", which is, by design one-to-one with git tree hashes. However, it does not seem like anyone currently depends on that exact reproducibility and it seems worth making this change in order to avoid confusing external consumers of our tarballs.Closes #103.