On Fri, Jan 13, 2017 at 12:52 PM, Jeff King <p...@peff.net> wrote: > On Fri, Jan 13, 2017 at 04:15:42AM -0500, John Szakmeister wrote: > >> > I did notice another interesting case when looking at this. Fsck ends up >> > in fsck_loose(), which has the sha1 and path of the loose object. It >> > passes the sha1 to fsck_sha1(), and ignores the path entirely! >> > >> > So if you have a duplicate copy of the object in a pack, we'd actually >> > find and check the duplicate. This can happen, e.g., if you had a loose >> > object and fetched a thin-pack which made a copy of the loose object to >> > complete the pack). >> > >> > Probably fsck_loose() should be more picky about making sure we are >> > reading the data from the loose version we found. >> >> Interesting find! Thanks for the information Peff! > > So I figured I would knock this out as a fun morning exercise. But > sheesh, it turned out to be a slog, because most of the functions rely > on map_sha1_file() to convert the sha1 to an object path at the lowest > level.
Yeah, I discovered the same thing when I took a look at it a week or so ago. :-( > But I finally got something working, so here it is. I found another bug > on the way, along with a few cleanups. And then I did the trailing > garbage detection at the end, because by that point I knew right where > it needed to go. :) I don't know if my opinion counts for much, but the changes look good to me. -John
