| Message ID | 20240220010936.GA1793660@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | bc47139f4ff6c0c41d4c854ca74c35b1006a464a |
| Headers | show |
| Series | trailer: fix comment/cut-line regression with opts->no_divider | expand |
Jeff King <peff@peff.net> writes: > On Mon, Feb 19, 2024 at 10:42:45AM -0800, Junio C Hamano wrote: > >> Thanks, both, for finding a rather unfortunate regression. Perhaps >> it is worth delaying 2.44 final by a week or so to include a fix (or >> a revert if it comes to it). > > Hmm, I had thought this was pre-2.44, but it was actually in the 2.43.x > maintenance series (so it is not a regression going from 2.43.2 to > 2.44.0, but it is from 2.43.0 to 2.44.0). I've been trying to be a bit aggressive this cycle to merge various clean-up topics, together with real bugfixes, to 'maint'. Those who often skip the -rcX but diligently follow numbered releases found a few potential regressions in 2.44-rc as a result, which I could say is a great success ;-). In addition, I was planning to have only one -rc release without going -rc2 before the final, but we may need one if only for this fix. The fix in the patch looks quite straight-forward. Thanks.
Jeff King <peff@peff.net> writes: > On Mon, Feb 19, 2024 at 10:42:45AM -0800, Junio C Hamano wrote: > [...] > > The fix itself is pretty simple: instead of returning early, no_divider > just skips the "---" handling but still calls ignored_log_message_bytes(). I realize I am late to the discussion, but this fix (and patch) looks right to me. FWIW I independently discovered the same problem and figured out a fix locally in my larger refactor of this area (with the same fix, to always call ignored_log_message_bytes() regardless of no_divider). Thank you Peff! Sorry for introducing the regression. My enthusiasm in changing things up without unit tests is regrettable.
diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh index b5bf7de7cd..d8e216613f 100755 --- a/t/t7502-commit-porcelain.sh +++ b/t/t7502-commit-porcelain.sh @@ -485,6 +485,24 @@ test_expect_success 'commit --trailer not confused by --- separator' ' test_cmp expected actual ' +test_expect_success 'commit --trailer with --verbose' ' + cat >msg <<-\EOF && + subject + + body + EOF + GIT_EDITOR=: git commit --edit -F msg --allow-empty \ + --trailer="my-trailer: value" --verbose && + { + cat msg && + echo && + echo "my-trailer: value" + } >expected && + git cat-file commit HEAD >commit.msg && + sed -e "1,/^\$/d" commit.msg >actual && + test_cmp expected actual +' + test_expect_success 'multiple -m' ' >negative && diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 97f10905d2..3343ad0eb6 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -1560,4 +1560,23 @@ test_expect_success 'suppress --- handling' ' test_cmp expected actual ' +test_expect_success 'suppressing --- does not disable cut-line handling' ' + echo "real-trailer: before the cut" >expected && + + git interpret-trailers --parse --no-divider >actual <<-\EOF && + subject + + This input has a cut-line in it; we should stop parsing when we see it + and consider only trailers before that line. + + real-trailer: before the cut + + # ------------------------ >8 ------------------------ + # Nothing below this line counts as part of the commit message. + not-a-trailer: too late + EOF + + test_cmp expected actual +' + test_done diff --git a/trailer.c b/trailer.c index 816f8b28a9..009ee80dee 100644 --- a/trailer.c +++ b/trailer.c @@ -837,16 +837,15 @@ static size_t find_end_of_log_message(const char *input, int no_divider) /* Assume the naive end of the input is already what we want. */ end = strlen(input); - if (no_divider) - return end; - /* Optionally skip over any patch part ("---" line and below). */ - for (s = input; *s; s = next_line(s)) { - const char *v; + if (!no_divider) { + for (s = input; *s; s = next_line(s)) { + const char *v; - if (skip_prefix(s, "---", &v) && isspace(*v)) { - end = s - input; - break; + if (skip_prefix(s, "---", &v) && isspace(*v)) { + end = s - input; + break; + } } }