Fix git-fsck to handle non-root trees #1332
Conversation
|
@Byron This was trivial logic bug introduced during the refactor of making Apologies for the silence during the past few months. Had a new baby boy added to the family, so took a bit to get back to my normal work capacity! |
- Trees except the commit's root tree were being skipped - Make `check_commit()` the only functioning marking trees as "seen" - Update unit tests to catch the failure (add a directory to test repo)
cesfahani
force-pushed
the
fix_fsck_tree_traversal
branch
from
0ca4562 to
5fdb440
Compare
There was a problem hiding this comment.
Apologies for the silence during the past few months. Had a new baby boy added to the family, so took a bit to get back to my normal work capacity!
Welcome back, and, I can totally imagine :)!
Regarding the PR, for now I treat the fsck as mostly maintained by you, so I don't have to fully understand or validate each and every bit as I would normally do. Having had a look at git fsck, I know it's actually a very complex program with a lot of tests, making this version humble beginnings tailor-made for a specific use-case.
With that in mind, I also think it's OK to have an fsck which is able to validate received packs, and do so quickly, and this crate is certainly a step in that direction. However, to be really fast, I think ultimately it has to be integrated into the pack-resolution algorithm as it also sees every object once with the decoding being 'free' and ultra-efficient, with the main problem being its multi-threadedness.
| @@ -80,9 +80,7 @@ where | |||
| match entry_ref.mode.kind() { | |||
| EntryKind::Tree => { | |||
| let tree_id = entry_ref.oid.to_owned(); | |||
| if self.seen.insert(tree_id) { | |||
There was a problem hiding this comment.
I do admit that I don't get how it solves the recursion issue, as I would have thought the seen check is to assure it doesn't look at the same tree twice.
check_commit()the only functioning marking trees as "seen"