On Thu, Aug 11, 2016 at 02:57:51AM -0400, Jeff King wrote:

> > One thing to be careful of is that there are more things this
> > drop_reused_delta() should be doing. But I looked through the rest of
> > check_object() and could not find anything else.
> Argh, I spoke too soon.
> It is true that the size lookup is the only part of check_object()
> we skip if we are reusing the delta. But what I didn't notice is that
> when we choose to reuse a delta, we overwrite entry->type (the real
> type!) with the in_pack_type (OFS_DELTA, etc). We need to undo that so
> that later stages see the real type.
> I'm not sure how my existing tests worked (I confirmed that they do
> indeed break the delta). It may be that only some code paths actually
> care about the real type. But when playing with the depth-limit (which
> uses the same drop_reused_delta() helper), I managed to make some pretty
> broken packs.
> So please disregard the v4 patch I just sent; I haven't triggered it,
> but I imagine it has the same problem, and I just didn't manage to
> trigger it.
> I'll clean that up and send out a new series.

Here it is. It ended up needing a few preparatory patches.

  [1/4]: provide an initializer for "struct object_info"
  [2/4]: sha1_file: make packed_object_info public
  [3/4]: pack-objects: break delta cycles before delta-search phase
  [4/4]: pack-objects: use mru list when iterating over packs

I had originally intended to include an extra patch to handle the
--depth limits better. But after writing it, I'm not sure it's actually
a good idea. I'll post it as an addendum with more discussion.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to