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

[Jeff King]

On Tue, Nov 13, 2012 at 09:35:18PM +0100, Felipe Contreras wrote:

  Yes, dying would be a regression, in that you would have to configure
  your name via the environment and re-run rather than type it at the
  prompt. You raise a good point that for people who _could_ take the
  implicit default, hitting enter is working fine now, and we would lose
  that.  I'd be fine with also just continuing to prompt in the implicit
  case.
 
  But that is a much smaller issue to me than having send-email fail to
  respect environment variables and silently use user.*, which is what
  started this whole discussion. And I agree it is worth considering as a
  regression we should avoid.
 
 It might be smaller, I don't think so. A hypothetical user that was
 relying on GIT_AUTHOR for whatever reason can switch to 'git
 send-email --from' (which is much easier) when they notice the
 failure, the same way somebody relying on fqdn would. The difference
 is that people with fqdn do exist, and they might be relying on this.
 
 Both are small issues, that I agree with.
 
 But the point is that you seem to be very adamant about _my_
 regressions, and not pay much attention about yours.

Really? I mentioned initially the possibility of dying instead of
prompting. You raised the point that it would regress a certain use
case. And then what happened? I said above you raise a good point[...].
I'd be fine with also just continuing to prompt[...]. I agree it is
worth considering as a regression we should avoid. And then I sent out
a patch series which does not have the regression.

In other words, my suggestion was a bad one, and once it was pointed
out, I did not pursue it.  If you want to call that not paying much
attention, feel free. But I'd much rather you point out problems in my
actual patch series.

 Why do you hold on to my first patch?

Because clearly I am confused by your behavior. You continued to argue
that the initial regression was not worth worrying about. Which I
thought meant you were still interested in pursuing that patch. But if
you are not, then there is not even any point in discussing it.  Which
is just fine with me, as that discussion seemed to be going nowhere.

 The second patch doesn't have this issue. It does change the behavior
 of 'git commit', yeah, but I think that's a benefit.

Changing git commit is even something I would entertain. It would be a
regression for some people, but at least it buys us something (increased
safety against people making bogus commits and failing to notice the
warning). I'm undecided on whether that is worth it or not.

But when you presented it, as far as I could tell the change in behavior
to git commit was accidental (which is why I pointed it out in
response). And as it was in the middle of a discussion about whether
regressions matter, it was not clear to me that your argument was I
have thought this through and the inconvenience is outweighted by the
benefit and not I did not bother to consider the implications for
users who are currently using this feature.

If you want to seriously propose changing the behavior of git commit,
I think the best thing would be to make a real patch, laying out the
pros and cons in the commit message, and post it. I would not be
surprised if the other list participants have stopped reading our thread
at this point, and the idea is going otherwise unnoticed.

  So you can either:
 
1. Reimplement the environment variable lookup that ident.c does,
   leaving implicit ident logic out completely.
 
2. Modify ident.c and git var to let send-email reuse the logic in
   ident.c, but avoid dropping the prompt when an implicit ident is
   used.
 
  Doing (2) sounds a lot more maintainable to me in the long run.
 
 Or:
 
 3. Change the meaning of the STRICT flag so that the values are
 explicit, which has benefits outside 'git send-email'. Yes, this would
 change the behavior in 'git commit' and other tools, but it's worth to
 investigate these changes, and most likely they would be desirable.

Yes, I think that is fine _if_ we want to change git-commit. Which is
not clear to me. If we don't, then it is not an option.

 Or:
 
 4. Just stop prompting
 
 I already sent a patch for 4. with all the details of why nobody (or
 very few, if any) would be affected negatively.

If doing (2) were really hard, that might be worth considering. But it's
not. I already did it. So I don't see how this is an attractive option,
unless my series is so unpalatable that we would rather accept a
regression.

[1/6]: ident: make user_ident_explicitly_given private
[2/6]: ident: keep separate explicit flags for author and committer
[3/6]: var: accept multiple variables on the command line
[4/6]: var: provide explicit/implicit ident information
[5/6]: Git.pm: teach ident to query explicitness
[6/6]: send-email: do not prompt for explicit repo ident
 
 I think this adds a lot of code that nobody would use.

A lot of code? It is 

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

[Felipe Contreras]

On Thu, Nov 15, 2012 at 1:07 AM, Jeff King p...@peff.net wrote:
 On Tue, Nov 13, 2012 at 09:35:18PM +0100, Felipe Contreras wrote:

  Yes, dying would be a regression, in that you would have to configure
  your name via the environment and re-run rather than type it at the
  prompt. You raise a good point that for people who _could_ take the
  implicit default, hitting enter is working fine now, and we would lose
  that.  I'd be fine with also just continuing to prompt in the implicit
  case.
 
  But that is a much smaller issue to me than having send-email fail to
  respect environment variables and silently use user.*, which is what
  started this whole discussion. And I agree it is worth considering as a
  regression we should avoid.

 It might be smaller, I don't think so. A hypothetical user that was
 relying on GIT_AUTHOR for whatever reason can switch to 'git
 send-email --from' (which is much easier) when they notice the
 failure, the same way somebody relying on fqdn would. The difference
 is that people with fqdn do exist, and they might be relying on this.

 Both are small issues, that I agree with.

 But the point is that you seem to be very adamant about _my_
 regressions, and not pay much attention about yours.

 Really? I mentioned initially the possibility of dying instead of
 prompting. You raised the point that it would regress a certain use
 case. And then what happened? I said above you raise a good point[...].
 I'd be fine with also just continuing to prompt[...]. I agree it is
 worth considering as a regression we should avoid. And then I sent out
 a patch series which does not have the regression.

 In other words, my suggestion was a bad one, and once it was pointed
 out, I did not pursue it.  If you want to call that not paying much
 attention, feel free. But I'd much rather you point out problems in my
 actual patch series.

