Re: [PATCH] sequencer.c: fix detection of duplicate s-o-b

[Willy Tarreau <w@1wt.eu>]

Hi Christian,

On Thu, Apr 07, 2016 at 04:06:59PM -0400, Christian Couder wrote:
> On Wed, Apr 6, 2016 at 12:37 PM, Willy Tarreau <w@1wt.eu> wrote:
> > On Wed, Apr 06, 2016 at 07:57:01AM -0700, Junio C Hamano wrote:
> >> This seems to have been lost, perhaps because the top part that was
> >> quite long didn't look like a patch submission message or something.
> >
> > Don't worry, we all know it's the submitter's responsibility to retransmit,
> > I apply the same principle :-)
> >
> >> Git 1.7.12 is a quite ancient release and I wouldn't be surprised if
> >> we made the behaviour change during the period leading to v2.6 on
> >> purpose, but nothing immediately comes to mind. Christian (as the
> >> advocate for the trailer machinery) and Brandon ("git shortlog
> >> sequencer.c" suggests you), can you take a look?
> 
> Ok, I will try to have a look at that next week.
> 
> > FWIW it wad changed in 1.8.3 by commit bab4d10 ("sequencer.c: teach
> > append_signoff how to detect duplicate s-o-b").
> 
> So the change is quite old and was made before I started working on
> the trailer machinery.
> 
> > The change made a lot of sense but it didn't assume that this practice
> > was common. And indeed I think this practice only happens in maintenance
> > branches where people have to make a lot of adaptations to existing
> > patches that they're cherry-picking. We do that a lot in stable kernels
> > to keep track of what we may need to revisit if we break something.
> 
> Yeah, we know for some time, but after the above patch breakage
> happened and after I worked on interpret-trailers, that some lines
> inside [] are added by kernel people in the trailer part and that the
> trailer machinery doesn't work properly with such lines.
> 
> Anyway if you want your patch to be applied, it will probably need tests.

Thanks. I've discussed with Junio yesterday who notified me that my patch
broke some existing tests that I hadn't updated (I didn't know they existed)
namely one supposed to make the distinction between a normal footer and a
special case where the s-o-b is in fact part of the last paragraph. So I
said I'll rework the patch to only consider the parts between brackets
above a footer. Thus no need to waste your time testing the current patch
next week.

Thanks,
Willy