Fix tree hashing with nested empty directories #1522
Conversation
When testing for directories to exclude from hashing, we must exclude not only empty directories, but also directories that themselves contain nothing but empty directories; in essence, we suppress adding directories that have no files contained within their entire subtree. While fixing this, it seemed prudent to eliminate the `names` argument to `tree_hash()`, especially as it was not actually used to iterate over. Fixes JuliaLang/julia#33979 (comment)
|
I'm explicitly requesting a review from @StefanKarpinski just to make sure that my changing the signature of |
When testing for directories to exclude from hashing, we must exclude not only empty directories, but also directories that themselves contain nothing but empty directories; in essence, we suppress adding directories that have no files contained within their entire subtree. While fixing this, it seemed prudent to eliminate the `names` argument to `tree_hash()`, especially as it was not actually used to iterate over. Fixes JuliaLang/julia#33979 (comment) (cherry picked from commit 648d66c, PR #1522)
Codecov Report
@@ Coverage Diff @@
## master #1522 +/- ##
==========================================
+ Coverage 86.73% 86.74% +<.01%
==========================================
Files 25 25
Lines 5451 5454 +3
==========================================
+ Hits 4728 4731 +3
Misses 723 723
Continue to review full report at Codecov.
|
|
Not using the names argument was a mistake. I meant to replace the call to readdir with it. Looks good to me. I have to say this makes it seems even wonkier there git doesn’t just allow empty directories. |
When testing for directories to exclude from hashing, we must exclude not only empty directories, but also directories that themselves contain nothing but empty directories; in essence, we suppress adding directories that have no files contained within their entire subtree. While fixing this, it seemed prudent to eliminate the `names` argument to `tree_hash()`, especially as it was not actually used to iterate over. (cherry picked from commit 49ab53e, PR #1522)
When testing for directories to exclude from hashing, we must exclude not only empty directories, but also directories that themselves contain nothing but empty directories; in essence, we suppress adding directories that have no files contained within their entire subtree. While fixing this, it seemed prudent to eliminate the `names` argument to `tree_hash()`, especially as it was not actually used to iterate over. (cherry picked from commit 49ab53e, PR #1522)
|
Yeah, |
|
It's bizarre for a number of reasons:
|
When testing for directories to exclude from hashing, we must exclude
not only empty directories, but also directories that themselves contain
nothing but empty directories; in essence, we suppress adding
directories that have no files contained within their entire subtree.
While fixing this, it seemed prudent to eliminate the
namesargumentto
tree_hash(), especially as it was not actually used to iterateover.
This will fix the error reported in JuliaLang/julia#33979 (comment)