But that I meant that when I introduce a regression it's like I'm
killing all that is good and sacred about git, and when you do it's
everything but that.

Yes, you sent a new patch. So did I.

 The second patch doesn't have this issue. It does change the behavior
 of 'git commit', yeah, but I think that's a benefit.

 Changing git commit is even something I would entertain. It would be a
 regression for some people, but at least it buys us something (increased
 safety against people making bogus commits and failing to notice the
 warning). I'm undecided on whether that is worth it or not.

 But when you presented it, as far as I could tell the change in behavior
 to git commit was accidental (which is why I pointed it out in
 response).

How could it be accidental if I said this: Not only will this fix
'git send-email', but it will also fix 'git
commit'.

 And as it was in the middle of a discussion about whether
 regressions matter,

That was not the discussion at all.

You can't say that all regressions are the same, and if I say
regression X doesn't matter, that means ALL regressions don't
matter. That's a hasty generalization.

 If you want to seriously propose changing the behavior of git commit,
 I think the best thing would be to make a real patch, laying out the
 pros and cons in the commit message, and post it. I would not be
 surprised if the other list participants have stopped reading our thread
 at this point, and the idea is going otherwise unnoticed.

I would, if I saw any chance in it actually going through.

 Or:

 4. Just stop prompting

 I already sent a patch for 4. with all the details of why nobody (or
 very few, if any) would be affected negatively.

 If doing (2) were really hard, that might be worth considering. But it's
 not. I already did it. So I don't see how this is an attractive option,
 unless my series is so unpalatable that we would rather accept a
 regression.

A matter of opinion. I think that series introduces way too much code
for a very very small gain that eventually would probably disappear.

[1/6]: ident: make user_ident_explicitly_given private
[2/6]: ident: keep separate explicit flags for author and committer
[3/6]: var: accept multiple variables on the command line
[4/6]: var: provide explicit/implicit ident information
[5/6]: Git.pm: teach ident to query explicitness
[6/6]: send-email: do not prompt for explicit repo ident

 I think this adds a lot of code that nobody would use.

 A lot of code? It is mostly refactoring,

Patch #1 and #3 are refactoring, the rest are not.

 which IMHO makes the resulting
 code cleaner, and it increases the utility of git var, and our test
 coverage. If you have review comments, then by all means, respond to the
 series.

I don't have any comments, except  that I don't think all that code is
needed. And why would I bother commenting there, if my opinion will be
ignored?

  I do not necessarily agree on git commit. Moreover, I feel like it is
  a separate issue. My series above _just_ implements the do not prompt
  when explicit behavior. It does not deal 

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

[Jeff King]

On Thu, Nov 15, 2012 at 02:41:50AM +0100, Felipe Contreras wrote:

 But that I meant that when I introduce a regression it's like I'm
 killing all that is good and sacred about git, and when you do it's
 everything but that.

The rhetoric in this statement is a good indication that there is
nothing productive to come from our discussing it anymore.

  If you want to seriously propose changing the behavior of git commit,
  I think the best thing would be to make a real patch, laying out the
  pros and cons in the commit message, and post it. I would not be
  surprised if the other list participants have stopped reading our thread
  at this point, and the idea is going otherwise unnoticed.
 
 I would, if I saw any chance in it actually going through.

Well, it certainly will not go through if you do not try.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Contreras]

On Thu, Nov 15, 2012 at 2:50 AM, Jeff King p...@peff.net wrote:
 On Thu, Nov 15, 2012 at 02:41:50AM +0100, Felipe Contreras wrote:

 But that I meant that when I introduce a regression it's like I'm
 killing all that is good and sacred about git, and when you do it's
 everything but that.

 The rhetoric in this statement is a good indication that there is
 nothing productive to come from our discussing it anymore.

The point is still true.

  If you want to seriously propose changing the behavior of git commit,
  I think the best thing would be to make a real patch, laying out the
  pros and cons in the commit message, and post it. I would not be
  surprised if the other list participants have stopped reading our thread
  at this point, and the idea is going otherwise unnoticed.

 I would, if I saw any chance in it actually going through.

 Well, it certainly will not go through if you do not try.

At least I wouldn't be wasting my time.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Contreras]

On Tue, Nov 13, 2012 at 8:47 AM, Jeff King p...@peff.net wrote:

 But I still don't see how that has anything to do with what send-email
 does or should do. That is why I said strawman above. You seem to
 think I am saying that send-email should use the system that generated
 those broken names, when I am saying the opposite.

No, I'm saying none should use that system, and that in fact 'git
commit' should be stricter... both should be stricter.

 Those people would also not be using a new version of git-send-email,
 and it will always prompt. I thought we were talking about what
 send-email should do in future versions. Namely, loosening that safety
 valve (the prompt) because it is inconvenient, but tightening the checks
 so that losing the safety valve is not a problem.

Yeah, but all I'm saying is that the issue happens, you seemed to
suggest that it doesn't.

 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.

 The second half of that sentence that you quoted above is ...instead
 only using the environment. As in, environment variables like
 GIT_AUTHOR_EMAIL, GIT_COMMITTER_EMAIL, and EMAIL. _Not_ implicit
 generation of the email from the username and hostname.

That sentence was *not* about 'git send-email', it was about git in
general, and 'git commit' is perfectly happy with implicit generation
of the email from the username and hostname.

