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

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

Doug Kelly [off-list ref] writes:

quoted

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