Re: [PATCH RFC] git-am: support any number of signatures

From: Christian Couder <chriscool@tuxfamily.org>
Date: Sep 28, 2014, 11:36 UTC
Message-ID: <20140928.133623.1635797151132857721.chriscool@tuxfamily.org>
In-Reply-To: <CAPc5daUTKU1JepLuyVC1Nk6LG4AZ2TA5SEYLsTSPbPKkauzNyQ@mail.gmail.com>

From: Junio C Hamano <gitster@pobox.com>

Show 27 quoted lines

> On Thu, Sep 25, 2014 at 3:04 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>>> To an existing message ends with Michael's Signed-off-by:, if his
>>> "git am --trailer arts" is called to add these three and then a
>>> Signed-off-by: from him, should it add an extra S-o-b (because his
>>> existing S-o-b will no longer be the last one after adding Acked and
>>> others), or should it refrain from doing so?  Can you express both
>>> preferences?
>>
>> The default for "trailer.where" is "end", and for "trailer.ifexists"
>> it is "addIfDifferentNeighbor".
>> That means that by default it will add the four new trailers at the end.
>>
>> If either "trailer.ifexists" or "trailer.S-o-b.ifexists" is set to
>> "addIfDifferent", then only the first 3 new trailers will be added at
>> the end. So yes you can express both preferences.
> 
> Suppose that the original ends with Sob, and Michael's "am" invokes
> interpret-trailers with Acked/Reviewed/Tested/Sob in this order. The first
> three are set to addifDifferent and Sob is set to addIfDifferentNeighbor,
> which would be the traditional and sensible default.
> 
> Now, how is addIfDifferentNeighbor interpreted? Adding the new Sob
> with this single request to add these four is done on a message that
> has the same Sob at the end (i.e. Neighbor). Does _you_ adding of
> Acked/Reviewed/Tested invalidate the Neighbor-ness and you add the
> Sob anew?

The trailers are processed in the order they appear on the command line. And when a trailer is processed, the input message it "sees" is the result from the processing of all the previous trailers on the command line. It is not any more the original input message.

So after Acked/Reviewed/Tested have been added, when the S-o-b at the end of the command line is processed, it "sees" an input message that has the Tested trailer at the end. That's why with addIfDifferentNeighbor the S-o-b will still be added at the end.

> If that is what happens, it is not a workable workaround to set Sob to
> addIfDifferent only for this invocation.

Setting S-o-b to addIfDifferent for this invocation would not add the S-o-b at the end, because another S-o-b still exists in the input message "seen" when the last S-o-b is processed.

> Alternatively, if Neighbor-ness is evaluated first before you add A/R/T
> in response to this request, then you'd refrain from adding a duplicate
> Sob. It wasn't quite clear from your description what your design was,
> and your explanation above is not still clear, at least to me.

I hope it is clearer now. Maybe I should add something in the doc to better explain how it works.

Thanks, Christian.