I don't thin 'git commit' should do that, and I don't think 'git
send-email' should do that. I'm criticizing the whole approach.

 I am tempted to fault myself for not communicating well, but I feel like
 I have made that point at least 3 times in this conversation now. Is
 that the source of the confusion?

I think you are the one that is not understanding what I'm saying. But
I don't think it matters.

This is what I'm saying; the current situation with 'git commit' is
not OK, _both_ 'git commit' and 'git send-email' should change.

 And you will survive if upstream git (whether it is me today or Junio
 tomorrow) does not pick up your patch.

Indeed I would, but there's other people that would benefit from this
patch. I'm sure I'm not the only person that doesn't have
sendmail.from configured, but does have user.name/user.email, and is
constantly typing enter.

And the difference is that I'm _real_, the hypothetical user that
sends patches with GIT_AUTHOR_NAME/EMAIL is not. I would be convinced
otherwise if some evidence was presented that such a user is real
though.

 I remember writing you a long
 email recently about how one of the responsibilities of the maintainer
 is to balance features versus regressions. I'll not bother repeating
 myself here.

And to balance you need to *measure*, and that means taking into
consideration who actually uses the features, if there's any. And it
looks to me this is a feature nobody uses.

But listen closely to what you said:

 I actually think it would make more sense to drop the prompt entirely and 
 just die when the user has not given us a usable ident.

Suppose somebody has a full name, and a fully qualified domain name,
and he can receive mails to it directly. Such a user would not need a
git configuration, and would not need $EMAIL, or anything.

Currently 'git send-email' will throw 'Felipe Contreras
feli...@felipec.org' which would actually work, but is not explicit.

You are suggesting to break that use-case. You are introducing a
regression. And this case is realistic, unlike the
GIT_AUTHOR_NAME/EMAIL. Isn't it?

I prefer to concentrate on real issues, but that's just me.

 As for whether they exist, what data do you have?

What data do _you_ have?

When there's no evidence either way, the rational response is to don't
believe. That's the default position.

 Are you aware that the
 test suite, for example, relies on setting GIT_AUTHOR_NAME but not
 having any user.* config?

What tests?  My patch doesn't seem to break anything there:
% make -C t t9001-send-email.sh
# passed all 96 test(s)

 When somebody comes on the list and asks why
 every git program in the entire system respects GIT_* environment
 variables as an override to user.* configuration _except_ for
 send-email, what should I say?

The same thing you say when somebody comes reporting a bug: yeah, we
should probably fix that.

But that's not going to happen. And in the unlikely event that it
does, it's not going to be a major issue.

It's all about proportion. Is it possible that we all are going to die
tomorrow because of an asteroid? Sure... but what's the point of
worrying about it if it's not likely?

 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

 That is dependent on your system. If 

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

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Tue, Nov 13, 2012 at 07:42:58AM +0100, Felipe Contreras wrote:
 ...
 5) GIT_COMMITTER
 
 Who should the emails appear to be from? [Felipe Contreras 2nd
 felipe.contrera...@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.contrera...@gmail.com 1352783223 +0100
 
 That's right, AUTHOR_IDENT would fall back to the default email and full 
 name.

 Yeah, I find that somewhat questionable in the current behavior, and I'd
 consider it a bug. Typically we prefer the committer ident when given a
 choice (e.g., for writing reflog entries).

Just to make sure I follow the discussion correctly, do you mean
that the bug is that we pick a fuzzy AUTHOR when COMMITTER is
specified more concretely and we usually use COMMIITTER for this
kind of thing in the first place but send-email does not in this
case (I do not see git var GIT_AUTHOR_IDENT returning value from
the implicit logic as a bug in this case---just making sure).

 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

 Doesn't that seem like a regression? It used to work.

I do not think we mind a change to favor GIT_COMMITTER setting over
GIT_AUTHOR to be consistent with others too much, but that indeed is
a big change.  People who are used to the inconsistency that
send-email favors AUTHOR over COMMITTER will certainly find it as a
regression.

 The explicitness needs to be tied to the specific ident we grabbed.
 Probably adding a git var GIT_AUTHOR_EXPLICIT would be enough, or
 alternatively, adding a flag to git var to error out rather than
 return a non-explicit ident (this may need to adjust the error
 handling of the git var calls from send-email).

Yeah, something like that would be necessary for git var users to
make this kind of decision.

 While simultaneously breaking git commit for people who are happily
 using the implicit generation. I can see the appeal of doing so; I was
 tempted to suggest it when I cleaned up IDENT_STRICT a few months back.
 But do we have any data on how many people are currently using that
 feature that would be annoyed by it?

As we didn't break git commit when you worked on IDENT_STRICT, I
do not think we have any data on it ;-)

 ...
 It may be something I would work on myself in the future, but I have
 other things to work on at the moment, and since you are interested in
 the topic, I thought you would be a good candidate to polish it enough
 to be suitable upstream. But instead I see a lot of push-back on what I
 considered to be a fairly straightforward technical comment on a
 regression.
 ...
 But I feel like I am fighting an uphill battle just to convince you that
 regressions are bad, and that I am having to make the same points
 repeatedly.  That makes me frustrated and less excited about reviewing
 your patches; and when I say it is not my itch, that is my most polite
 way of saying If that is going to be your attitude, then I do not feel
 like dealing with you anymore on this topic.

For a change with low benefit/cost ratio like this, the best course
of action often is to stop paying attention to the thread that has
become unproductive and venomous, and resurrect the topic after
people forgot about it and doing it right yourself ;-).

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jeff King]

