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

From: Jeff King <peff@peff.net>
Date: Nov 4, 2015, 20:02 UTC
Message-ID: <20151104200249.GC16101@sigill.intra.peff.net>
In-Reply-To: <CAEtYS8Rp0Eb7uHB8kJ=muVWy6u+beB7kAAWZqPgTYqfuKx3P2A@mail.gmail.com>

On Wed, Nov 04, 2015 at 01:56:38PM -0600, Doug Kelly wrote:

Show 8 quoted lines

> > 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)?
> 
> I think you're right -- this would be something worth following up on.
> At least, t5304 doesn't cover this case explicitly, but when I tried
> adding an empty bitmap with a bogus name, I did see a "no
> corresponding .idx or .pack" error, similar to the stale .keep file.

Yeah, that should be harmless warning (although note because the bitmap code only really handles a single bitmap, it can prevent loading of the "real" bitmap; so you'd want to clean it up, for sure).

Show 6 quoted lines

> I'd trust your (and Jeff's) knowledge on this far more than my own,
> but would it be a bad idea to clean up .keep and .bitmap files if the
> .idx/.pack pair are missing?  I think we may have had a discussion
> previously on how things along these lines might be racey -- but I
> don't know what order the .keep file is created in relation to the
> .idx/.pack.

Definitely cleaning up the .bitmap is sane and not racy (it's in the same boat as the .idx, I think).

.keep files are more tricky. I'd have to go over the receive-pack code to confirm, but I think they _are_ racy. That is, receive-pack will create them as a lockfile before moving the pack into place. That's OK, though, if we use mtimes to give ourselves a grace period (I haven't looked at your series yet).

But moreover, .keep files can be created manually by the user. If the pack they referenced goes away, they are not really serving any purpose. But it's possible that the user would want to salvage the content of the file, or know that it was there.

So I'd argue we should leave them. Or at least leave ones that do not have the generic "{receive,fetch}-pack $pid on $host comment in them, which were clearly created as lockfiles.

-Peff