Re: [PATCH 0/5] not making corruption worse

From: Jeff King <peff@peff.net>
Date: Mar 17, 2015, 07:37 UTC
Message-ID: <20150317073730.GA25267@peff.net>
In-Reply-To: <20150317072750.GA22155@peff.net>

On Tue, Mar 17, 2015 at 03:27:50AM -0400, Jeff King wrote:

Show 13 quoted lines

> The general strategy for these is to use for_each_rawref traversals in
> these situations. That doesn't cover _every_ possible scenario. For
> example, you could do:
> 
>   git clone --no-local repo.git backup.git &&
>   rm -rf repo.git
> 
> and you might be disappointed if "backup.git" omitted some broken refs
> (upload-pack will simply skip the broken refs in its advertisement).  We
> could tighten this, but then it becomes hard to access slightly broken
> repositories (e.g., you might prefer to clone what you can, and not have
> git die() when it tries to serve the breakage). Patch 2 provides a
> tweakable safety valve for this.

One thing I thought about while working on this was whether we should just make _all_ ref iterations for_each_rawref. The benefit to not doing so in the hypothetical above is that you might be able to clone "foo" even if "bar" is broken.

But it strikes me as weird that we consider the _tips_ of history to be special for ignoring breakage. If the tip of "bar" is broken, we omit it. But if the tip is fine, and there's breakage three commits down in the history, then doing a clone is going to fail horribly, as pack-objects realizes it can't generate the pack. So in practice, I'm not sure how much you're buying with the "don't mention broken refs" code.

OTOH, there are probably _some_ situations that can be recovered with the current code that could not otherwise. For example, in the current code, I can still fetch "foo" even if "bar" is broken 3 commits down. Whereas if the tip is broken, there's a reasonable chance that "upload-pack" would just barf and I could fetch nothing.

So I stuck to the status quo in most cases, and only turned on the more aggressive behavior for destructive operations (and people who want to go wild can set GIT_REF_PARANOIA=1 for their every day operations if they want to).

-Peff