On Tue, Nov 13, 2012 at 10:06:26AM +0100, Felipe Contreras wrote:

  Those people would also not be using a new version of git-send-email,
  and it will always prompt. I thought we were talking about what
  send-email should do in future versions. Namely, loosening that safety
  valve (the prompt) because it is inconvenient, but tightening the checks
  so that losing the safety valve is not a problem.
 
 Yeah, but all I'm saying is that the issue happens, you seemed to
 suggest that it doesn't.

What is it?

If it is bad guesses like user@host.(none) on unconfigured systems,
then I did not suggest it doesn't happen. I said that it happened with
older versions of git, but that it has now been fixed. Of course it will
continue to happen for people on older versions of git until they
upgrade. I cannot go back in time and fix released versions.

If it is guessing at all to end up with user@host.domain for a host
that does not receive mail, yes, that happens, and I have been very
clear that it does. The safety valve is showing the ident and a warning
to the user during the commit process.

I do not see any point in discussing the former when considering future
changes to git. It is already disallowed by current versions of git, and
we are just waiting for the whole world to upgrade to a fixed version.

I can see the argument for tightening the check to disallow the latter.
But it would also hurt real people who are relying on the feature.
Perhaps it would make sense to adopt the implicit_ident_advice in other
code paths besides git commit (e.g., via git var).

 I think you are the one that is not understanding what I'm saying. But
 I don't think it matters.
 
 This is what I'm saying; the current situation with 'git commit' is
 not OK, _both_ 'git commit' and 'git send-email' should change.

Perhaps I am being dense, but this is the first time it became apparent
to me that you are arguing for a change in git commit.

 Indeed I would, but there's other people that would benefit from this
 patch. I'm sure I'm not the only person that doesn't have
 sendmail.from configured, but does have user.name/user.email, and is
 constantly typing enter.
 
 And the difference is that I'm _real_, the hypothetical user that
 sends patches with GIT_AUTHOR_NAME/EMAIL is not. I would be convinced
 otherwise if some evidence was presented that such a user is real
 though.

Sadly we cannot poll the configuration of every user, nor expect them
all to pay attention to this discussion on the list. So we cannot take
the absence of comment from such users as evidence that they do not
exist. Instead we must use our judgement about what is being changed,
and we tend to err on the side of keeping the status quo, since that is
what the silent majority is busy _not_ complaining about.

We use the same judgement on the other side, too. Right now your
evidence is 1 user wants this, 0 users do not. But we can guess that
there are other people who would like the intent of your patch, but did
not care enough or are not active enough on the list to write the patch
themselves or comment on this thread.

It is also very easy to me accept the status quo when it is not in
fundamental conflict with the proposed improvement, but simply a matter
of implementation (which it is the case for send-email stopping
prompting in some cases, though it is not for changing the behavior of
git-commit).

 And to balance you need to *measure*, and that means taking into
 consideration who actually uses the features, if there's any. And it
 looks to me this is a feature nobody uses.

You said measure and then it looks to me like. What did you measure?
Did you consider systematic bias in your measurement, like the fact that
people who are using the feature have no reason to come on the list and
announce it?

 But listen closely to what you said:
 
  I actually think it would make more sense to drop the prompt entirely and 
  just die when the user has not given us a usable ident.
 
 Suppose somebody has a full name, and a fully qualified domain name,
 and he can receive mails to it directly. Such a user would not need a
 git configuration, and would not need $EMAIL, or anything.
 
 Currently 'git send-email' will throw 'Felipe Contreras
 feli...@felipec.org' which would actually work, but is not explicit.
 
 You are suggesting to break that use-case. You are introducing a
 regression. And this case is realistic, unlike the
 GIT_AUTHOR_NAME/EMAIL. Isn't it?

Yes, dying would be a regression, in that you would have to configure
your name via the environment and re-run rather than type it at the
prompt. You raise a good point that for people who _could_ take the
implicit default, hitting enter is working fine now, and we would lose
that.  I'd be fine with also just continuing to prompt in the implicit
case.

But that is a much smaller issue to me than having send-email fail to
respect environment variables and silently use user.*, which is what
started this whole 

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

[Jeff King]

On Tue, Nov 13, 2012 at 08:13:04AM -0800, Junio C Hamano wrote:

  That's right, AUTHOR_IDENT would fall back to the default email and full 
  name.
 
  Yeah, I find that somewhat questionable in the current behavior, and I'd
  consider it a bug. Typically we prefer the committer ident when given a
  choice (e.g., for writing reflog entries).
 
 Just to make sure I follow the discussion correctly, do you mean
 that the bug is that we pick a fuzzy AUTHOR when COMMITTER is
 specified more concretely and we usually use COMMIITTER for this
 kind of thing in the first place but send-email does not in this
 case (I do not see git var GIT_AUTHOR_IDENT returning value from
 the implicit logic as a bug in this case---just making sure).

Having discussed more, I think there are two questionable things:

  1. Preferring author over committer

  2. Failing to fall back to committer when author is implicit or bogus
 (because git var dies).

I think (1) may fall into the this is not how I would do it today, but
it is not worth a possible regression category.

I think (2) might be worth fixing, though. Certainly when the author is
bogus (by IDENT_STRICT rules), which I think was the original intent of
the $repoauthor || $repocommitter code. Probably when the author ident
is implicit, though that is more hazy to me.

 For a change with low benefit/cost ratio like this, the best course
 of action often is to stop paying attention to the thread that has
 become unproductive and venomous, and resurrect the topic after
 people forgot about it and doing it right yourself ;-).

