Hi,
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 <[email protected]>
[bwh: Backported to 3.2:
- Adjust numbering in the comment
- Adjust filename]
Signed-off-by: Ben Hutchings <[email protected]>
Cc: Byungchul Park <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Willy Tarreau <[email protected]>
Used to be turned into this :
Signed-off-by: Mike Galbraith <[email protected]>
[bwh: Backported to 3.2:
- Adjust numbering in the comment
- Adjust filename]
Signed-off-by: Ben Hutchings <[email protected]>
Cc: Byungchul Park <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Willy Tarreau <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
And is now properly converted to :
Signed-off-by: Mike Galbraith <[email protected]>
[bwh: Backported to 3.2:
- Adjust numbering in the comment
- Adjust filename]
Signed-off-by: Ben Hutchings <[email protected]>
Cc: Byungchul Park <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Willy Tarreau <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
Also, cherry-picking the last commit above again would produce this
before :
Signed-off-by: Mike Galbraith <[email protected]>
[bwh: Backported to 3.2:
- Adjust numbering in the comment
- Adjust filename]
Signed-off-by: Ben Hutchings <[email protected]>
Cc: Byungchul Park <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Willy Tarreau <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
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 <[email protected]>
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 <[email protected]>
Acked-by: Clemens Ladisch <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Ben Hutchings <[email protected]>
[wt: file is sound/midi/usbmidi.c in 2.6.32]
Signed-off-by: Willy Tarreau <[email protected]>
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 <[email protected]>
Acked-by: Clemens Ladisch <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Ben Hutchings <[email protected]>
[wt: file is sound/midi/usbmidi.c in 2.6.32]
Signed-off-by: Willy Tarreau <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
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 <[email protected]>
---
sequencer.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
