git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] send-email: add proper default sender

From
Felipe Contreras <felipe.contreras@gmail.com>
Date
Nov 13, 2012, 06:42 UTC
Message-ID
<CAMP44s1w3oZhEUM-cnO=kECH2bhdOTGVuKy8JS4uhWFbA_oi3w@mail.gmail.com>
In-Reply-To
<20121113040104.GA9361@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 5:01 AM, Jeff King <peff@peff.net> wrote:
Show 10 quoted lines
> On Tue, Nov 13, 2012 at 04:55:25AM +0100, Felipe Contreras wrote:
>
>> > No, it's not. Those broken names do not come from the environment, but
>> > from our last-resort guess of the hostname.
>>
>> That depends how you define environment, but fine, the point is that
>> it happens.
>
> If you have a strawman definition that does not have anything to do with
> what I said in my original email, then yes, it could happen.
It happens, I've seen commits with (none) not that long ago.
> But as I
> said already, "git var" uses IDENT_STRICT and will not allow such broken
> names.

Since 1.7.11, sure. But not everyone is using such a recent version of git, and people with fully qualified domains would still get unwanted behavior.

Show 10 quoted lines
>> > We long ago switched to
>> > printing the name as a warning when we have made such a guess (bb1ae3f),
>> > then more recently started rejecting them outright (8c5b1ae).
>>
>> Right, but these would still happen:
>>
>> michael <michael@michael-laptop.michael.org>
>
> Did you read my email? I explicitly proposed that we would _not_ allow
> send-email to use implicit email addresses constructed in that way.

I'm not talking about git send-email, I'm talking about your comment 'it has always been the case that you can use git without setting user.*', which has caused issues with wrong author/commmitter names in commits, and will probably continue to do so.

Show 10 quoted lines
>> > But in the meantime you are causing a regression for anybody who expects
>> > GIT_AUTHOR_NAME to override user.email when running git-send-email (and
>> > you have taken away the prompt that they could have used to notice and
>> > correct it).
>>
>> I think they can survive. If anybody like this exists.
>
> Sorry, but that is not how things work on this project. You do not get
> to cause regressions because you are too lazy to implement the feature
> _you_ want in a way that does not break other people.

That doesn't change the fact that they would survive, and the fact that those users don't actually exist.

But let's look at the current situation closely:
PERL5LIB=~/dev/git/perl ./git-send-email.perl --confirm=always -1
1) No information at all
fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
2) Full Name + full hostname

Who should the emails appear to be from? [Felipe Contreras <felipec@nysa.felipec.org>]

That's right, ident doesn't fail, and that's not the mail address I specified, it's *implicit*.

3) Full Name + EMAIL

Who should the emails appear to be from? [Felipe Contreras <felipe.contreras@gmail.com>]

4) config user

Who should the emails appear to be from? [Felipe Contreras 2nd <felipe.contreras+2@gmail.com>]

5) GIT_COMMITTER

Who should the emails appear to be from? [Felipe Contreras 2nd <felipe.contreras+2@gmail.com>]

Whoa, what happened there?
Well:
  $sender = $repoauthor || $repocommitter || '';
  ($repoauthor) = Git::ident_person(@repo, 'author');
  % ./git var GIT_AUTHOR_IDENT
  Felipe Contreras 2nd <felipe.contreras+2@gmail.com> 1352783223 +0100
That's right, AUTHOR_IDENT would fall back to the default email and full name.
Hmm, I wonder...
5.1) GIT_COMMITER without anything else

fatal: empty ident name (for <felipec@nysa.(none)>) not allowed var GIT_AUTHOR_IDENT: command returned error: 128

Why? Because:

% PERL5LIB=~/dev/git/perl perl -e 'use Git; printf("%s\n", Git::ident_person(@repo, 'author'));' fatal: empty ident name (for <felipec@nysa.(none)>) not allowed

($repoauthor) = Git::ident_person(@repo, 'author'); ($repocommitter) = Git::ident_person(@repo, 'committer');

