Re: [PATCH] pack-objects: do not reuse packfiles without --delta-base-offset

On Wed, Apr 02, 2014 at 10:39:13AM -0700, Junio C Hamano wrote:

quoted

However, it's possible that the other side cannot read our
packfile verbatim. For example, we may have objects stored
as OFS_DELTA, but the client is an antique version of git
that only understands REF_DELTA. We negotiate this
capability over the fetch protocol. A normal pack-objects
run will convert OFS_DELTA into REF_DELTA on the fly, but
the "reuse pack" code path never even looks at the objects.

The above makes it sound like "reuse pack" codepath is broken.

It is broken (without this patch), though in practice only for ancient
(pre-1.4.x) clients.

Is it too much hassle to peek at the initial bytes of each object to
see how they are encoded? Would it be possible to convert OFS_DELTA to
REF_DELTA on the fly on that codepath as well, instead of disabling
the reuse altogether?

It's a mistake to peek ahead of time. Part of the point of the
pack-reuse optimization is to start sending out bytes as soon as
possible, since the network is quite often the bottleneck. So we would
not want to look through all of the to-be-sent data before sending out
the first byte.

We could convert OFS_DELTA to REF_DELTA on the fly. That _may_ have a
performance impact. Right now, we are basically doing the equivalent of
sendfile(), and conversion would involve iterating through each object
and examining the header.  I think that's probably not too bad, though.
The most expensive part of that, stepping to the next object, requires
scanning through the zlib packets, but we should be able to use the
revidx to avoid that.

I'm not sure it's even worth the code complexity, though. The non-reuse
codepath is not that much slower, and it should be extremely rare for a
client not to support OFS_DELTA these days.

-Peff