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

[Jeff King <peff@peff.net>]

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

> > 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