On 2024.08.19 17:07, Jacob Keller wrote:
> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>
> git check-mailmap splits each provided contact using split_ident_line.
> This function requires that the contact either be of the form "Name
> <user@host>" or of the form "<user@host>". In particular, if the mail
> portion of the contact is not surrounded by angle brackets,
> split_ident_line will reject it.
>
> This results in git check-mailmap rejecting attempts to translate simple
> email addresses:
>
> $ git check-mailmap user@host
> fatal: unable to parse contact: user@host
>
> This limits the usability of check-mailmap as it requires placing angle
> brackets around plain email addresses.
>
> In particular, attempting to use git check-mailmap to support mapping
> addresses in git send-email is not straight forward. The sanitization
> and validation functions in git send-email strip angle brackets from
> plain email addresses. It is not trivial to add brackets prior to
> invoking git check-mailmap.
>
> Instead, modify check_mailmap() to allow such strings as contacts. In
> particular, treat any line which cannot be split by split_ident_line as
> a simple email address.
>
> No attempt is made to actually parse the address line to validate that
> it is actually an address. Doing so is non-trivial, and provides little
> value. Either the provided input will correctly map via the map_user
> call, or it will fail and be printed out, surrounded by angle brackets:
>
> $ git check-mailmap user@host
> <user@host>
>
> Signed-off-by: Jacob Keller <jacob.keller@xxxxxxxxx>
> ---
> builtin/check-mailmap.c | 18 +++++++++++-------
> Documentation/git-check-mailmap.txt | 8 ++++----
> t/t4203-mailmap.sh | 33 +++++++++++++++++++++++++++++++--
> 3 files changed, 46 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c
> index b8a05b8e07b5..6b7fb53494f0 100644
> --- a/builtin/check-mailmap.c
> +++ b/builtin/check-mailmap.c
> @@ -25,13 +25,17 @@ static void check_mailmap(struct string_list *mailmap, const char *contact)
> size_t namelen, maillen;
> struct ident_split ident;
>
> - if (split_ident_line(&ident, contact, strlen(contact)))
> - die(_("unable to parse contact: %s"), contact);
> -
> - name = ident.name_begin;
> - namelen = ident.name_end - ident.name_begin;
> - mail = ident.mail_begin;
> - maillen = ident.mail_end - ident.mail_begin;
> + if (!split_ident_line(&ident, contact, strlen(contact))) {
> + name = ident.name_begin;
> + namelen = ident.name_end - ident.name_begin;
> + mail = ident.mail_begin;
> + maillen = ident.mail_end - ident.mail_begin;
> + } else {
> + name = NULL;
> + namelen = 0;
> + mail = contact;
> + maillen = strlen(contact);
> + }
>
> map_user(mailmap, &mail, &maillen, &name, &namelen);
>
> diff --git a/Documentation/git-check-mailmap.txt b/Documentation/git-check-mailmap.txt
> index 02f441832321..7747e38e25e3 100644
> --- a/Documentation/git-check-mailmap.txt
> +++ b/Documentation/git-check-mailmap.txt
> @@ -15,10 +15,10 @@ SYNOPSIS
> DESCRIPTION
> -----------
>
> -For each ``Name $$<user@host>$$'' or ``$$<user@host>$$'' from the command-line
> -or standard input (when using `--stdin`), look up the person's canonical name
> -and email address (see "Mapping Authors" below). If found, print them;
> -otherwise print the input as-is.
> +For each ``Name $$<user@host>$$'', ``$$<user@host>$$'', or ``$$user@host$$''
> +from the command-line or standard input (when using `--stdin`), look up the
> +person's canonical name and email address (see "Mapping Authors" below). If
> +found, print them; otherwise print the input as-is.
>
>
> OPTIONS
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
> index 79e5f42760d9..0c1efe0b2e17 100755
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -73,11 +73,40 @@ test_expect_success 'check-mailmap --stdin arguments: mapping' '
> '
>
> test_expect_success 'check-mailmap bogus contact' '
> - test_must_fail git check-mailmap bogus
> + cat >expect <<-EOF &&
> + <bogus>
> + EOF
> + git check-mailmap bogus >actual &&
> + test_cmp expect actual
> '
I think I'd just remove this test case altogether, IIUC it' doesn't
provide any additional value vs. the "check-mailmap simple address: no
mapping" test below.
> test_expect_success 'check-mailmap bogus contact --stdin' '
> - test_must_fail git check-mailmap --stdin bogus </dev/null
> + cat >expect <<-EOF &&
> + <bogus>
> + EOF
> + cat >stdin <<-EOF &&
> + bogus
> + EOF
> + git check-mailmap --stdin <stdin >actual &&
> + test_cmp expect actual
> +'
Similarly, I might change this to use a real address instead of "bogus",
as we're no longer checking for invalid input.
> +test_expect_success 'check-mailmap simple address: mapping' '
> + test_when_finished "rm .mailmap" &&
> + cat >.mailmap <<-EOF &&
> + New Name <$GIT_AUTHOR_EMAIL>
> + EOF
> + cat .mailmap >expect &&
> + git check-mailmap "$GIT_AUTHOR_EMAIL" >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'check-mailmap simple address: no mapping' '
> + cat >expect <<-EOF &&
> + <bugs@xxxxxxxxxx>
> + EOF
> + git check-mailmap "bugs@xxxxxxxxxx" >actual &&
> + test_cmp expect actual
> '
>
> test_expect_success 'No mailmap' '
>
> --
> 2.46.0.124.g2dc1a81c8933
>
