Re: [PATCH v3 1/4] replace: add --graft option

From: Christian Couder <christian.couder@gmail.com>
Date: Jun 29, 2014, 06:34 UTC
Message-ID: <CAP8UFD25k4X0vVUDDTOj_XDqmShz-aHXFLKD5bruUYYLX6awow@mail.gmail.com>
In-Reply-To: <CAPc5daWBycdmKBZXGhhy4_649p_JFfGf7RQbqa08XA1hL9mFTg@mail.gmail.com>

On Sun, Jun 8, 2014 at 10:18 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 19 quoted lines

>
> On Sat, Jun 7, 2014 at 11:49 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>>
>> On Fri, Jun 6, 2014 at 5:44 PM, Christian Couder
>> <christian.couder@gmail.com> wrote:
>> >
>> >         /* find existing parents */
>> >         strbuf_addstr(&buf, commit->buffer);
>>
>> Unfortunately, it looks like the above will not work if the commit->buffer
>> contains an embedded NUL. I wonder if it is a real problem or not.
>
> Yes, it is a real problem (there was another thread on this regarding the
> code path that verifies GPG signature on the commit itself), which
> incidentally reminds us to another thing to think about in your patch as
> well. I *think* you shoud drop the GPG signature on the commit itself, and
> you also should drop the merge-tag of a parent you are not going to keep,
> but should keep the merge-tag of a parent you are keeping.

In the v5 of the patch series, I now drop the GPG signature on the commit itself.

Now, after having read the recent thread about "git verify-commit", I understand that you also want me to drop the signature of a tag that was merged, because such signatures are added to the commit message.

But I wonder how far should we go in this path. For example merge commits have a title like "Merge branch 'dev'" or "Merge tag 'stuff'", but this does not make sense any more if the replacement commit drops the parent corresponding to 'dev' or 'stuff'. And I don't think we should change the commit title.

Thanks, Christian.