Re: [PATCH 1/2] prepare_packed_git(): refactor garbage reporting in pack directory

From: Jeff King <peff@peff.net>
Date: Nov 4, 2015, 19:56 UTC
Message-ID: <20151104195649.GB16101@sigill.intra.peff.net>
In-Reply-To: <xmqqr3k5a76v.fsf@gitster.mtv.corp.google.com>

On Wed, Nov 04, 2015 at 11:35:52AM -0800, Junio C Hamano wrote:

Show 14 quoted lines

> Doug Kelly <dougk.ff7@gmail.com> writes:
> 
> > I think the patches I sent (a bit prematurely) address the
> > remaining comments... I did find there was a relevant test in
> > t5304 already, so I added a new test in the same section (and
> > cleaned up some of the garbage it wasn't removing before).  I'm
> > not sure if it's poor form to move tests around like this, but I
> > figured it might be best to keep them logically grouped.
> 
> OK, will queue as I didn't spot anything glaringly wrong ;-)
> 
> I did wonder if we want to say anything about .bitmap files, though.
> If there is one without matching .idx and .pack, shouldn't we report
> just like we report .idx without .pack (or vice versa)?

Yeah, I think so. The logic should really extend to anything without a matching .pack. And I think the sane rule is probably:

  If we have pack-$sha.$ext, but not pack-$sha.pack, then:

    1. if $ext is known to us as a cache that can be regenerated from the
       .pack (i.e., .idx, .bitmap), then delete it

    2. if $ext is known to us as precious, do nothing (there is nothing
       in this category right now, though)

    3. if $ext is not known to us, warn but do not delete (in case a
       future version adds something precious)

The conservatism in (3) is the right thing to do, I think, but I doubt it will ever matter, because we probably cannot ever add non-cache auxiliary files to the pack. Old versions of git would not delete such precious files, but nor would they carry them forward during a repack. So short of a repo-version bump, I think we are effectively limited to adding only caches which can be re-generated from an original .pack.

-Peff