Re: [PATCH] cat-file: fix mailmap application for different author and committer
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 11, 2025, 15:53 UTC
- Message-ID
- <xmqqy0tyi8aj.fsf@gitster.g>
- In-Reply-To
- <20250611062643.8639-1-siddharthasthana31@gmail.com>
siddharthasthana31@gmail.com writes:
Show 10 quoted lines
> From: Siddharth Asthana <siddharthasthana31@gmail.com> > > The git cat-file command with --mailmap option fails to apply mailmap > transformations to the committer field when the author and committer > identities are different. This occurs due to a missing newline handling > in apply_mailmap_to_header() after processing each identity line. > ... > This ensures that all identity headers in commit and tag objects are > consistently processed regardless of whether the author and committer > are the same person.
Nicely described.
While the above explains what is wrong in the current code, it does not tell if that was buggy from the beginning or we unintentionally broke it. It seems that this logic came from e9c1b0e3 (revision: improve commit_rewrite_person(), 2022-07-19) when a much simpler version of commit_rewrite_person() that worked on one "person header" at a time (as there are only two, author and committer, anyway) was rewritten to use this function, and it was broken during the rewrite?
Show 21 quoted lines
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh > index 4a6242ff99..98dd0ae12f 100755 > --- a/t/t4203-mailmap.sh > +++ b/t/t4203-mailmap.sh > @@ -1133,4 +1133,37 @@ test_expect_success 'git cat-file --batch-command returns correct size with --us > test_cmp expect actual > ' > > +test_expect_success 'git cat-file --mailmap works with different author and committer' ' > + test_when_finished "rm .mailmap" && > + cat >.mailmap <<-\EOF && > + Mailmapped User <mailmapped-user@gitlab.com> C O Mitter <committer@example.com> > + EOF > + git commit --allow-empty -m "different author/committer" \ > + --author="Different Author <different@example.com>" && > + cat >expect <<-\EOF && > + author Different Author <different@example.com> > + committer Mailmapped User <mailmapped-user@gitlab.com> > + EOF > + git cat-file --mailmap commit HEAD >log && > + sed -n "/^author /s/\([^>]*>\).*/\1/p; /^committer /s/\([^>]*>\).*/\1/p" log >actual &&
Perhaps just a matter of taste, but
sed -n -e "/^author /s/>.*/>/p" -e "/^committer /s/>.*/>/p"
may be easier to read and more portable (as some implementation of sed is picky about semicolon concatenated multiple commands).
Show 21 quoted lines
> + test_cmp expect actual > +' > + > +test_expect_success 'git cat-file --mailmap maps both author and committer when both need mapping' ' > + test_when_finished "rm .mailmap" && > + cat >.mailmap <<-\EOF && > + Mapped Author <mapped-author@example.com> <different@example.com> > + Mapped Committer <mapped-committer@example.com> C O Mitter <committer@example.com> > + EOF > + git commit --allow-empty -m "both author and committer mapped" \ > + --author="Different Author <different@example.com>" && > + cat >expect <<-\EOF && > + author Mapped Author <mapped-author@example.com> > + committer Mapped Committer <mapped-committer@example.com> > + EOF > + git cat-file --mailmap commit HEAD >log && > + sed -n "/^author /s/\([^>]*>\).*/\1/p; /^committer /s/\([^>]*>\).*/\1/p" log >actual && > + test_cmp expect actual > +' > + > test_done