So $repoauthor || $repocommiter is pointless.
6) GIT_AUTHOR

Who should the emails appear to be from? [Felipe Contreras 4th <felipe.contreras+4@gmail.com>]

What about after my change?
6.1) GIT_AUTHOR without anything else

fatal: empty ident name (for <felipec@nysa.(none)>) not allowed var GIT_COMMITTER_IDENT: command returned error: 128

4) config user
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
5) GIT_COMMITTER
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
6) GIT_AUTHOR
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
And what about your proposed change?
2) Full Name + full hostname

./git var GIT_EXPLICIT_IDENT 0

6.1) GIT_AUTHOR without anything else
Even if the previous problem was solved:

export GIT_AUTHOR_NAME='Felipe Contreras 4th'; export GIT_AUTHOR_EMAIL='felipe.contreras+4@gmail.com' ./git var GIT_EXPLICIT_IDENT 0

No explicit ident? This is most certainly not what the user would expect.
And then:
5.2) GIT_COMMITTER with Full Name and full hostname

export GIT_COMMITTER_NAME='Felipe Contreras 3nd'; export GIT_COMMITTER_EMAIL='felipe.contreras+3@gmail.com' ./git var GIT_EXPLICIT_IDENT 1

From: Felipe Contreras <felipec@nysa.felipec.org>

It is explicit, yeah, but 'git send-email' would not be picking the committer, it would pick the author.

> I tried to help you by pointing you in the right direction and even
> providing a sample "git var" patch.
Are you 100% sure that was the right direction?
I think the right approach is more along these lines:
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,16 +748,11 @@ if (!$force) {
        }
 }

-my $prompting = 0;
 if (!defined $sender) {
        $sender = $repoauthor || $repocommitter || '';
-       $sender = ask("Who should the emails appear to be from? [$sender] ",
-                     default => $sender,
-                     valid_re => qr/\@.*\./, confirm_only => 1);
-       print "Emails will be sent from: ", $sender, "\n";
-       $prompting++;
 }

+my $prompting = 0;
 if (!@initial_to && !defined $to_cmd) {
        my $to = ask("Who should the emails be sent to (if any)? ",
                     default => "",
diff --git a/ident.c b/ident.c
index a4bf206..c73ba82 100644
--- a/ident.c
+++ b/ident.c
@@ -291,9 +291,9 @@ const char *fmt_ident(const char *name, const char *email,
        }

        if (strict && email == git_default_email.buf &&
-           strstr(email, "(none)")) {
+               !(user_ident_explicitly_given & IDENT_MAIL_GIVEN)) {
                fputs(env_hint, stderr);
-               die("unable to auto-detect email address (got '%s')", email);
+               die("no explicit email address");
        }

        if (want_date) {

With that we get:

2) Full Name + full hostname

fatal: no explicit email address

3) Full Name + EMAIL

From: Felipe Contreras <felipe.contreras@gmail.com>

4) config user

From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>

5) GIT_COMMITTER

From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>

(as buggy as before)

6) GIT_AUTHOR

From: Felipe Contreras 4th <felipe.contreras+4@gmail.com>

Not only will this fix 'git send-email', but it will also fix 'git
commit' so that we don't end up with authors such as 'Felipe Contreras
<felipec@nysa.felipec.org>' ever again.

> But it is not my itch to scratch.

Suit yourself, it's only git users that would get hurt. I can always
use my own 'git send-email' (as I am doing right now).

Cheers.

