git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 7/9] sequencer: ignore "---" divider when parsing trailers

From
Jeff King <peff@peff.net>
Date
Aug 23, 2018, 00:50 UTC
Message-ID
<20180823005037.GG3126@sigill.intra.peff.net>
In-Reply-To
<20180823004300.GA1355@sigill.intra.peff.net>

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 <peff@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
Previous: Jeff KingNext: Jeff King
Message 48 of 53 in “test-tool.h: include git-compat-util.h”
  1. test-tool.h: include git-compat-util.hJeff King, Aug 21, 2018
  2. Junio C HamanoAug 21, 2018
  3. 1/6 t/perf: factor boilerplate out of test_perfJeff King, Aug 21, 2018
  4. 2/6 t/perf: factor out percent calculationsJeff King, Aug 21, 2018
  5. 3/6 t/perf: add infrastructure for measuring sizesJeff King, Aug 21, 2018
  6. Derrick StoleeAug 22, 2018
  7. Jeff KingAug 22, 2018
  8. 4/6 t/perf: add perf tests for fetches from a bitmapped serverJeff King, Aug 21, 2018
  9. 5/6 pack-bitmap: save "have" bitmap from walkJeff King, Aug 21, 2018
  10. Derrick StoleeAug 21, 2018
  11. Jeff KingAug 21, 2018
  12. Ævar Arnfjörð BjarmasonAug 31, 2018
  13. Jeff KingAug 31, 2018
  14. 0/4 un-breaking pack-objects with bitmapsJeff King, Sep 1, 2018
  15. 1/4 bitmap_has_sha1_in_uninteresting(): drop BUG checkJeff King, Sep 1, 2018
  16. 2/4 t5310: test delta reuse with bitmapsJeff King, Sep 1, 2018
  17. Jeff KingSep 1, 2018
  18. Ævar Arnfjörð BjarmasonSep 1, 2018
  19. Ben PeartSep 1, 2018
  20. Jeff KingSep 2, 2018
  21. Stefan BellerSep 4, 2018
  22. Junio C HamanoSep 4, 2018
  23. Jeff KingSep 4, 2018
  24. 3/4 traverse_bitmap_commit_list(): don't free resultJeff King, Sep 1, 2018
  25. 4/4 pack-bitmap: drop "loaded" flagJeff King, Sep 1, 2018
  26. Stefan BellerSep 4, 2018
  27. Jeff KingSep 4, 2018
  28. Ævar Arnfjörð BjarmasonSep 8, 2018
  29. Junio C HamanoSep 10, 2018
  30. Jeff KingSep 10, 2018
  31. Junio C HamanoSep 10, 2018
  32. 6/6 pack-objects: reuse on-disk deltas for thin "have" objectsJeff King, Aug 21, 2018
  33. Junio C HamanoAug 21, 2018
  34. Junio C HamanoAug 21, 2018
  35. Jeff KingAug 21, 2018
  36. Jeff KingAug 21, 2018
  37. Junio C HamanoAug 21, 2018
  38. Jeff KingAug 21, 2018
  39. Junio C HamanoAug 21, 2018
  40. Jeff KingAug 21, 2018
  41. 0/9 trailer-parsing false positivesJeff King, Aug 23, 2018
  42. 1/9 trailer: use size_t for string offsetsJeff King, Aug 23, 2018
  43. 2/9 trailer: use size_t for iterating trailer listJeff King, Aug 23, 2018
  44. 3/9 trailer: pass process_trailer_opts to trailer_info_get()Jeff King, Aug 23, 2018
  45. 4/9 interpret-trailers: tighten check for "---" patch boundaryJeff King, Aug 23, 2018
  46. 5/9 interpret-trailers: allow suppressing "---" dividerJeff King, Aug 23, 2018
  47. 6/9 pretty, ref-filter: format %(trailers) with no_divider optionJeff King, Aug 23, 2018
  48. 7/9 sequencer: ignore "---" divider when parsing trailersJeff King, Aug 23, 2018
  49. 8/9 append_signoff: use size_t for string offsetsJeff King, Aug 23, 2018
  50. 9/9 sequencer: handle ignore_footer when parsing trailersJeff King, Aug 23, 2018
  51. Junio C HamanoAug 23, 2018
  52. Jeff KingAug 24, 2018
  53. Jeff KingAug 21, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.