Re: [PATCH RFC] parse_object: pass on the original sha1, not the replaced one

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Aug 2, 2010, 09:31 UTC
Message-ID: <AANLkTin0eymuYFv_6hamDuan0TELogtCyW3Xwjm=pAwH@mail.gmail.com>
In-Reply-To: <201008020942.58137.chriscool@tuxfamily.org>

2010/8/2 Christian Couder <chriscool@tuxfamily.org>:

Show 16 quoted lines

> On Saturday 31 July 2010 14:36:42 Nguyễn Thái Ngọc Duy wrote:
>> Commit 0e87c36 (object: call "check_sha1_signature" with the
>> replacement sha1) did this. I'm not sure if it's should be done this
>> way.
>>
>> With "repl" as the first argument to parse_object_buffer, the returned
>> obj pointer will have the replaced SHA1 in obj->sha1, not the original
>> one. I sort of expect that, no matter the object is replaced,
>> obj->sha1 should stay the same.
>>
>> This was observed by replacing commit tip. git log would show the
>> replaced sha1, not the original one.
>
> I am not sure I understand what you are saying. Do you mean that git log
> should show the original sha1 but the content of the replacement commit,
> instead of both the sha1 and the content of the replacement commit?

"original sha1, but the content of replacement commit", yes. Isn't that the intention of git-replace?

> I just tested your patch and indeed with it it seems to me that the result
> shown by git log is not consistent, as for example the commit message is the
> one of the replacement commit but the commit sha1 is the one of the original
> commit.

The consistency is already there. Suppose I want to replace commit A with B. Depends on what function I call to fetch "A", I get different object->sha1. (the content is always from B though).

With parse_object(A), I get an object pointer whose sha1 is B.

With  lookup_commit(A), I get an object pointer whose sha1 is A.

> Could you explain why you need the object returned by parse_object_buffer to
> not have the replacement SHA1 in obj->sha1 when it is parsing the buffer from
> the replacement object?

As I said above, it's inconsistent. I'm not saying my way is correct. Just wondering.

-- 
Duy