When the sequencer code appends a signoff or cherry-pick
origin, it uses the default trailer-parsing options, which
treat "---" as the end of the commit message. As a result,
it may be fooled by a commit message that contains that
string and fail to find the existing trailer block. Even
more confusing, the actual append code does not know about
"---", and always appends to the end of the string. This can
lead to bizarre results. E.g., appending a signoff to a
commit message like this:

  subject

  body
  ---
  these dashes confuse the parser!

  Signed-off-by: A

results in output with a final block like:

  Signed-off-by: A

  Signed-off-by: A

The parser thinks the final line of the message is "body",
and ignores everything else, claiming there are no trailers.
So we output an extra newline separator (wrong) and add a
duplicate signoff (also wrong).

Since we know we are feeding a pure commit message, we can
simply tell the parser to ignore the "---" divider.

Signed-off-by: Jeff King <p...@peff.net>
---
 sequencer.c       |  2 ++
 t/t7501-commit.sh | 16 ++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 3e15faa94e..25bdbfcce1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -232,6 +232,8 @@ static int has_conforming_footer(struct strbuf *sb, struct 
strbuf *sob,
        size_t i;
        int found_sob = 0, found_sob_last = 0;
 
+       opts.no_divider = 1;
+
        trailer_info_get(&info, sb->buf, &opts);
 
        if (info.trailer_start == info.trailer_end)
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 51646d8019..1cabb8b8ed 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -517,6 +517,22 @@ Myfooter: x" &&
        test_cmp expected actual
 '
 
+test_expect_success 'signoff not confused by ---' '
+       cat >expected <<-EOF &&
+               subject
+
+               body
+               ---
+               these dashes confuse the parser!
+
+               Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+       EOF
+       # should be a noop, since we already signed
+       git commit --allow-empty --signoff -F expected &&
+       git log -1 --pretty=format:%B >actual &&
+       test_cmp expected actual
+'
+
 test_expect_success 'multiple -m' '
 
        >negative &&
-- 
2.19.0.rc0.412.g7005db4e88

Reply via email to