Re: Possible bug regarding trailers
From: Kristoffer Haugsbakk <hidden>
Date: 2023-06-15 19:05:21
Hi On Thu, Jun 15, 2023, at 19:46, eric.frederich@siemens.com wrote:
I am able to produce a commit with a trailer which does not show up in:
`git log
--format="%(trailers:key=old-scm-change-id,valueonly,separator=%x2c) %H
%T" HEAD`
But does show up in:
`git cat-file commit HEAD | git interpret-trailers --parse`
It seems that the `--- ` (note the space) is interpreted as marking the
start of the patch part (as in a patch file which contains a commit
message followed by a patch).
See `trailers.c:find_patch_start` (here on d7d8841f67 (Start the 2.42
cycle, 2023-06-13):
for (s = str; *s; s = next_line(s)) {
const char *v;
if (skip_prefix(s, "---", &v) && isspace(*v))
return s - str;
}
I’m not good with C but it seems that this line will match:
--- let's mess stuff up ---
Which instructs the code to put the trailer *before* this “patch part”.
(Or at least: if I remove this if-block then your script seems to work
like you want it to.)
This seems to be in line with the documentation in `man git
interpret-trailers`:
The group must either be at the end of the message or be the last non-whitespace lines before a line that starts with --- (followed by a space or the end of the line). Such three minus signs start the patch part of the message. See also --no-divider below.
Note “by a space or the end of the line”. This check used to be simpler: before it only checked for a line that started with `---`, no matter what came after on that line. But that was changed to match on `---` followed by `isspace(v*)` in c188668e38 (interpret-trailers: tighten check for "---" patch boundary, 2018-08-22). Reading the commit message it seems that the change was conservative. Maybe it would have been more strict (like demanding only lines like either `---\n` or `---\n `) if there weren’t concerns about how the behavior had been documented to match loosely up until that point. (+Cc the commit author) -- Kristoffer Haugsbakk