From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
jrnieder@gmail.com, gitster@pobox.com,
Johannes.Schindelin@gmx.de
Subject: [PATCH v3] sequencer: add newline before adding footers
Date: Wed, 26 Apr 2017 13:50:03 -0700 [thread overview]
Message-ID: <20170426205003.30825-1-jonathantanmy@google.com> (raw)
In-Reply-To: <20170425190651.8910-1-jonathantanmy@google.com>
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>
---
The failure in the case of non-footers not being terminated by a newline
turns out to be quite easy to fix, so I've added that fix here. I think
this makes the overall fix more obvious too.
I've used Jonathan Nieder's and Dscho's suggestions for the tests
(except for --format vs --pretty, since --format seems to add an extra
newline to the output).
sequencer.c | 11 ++++-------
t/t3511-cherry-pick-x.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 77afecaeb..ffac95545 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1045,6 +1045,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
strbuf_addstr(&msgbuf, p);
if (opts->record_origin) {
+ strbuf_complete_line(&msgbuf);
if (!has_conforming_footer(&msgbuf, NULL, 0))
strbuf_addch(&msgbuf, '\n');
strbuf_addstr(&msgbuf, cherry_picked_prefix);
@@ -2335,6 +2336,9 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
getenv("GIT_COMMITTER_EMAIL")));
strbuf_addch(&sob, '\n');
+ if (!ignore_footer)
+ strbuf_complete_line(msgbuf);
+
/*
* If the whole message buffer is equal to the sob, pretend that we
* found a conforming footer with a matching sob
@@ -2355,13 +2359,6 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
* the title and body to be filled in by the user.
*/
append_newlines = "\n\n";
- } else if (msgbuf->buf[len - 1] != '\n') {
- /*
- * Incomplete line. Complete the line and add a
- * blank one so that there is an empty line between
- * the message body and the sob.
- */
- append_newlines = "\n\n";
} else if (len == 1) {
/*
* Buffer contains a single newline. Add another
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index bf0a5c988..9888bf34b 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -208,6 +208,50 @@ test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists fo
test_cmp expect actual
'
+test_expect_success 'cherry-pick -x handles commits with no NL at end of message' '
+ pristine_detach initial &&
+ printf "title\n\nSigned-off-by: A <a@example.com>" >msg &&
+ sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) &&
+ git cherry-pick -x $sha1 &&
+ git log -1 --pretty=format:%B >actual &&
+
+ printf "\n(cherry picked from commit %s)\n" $sha1 >>msg &&
+ test_cmp msg actual
+'
+
+test_expect_success 'cherry-pick -x handles commits with no footer and no NL at end of message' '
+ pristine_detach initial &&
+ printf "title\n\nnot a footer" >msg &&
+ sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) &&
+ git cherry-pick -x $sha1 &&
+ git log -1 --pretty=format:%B >actual &&
+
+ printf "\n\n(cherry picked from commit %s)\n" $sha1 >>msg &&
+ test_cmp msg actual
+'
+
+test_expect_success 'cherry-pick -s handles commits with no NL at end of message' '
+ pristine_detach initial &&
+ printf "title\n\nSigned-off-by: A <a@example.com>" >msg &&
+ sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) &&
+ git cherry-pick -s $sha1 &&
+ git log -1 --pretty=format:%B >actual &&
+
+ printf "\nSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n" >>msg &&
+ test_cmp msg actual
+'
+
+test_expect_success 'cherry-pick -s handles commits with no footer and no NL at end of message' '
+ pristine_detach initial &&
+ printf "title\n\nnot a footer" >msg &&
+ sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) &&
+ git cherry-pick -s $sha1 &&
+ git log -1 --pretty=format:%B >actual &&
+
+ printf "\n\nSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n" >>msg &&
+ test_cmp msg actual
+'
+
test_expect_success 'cherry-pick -x treats "(cherry picked from..." line as part of footer' '
pristine_detach initial &&
sha1=$(git rev-parse mesg-with-cherry-footer^0) &&
--
2.13.0.rc0.306.g87b477812d-goog
prev parent reply other threads:[~2017-04-26 20:50 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-21 22:01 [GIT 2.12.2 REGRESSION] git cherry-pick -x Brian Norris
2017-04-21 22:10 ` Stefan Beller
2017-04-21 22:13 ` Jeff King
2017-04-25 19:06 ` [PATCH] sequencer: require trailing NL in footers Jonathan Tan
2017-04-25 21:04 ` Stefan Beller
2017-04-25 21:51 ` Jonathan Tan
2017-04-25 21:56 ` Stefan Beller
2017-04-25 22:05 ` Johannes Schindelin
2017-04-25 23:39 ` Jonathan Nieder
2017-04-25 22:30 ` Brian Norris
2017-04-26 20:31 ` Brian Norris
2017-04-25 21:56 ` Johannes Schindelin
2017-04-25 23:34 ` Jonathan Nieder
2017-04-25 23:57 ` [PATCH v2] " Jonathan Tan
2017-04-26 0:07 ` Jonathan Nieder
2017-04-26 2:08 ` Junio C Hamano
2017-04-26 9:09 ` Johannes Schindelin
2017-04-26 20:50 ` Jonathan Tan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170426205003.30825-1-jonathantanmy@google.com \
--to=jonathantanmy@google.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).