Willy TarreauHi,
after I upgraded my machine, I switched from git 1.7.12.2 to 2.6.4 and experienced an annoying regression when dealing with stable kernel backports.
I'm using a "dorelease" script which relies on git-cherry-pick's ability to properly detect duplicate s-o-b to ensure that all merged commits are properly signed in a release. Today while preparing the last 2.6.32 release, I did a git log before pushing and found some commits having two s-o-b lines with myself. I found that these ones were always those containing some backporting notes between the s-o-b lines (which we all do in stable branches to indicate what was changed in the backport process).
I didn't feel brave enough to individually deal with each offending patch by hand so instead I bisected the git changes and found that the behaviour changed with commit bab4d10 ("sequencer.c: teach append_signoff how to detect duplicate s-o-b").
The reason is that function has_conforming_footer() immediately stops after the first non-conforming line without checking if there are conforming lines after. But if someone added signed-off-by anywhere after a non-conforming block, it should always be considered as part of the footer. Thus I adjusted the logic to check till the end of the footer and report the presence of valid rfc2822 or cherry-picked lines after the last non-conformant one and now it correctly handles all types of commits I had to deal with (ie: only adds s-o-b when it doesn't match the last one and doesn't add an empty line after a conformant one). For example, this footer :
Signed-off-by: Mike Galbraith <umgwanakikbuti@gmail.com>
[bwh: Backported to 3.2:
- Adjust numbering in the comment
- Adjust filename]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: Byungchul Park <byungchul.park@lge.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Willy Tarreau <w@1wt.eu>Used to be turned into this :
Signed-off-by: Mike Galbraith <umgwanakikbuti@gmail.com>
[bwh: Backported to 3.2:
- Adjust numbering in the comment
- Adjust filename]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: Byungchul Park <byungchul.park@lge.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Willy Tarreau <w@1wt.eu>Signed-off-by: Willy Tarreau <w@1wt.eu>
And is now properly converted to :
Signed-off-by: Mike Galbraith <umgwanakikbuti@gmail.com>
[bwh: Backported to 3.2:
- Adjust numbering in the comment
- Adjust filename]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: Byungchul Park <byungchul.park@lge.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Willy Tarreau <w@1wt.eu>
Signed-off-by: Willy Tarreau <w@1wt.eu>Also, cherry-picking the last commit above again would produce this before :
Signed-off-by: Mike Galbraith <umgwanakikbuti@gmail.com>
[bwh: Backported to 3.2:
- Adjust numbering in the comment
- Adjust filename]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: Byungchul Park <byungchul.park@lge.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Willy Tarreau <w@1wt.eu>
Signed-off-by: Willy Tarreau <w@1wt.eu>Signed-off-by: Willy Tarreau <w@1wt.eu>
And it now is properly left untouched since the last s-o-b line is properly matched.
I'm appending the patch, please include it upstream.
Thanks! Willy
>From be9624a0df4c649d452f898925953a81dc9163fc Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu> Date: Sat, 12 Mar 2016 13:35:35 +0100 Subject: sequencer.c: fix detection of duplicate s-o-b
Commit bab4d10 ("sequencer.c: teach append_signoff how to detect duplicate s-o-b") changed the method used to detect duplicate s-o-b, but it introduced a regression for a case where some non-compliant information are present in the footer. In maintenance branches, it's very common to add some elements after the signed-off and to add your s-o-b after. This is used a lot in the stable kernel series, for example this commit backported from 3.2 to 2.6.32 :
ALSA: usb-audio: avoid freeing umidi object twice
commit 07d86ca93db7e5cdf4743564d98292042ec21af7 upstream.
The 'umidi' object will be free'd on the error path by snd_usbmidi_free()
when tearing down the rawmidi interface. So we shouldn't try to free it
in snd_usbmidi_create() after having registered the rawmidi interface.Found by KASAN.
Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
[wt: file is sound/midi/usbmidi.c in 2.6.32]
Signed-off-by: Willy Tarreau <w@1wt.eu>Prior to the commit above, a cherry-pick -s would not append an extra s-o-b. After this commit, a new line and a second s-o-b are added, making the footer look like this :
Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
[wt: file is sound/midi/usbmidi.c in 2.6.32]
Signed-off-by: Willy Tarreau <w@1wt.eu>Signed-off-by: Willy Tarreau <w@1wt.eu>
This patch improves the parsing of the footer by considering the presence of a valid rfc2822 line after possibly non-conformant lines. Indeed, if someone added an s-o-b or CC after some stuff, this line must properly be considered as part of the footer and not of the body.
Signed-off-by: Willy Tarreau <w@1wt.eu> --- sequencer.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
Show changes to sequencer.c +10 −5
diff --git a/sequencer.c b/sequencer.c index e66f2fe..ab2c18d 100644 --- a/sequencer.c +++ b/sequencer.c @@ -64,6 +64,8 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, int len = sb->len - ignore_footer; const char *buf = sb->buf; int found_sob = 0; + int found_valid = 0; + int found_other = 0; /* footer must end with newline */ if (!len || buf[len - 1] != '\n') @@ -96,15 +98,18 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, if (found_rfc2822 && sob && !strncmp(buf + i, sob->buf, sob->len)) found_sob = k; - - if (!(found_rfc2822 || - is_cherry_picked_from_line(buf + i, k - i - 1))) - return 0; + else if (found_rfc2822 || + is_cherry_picked_from_line(buf + i, k - i - 1)) + found_valid = k; + else + found_other = k; } if (found_sob == i) return 3; - if (found_sob) + if (found_sob > found_other) return 2; + if (found_other > found_valid) + return 0; return 1; }
-- 2.6.4
