From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Mon, 22 Dec 2008 02:08:28 GMT
Subject: Re: [PATCH 2/2] fast-import: add special '-' blob reference to use the previous one.
Message-ID: <94a0d4530812211808r147cbd40na7266abe5ac0e6f3@mail.gmail.com>
In-Reply-To: <20081221221149.GB17355@spearce.org>

On Mon, Dec 22, 2008 at 12:11 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> @@ -1862,7 +1864,7 @@ static void file_change_m(struct branch *b)
>>       const char *endp;
>>       struct object_entry *oe = oe;
>>       unsigned char sha1[20];
>> -     uint16_t mode, inline_data = 0;
>> +     uint16_t mode, inline_data = 0, empty_blob = 0;
>
> Its not the empty blob, its the inherited/assumed blob...

Right. I thought: in order to use the inherited blob, you should not
specify any blob (leave it empty, or blank).

But yeah, 'inherited' does the job too.

>> @@ -1893,6 +1895,10 @@ static void file_change_m(struct branch *b)
>>       } else if (!prefixcmp(p, "inline")) {
>>               inline_data = 1;
>>               p += 6;
>> +     } else if (!prefixcmp(p, "- ")) {
>> +             hashclr(sha1);
>> +             empty_blob = 1;
>> +             p += 1;
>
> Hmph, so if create a new path with a blob of "-" the repository
> will be corrupt because the zero id was used and error was produced.
>
> Actually I think you have the same bug in the prior patch with the
> mode being inherited.  I wonder if we shouldn't put error checking
> in too to validate that versions[0] describes a file entry.

Yes, in my tests I found that issue in the previous patch and I have a
fix for that (set a default mode), but I haven't fixed this one. Do
you know what should be the behavior? I think it should 'die'.

-- 
Felipe Contreras