Re: [PATCH] drop support for "experimental" loose objects

[Keshav Kini <keshav.kini@gmail.com>]

Duy Nguyen <pclouds@gmail.com> writes:

> On Thu, Nov 21, 2013 at 6:48 PM, Jeff King <peff@peff.net> wrote:
>> @@ -1514,14 +1469,6 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
>>
>>  int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz)
>>  {
>> -       unsigned long size, used;
>> -       static const char valid_loose_object_type[8] = {
>> -               0, /* OBJ_EXT */
>> -               1, 1, 1, 1, /* "commit", "tree", "blob", "tag" */
>> -               0, /* "delta" and others are invalid in a loose object */
>> -       };
>> -       enum object_type type;
>> -
>>         /* Get the data stream */
>>         memset(stream, 0, sizeof(*stream));
>>         stream->next_in = map;
>> @@ -1529,27 +1476,6 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma
>>         stream->next_out = buffer;
>>         stream->avail_out = bufsiz;
>>
>> -       if (experimental_loose_object(map)) {
>
> Perhaps keep this..
>
>> -               /*
>> -                * The old experimental format we no longer produce;
>> -                * we can still read it.
>> -                */
>> -               used = unpack_object_header_buffer(map, mapsize, &type, &size);
>> -               if (!used || !valid_loose_object_type[type])
>> -                       return -1;
>> -               map += used;
>> -               mapsize -= used;
>> -
>> -               /* Set up the stream for the rest.. */
>> -               stream->next_in = map;
>> -               stream->avail_in = mapsize;
>> -               git_inflate_init(stream);
>> -
>> -               /* And generate the fake traditional header */
>> -               stream->total_out = 1 + snprintf(buffer, bufsiz, "%s %lu",
>> -                                                typename(type), size);
>> -               return 0;
>
> and replace all this with
>
> die("detected an object in obsolete format, please repack the
> repository using a version before XXX");
>
> ?

Wouldn't that fail to solve the issue of `git fsck` dying on corrupt
data?  experimental_loose_object() would need to be rewritten to be more
conservative in deciding that an object was in the experimental loose
object format.

-Keshav