sequencer: add newline before adding footers

When encountering a commit message that does not end in a newline, sequencer does not complete the line before determining if a blank line should be added. This causes the "(cherry picked..." and sign-off lines to sometimes appear on the same line as the last line of the commit message. This behavior was introduced by commit 967dfd4 ("sequencer: use trailer's trailer layout", 2016-11-29). However, a revert of that commit would not resolve this issue completely: prior to that commit, a conforming footer was deemed to be non-conforming by has_conforming_footer() if there was no terminating newline, resulting in both conforming and non-conforming footers being treated the same when they should not be. Resolve this issue, both for conforming and non-conforming footers, and in both do_pick_commit() and append_signoff(), by always adding a newline to the commit message if it does not end in one before checking the footer for conformity. Reported-by: Brian Norris <computersforpeace@gmail.com> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Apr 26, 2017 at 13:50 UTC 44dc738a39f2c4dba41c0ddacae280d0f88dc71f
2 files changed +48 -7
sequencer.c
+4 -7
@@ -698,6 +698,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
698 strbuf_addstr(&msgbuf, skip_blank_lines(p + 2));
699
700 if (opts->record_origin) {
701 + strbuf_complete_line(&msgbuf);
702 if (!has_conforming_footer(&msgbuf, NULL, 0))
703 strbuf_addch(&msgbuf, '\n');
704 strbuf_addstr(&msgbuf, cherry_picked_prefix);
@@ -1362,6 +1363,9 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
1363 getenv("GIT_COMMITTER_EMAIL")));
1364 strbuf_addch(&sob, '\n');
1365
1366 + if (!ignore_footer)
1367 + strbuf_complete_line(msgbuf);
1368 +
1369 /*
1370 * If the whole message buffer is equal to the sob, pretend that we
1371 * found a conforming footer with a matching sob
@@ -1382,13 +1386,6 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
1386 * the title and body to be filled in by the user.
1387 */
1388 append_newlines = "\n\n";
1385 - } else if (msgbuf->buf[len - 1] != '\n') {
1386 - /*
1387 - * Incomplete line. Complete the line and add a
1388 - * blank one so that there is an empty line between
1389 - * the message body and the sob.
1390 - */
1391 - append_newlines = "\n\n";
1389 } else if (len == 1) {
1390 /*
1391 * Buffer contains a single newline. Add another
t/t3511-cherry-pick-x.sh
+44
@@ -208,6 +208,50 @@ test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists fo
208 test_cmp expect actual
209 '
210
211 +test_expect_success 'cherry-pick -x handles commits with no NL at end of message' '
212 + pristine_detach initial &&
213 + printf "title\n\nSigned-off-by: A <a@example.com>" >msg &&
214 + sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) &&
215 + git cherry-pick -x $sha1 &&
216 + git log -1 --pretty=format:%B >actual &&
217 +
218 + printf "\n(cherry picked from commit %s)\n" $sha1 >>msg &&
219 + test_cmp msg actual
220 +'
221 +
222 +test_expect_success 'cherry-pick -x handles commits with no footer and no NL at end of message' '
223 + pristine_detach initial &&
224 + printf "title\n\nnot a footer" >msg &&
225 + sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) &&
226 + git cherry-pick -x $sha1 &&
227 + git log -1 --pretty=format:%B >actual &&
228 +
229 + printf "\n\n(cherry picked from commit %s)\n" $sha1 >>msg &&
230 + test_cmp msg actual
231 +'
232 +
233 +test_expect_success 'cherry-pick -s handles commits with no NL at end of message' '
234 + pristine_detach initial &&
235 + printf "title\n\nSigned-off-by: A <a@example.com>" >msg &&
236 + sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) &&
237 + git cherry-pick -s $sha1 &&
238 + git log -1 --pretty=format:%B >actual &&
239 +
240 + printf "\nSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n" >>msg &&
241 + test_cmp msg actual
242 +'
243 +
244 +test_expect_success 'cherry-pick -s handles commits with no footer and no NL at end of message' '
245 + pristine_detach initial &&
246 + printf "title\n\nnot a footer" >msg &&
247 + sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) &&
248 + git cherry-pick -s $sha1 &&
249 + git log -1 --pretty=format:%B >actual &&
250 +
251 + printf "\n\nSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n" >>msg &&
252 + test_cmp msg actual
253 +'
254 +
255 test_expect_success 'cherry-pick -x treats "(cherry picked from..." line as part of footer' '
256 pristine_detach initial &&
257 sha1=$(git rev-parse mesg-with-cherry-footer^0) &&