RE: Possible bug regarding trailers

From: Linus Arver <linusa@google.com>
Date: Jun 16, 2023, 23:49 UTC
Message-ID: <owlywn0355no.fsf@fine.c.googlers.com>
In-Reply-To: <PH7PR07MB9467AD34C96AEE60D5A4369F8C58A@PH7PR07MB9467.namprd07.prod.outlook.com>

Hello Eric,

"eric.frederich@siemens.com" <eric.frederich@siemens.com> writes:

Show 6 quoted lines

> B)
> Should anything that is retrieved via:
>      `git cat-file commit $SHA | git interpret-trailers --parse`
> also be displayed via:
>     `git log -1  
> --format="%(trailers:key=some-key,valueonly,separator=%x2c) %H %T" $SHA`

> ... why is there a difference?  (Explicit call to interpret-trailers  
> shows the trailer, but the log command does not).

For reference (using your repro script), I get:

     $ git cat-file commit HEAD~ | sed -e '1,/^$/d'
     A bad commit

     old-scm-change-id: THIS TRAILER GETS LOST

     --- let's mess stuff up ---

     Have fun
     $ git cat-file commit HEAD~ | git interpret-trailers --parse
     old-scm-change-id: THIS TRAILER GETS LOST
     $ git cat-file commit HEAD~ | git interpret-trailers --parse  
--no-divider
     $ git log -1  
--format="%(trailers:key=some-key,valueonly,separator=%x2c) %H %T" HEAD~
      a1d5cfede3cbd109a353235a495e66f86e7456e4  
1dd017bdd070190005e5e162d87afebcd61e0fa8

I agree with Peff that interpret-trailers and git-log are both behaving correctly. For

     $ git cat-file commit HEAD~ | git interpret-trailers --parse
     old-scm-change-id: THIS TRAILER GETS LOST

we detect the trailer because interpret-trailers treats the "--- let's mess stuff up ---" line as a git-format-patch-like divider separating the commit message from the patch diff. This is working as intended.

Contrast this with the invocation where we pass in "--no-divider" and interpret-trailers (correctly) treats the "---" as _not_ a divider and so considers the "old-scm-change-id: THIS TRAILER GETS LOST" line as part of the commit message's body text. As there are no trailers at the bottom, it parses no trailers and nothing is printed on the output. This is working as intended.

For
     $ git log -1  
--format="%(trailers:key=some-key,valueonly,separator=%x2c) %H %T" HEAD~
      a1d5cfede3cbd109a353235a495e66f86e7456e4  
1dd017bdd070190005e5e162d87afebcd61e0fa8

the git-log command already knows that it is dealing with input that is only the commit message. And so it behaves the same way as interpret-trailers with the "--no-divider" flag (treating the entire input as a commit message). This is working as intended.

The issue (as Peff pointed out) is the insertion of the trailer by git-commit before the "--- let's mess stuff up ---" line. Instead we should make git-commit insert the

     old-scm-change-id: THIS TRAILER GETS LOST

line _after_ the "Have fun" line. If so, then the calls to interpret-trailers and git-log should behave more intuitively.