I came to the same conclusion, but decided to simply do it right now
while it was in my head. Hopefully we can progress by reviewing the
series I just posted.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Erik Faye-Lund]

On Tue, Nov 13, 2012 at 12:35 AM, Jeff King p...@peff.net wrote:
 On Sun, Nov 11, 2012 at 06:06:50PM +0100, Felipe Contreras wrote:

 There's no point in asking this over and over if the user already
 properly configured his/her name and email.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---

 I got really tired of 'git send-email' always asking me from which address 
 to send mails... that's already configured.

 It should be defaulting to your regular git ident, and you just have to
 hit enter, right?

 I think it's probably reasonable to skip that enter in most cases. But
 I'm not sure why we ever asked in the first place. What do people input
 there if they are not taking the default?


I usually input Linus Torvalds torva...@linux-foundation.org, to
give my patch high priority ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Tue, Nov 13, 2012 at 08:13:04AM -0800, Junio C Hamano wrote:

  That's right, AUTHOR_IDENT would fall back to the default email and full 
  name.
 
  Yeah, I find that somewhat questionable in the current behavior, and I'd
  consider it a bug. Typically we prefer the committer ident when given a
  choice (e.g., for writing reflog entries).
 
 Just to make sure I follow the discussion correctly, do you mean
 that the bug is that we pick a fuzzy AUTHOR when COMMITTER is
 specified more concretely and we usually use COMMIITTER for this
 kind of thing in the first place but send-email does not in this
 case (I do not see git var GIT_AUTHOR_IDENT returning value from
 the implicit logic as a bug in this case---just making sure).

 Having discussed more, I think there are two questionable things:

   1. Preferring author over committer

   2. Failing to fall back to committer when author is implicit or bogus
  (because git var dies).

 I think (1) may fall into the this is not how I would do it today, but
 it is not worth a possible regression category.

 I think (2) might be worth fixing, though. Certainly when the author is
 bogus (by IDENT_STRICT rules), which I think was the original intent of
 the $repoauthor || $repocommitter code. Probably when the author ident
 is implicit, though that is more hazy to me.

I agree with both points.  Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Contreras]

On Tue, Nov 13, 2012 at 5:48 PM, Jeff King p...@peff.net wrote:
 On Tue, Nov 13, 2012 at 10:06:26AM +0100, Felipe Contreras wrote:

 I think you are the one that is not understanding what I'm saying. But
 I don't think it matters.

 This is what I'm saying; the current situation with 'git commit' is
 not OK, _both_ 'git commit' and 'git send-email' should change.

 Perhaps I am being dense, but this is the first time it became apparent
 to me that you are arguing for a change in git commit.

Miscomunication then. When you mentioned 'it has always been the case
that you can use git without setting
user.*', I directed my comments at git in general, not git send-email.

 Indeed I would, but there's other people that would benefit from this
 patch. I'm sure I'm not the only person that doesn't have
 sendmail.from configured, but does have user.name/user.email, and is
 constantly typing enter.

 And the difference is that I'm _real_, the hypothetical user that
 sends patches with GIT_AUTHOR_NAME/EMAIL is not. I would be convinced
 otherwise if some evidence was presented that such a user is real
 though.

 Sadly we cannot poll the configuration of every user, nor expect them
 all to pay attention to this discussion on the list. So we cannot take
 the absence of comment from such users as evidence that they do not
 exist.

And we cannot take it as evidence that these users do exist either.

The absence of evidence simply means that... we don't have evidence.

 Instead we must use our judgement about what is being changed,
 and we tend to err on the side of keeping the status quo, since that is
 what the silent majority is busy _not_ complaining about.

Yes, the keyword is *tend*; this wouldn't be first or the last time
that a behavior change happens.

We have to be careful about these changes, but we do them.

 We use the same judgement on the other side, too. Right now your
 evidence is 1 user wants this, 0 users do not.

1 is infinitely greater than 0.

 But we can guess that
 there are other people who would like the intent of your patch, but did
 not care enough or are not active enough on the list to write the patch
 themselves or comment on this thread.

Yes, that would be an educated guess. IMO the fact that people use
GIT_AUTHOR_ variables to send mail is not. There's many ways to
achieve that: sendemail.from, --from, user configuration, $EMAIL,
--compose and From:, etc. each and every one of them much more likely
to be used than GIT_AUTHOR_.

But I'm tired of arguing how extremely unlikely it is that such people
don't exist. Lets agree to disagree. Either way it doesn't matter,
because nobody is proposing a patch that would affect these
hypothetical users.

 And to balance you need to *measure*, and that means taking into
 consideration who actually uses the features, if there's any. And it
 looks to me this is a feature nobody uses.

 You said measure and then it looks to me like. What did you measure?
 Did you consider systematic bias in your measurement, like the fact that
 people who are using the feature have no reason to come on the list and
 announce it?

measure != scientific measurement.

I used common sense, because that's the only tool available.

GIT_AUTHOR is plumbing, not very well known, it's cumbersome (you need
to export two variables), it can be easily confused with
GIT_COMMITTER, which wouldn't work on this case. And there's plenty of
tools that much simpler to use, starting with 'git send-email --from',
which is so user friendly it's in the --help. There's absolutely no
reason why anybody would want to use GIT_AUTHOR.

