Re: [PATCH 2/2] index-pack: handle duplicate base objects gracefully

[Shawn Pearce <spearce@spearce.org>]

On Sat, Aug 30, 2014 at 6:16 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Aug 29, 2014 at 07:59:32PM -0700, Shawn Pearce wrote:
>
>> > I agree it is probably a bug on the sending side, but I think last time
>> > this came up we decided to try to be liberal in what we accept.  c.f.
>> > http://thread.gmane.org/gmane.comp.version-control.git/232305/focus=232310
>>
>> IIRC they aren't valid pack files to contain duplicates.
>>
>> Once upon a time JGit had a bug and android.googlesource.com returned
>> duplicate objects in a Linux kernel repository. This caused at least
>> some versions of git-core to fail very badly in binary search at
>> object lookup time or something. We had a lot of users angry with us.
>> :)
>>
>> I know Nico said its OK last year, but its really not. I don't think
>> implementations are capable of handling it.
>
> We do detect and complain if --strict is given. Should we make it the
> default instead? I think it is still worthwhile to have a mode that can
> handle these packs. It may be the only reasonable way to recover the
> data from such a broken pack (and that broken pack may be the only copy
> of the data you have, if you are stuck getting it out of a broken
> implementation on a remote server).

Eh, OK, that does make a lot of sense.

Unfortunately accepting it by default encourages broken
implementations to stay broken and ship packs with duplicate objects,
which they shouldn't do since there are readers out there that cannot
handle it.

In my opinion we should make --strict default, but its an excellent
point made that users should have an escape hatch to disable this
check, attempt accepting a broken pack and try fixing it locally with
repack.