Re: Zero padded file modes...

[Jeff King <peff@peff.net>]

On Thu, Sep 05, 2013 at 10:00:39AM -0400, John Szakmeister wrote:

> I went to clone a repository from GitHub today and discovered
> something interesting:
> 
>     :: git clone https://github.com/liebke/incanter.git
>     Cloning into 'incanter'...
>     remote: Counting objects: 10457, done.
>     remote: Compressing objects: 100% (3018/3018), done.
>     error: object 4946e1ba09ba5655202a7a5d81ae106b08411061:contains
> zero-padded file modes
>     fatal: Error in object
>     fatal: index-pack failed

Yep. These were mostly caused by a bug in Grit that is long-fixed.  But
the objects remain in many histories. It would have painful to rewrite
them back then, and it would be even more painful now.

> > This is going to screw up pack v4 (yes, someday I'll have the
> > time to make it real).
> 
> I don't know if this is still true, but given that patches are
> being sent out about it, I thought it relevant.

I haven't looked carefully at the pack v4 patches yet, but I suspect
that yes, it's still a problem. The premise of pack v4 is that we can do
better by not storing the raw git object bytes, but rather storing
specialized representations of the various components. For example, by
using an integer to store the mode rather than the ascii representation.
But that representation does not represent the "oops, I have a 0-padded
mode" quirk. And we have to be able to recover the original object, byte
for byte, from the v4 representation (to verify sha1, or to generate a
loose object or v2 pack).

There are basically two solutions:

  1. Add a single-bit flag for "I am 0-padded in the real data". We
     could probably even squeeze it into the same integer.

  2. Have a "classic" section of the pack that stores the raw object
     bytes. For objects which do not match our expectations, store them
     raw instead of in v4 format. They will not get the benefit of v4
     optimizations, but if they are the minority of objects, that will
     only end up with a slight slow-down.

As I said, I have not looked carefully at the v4 patches, so maybe they
handle this case already. But of the two solutions, I prefer (2). Doing
(1) can solve _this_ problem, but it complicates the format, and does
nothing for any future compatibility issues. Whereas (2) is easy to
implement, since it is basically just pack v2 (and implementations would
need a pack v2 reader anyway).

-Peff