-- 
Felipe Contreras
Previous: Jeff KingNext: Felipe Contreras
Message 14 of 76 in “send-email: add proper default sender”
  1. send-email: add proper default senderFelipe Contreras, Nov 11, 2012
  2. Ramkumar RamachandraNov 11, 2012
  3. Felipe ContrerasNov 11, 2012
  4. Jeff KingNov 12, 2012
  5. Felipe ContrerasNov 12, 2012
  6. Jeff KingNov 13, 2012
  7. Jeff KingNov 13, 2012
  8. Junio C HamanoNov 13, 2012
  9. Felipe ContrerasNov 13, 2012
  10. Jeff KingNov 13, 2012
  11. Jeff KingNov 13, 2012
  12. Felipe ContrerasNov 13, 2012
  13. Jeff KingNov 13, 2012
  14. Felipe ContrerasNov 13, 2012
  15. Felipe ContrerasNov 13, 2012
  16. Jeff KingNov 13, 2012
  17. Felipe ContrerasNov 13, 2012
  18. Jeff KingNov 13, 2012
  19. 1/6 ident: make user_ident_explicitly_given privateJeff King, Nov 13, 2012
  20. Jonathan NiederNov 14, 2012
  21. Jeff KingNov 14, 2012
  22. 2/6 ident: keep separate "explicit" flags for author and committerJeff King, Nov 13, 2012
  23. 3/6 var: accept multiple variables on the command lineJeff King, Nov 13, 2012
  24. Jonathan NiederNov 14, 2012
  25. Jeff KingNov 14, 2012
  26. 4/6 var: provide explicit/implicit ident informationJeff King, Nov 13, 2012
  27. Jonathan NiederNov 14, 2012
  28. Jeff KingNov 14, 2012
  29. 5/6 Git.pm: teach "ident" to query explicitnessJeff King, Nov 13, 2012
  30. Jonathan NiederNov 14, 2012
  31. Jeff KingNov 14, 2012
  32. 6/6 send-email: do not prompt for explicit repo identJeff King, Nov 13, 2012
  33. Jonathan NiederNov 14, 2012
  34. Jeff KingNov 14, 2012
  35. Jeff KingNov 14, 2012
  36. Felipe ContrerasNov 13, 2012
  37. Jeff KingNov 15, 2012
  38. Felipe ContrerasNov 15, 2012
  39. Jeff KingNov 15, 2012
  40. Felipe ContrerasNov 15, 2012
  41. 0/8 loosening "sender" prompt in send-emailJeff King, Nov 15, 2012
  42. 1/8 test-lib: allow negation of prerequisitesJeff King, Nov 15, 2012
  43. Jonathan NiederNov 15, 2012
  44. Jeff KingNov 15, 2012
  45. Jonathan NiederNov 15, 2012
  46. 2/8 t7502: factor out autoident prerequisiteJeff King, Nov 15, 2012
  47. Jonathan NiederNov 15, 2012
  48. 3/8 ident: make user_ident_explicitly_given staticJeff King, Nov 15, 2012
  49. Jonathan NiederNov 15, 2012
  50. 4/8 ident: keep separate "explicit" flags for author and committerJeff King, Nov 15, 2012
  51. Jonathan NiederNov 15, 2012
  52. 5/8 var: accept multiple variables on the command lineJeff King, Nov 15, 2012
  53. Jonathan NiederNov 15, 2012
  54. 6/8 var: provide explicit/implicit ident informationJeff King, Nov 15, 2012
  55. 7/8 Git.pm: teach "ident" to query explicitnessJeff King, Nov 15, 2012
  56. Jonathan NiederNov 15, 2012
  57. 8/8 send-email: do not prompt for explicit repo identJeff King, Nov 15, 2012
  58. Felipe ContrerasNov 15, 2012
  59. Jeff KingNov 15, 2012
  60. Felipe ContrerasNov 15, 2012
  61. Jeff KingNov 15, 2012
  62. Jeff KingNov 15, 2012
  63. Jeff KingNov 15, 2012
  64. Junio C HamanoNov 15, 2012
  65. Jeff KingNov 15, 2012
  66. Junio C HamanoNov 16, 2012
  67. Jeff KingNov 16, 2012
  68. Felipe ContrerasNov 16, 2012
  69. Jeff KingNov 16, 2012
  70. Junio C HamanoNov 16, 2012
  71. Jonathan NiederNov 15, 2012
  72. Junio C HamanoNov 13, 2012
  73. Jeff KingNov 13, 2012
  74. Junio C HamanoNov 13, 2012
  75. Erik Faye-LundNov 13, 2012
  76. Jeff KingNov 13, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.