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

[Jeff King <peff@peff.net>]

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

> 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