But lets agree to disagree.

 But listen closely to what you said:

  I actually think it would make more sense to drop the prompt entirely and 
  just die when the user has not given us a usable ident.

 Suppose somebody has a full name, and a fully qualified domain name,
 and he can receive mails to it directly. Such a user would not need a
 git configuration, and would not need $EMAIL, or anything.

 Currently 'git send-email' will throw 'Felipe Contreras
 feli...@felipec.org' which would actually work, but is not explicit.

 You are suggesting to break that use-case. You are introducing a
 regression. And this case is realistic, unlike the
 GIT_AUTHOR_NAME/EMAIL. Isn't it?

 Yes, dying would be a regression, in that you would have to configure
 your name via the environment and re-run rather than type it at the
 prompt. You raise a good point that for people who _could_ take the
 implicit default, hitting enter is working fine now, and we would lose
 that.  I'd be fine with also just continuing to prompt in the implicit
 case.

 But that is a much smaller issue to me than having send-email fail to
 respect environment variables and silently use user.*, which is what
 started this whole discussion. And I agree it is worth considering as a
 regression we should avoid.

It might be smaller, I don't think so. A hypothetical user that was
relying on GIT_AUTHOR for 

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

[Jeff King]

On Sun, Nov 11, 2012 at 06:06:50PM +0100, Felipe Contreras wrote:

 There's no point in asking this over and over if the user already
 properly configured his/her name and email.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
 
 I got really tired of 'git send-email' always asking me from which address to 
 send mails... that's already configured.

It should be defaulting to your regular git ident, and you just have to
hit enter, right?

I think it's probably reasonable to skip that enter in most cases. But
I'm not sure why we ever asked in the first place. What do people input
there if they are not taking the default?

 diff --git a/git-send-email.perl b/git-send-email.perl
 index aea66a0..65b8328 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -748,6 +748,17 @@ if (!$force) {
   }
  }
  
 +my $name = Git::config('user.name');
 +my $email = Git::config('user.email');
 +
 +if (defined $email) {
 + if (defined $name) {
 + $sender = sprintf(%s %s, $name, $email);
 + } else {
 + $sender = $email;
 + }
 +}

Why not use Git::ident_person() here? It saves some code, and would also
respect environment variables. Or better yet...

  my $prompting = 0;
  if (!defined $sender) {
   $sender = $repoauthor || $repocommitter || '';

Why not just use $repoauthor or $repocommitter, as the prompt default
already does?

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Contreras]

On Tue, Nov 13, 2012 at 12:35 AM, Jeff King p...@peff.net wrote:
 On Sun, Nov 11, 2012 at 06:06:50PM +0100, Felipe Contreras wrote:

 There's no point in asking this over and over if the user already
 properly configured his/her name and email.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---

 I got really tired of 'git send-email' always asking me from which address 
 to send mails... that's already configured.

 It should be defaulting to your regular git ident, and you just have to
 hit enter, right?

Yes.

 I think it's probably reasonable to skip that enter in most cases. But
 I'm not sure why we ever asked in the first place. What do people input
 there if they are not taking the default?

Beats me.

 Why not use Git::ident_person() here? It saves some code, and would also
 respect environment variables. Or better yet...

I assume there was a reason why that code was asking for input;
precisely because it would use the environment variables. For some
reason the user might have exported GIT_AUTHOR_EMAIL, or maybe EMAIL
is not right, or the full name config.

OTOH user.name/.email configurations come clearly from the user.

  my $prompting = 0;
  if (!defined $sender) {
   $sender = $repoauthor || $repocommitter || '';

 Why not just use $repoauthor or $repocommitter, as the prompt default
 already does?

See above.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jeff King]

On Tue, Nov 13, 2012 at 12:42:02AM +0100, Felipe Contreras wrote:

  Why not use Git::ident_person() here? It saves some code, and would also
  respect environment variables. Or better yet...
 
 I assume there was a reason why that code was asking for input;
 precisely because it would use the environment variables. For some
 reason the user might have exported GIT_AUTHOR_EMAIL, or maybe EMAIL
 is not right, or the full name config.
 
 OTOH user.name/.email configurations come clearly from the user.

But we use the environment to default the field, so the distinction
doesn't make much sense to me.  Plus, it has always been the case that
you can use git without setting user.*, but instead only using the
environment. I don't see any reason not to follow that principle here,
too.

The one distinction that would make sense to me is pausing to ask when
we use implicit methods to look up the ident, like concatenating the
username with the hostname to get the email.

Git::ident uses git var to do its lookup, which will use IDENT_STRICT;
that stops most junk like empty names and bogus domains. But I think we
would want to go one step further and actually check
user_ident_sufficiently_given.  Unfortunately, that is not currently
available outside of C. You'd probably want something like:

diff --git a/builtin/var.c b/builtin/var.c
index aedbb53..eaf324e 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -26,6 +26,12 @@ static const char *pager(int flag)
return pgm;
 }
 
+static const char *explicit_ident(int flag)
+{
+   git_committer_info(flag);
+   return user_ident_sufficiently_given() ? 1 : 0;
+}
+
 struct git_var {
const char *name;
const char *(*read)(int);
@@ -35,6 +41,7 @@ static struct git_var git_vars[] = {
{ GIT_AUTHOR_IDENT,   git_author_info },
{ GIT_EDITOR, editor },
{ GIT_PAGER, pager },
+   { GIT_EXPLICIT_IDENT, explicit_ident },
{ , NULL },
 };

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jeff King]

On Mon, Nov 12, 2012 at 07:02:17PM -0500, Jeff King wrote:

 The one distinction that would make sense to me is pausing to ask when
 we use implicit methods to look up the ident, like concatenating the
 username with the hostname to get the email.

