git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Shawn Pearce <spearce@spearce.org>
Date
Aug 31, 2014, 01:10 UTC
Message-ID
<CAJo=hJu-DCMv=jepMJvcmR9EOedkynCyL0kD_hB+UGWxbErDfA@mail.gmail.com>
In-Reply-To
<20140830131649.GA26833@peff.net>
On Sat, Aug 30, 2014 at 6:16 AM, Jeff King <peff@peff.net> wrote:
Show 23 quoted lines
> 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.

Previous: René ScharfeNext: Jeff King
Message 24 of 30 in “[BUG] resolved deltas”
  1. Petr StodulkaAug 21, 2014
  2. Petr StodulkaAug 21, 2014
  3. Martin von GagernAug 22, 2014
  4. René ScharfeAug 23, 2014
  5. Jeff KingAug 23, 2014
  6. Jeff KingAug 23, 2014
  7. Jeff KingAug 23, 2014
  8. René ScharfeAug 25, 2014
  9. Jeff KingAug 28, 2014
  10. Jeff KingAug 28, 2014
  11. Jeff KingAug 28, 2014
  12. Jeff KingAug 28, 2014
  13. Junio C HamanoAug 28, 2014
  14. Jeff KingAug 29, 2014
  15. 1/2 index-pack: fix race condition with duplicate basesJeff King, Aug 29, 2014
  16. 2/2 index-pack: handle duplicate base objects gracefullyJeff King, Aug 29, 2014
  17. Junio C HamanoAug 29, 2014
  18. Jeff KingAug 29, 2014
  19. Shawn PearceAug 30, 2014
  20. Jeff KingAug 30, 2014
  21. René ScharfeAug 30, 2014
  22. Jeff KingAug 31, 2014
  23. René ScharfeAug 31, 2014
  24. Shawn PearceAug 31, 2014
  25. Jeff KingAug 31, 2014
  26. Junio C HamanoAug 31, 2014
  27. 3/2 t5309: mark delta-cycle failover tests as passingJeff King, Aug 30, 2014
  28. Jeff KingAug 31, 2014
  29. Junio C HamanoSep 2, 2014
  30. Shawn PearceAug 25, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.