By the way, I suspect this is the answer to what do people type for
this prompt. It is probably more about a safety on bad ident than it is
about people routinely updating the information. I actually think it
would make more sense to drop the prompt entirely and just die when the
user has not given us a usable ident. But maybe people who do one-off
format-patches would rather type their name in a prompt than set an
environment variable and re-run the program.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Contreras]

On Tue, Nov 13, 2012 at 1:02 AM, Jeff King p...@peff.net wrote:
 On Tue, Nov 13, 2012 at 12:42:02AM +0100, Felipe Contreras wrote:

  Why not use Git::ident_person() here? It saves some code, and would also
  respect environment variables. Or better yet...

 I assume there was a reason why that code was asking for input;
 precisely because it would use the environment variables. For some
 reason the user might have exported GIT_AUTHOR_EMAIL, or maybe EMAIL
 is not right, or the full name config.

 OTOH user.name/.email configurations come clearly from the user.

 But we use the environment to default the field, so the distinction
 doesn't make much sense to me.  Plus, it has always been the case that
 you can use git without setting user.*, but instead only using the
 environment. I don't see any reason not to follow that principle here,
 too.

And that's why a lot of commits end up like michael
michael@michael-laptop.(none).

 The one distinction that would make sense to me is pausing to ask when
 we use implicit methods to look up the ident, like concatenating the
 username with the hostname to get the email.

 Git::ident uses git var to do its lookup, which will use IDENT_STRICT;
 that stops most junk like empty names and bogus domains. But I think we
 would want to go one step further and actually check
 user_ident_sufficiently_given.  Unfortunately, that is not currently
 available outside of C. You'd probably want something like:

 diff --git a/builtin/var.c b/builtin/var.c
 index aedbb53..eaf324e 100644
 --- a/builtin/var.c
 +++ b/builtin/var.c
 @@ -26,6 +26,12 @@ static const char *pager(int flag)
 return pgm;
  }

 +static const char *explicit_ident(int flag)
 +{
 +   git_committer_info(flag);
 +   return user_ident_sufficiently_given() ? 1 : 0;
 +}
 +
  struct git_var {
 const char *name;
 const char *(*read)(int);
 @@ -35,6 +41,7 @@ static struct git_var git_vars[] = {
 { GIT_AUTHOR_IDENT,   git_author_info },
 { GIT_EDITOR, editor },
 { GIT_PAGER, pager },
 +   { GIT_EXPLICIT_IDENT, explicit_ident },
 { , NULL },
  };

Probably. But what I really want is to stop 'git send-email' from
asking. I think the one next step further can be done later.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Mon, Nov 12, 2012 at 07:02:17PM -0500, Jeff King wrote:

 The one distinction that would make sense to me is pausing to ask when
 we use implicit methods to look up the ident, like concatenating the
 username with the hostname to get the email.

 By the way, I suspect this is the answer to what do people type for
 this prompt. It is probably more about a safety on bad ident than it is
 about people routinely updating the information. I actually think it
 would make more sense to drop the prompt entirely and just die when the
 user has not given us a usable ident.

Yeah, I agree with pretty much everything you said in this thread
(including that environment and config are equally likely to reflect
user's wish and it does not make much sense to treat environment as
more suspicious).

 But maybe people who do one-off format-patches would rather type
 their name in a prompt than set an environment variable and re-run
 the program.

s/one-off format-patches/one-off send-email/.  I think dying will
force them to configure their names once (so that later invocations
do not have to stop) while prompting will force them to type their
names every time, so the current behaviour is probably a false
economy.  As long as we caution users in the release notes, it
probably is OK to change the command to die.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jeff King]

On Tue, Nov 13, 2012 at 01:54:59AM +0100, Felipe Contreras wrote:

  But we use the environment to default the field, so the distinction
  doesn't make much sense to me.  Plus, it has always been the case that
  you can use git without setting user.*, but instead only using the
  environment. I don't see any reason not to follow that principle here,
  too.
 
 And that's why a lot of commits end up like michael
 michael@michael-laptop.(none).

No, it's not. Those broken names do not come from the environment, but
from our last-resort guess of the hostname. 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).

And I have proposed exactly the same behavior here: respect the
environment and the config, but do not trust the implicit guesses.

 Probably. But what I really want is to stop 'git send-email' from
 asking. I think the one next step further can be done later.

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).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jeff King]

On Mon, Nov 12, 2012 at 10:27:27PM -0500, Jeff King wrote:

 On Tue, Nov 13, 2012 at 01:54:59AM +0100, Felipe Contreras wrote:
 
   But we use the environment to default the field, so the distinction
   doesn't make much sense to me.  Plus, it has always been the case that
   you can use git without setting user.*, but instead only using the
   environment. I don't see any reason not to follow that principle here,
   too.
  
  And that's why a lot of commits end up like michael
  michael@michael-laptop.(none).
 
 No, it's not. Those broken names do not come from the environment, but
 from our last-resort guess of the hostname. 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).
 
 And I have proposed exactly the same behavior here: respect the
 environment and the config, but do not trust the implicit guesses.

Re-reading this, I think them at the end of the second paragraph is
slightly unclear. Let me rephrase.

The lack of a name or the presence of an obviously bogus email address
(e.g., with (none)) is disallowed by commit. We still allow implicit
idents on commit as long they are not obviously wrong, but show them to
the user so that they can notice and correct via commit --amend.

So if it dies on an implicit ident, send-email would actually be taking
an even stronger stance against bogus idents. Which makes sense, since
there is no --amend for fixing a broken email that has been sent.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Contreras]

On Tue, Nov 13, 2012 at 4:27 AM, Jeff King p...@peff.net wrote:
 On Tue, Nov 13, 2012 at 01:54:59AM +0100, Felipe Contreras wrote:

  But we use the environment to default the field, so the distinction
  doesn't make much sense to me.  Plus, it has always been the case that
  you can use git without setting user.*, but instead only using the
  environment. I don't see any reason not to follow that principle here,
  too.

 And that's why a lot of commits end up like michael
 michael@michael-laptop.(none).

 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.

 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 mich...@michael-laptop.michael.org

 Probably. But what I really want is to stop 'git send-email' from
 asking. I think the one next step further can be done later.

 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.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jeff King]

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. But as I
said already, git var uses IDENT_STRICT and will not allow such broken
names.

  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 mich...@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.

  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.

I tried to help you by pointing you in the right direction and even
providing a sample git var patch. But it is not my itch to scratch.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Contreras]

On Tue, Nov 13, 2012 at 5:01 AM, Jeff King p...@peff.net wrote:
 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.

  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 mich...@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.

  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
feli...@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.contre...@gmail.com]

4) config user

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

5) GIT_COMMITTER

Who should the emails appear to be from? [Felipe Contreras 2nd
felipe.contrera...@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.contrera...@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.contrera...@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.contrera...@gmail.com

5) GIT_COMMITTER

From: Felipe Contreras 2nd felipe.contrera...@gmail.com

6) GIT_AUTHOR

From: Felipe Contreras 2nd felipe.contrera...@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.contrera...@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.contrera...@gmail.com'
./git var GIT_EXPLICIT_IDENT
1

From: Felipe Contreras feli...@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 

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

[Felipe Contreras]

On Tue, Nov 13, 2012 at 7:42 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:

 6) GIT_AUTHOR

 Who should the emails appear to be from? [Felipe Contreras 4th
 felipe.contrera...@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

This was supposed to be above (before my change).

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jeff King]

On Tue, Nov 13, 2012 at 07:42:58AM +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.

There was a bug that caused the check to fail in some cases. I fixed it
in f20f387 (this July).

But I still don't see how that has anything to do with what send-email
does or should do. That is why I said strawman above. You seem to
think I am saying that send-email should use the system that generated
those broken names, when I am saying the opposite.

  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.

Those people would also not be using a new version of git-send-email,
and it will always prompt. I thought we were talking about what
send-email should do in future versions. Namely, loosening that safety
valve (the prompt) because it is inconvenient, but tightening the checks
so that losing the safety valve is not a problem.

  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.

The second half of that sentence that you quoted above is ...instead
only using the environment. As in, environment variables like
GIT_AUTHOR_EMAIL, GIT_COMMITTER_EMAIL, and EMAIL. _Not_ implicit
generation of the email from the username and hostname.

I am tempted to fault myself for not communicating well, but I feel like
I have made that point at least 3 times in this conversation now. Is
that the source of the confusion?

  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.

And you will survive if upstream git (whether it is me today or Junio
tomorrow) does not pick up your patch. I remember writing you a long
email recently about how one of the responsibilities of the maintainer
is to balance features versus regressions. I'll not bother repeating
myself here.

As for whether they exist, what data do you have? Are you aware that the
test suite, for example, relies on setting GIT_AUTHOR_NAME but not
having any user.* config? When somebody comes on the list and asks why
every git program in the entire system respects GIT_* environment
variables as an override to user.* configuration _except_ for
send-email, what should I say?

 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

That is dependent on your system. If you have a non-empty name in your
GECOS field, and if your machine has a FQDN, it will currently work (and
prompt).

 2) Full Name + full hostname
 
 Who should the emails appear to be from? [Felipe Contreras
 feli...@nysa.felipec.org]
 
 That's right, ident doesn't fail, and that's not the mail address I
 specified, it's *implicit*.

Right. I never said it did. I said it currently rejected obviously bogus
stuff (like the .(none) above) due to IDENT_STRICT, but currently
allowed implicit definitions. And I also said that if we get rid of the
prompt, we should disallow implicit definitions like this, because the
prompt is the safety valve on sending out mails with broken from
addresses. I even wrote a patch that let you find out whether the ident
was generated implicitly.

 3) Full Name + EMAIL
 
 Who should the emails appear to be from? [Felipe Contreras
 felipe.contre...@gmail.com]

Which sounds fine to me. EMAIL is considered explicit, and I have not
seen any evidence that people are putting bogus values in their EMAIL
variable and complaining that it is git's fault for respecting it.

 4) config user
 
 Who should the emails appear to be from? [Felipe Contreras 2nd
 felipe.contrera...@gmail.com]

OK.

 5) GIT_COMMITTER
 
 Who should the emails appear to be from? [Felipe Contreras 2nd
 felipe.contrera...@gmail.com]
 
 Whoa, what happened there?
 
 Well:
 
   $sender = $repoauthor || $repocommitter || '';
   ($repoauthor) = Git::ident_person(@repo, 'author');
   % ./git var GIT_AUTHOR_IDENT
   

[PATCH] send-email: add proper default sender

[Felipe Contreras]

There's no point in asking this over and over if the user already
properly configured his/her name and email.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---

I got really tired of 'git send-email' always asking me from which address to 
send mails... that's already configured.

 git-send-email.perl | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..65b8328 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,6 +748,17 @@ if (!$force) {
}
 }
 
+my $name = Git::config('user.name');
+my $email = Git::config('user.email');
+
+if (defined $email) {
+   if (defined $name) {
+   $sender = sprintf(%s %s, $name, $email);
+   } else {
+   $sender = $email;
+   }
+}
+
 my $prompting = 0;
 if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
-- 
1.8.0

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html