Re: git should not use a default user.email config value

Re: git should not use a default user.email config value

[Jonathan Nieder]

Hi Thorsten,

Thorsten Glaser wrote[1]:

> git config user.email SHOULD NOT default to $(id -un)@$(hostname -f)
> because just too many cow-orkers seem to be unable to follow basic
> instructions

Heh.

Can you say a little more about your setup?  In a university
environment with sysadmin-managed email and /etc/mailname set up
correctly it is handy that people can start working without doing
anything special to configure git's "[user] email" setting.  On the
other hand it is obnoxious to receive patches with wrong authorship
information.  So I'm wondering if there's some detail that
distinguishes between these cases.

Incidentally, it's been a long time since I looked at the "Please
configure your email address; I've made something up, but you'll want
to check it" message:

	Your name and email address were configured automatically based
	on your username and hostname. Please check that they are accurate.
	You can suppress this message by setting them explicitly:

	    git config --global user.name "Your Name"
	    git config --global user.email you@example.com

	After doing this, you may fix the identity used for this commit with:

	    git commit --amend --reset-author

I wonder if it's too gentle and long to get the point across.  Would
something the following (including the guesses in the message for
easier copy-pasting) help?

	No name and email address configured, so I had to guess.  You
	can suppress this message by setting your identity explicitly:

		git config --global user.name "Thorsten Glaser"
		git config --global user.email tg@mirbsd.de

	After doing so, you may fix the identity used for this commit
	with "git commit --amend --reset-author".

It may also make sense to distinguish between cases where a mailname
is set and not set.  Git already notices the cases where the guessed
email address ends with ".(none)" and errors out, and it could make
sense to be more aggressive.

Hope that helps,
Jonathan

[1] http://bugs.debian.org/719226

Re: git should not use a default user.email config value

[Thorsten Glaser]

Jonathan Nieder dixit:

>Can you say a little more about your setup?  In a university
>environment with sysadmin-managed email and /etc/mailname set up
>correctly it is handy that people can start working without doing

Ah okay. We don’t have /etc/mailname set up I think and,
additionally, the Unix user name doesn’t match the eMail
localpart, so that won’t work anyway.

Though we’re having a very heterogenous desktop environment
nowadays so I can’t really know all specifics.

At least, I think, most devs seem to use the Unix git client
now, whereas for svn they use the one that comes with Eclipse…

>I wonder if it's too gentle and long to get the point across.  Would
>something the following (including the guesses in the message for
>easier copy-pasting) help?

Definitely not. It needs to fail hard if user.email is not set,
i.e. refuse to accept the commit.

>is set and not set.  Git already notices the cases where the guessed
>email address ends with ".(none)" and errors out, and it could make
>sense to be more aggressive.

The guessed addresses are like 'denge@pc-bn-041.lan.tarent.de'
instead of 'd.enge@tarent.de' which is the correct Kolab address
(this information can be publicly accessed since the project I
noticed it in is on our public FusionForge instance, so I don’t
think sharing specifics is bad here, but please don’t hammer our
poor trainee with spam now). So they’re a “correct” unix username
at a correct FQDN (which, thanks to split-horizon, even would
work internally, except there’s of course no MTA set up) and
won’t be caught by *.(none) matches.

Hope this helps.

Thanks,
//mirabilos
-- 
tarent solutions GmbH
Rochusstraße 2-4, D-53123 Bonn • http://www.tarent.de/
Tel: +49 228 54881-393 • Fax: +49 228 54881-314
HRB AG Bonn 5168 • USt-ID (VAT): DE122264941
Geschäftsführer: Boris Esser, Sebastian Mancke

Re: git should not use a default user.email config value

[Felipe Contreras]

On Fri, Aug 9, 2013 at 3:00 PM, Thorsten Glaser <tg@mirbsd.de> wrote:
> Jonathan Nieder dixit:

>>I wonder if it's too gentle and long to get the point across.  Would
>>something the following (including the guesses in the message for
>>easier copy-pasting) help?
>
> Definitely not. It needs to fail hard if user.email is not set,
> i.e. refuse to accept the commit.

Completely agree, and I argued this point some time ago.

>>is set and not set.  Git already notices the cases where the guessed
>>email address ends with ".(none)" and errors out, and it could make
>>sense to be more aggressive.
>
> The guessed addresses are like 'denge@pc-bn-041.lan.tarent.de'
> instead of 'd.enge@tarent.de' which is the correct Kolab address
> (this information can be publicly accessed since the project I
> noticed it in is on our public FusionForge instance, so I don’t
> think sharing specifics is bad here, but please don’t hammer our
> poor trainee with spam now). So they’re a “correct” unix username
> at a correct FQDN (which, thanks to split-horizon, even would
> work internally, except there’s of course no MTA set up) and
> won’t be caught by *.(none) matches.

This is how to implement that:

From f1feaa05ce3772d8006078c4aeabcbd55b52d58e Mon Sep 17 00:00:00 2001
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
Date: Tue, 13 Nov 2012 07:33:12 +0100
Subject: [PATCH] ident: don't allow implicit email addresses

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 ident.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ident.c b/ident.c
index 1c123e6..85fc729 100644
--- a/ident.c
+++ b/ident.c
@@ -301,9 +301,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) {
-- 
1.8.3.267.gbb4989f


-- 
Felipe Contreras

Re: git should not use a default user.email config value

[Jeff King]

On Fri, Aug 09, 2013 at 12:42:14PM -0700, Jonathan Nieder wrote:

> I wonder if it's too gentle and long to get the point across.  Would
> something the following (including the guesses in the message for
> easier copy-pasting) help?
> 
> 	No name and email address configured, so I had to guess.  You
> 	can suppress this message by setting your identity explicitly:
> 
> 		git config --global user.name "Thorsten Glaser"
> 		git config --global user.email tg@mirbsd.de
> 
> 	After doing so, you may fix the identity used for this commit
> 	with "git commit --amend --reset-author".

I don't know if including the name and email helps that much. It should
already be printed along with that message, like:

  $ git commit --allow-empty -m foo
  [master ba77f94] foo
   Committer: Jeff King <peff@sigill.intra.peff.net>
  Your name and email address were configured automatically based
  on your username and hostname. Please check that they are accurate.
  You can suppress this message by setting them explicitly:

      git config --global user.name "Your Name"
      git config --global user.email you@example.com

  After doing this, you may fix the identity used for this commit with:

      git commit --amend --reset-author

> It may also make sense to distinguish between cases where a mailname
> is set and not set.  Git already notices the cases where the guessed
> email address ends with ".(none)" and errors out, and it could make
> sense to be more aggressive.

Yeah, there are basically three levels of ident:

  1. The user told us explicitly (e.g., $EMAIL, user.email). Trust it.

  2. We guessed and it looks reasonable (e.g., hostname is FQDN). Warn
     but use it.

  3. It looks obviously bogus (e.g., we do not have a domain name).
     Reject it.

We can move some cases from (2) down to (3), like when we use
gethostname rather than /etc/mailname.  But we risk breaking people's
existing setups. I don't think we know how many people rely on the
implicit hostname selection and would be affected. I don't know if there
is a good way to find out short of changing it and seeing who screams.
We can put a deprecation warning in the release notes, but people tend
to ignore those. Or perhaps now that we have had the long obnoxious
implicit-ident warning for several versions, everybody has finally set
user.email and the time is right to change.

Another option could to add an option to control the strictness. We
usually have a chicken-and-egg problem here with individual installs
(i.e., any person who could set "user.trustHostname = false" could just
as easily have set "user.email"). But in an institutional setting, the
admin could set such a config in /etc/gitconfig for everybody. Or for a
system like Debian, the packager could include the option, knowing that
any reasonably configured system should have /etc/mailname set up (which
is not something we can necessarily count on for other operating
systems).

-Peff

Re: git should not use a default user.email config value

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Yeah, there are basically three levels of ident:
>
>   1. The user told us explicitly (e.g., $EMAIL, user.email). Trust it.
>
>   2. We guessed and it looks reasonable (e.g., hostname is FQDN). Warn
>      but use it.
>
>   3. It looks obviously bogus (e.g., we do not have a domain name).
>      Reject it.
>
> We can move some cases from (2) down to (3), like ...

Judging from Thorsten's earlier response, I am afraid no amount of
autodetection would help the users of that site.  If we were to do
something, /etc/gitconfig as you outlined below would be the way to
go, even though it makes me feel dirty.

> Another option could to add an option to control the strictness. We
> usually have a chicken-and-egg problem here with individual installs
> (i.e., any person who could set "user.trustHostname = false" could just
> as easily have set "user.email"). But in an institutional setting, the
> admin could set such a config in /etc/gitconfig for everybody. Or for a
> system like Debian, the packager could include the option, knowing that
> any reasonably configured system should have /etc/mailname set up (which
> is not something we can necessarily count on for other operating
> systems).
>
> -Peff

Re: git should not use a default user.email config value

[Jonathan Nieder]

Jeff King wrote:

> Yeah, there are basically three levels of ident:
>
>   1. The user told us explicitly (e.g., $EMAIL, user.email). Trust it.
>
>   2. We guessed and it looks reasonable (e.g., hostname is FQDN). Warn
>      but use it.
>
>   3. It looks obviously bogus (e.g., we do not have a domain name).
>      Reject it.
>
> We can move some cases from (2) down to (3), like when we use
> gethostname rather than /etc/mailname.  But we risk breaking people's
> existing setups. I don't think we know how many people rely on the
> implicit hostname selection and would be affected. I don't know if there
> is a good way to find out short of changing it and seeing who screams.

Yes.  The result from a reverse DNS lookup is almost never the right
mailname.

 * Small installations tend to use a smarthost.
 * Large installations tend to use more than one machine, and only
   one machine's name gets the MX record.
 
So except for cases where someone doesn't actually care about the
recorded author and just has a script making commits (such users
already suffer from the ".(none)" heuristic), I don't think this would
hurt anyone.

> We can put a deprecation warning in the release notes, but people tend
> to ignore those.

Not so much a deprecation warning as an "Here is one of the more
noticeable changes in this release" announcement.

I'm pretty sure a deprecation warning would not help here.  Either
people are affected and we say "WARNING: You were doing something
perfectly reasonable, but now we discourage it", or, more likely,
people are not affected.  Announcing a change too loudly to users not
affected by it has a very bad side effect of training them not to pay
much attention to release notes.

[...]
> Another option could to add an option to control the strictness.

I suspect a new config item for this is a bad idea, given how simple
it is to choose a good default for everyone.

Thanks,
Jonathan

Re: git should not use a default user.email config value

[Jeff King]

On Fri, Aug 09, 2013 at 04:06:16PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Yeah, there are basically three levels of ident:
> >
> >   1. The user told us explicitly (e.g., $EMAIL, user.email). Trust it.
> >
> >   2. We guessed and it looks reasonable (e.g., hostname is FQDN). Warn
> >      but use it.
> >
> >   3. It looks obviously bogus (e.g., we do not have a domain name).
> >      Reject it.
> >
> > We can move some cases from (2) down to (3), like ...
> 
> Judging from Thorsten's earlier response, I am afraid no amount of
> autodetection would help the users of that site.  If we were to do
> something, /etc/gitconfig as you outlined below would be the way to
> go, even though it makes me feel dirty.

It was not clear to me whether his site has /etc/mailname. If it does
not, then the new rule could be to leave "/etc/mailname" in group 2, and
put "gethostname/gethostbyname" into group 3 (right now we do so only
when the results from those functions are obviously not
fully-qualified).

But from his description, the machine may even have a split-horizon name
in /etc/mailname, and we can do nothing at all about that.

Even if it worked, though, I am not sure it would be worth such a rule.
The /etc/mailname file is not a standard, so you would effectively be
cutting off the auto-ident behavior for people on every other system. If
we are going to do that, we might as well do it uniformly.

-Peff

Re: git should not use a default user.email config value

[Jonathan Nieder]

Jeff King wrote:

> Even if it worked, though, I am not sure it would be worth such a rule.
> The /etc/mailname file is not a standard, so you would effectively be
> cutting off the auto-ident behavior for people on every other system. If
> we are going to do that, we might as well do it uniformly.

I don't fully follow.  Do you mean that because other operating
systems choose not to make full use of an /etc/mailname file when it
is present (and instead use per-MTA configuration), git should not
take advantage of it to choose an appropriate email address?

Or do you mean that on non-Debian systems, the FQDN for localhost is
reliably the mailname, just like on Debian systems /etc/mailname is
supposed to be?

Confused,
Jonathan

Re: git should not use a default user.email config value

[Jeff King]

On Fri, Aug 09, 2013 at 04:19:28PM -0700, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > Yeah, there are basically three levels of ident:
> >
> >   1. The user told us explicitly (e.g., $EMAIL, user.email). Trust it.
> >
> >   2. We guessed and it looks reasonable (e.g., hostname is FQDN). Warn
> >      but use it.
> >
> >   3. It looks obviously bogus (e.g., we do not have a domain name).
> >      Reject it.
> >
> > We can move some cases from (2) down to (3), like when we use
> > gethostname rather than /etc/mailname.  But we risk breaking people's
> > existing setups. I don't think we know how many people rely on the
> > implicit hostname selection and would be affected. I don't know if there
> > is a good way to find out short of changing it and seeing who screams.
> 
> Yes.  The result from a reverse DNS lookup is almost never the right
> mailname.

Just to nitpick, the name we guess is not necessarily from DNS (and if
the FQDN comes from DNS, it is not a reverse lookup, but rather
following the search rules in resolv.conf, or even /etc/hosts). But I
think the point is the same: we somehow arrive at the hostname through
some accurate means, but that hostname does not reflect the user's
actual email address.

>  * Small installations tend to use a smarthost.
>  * Large installations tend to use more than one machine, and only
>    one machine's name gets the MX record.

I'm not sure the second one is true. Many large installations will MX
all of their workstations names to a smarthost. So mail to
user@randommachine.example.com _is_ deliverable. It has (thankfully)
been a long time since I have been involved in large network IT, but
that was standard practice at one time.

But I think MX records and deliverability is beside the point. Even in a
case where we come up with a valid, deliverable address, is that what
the user wants to have in their commit history for all time?

> So except for cases where someone doesn't actually care about the
> recorded author and just has a script making commits (such users
> already suffer from the ".(none)" heuristic), I don't think this would
> hurt anyone.

I think the other case is "people who actually think the per-machine
information is useful". I recall Linus arguing for this early on, but he
seems to have relented. I am not sure whether anyone else in the world
has that view (or ever did).

There are certainly people in the "I don't care, just make it work"
camp, judging from the repositories I sometimes see on GitHub. Whether
we would be harming them (because their workflow breaks) or helping them
(because they had no idea they had these crappy idents in their history
and we would be letting them know) is not clear to me, though.

> > We can put a deprecation warning in the release notes, but people tend
> > to ignore those.
> 
> Not so much a deprecation warning as an "Here is one of the more
> noticeable changes in this release" announcement.

I meant a warning to give people a chance to comment before the change
comes. Following this mailing list is another source for people to find out
about it, but I suspect most casual users do not read the list. Perhaps
it would be worth taking a straw poll over G+ or another more casual
medium?

-Peff

Re: git should not use a default user.email config value

[Jeff King]

On Fri, Aug 09, 2013 at 11:40:56PM -0700, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > Even if it worked, though, I am not sure it would be worth such a rule.
> > The /etc/mailname file is not a standard, so you would effectively be
> > cutting off the auto-ident behavior for people on every other system. If
> > we are going to do that, we might as well do it uniformly.
> 
> I don't fully follow.  Do you mean that because other operating
> systems choose not to make full use of an /etc/mailname file when it
> is present (and instead use per-MTA configuration), git should not
> take advantage of it to choose an appropriate email address?
> 
> Or do you mean that on non-Debian systems, the FQDN for localhost is
> reliably the mailname, just like on Debian systems /etc/mailname is
> supposed to be?

Sorry to be unclear. I meant that treating /etc/mailname and gethostname
differently might be justified on Debian under the logic "if you have
/etc/mailname, that is a trustworthy address, and if you do not, then we
cannot guess at a trustworthy address (because putting it in
/etc/mailname is the accepted way to do so on Debian)".

But such logic would not extend to other operating systems, where
/etc/mailname does not have such a status.

I am guessing, too, about what people even put in /etc/mailname. If they
relay mail from the machine to a smarthost, do they put the individual
hostname into /etc/mailname? Or do they put in the domain name that
represents a real deliverable address? If the former, then it is no
better than gethostname anyway.

-Peff

Re: git should not use a default user.email config value

[Jonathan Nieder]

Jeff King wrote:

> Sorry to be unclear. I meant that treating /etc/mailname and gethostname
> differently might be justified on Debian under the logic "if you have
> /etc/mailname, that is a trustworthy address, and if you do not, then we
> cannot guess at a trustworthy address (because putting it in
> /etc/mailname is the accepted way to do so on Debian)".
>
> But such logic would not extend to other operating systems, where
> /etc/mailname does not have such a status.

I thought that on other operating systems people typically don't have
an /etc/mailname.  How does trusting the file when present hurt?

> I am guessing, too, about what people even put in /etc/mailname. If they
> relay mail from the machine to a smarthost, do they put the individual
> hostname into /etc/mailname? Or do they put in the domain name that
> represents a real deliverable address? If the former, then it is no
> better than gethostname anyway.

Debian policy explains:

	If your package needs to know what hostname to use on (for
	example) outgoing news and mail messages which are generated
	locally, you should use the file /etc/mailname. It will contain
	the portion after the username and @ (at) sign for email
	addresses of users on the machine (followed by a newline).

	Such a package should check for the existence of this file when
	it is being configured. If it exists, it should be used without
	comment, although an MTA's configuration script may wish to
	prompt the user even if it finds that this file exists. If the
	file does not exist, the package should prompt the user for the
	value (preferably using debconf) and store it in /etc/mailname as
	well as using it in the package's configuration. The prompt
	should make it clear that the name will not just be used by that
	package.

So on a properly configured Debian system, /etc/mailname contains
something appropriate to put after the @ sign in an email address
and the sysadmin expects it to be used for that.

As far as I can tell, to the extent that other distros support
/etc/mailname, it is only as a side effect of handling that Debian
requirement.  I don't think e.g. Fedora or Solaris systems typically
will have a /etc/mailname file.

I *am* a bit worried about what people might put in /etc/mailname on
Debian systems when there is no appropriate host to put there (as on
Thorsten's machine).

Jonathan

Re: git should not use a default user.email config value

[Jeff King]

On Sat, Aug 10, 2013 at 12:03:00AM -0700, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > Sorry to be unclear. I meant that treating /etc/mailname and gethostname
> > differently might be justified on Debian under the logic "if you have
> > /etc/mailname, that is a trustworthy address, and if you do not, then we
> > cannot guess at a trustworthy address (because putting it in
> > /etc/mailname is the accepted way to do so on Debian)".
> >
> > But such logic would not extend to other operating systems, where
> > /etc/mailname does not have such a status.
> 
> I thought that on other operating systems people typically don't have
> an /etc/mailname.  How does trusting the file when present hurt?

I guess I am not explaining myself well. Trusting the file when present
does not hurt at all. But the logic above is making assumptions about
the state when the file is _not_ present (i.e., the "if you do not..."
clause above). On Debian, we might assume that if /etc/mailname is not
present that this is a clue that the machine cannot produce a useful
address.  But on other operating systems, that is not a useful clue (it
is simply that /etc/mailname is not used on that system). Dying on such
a system when /etc/mailname is not present would be a regression.

Does that make more sense?

> I *am* a bit worried about what people might put in /etc/mailname on
> Debian systems when there is no appropriate host to put there (as on
> Thorsten's machine).

Yeah. Or even in a split-horizon setup where the mail is deliverable but
does not reflect the public identity of the user. I think we are getting
down to the question I mentioned elsewhere: it is not about whether we
have a deliverable address or not, but what users want to cement in
history for all time as their identity.

So thinking too much about /etc/mailname versus gethostname is probably
not useful. Either it is worth breaking the few (if any) users who
depend on the auto ident in favor of fewer accidental implicit idents
making their way into the wild, or it is not.

-Peff

Re: git should not use a default user.email config value

[Michael Haggerty]

On 08/10/2013 08:47 AM, Jeff King wrote:
> But I think MX records and deliverability is beside the point. Even in a
> case where we come up with a valid, deliverable address, is that what
> the user wants to have in their commit history for all time?

I intentionally don't set user.email in my ~/.gitconfig because I use
different identities (on the same machine) depending on what project I
am committing to (open-source vs. work).  After I clone a repo, I *rely*
on Git reminding me to set user.email on my first commit, because I
invariably forget to set it myself.  And for me, *any* universal,
heuristically-determined email address would be wrong for me for at
least some repos.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: git should not use a default user.email config value

[Jeff King]

On Sat, Aug 10, 2013 at 11:59:21AM +0200, Michael Haggerty wrote:

> On 08/10/2013 08:47 AM, Jeff King wrote:
> > But I think MX records and deliverability is beside the point. Even in a
> > case where we come up with a valid, deliverable address, is that what
> > the user wants to have in their commit history for all time?
> 
> I intentionally don't set user.email in my ~/.gitconfig because I use
> different identities (on the same machine) depending on what project I
> am committing to (open-source vs. work).  After I clone a repo, I *rely*
> on Git reminding me to set user.email on my first commit, because I
> invariably forget to set it myself.  And for me, *any* universal,
> heuristically-determined email address would be wrong for me for at
> least some repos.

So if I understand your use case, then you would be even happier if
rather than giving a warning, git simply barfed and said "please set
your identity before committing"?

-Peff

Re: git should not use a default user.email config value

[Michael Haggerty]

On 08/10/2013 12:28 PM, Jeff King wrote:
> On Sat, Aug 10, 2013 at 11:59:21AM +0200, Michael Haggerty wrote:
> 
>> On 08/10/2013 08:47 AM, Jeff King wrote:
>>> But I think MX records and deliverability is beside the point. Even in a
>>> case where we come up with a valid, deliverable address, is that what
>>> the user wants to have in their commit history for all time?
>>
>> I intentionally don't set user.email in my ~/.gitconfig because I use
>> different identities (on the same machine) depending on what project I
>> am committing to (open-source vs. work).  After I clone a repo, I *rely*
>> on Git reminding me to set user.email on my first commit, because I
>> invariably forget to set it myself.  And for me, *any* universal,
>> heuristically-determined email address would be wrong for me for at
>> least some repos.
> 
> So if I understand your use case, then you would be even happier if
> rather than giving a warning, git simply barfed and said "please set
> your identity before committing"?

Yes, definitely.

For the particular use case that I described, I wouldn't mind setting a
global setting "barfOnMissingEmail = true" because I always use the same
Linux account.  But for other uses cases that arise at my company,
people have to jump around from one computer to another, and it would be
more convenient if the barfing behavior was the default without the need
for a setting.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: git should not use a default user.email config value

[Thorsten Glaser]

Jeff King dixit:

>It was not clear to me whether his site has /etc/mailname. If it does

Some may, some may not but…

>But from his description, the machine may even have a split-horizon name
>in /etc/mailname, and we can do nothing at all about that.

… that won’t happen. The problem is that they may have
the correct domain there but the localpart will still
be wrong because Kolab localparts are not Unix usernames.


Jonathan Nieder dixit:

>I thought that on other operating systems people typically don't have
>an /etc/mailname.  How does trusting the file when present hurt?

Right, MirBSD doesn’t have it, and I don’t think OpenBSD
added it since we forked.


Jeff King dixit:

>On Sat, Aug 10, 2013 at 11:59:21AM +0200, Michael Haggerty wrote:
>
>> I intentionally don't set user.email in my ~/.gitconfig because I use
>> different identities (on the same machine) depending on what project I

For me that’s also true, but I set a default one at the moment
which is still better than having an unroutable one (on my private
laptop, ${unix_username}@${fqdn} does work, but only as long as my
laptop is powered on, has got IPv6 Internet, and the sending MTA
has IPv6 Internet, so… it’s mostly unroutable).

While I used a fallback for this scenario (me, privately), I’d
also benefit from git refusing to accept commits by default.

>So if I understand your use case, then you would be even happier if
>rather than giving a warning, git simply barfed and said "please set
>your identity before committing"?

Exactly. That’s what I think he said, and what I asked for too.

Thanks,
//mirabilos (working with many OSS projects)
-- 
I believe no one can invent an algorithm. One just happens to hit upon it
when God enlightens him. Or only God invents algorithms, we merely copy them.
If you don't believe in God, just consider God as Nature if you won't deny
existence.		-- Coywolf Qi Hunt

Re: git should not use a default user.email config value

[Andreas Schwab]

Jeff King <peff@peff.net> writes:

> So if I understand your use case, then you would be even happier if
> rather than giving a warning, git simply barfed and said "please set
> your identity before committing"?

FWIW, this is what both hg and bzr do.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: git should not use a default user.email config value

[Junio C Hamano]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 08/10/2013 08:47 AM, Jeff King wrote:
>> But I think MX records and deliverability is beside the point. Even in a
>> case where we come up with a valid, deliverable address, is that what
>> the user wants to have in their commit history for all time?
>
> I intentionally don't set user.email in my ~/.gitconfig because I use
> different identities (on the same machine) depending on what project I
> am committing to (open-source vs. work).  After I clone a repo, I *rely*
> on Git reminding me to set user.email on my first commit, because I
> invariably forget to set it myself.  And for me, *any* universal,
> heuristically-determined email address would be wrong for me for at
> least some repos.

Interesting.

If we tweaked the "template" mechanism used by the init_db() more
accessible, because both "git init" and "git clone" know to honor
the templates, you could prepare ~/.git-profile/{work,open}/config
files that define user.email/user.name in there.  The existing way
to use "template" mechanism is a bit too heavy-handed in the sense
that if you want to tweak the "config" using it, you also have to
have everything else in the templates, which makes it unwieldy to
use.  Perhaps we need a lighter-weight mechanism

	git init --profile=open
        git clone --profile=open git://git.kernel.org/git.git

that does:

 (1) exactly the same as what the current code do without the new
     option, then

 (2) configure "include.path" to point at "~/.git-profile/open" at
     the very end

or something?  Then the "profile" files can have a shared setting
for the kinds of projects (the above examples are "open" projects,
and you would have another kind, "work" projects) that will apply to
all the projects of that nature. You update ~/.git-profile/open and
then that update applys to all repositories on your open projects.

The above is a tangent and independent from allowing the site owner
to set "user.requireExplicit = true" in /etc/gitconfig.

Re: git should not use a default user.email config value

[Aaron Schrab]

At 11:59 +0200 10 Aug 2013, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>I intentionally don't set user.email in my ~/.gitconfig because I use
>different identities (on the same machine) depending on what project I
>am committing to (open-source vs. work).  After I clone a repo, I *rely*
>on Git reminding me to set user.email on my first commit, because I
>invariably forget to set it myself.  And for me, *any* universal,
>heuristically-determined email address would be wrong for me for at
>least some repos.

I was in a similar situation for awhile.  Except in my case I had $EMAIL 
set for other reasons, so I didn't get the reminder even if git wasn't 
configured.

The solution I came up with was to use a template directory to have the 
following script installed as a pre-commit hook in all new repos:

  #!/bin/sh
  git config user.email > /dev/null && exit
  echo 'Set email address with `git config user.email` first' >&2
  exit 1

Re: git should not use a default user.email config value

[Andrew Ardill]

On 11 August 2013 02:58, Junio C Hamano <gitster@pobox.com> wrote:
> Perhaps we need a lighter-weight mechanism
>
>         git init --profile=open
>         git clone --profile=open git://git.kernel.org/git.git

This is something I would definitely use.

All of my work git directories are in a separate folder to my other
git directories, and as such it would be extremely convenient if every
repository under that folder defaulted to the same profile. That may
be asking for too much though!

Regards,

Andrew Ardill

Re: git should not use a default user.email config value

[Jeff King]

On Mon, Aug 12, 2013 at 09:52:45PM +1000, Andrew Ardill wrote:

> On 11 August 2013 02:58, Junio C Hamano <gitster@pobox.com> wrote:
> > Perhaps we need a lighter-weight mechanism
> >
> >         git init --profile=open
> >         git clone --profile=open git://git.kernel.org/git.git
> 
> This is something I would definitely use.
> 
> All of my work git directories are in a separate folder to my other
> git directories, and as such it would be extremely convenient if every
> repository under that folder defaulted to the same profile. That may
> be asking for too much though!

We could do something like the patch below, which allows:

  $ git config --global include./magic/.path .gitconfig-magic

to read ~/.gitconfig-magic only when we are in a repository with a
directory component "/magic/".

I can see how such a thing might be useful, even though I do not have a
use for that much flexibility myself. I find myself doing this trick for
things like editor settings, but not for git config. So do not count
this necessarily as a vote for doing this; it was a fun exercise for me
that others might find useful.

Comparing this against a "profile" type of solution:

  1. This handles only config, not full templates (so no custom hooks;
     however, we could provide a level of indirection for hooks inside
     the config).

  2. Unlike a profile that is used during repository init, this is
     resolved at runtime, so it keeps up to date as you change
     ~/.gitconfig-magic.

---
diff --git a/config.c b/config.c
index e13a7b6..a31dc85 100644
--- a/config.c
+++ b/config.c
@@ -119,10 +119,45 @@ int git_config_include(const char *var, const char *value, void *data)
 	return ret;
 }
 
+static NORETURN void die_bad_regex(int err, regex_t *re)
+{
+	char errbuf[1024];
+	regerror(err, re, errbuf, sizeof(errbuf));
+	if (cf && cf->name)
+		die("bad regex (at %s:%d): %s", cf->name, cf->linenr, errbuf);
+	else
+		die("bad regex: %s", errbuf);
+}
+
+static int match_repo_path(const char *re_str)
+{
+	regex_t re;
+	int ret;
+	const char *repo_path;
+
+	ret = regcomp(&re, re_str, REG_EXTENDED);
+	if (ret)
+		die_bad_regex(ret, &re);
+
+	repo_path = absolute_path(get_git_dir());
+	ret = regexec(&re, repo_path, 0, NULL, 0);
+	regfree(&re);
+	return !ret;
+}
+
+static int match_repo_path_mem(const char *re_buf, int len)
+{
+	char *re_str = xmemdupz(re_buf, len);
+	int ret = match_repo_path(re_str);
+	free(re_str);
+	return ret;
+}
+
 int git_config_include(const char *var, const char *value, void *data)
 {
 	struct config_include_data *inc = data;
-	const char *type;
+	const char *match, *type;
+	int match_len;
 	int ret;
 
 	/*
@@ -133,8 +168,9 @@ int git_config_include(const char *var, const char *value, void *data)
 	if (ret < 0)
 		return ret;
 
-	type = skip_prefix(var, "include.");
-	if (!type)
+	if (parse_config_key(var, "include", &match, &match_len, &type))
+		return ret;
+	if (match && !match_repo_path_mem(match, match_len))
 		return ret;
 
 	if (!strcmp(type, "path"))

Re: git should not use a default user.email config value

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]


Jeff King <peff@peff.net> writes:

> On Sat, Aug 10, 2013 at 11:59:21AM +0200, Michael Haggerty wrote:
>
>> I intentionally don't set user.email in my ~/.gitconfig because I use
>> different identities (on the same machine) depending on what project I
>> am committing to (open-source vs. work).  After I clone a repo, I *rely*
>> on Git reminding me to set user.email on my first commit, because I
>> invariably forget to set it myself.  And for me, *any* universal,
>> heuristically-determined email address would be wrong for me for at
>> least some repos.
>
> So if I understand your use case, then you would be even happier if
> rather than giving a warning, git simply barfed and said "please set
> your identity before committing"?

I also think it's a bug that git will create commits without an
explicitly-set author.  I've seen multiple cases of the author being
something unreasonable in a shared/official repository because of this.
One was a person's personal email address on a work-repo commit,
apparently because on Mac there was some magic extraction of primary
email address from Mail.app (but I'm not 100% clear on what happened).
If name/mail are not explicitly set, failing and making the user set
them seems like the right thing.

I find all the discussion of /etc/mailname to be a bit perplexing.  The
notion that the externally-visible email of a person making a commit
should be the same as if they sent mail from that machine seems to be a
bit of a stretch.  And their username might be different.  I don't think
it's possible to reliably figure out what ought to be in the git author
field.

Another reason to fail rather than use a possibly-wrong default is that
it's very difficult (if not impossible, depending on local CM policy
about forced updates in shared repos) to recover from pushing a commit
with a bad email address.  (And the people that don't set their email
right are the same people that won't run "git log -p @{u}.." before
pushing.)  But failing and having to set it manually is easy (people who
are already competent will be slowed down a minute or two, and the
others need to learn anyway), results in something that should have been
done anyway, and has no long-term negative consequences.


[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

Re: git should not use a default user.email config value

[Michael Haggerty]

On 08/12/2013 02:39 PM, Jeff King wrote:
> On Mon, Aug 12, 2013 at 09:52:45PM +1000, Andrew Ardill wrote:
> 
>> On 11 August 2013 02:58, Junio C Hamano <gitster@pobox.com> wrote:
>>> Perhaps we need a lighter-weight mechanism
>>>
>>>         git init --profile=open
>>>         git clone --profile=open git://git.kernel.org/git.git
>>
>> This is something I would definitely use.
>>
>> All of my work git directories are in a separate folder to my other
>> git directories, and as such it would be extremely convenient if every
>> repository under that folder defaulted to the same profile. That may
>> be asking for too much though!
> 
> We could do something like the patch below, which allows:
> 
>   $ git config --global include./magic/.path .gitconfig-magic
> 
> to read ~/.gitconfig-magic only when we are in a repository with a
> directory component "/magic/".
> 
> I can see how such a thing might be useful, even though I do not have a
> use for that much flexibility myself. I find myself doing this trick for
> things like editor settings, but not for git config. So do not count
> this necessarily as a vote for doing this; it was a fun exercise for me
> that others might find useful.

We could satisfy a whole class of wishes by supporting
user-wide/system-wide git hooks like

    ~/.githooks/{pre,post}-clone     /etc/githooks/{pre,post}-clone
    ~/.githooks/{pre,post}-init      /etc/githooks/{pre,post}-init

I suppose similar functionality could be implemented via git aliases,
but hook scripts are easier to install and share.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: git should not use a default user.email config value

[Andrew Ardill]

On 12 August 2013 22:39, Jeff King <peff@peff.net> wrote:
> We could do something like the patch below, which allows:
>
>   $ git config --global include./magic/.path .gitconfig-magic
>
> to read ~/.gitconfig-magic only when we are in a repository with a
> directory component "/magic/".

Thanks, this looks great! I'll have a play with it tomorrow.

Would locally configured config options override this one? From a
quick read of the patch there doesn't look like there is a way of
turning this off for a specific repository, but perhaps that is
unnecessary. I think after a bit of use the edge cases will be a bit
clearer.

Again thanks, this will scratch an itch I didn't even realise I had.

Regards,

Andrew Ardill

Re: git should not use a default user.email config value

[Jeff King]

On Mon, Aug 12, 2013 at 11:01:03PM +1000, Andrew Ardill wrote:

> On 12 August 2013 22:39, Jeff King <peff@peff.net> wrote:
> > We could do something like the patch below, which allows:
> >
> >   $ git config --global include./magic/.path .gitconfig-magic
> >
> > to read ~/.gitconfig-magic only when we are in a repository with a
> > directory component "/magic/".
> 
> Thanks, this looks great! I'll have a play with it tomorrow.
> 
> Would locally configured config options override this one? From a
> quick read of the patch there doesn't look like there is a way of
> turning this off for a specific repository, but perhaps that is
> unnecessary. I think after a bit of use the edge cases will be a bit
> clearer.

Yes, the usual config and include rules apply; the patch just selectively
ignores the include based on the subsection regex. So if you put the
magic include in your ~/.gitconfig, anything in the repo's .git/config
will override it.

But that also means the usual restrictions apply, too. There is no way
to "unset" a variable as if it had never been specified in the first
place. And multi-valued variables will always append (e.g.,
remote.*.fetch).

The matcher is a regex, so depending on how tortured you want your regex
to get, you can probably exclude a particular directory with that. :)

-Peff

Re: git should not use a default user.email config value

[Jeff King]

On Mon, Aug 12, 2013 at 02:54:13PM +0200, Michael Haggerty wrote:

> We could satisfy a whole class of wishes by supporting
> user-wide/system-wide git hooks like
> 
>     ~/.githooks/{pre,post}-clone     /etc/githooks/{pre,post}-clone
>     ~/.githooks/{pre,post}-init      /etc/githooks/{pre,post}-init
> 
> I suppose similar functionality could be implemented via git aliases,
> but hook scripts are easier to install and share.

I don't mind something like that, as it is very flexible. But I have a
feeling most uses would end up just symlinking some template hooks or
config.  At which point we might be better serving the user to provide a
solution that is simpler to use (e.g., a ~/.githooks directory that is
checked for all hooks; the tricky part there would be making rules for
the case that there are system, user, and repo-level scripts for a
particular hook).

-Peff

Re: git should not use a default user.email config value

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

>  (2) configure "include.path" to point at "~/.git-profile/open" at
>      the very end

I'd rather have it ~/.config/git/profile/ (or
$XDG_CONFIG_HOME/git/profile if $XDG_CONFIG_HOME is set), but the
proposal makes sense.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git should not use a default user.email config value

[Matthieu Moy]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Hi Thorsten,
>
> Thorsten Glaser wrote[1]:
>
>> git config user.email SHOULD NOT default to $(id -un)@$(hostname -f)
>> because just too many cow-orkers seem to be unable to follow basic
>> instructions
>
> Heh.
>
> Can you say a little more about your setup?  In a university
> environment with sysadmin-managed email and /etc/mailname set up
> correctly it is handy that people can start working without doing
> anything special to configure git's "[user] email" setting.

I also work with a university environment. The guessed user.email is
almost right (actually, it's not the official email address, but an
internal one we ask students not to use). Still, I'd love to see Git
error out by default, as most students use Git from several machines.
They usually learn and write their first ~/.gitconfig on the school's
machines, and then start working from their personal laptops, where the
guessed user.email is plain wrong.

We do teach them to set user.email in ~/.gitconfig as a very first step,
but many don't (because they don't read the tutorial, or because they do
something wrong like putting .gitconfig in the wrong directory). We do
tell them to set up ~/.gitconfig on every host they work from, but many
don't either. And unfortunately, the warning is not scary enough for
some of them :-\ ("Err, did I get a warning? where?").

An opt-in auto-detection would be cool for people who really work in a
controlled environment, so that the sysadmin could enable it from
/etc/gitconfig.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git should not use a default user.email config value

[Matthieu Moy]

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

> This is how to implement that:
>
> From f1feaa05ce3772d8006078c4aeabcbd55b52d58e Mon Sep 17 00:00:00 2001
> From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
> Date: Tue, 13 Nov 2012 07:33:12 +0100
> Subject: [PATCH] ident: don't allow implicit email addresses
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  ident.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ident.c b/ident.c
> index 1c123e6..85fc729 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -301,9 +301,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) {

That's a first step, but something should also be done in
builtin/commit.c, which currently displays a detailed warning
(implicit_ident_advice) /after/ performing the commit. I think your
patch would turn this warning into dead code.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git should not use a default user.email config value

[Thorsten Glaser]

Matthieu Moy dixit:

>An opt-in auto-detection would be cool for people who really work in a
>controlled environment, so that the sysadmin could enable it from

Sounds like a plan ;-)

I think with several people chiming in on this, while that proposal
would affect a majority of people, it would do so in a less intrusive
way as the current behaviour of autodetection which negatively affects
some users, although not few either, in a strong way.

bye,
//mirabilos
-- 
> emacs als auch vi zum Kotzen finde (joe rules) und pine für den einzig
> bedienbaren textmode-mailclient halte (und ich hab sie alle ausprobiert). ;)
Hallooooo, ich bin der Holger ("Hallo Holger!"), und ich bin ebenfalls
... pine-User, und das auch noch gewohnheitsmäßig ("Oooooooohhh").  [aus dasr]

Re: git should not use a default user.email config value

[Andrew Ardill]

On Mon, Aug 12, 2013 at 11:01:03PM +1000, Andrew Ardill wrote:
>On 12 August 2013 22:39, Jeff King <peff@peff.net> wrote:
>> We could do something like the patch below, which allows:
>>
>>   $ git config --global include./magic/.path .gitconfig-magic
>>
>> to read ~/.gitconfig-magic only when we are in a repository with a
>> directory component "/magic/".
>
> Thanks, this looks great! I'll have a play with it tomorrow.

I applied this on top of latest next (1da3ebde8999d07), and it worked
perfectly for my use case.

For what it's worth, it also passed the test suite!

Would be great to see this, or something on the same theme, get into
master. I'd be happy to review patches/write tests/write documentation
if needed.

Regards,

Andrew Ardill

Re: git should not use a default user.email config value

[Jeff King]

On Tue, Aug 13, 2013 at 09:05:40PM +1000, Andrew Ardill wrote:

> I applied this on top of latest next (1da3ebde8999d07), and it worked
> perfectly for my use case.
> 
> For what it's worth, it also passed the test suite!
> 
> Would be great to see this, or something on the same theme, get into
> master. I'd be happy to review patches/write tests/write documentation
> if needed.

Like I said, I do not have a particular use for it, but I don't think it
would hurt anybody who does not use it. If you want to polish it up into
a real patch with docs and tests, I don't mind.

The only downside I can think of is that we might want to use the
subsection in "include.SUBSECTION.*" for some other limiting conditions
(e.g., "only include this config when running version >= X.Y", or even
"include only when environment variable FOO is true").

I guess we could do something like:

  [include "repo:...your regex here..."]
    path = .gitconfig-only-for-some-repos
  [include "env:USE_MY_MAGIC_CONFIG"]
    path = .gitconfig-only-when-magic-env-set

Adding the "repo:" prefix for this repo-dir matching is pretty trivial.
Adding a similar env-matching is only slightly less trivial; but does
anybody actually want it?

-Peff

Re: git should not use a default user.email config value

[Jeff King]

On Tue, Aug 13, 2013 at 07:46:35AM -0400, Jeff King wrote:

> The only downside I can think of is that we might want to use the
> subsection in "include.SUBSECTION.*" for some other limiting conditions
> (e.g., "only include this config when running version >= X.Y", or even
> "include only when environment variable FOO is true").
> 
> I guess we could do something like:
> 
>   [include "repo:...your regex here..."]
>     path = .gitconfig-only-for-some-repos
>   [include "env:USE_MY_MAGIC_CONFIG"]
>     path = .gitconfig-only-when-magic-env-set
> 
> Adding the "repo:" prefix for this repo-dir matching is pretty trivial.
> Adding a similar env-matching is only slightly less trivial; but does
> anybody actually want it?

Here it is with the "repo:" prefix, if you want to build on that.

Adding the "env" spec is as easy as doing this on top:


	diff --git a/config.c b/config.c
	index f1ca6fa..64ba141 100644
	--- a/config.c
	+++ b/config.c
	@@ -150,6 +150,8 @@ static int match_config_include(const char *spec)
	 	const char *val;
	 	if ((val = skip_prefix(spec, "repo:")))
	 		return match_repo_path(val);
	+	if ((val = skip_prefix(spec, "env:")))
	+		return git_env_bool(val, 0);
	 
	 	/* Unknown specs are considered "no match". */
	 	return 0;

---
diff --git a/config.c b/config.c
index e13a7b6..f1ca6fa 100644
--- a/config.c
+++ b/config.c
@@ -119,10 +119,55 @@ int git_config_include(const char *var, const char *value, void *data)
 	return ret;
 }
 
+static NORETURN void die_bad_regex(int err, regex_t *re)
+{
+	char errbuf[1024];
+	regerror(err, re, errbuf, sizeof(errbuf));
+	if (cf && cf->name)
+		die("bad regex (at %s:%d): %s", cf->name, cf->linenr, errbuf);
+	else
+		die("bad regex: %s", errbuf);
+}
+
+static int match_repo_path(const char *re_str)
+{
+	regex_t re;
+	int ret;
+	const char *repo_path;
+
+	ret = regcomp(&re, re_str, REG_EXTENDED);
+	if (ret)
+		die_bad_regex(ret, &re);
+
+	repo_path = absolute_path(get_git_dir());
+	ret = regexec(&re, repo_path, 0, NULL, 0);
+	regfree(&re);
+	return !ret;
+}
+
+static int match_config_include(const char *spec)
+{
+	const char *val;
+	if ((val = skip_prefix(spec, "repo:")))
+		return match_repo_path(val);
+
+	/* Unknown specs are considered "no match". */
+	return 0;
+}
+
+static int match_config_include_mem(const char *spec, int spec_len)
+{
+	char *spec_str = xmemdupz(spec, spec_len);
+	int ret = match_config_include(spec_str);
+	free(spec_str);
+	return ret;
+}
+
 int git_config_include(const char *var, const char *value, void *data)
 {
 	struct config_include_data *inc = data;
-	const char *type;
+	const char *match, *type;
+	int match_len;
 	int ret;
 
 	/*
@@ -133,8 +178,9 @@ int git_config_include(const char *var, const char *value, void *data)
 	if (ret < 0)
 		return ret;
 
-	type = skip_prefix(var, "include.");
-	if (!type)
+	if (parse_config_key(var, "include", &match, &match_len, &type))
+		return ret;
+	if (match && !match_config_include_mem(match, match_len))
 		return ret;
 
 	if (!strcmp(type, "path"))

Re: git should not use a default user.email config value

[Andrew Ardill]

On 13 August 2013 21:46, Jeff King <peff@peff.net> wrote:

> Like I said, I do not have a particular use for it, but I don't think it
> would hurt anybody who does not use it. If you want to polish it up into
> a real patch with docs and tests, I don't mind.

I'll have a go at this.

> The only downside I can think of is that we might want to use the
> subsection in "include.SUBSECTION.*" for some other limiting conditions
> (e.g., "only include this config when running version >= X.Y", or even
> "include only when environment variable FOO is true").

It seems as though gitconfig doesn't have a standard way of dealing
with 'sub-subsections', which is essentially what this is trying to
implement.

It makes sense that there could be different 'modes' of includes.
These could be the ones you mentioned already, such as repo and env,
but could also be things like branch where the config changes
depending on which branch you are on. Ideally, multiple entries per
mode would be allowed.
Implementing all that initially would be overkill however if this sort
of functionality is desirable the ability to easily add new modes
would be a great boon down the track.

The four pieces of information we need to include are that this is an
include, the path to the include, the mode, and the mode specific
parameter. Your proposal is to allow the sub-subsection by
concatenating with a ":" like this

[include "<mode>:<mode-param>]
  path = <path>

Alternatively, we could allow chaining of subsections (couldn't find
any previous discussion on this) by adding whitespace between each
subsection. Seems like lots of potentially unnecessary work, but maybe
this has already been discussed or is the most appropriate way of
doing it.

$ git config --global include.repo./magic/.path ~/.gitconfig-magic

[include repo "/magic/"]
   path = .gitconfig-magic

We could also require a unique key that grouped the options together.
This seems like the easiest and most flexible method, and doesn't
require any 'special' considerations for the subsection. It would be
harder for a user to configure, and the concept of a mode seems less
intuitive.

$ git config --global include.magicrepos.mode repo
$ git config --global include.magicrepos.param /magic/
$ git config --global include.magicrepos.path ~/.gitconfig-magic

[include "magicrepos"]
  mode = repo
  param = "/magic/"
  path = ~/.gitconfig-magic

Of the three I probably think the subsection chaining is the nicest
overall, though your original "repo:" proposal seems to be the easiest
to implement.

Regards,

Andrew Ardill

Re: git should not use a default user.email config value

[Jeff King]

On Tue, Aug 13, 2013 at 10:52:34PM +1000, Andrew Ardill wrote:

> > The only downside I can think of is that we might want to use the
> > subsection in "include.SUBSECTION.*" for some other limiting conditions
> > (e.g., "only include this config when running version >= X.Y", or even
> > "include only when environment variable FOO is true").
> 
> It seems as though gitconfig doesn't have a standard way of dealing
> with 'sub-subsections', which is essentially what this is trying to
> implement.

Right. Syntactically, the config keys are:

  SECTION.SUBSECTION.KEY

where SUBSECTION is optional. SECTION and KEY cannot contain spaces or
dots and are case insensitive, but SUBSECTION is handled literally. It
can contain whatever data is useful to the config parser (for example,
remote names, branch names, or even URLs), including spaces or dots.

We could introduce the notion of sub-subsections, but that would not
play well with existing uses of subsection, which assume that they can
put arbitrary data into it.

> It makes sense that there could be different 'modes' of includes.
> These could be the ones you mentioned already, such as repo and env,
> but could also be things like branch where the config changes
> depending on which branch you are on. Ideally, multiple entries per
> mode would be allowed.
>
> Implementing all that initially would be overkill however if this sort
> of functionality is desirable the ability to easily add new modes
> would be a great boon down the track.

Right. We don't have to decide on all of it now; we just have to leave
the door open syntactically for future growth.

> The four pieces of information we need to include are that this is an
> include, the path to the include, the mode, and the mode specific
> parameter. Your proposal is to allow the sub-subsection by
> concatenating with a ":" like this
> 
> [include "<mode>:<mode-param>]
>   path = <path>

Right. The config parser does not care about the sub-subsection; it is
up to the interpreter of the key to split the subsection if it chooses.
I arbitrarily chose ":" as the internal delimiter because I thought it
looked nice. You could make it dot or space, too.

> Alternatively, we could allow chaining of subsections (couldn't find
> any previous discussion on this) by adding whitespace between each
> subsection. Seems like lots of potentially unnecessary work, but maybe
> this has already been discussed or is the most appropriate way of
> doing it.
> 
> $ git config --global include.repo./magic/.path ~/.gitconfig-magic
> 
> [include repo "/magic/"]
>    path = .gitconfig-magic

I don't think it has been discussed before. But as I mentioned above,
you would not want to apply this everywhere. For existing config
callbacks, they want to take the section literally. So it is going to be
up to the callback to parse the section into subsections anyway, at
which point it does not really matter what syntax you use.

We could teach the config parser to normalize:

  [section with many spaces]
    key

as "section.with.many.spaces.key" or "section.with many spaces.key" (I
do not think it is even valid in today's code, but I didn't check). But
personally I do not find that any easier to read or understand than the
colon syntax.

> This seems like the easiest and most flexible method, and doesn't
> require any 'special' considerations for the subsection. It would be
> harder for a user to configure, and the concept of a mode seems less
> intuitive.
> 
> $ git config --global include.magicrepos.mode repo
> $ git config --global include.magicrepos.param /magic/
> $ git config --global include.magicrepos.path ~/.gitconfig-magic
> 
> [include "magicrepos"]
>   mode = repo
>   param = "/magic/"
>   path = ~/.gitconfig-magic

Yeah, that is the most flexible. You could introduce multiple conditions
or other options, as well (e.g., instead of mode and param, have
include.magic.repo, include.magic.env, etc). But it seems like
over-engineering. I do not mind making the code a little harder to
write, but it seems unnecessarily complicated for the user, too.

> Of the three I probably think the subsection chaining is the nicest
> overall, though your original "repo:" proposal seems to be the easiest
> to implement.

I think I favor the colon proposal because of its simplicity. And
because the sub-section chaining cannot be applied consistently across
config keys, I don't think there is much value in trying to introduce a
new general config syntax.

-Peff

Re: git should not use a default user.email config value

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> diff --git a/config.c b/config.c
> index e13a7b6..a31dc85 100644
> --- a/config.c
> +++ b/config.c
> @@ -119,10 +119,45 @@ int git_config_include(const char *var, const char *value, void *data)
>  	return ret;
>  }
>  
> +static NORETURN void die_bad_regex(int err, regex_t *re)
> +{
> +	char errbuf[1024];
> +	regerror(err, re, errbuf, sizeof(errbuf));
> +	if (cf && cf->name)
> +		die("bad regex (at %s:%d): %s", cf->name, cf->linenr, errbuf);
> +	else
> +		die("bad regex: %s", errbuf);
> +}
> +
> +static int match_repo_path(const char *re_str)
> +{
> +	regex_t re;
> +	int ret;
> +	const char *repo_path;
> +
> +	ret = regcomp(&re, re_str, REG_EXTENDED);
> +	if (ret)
> +		die_bad_regex(ret, &re);
> +
> +	repo_path = absolute_path(get_git_dir());
> +	ret = regexec(&re, repo_path, 0, NULL, 0);
> +	regfree(&re);
> +	return !ret;

We do this every time during the parsing?

Hmph, if you had "include.repo:/home/junio/frotz/.path" and
"include.repo:/srv/project/git.git/.path" in your ~/.gitconfig,
then a single regexp that is lazily prepared once will not cut it,
so I guess that you cannot avoid it.

Unlike "git init|clone --profile=foo" that requires you to be
explicit about your profile upon invocation, this mechanism is much
easier to use by having include.<magic>.path in some global
configuration, and the existing precedence rule makes it perfect.
By starting /etc/gitconfig and/or your $HOME/.gitconfig with series
of include.<magic>.path, you can have the default definitions
included from these magic include to take effect before anything
else, and settings from other configuration files can override it.

Re: git should not use a default user.email config value

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I guess we could do something like:
>
>   [include "repo:...your regex here..."]
>     path = .gitconfig-only-for-some-repos
>   [include "env:USE_MY_MAGIC_CONFIG"]
>     path = .gitconfig-only-when-magic-env-set

I am not sure if "env" is very useful, but there certainly are other
possibilities (e.g. apply this only on this host, only for members
of this UNIX group, etc.), so having "repo:" prefix even if we only
support the repository path mapping in the initial version is a good
way forward.

Thanks.

Re: git should not use a default user.email config value

[Michael Haggerty]

On 08/13/2013 01:46 PM, Jeff King wrote:
> On Tue, Aug 13, 2013 at 09:05:40PM +1000, Andrew Ardill wrote:
> 
>> I applied this on top of latest next (1da3ebde8999d07), and it worked
>> perfectly for my use case.
>>
>> For what it's worth, it also passed the test suite!
>>
>> Would be great to see this, or something on the same theme, get into
>> master. I'd be happy to review patches/write tests/write documentation
>> if needed.
> 
> Like I said, I do not have a particular use for it, but I don't think it
> would hurt anybody who does not use it. If you want to polish it up into
> a real patch with docs and tests, I don't mind.
> 
> The only downside I can think of is that we might want to use the
> subsection in "include.SUBSECTION.*" for some other limiting conditions
> (e.g., "only include this config when running version >= X.Y", or even
> "include only when environment variable FOO is true").
> 
> I guess we could do something like:
> 
>   [include "repo:...your regex here..."]
>     path = .gitconfig-only-for-some-repos
>   [include "env:USE_MY_MAGIC_CONFIG"]
>     path = .gitconfig-only-when-magic-env-set
> 
> Adding the "repo:" prefix for this repo-dir matching is pretty trivial.
> Adding a similar env-matching is only slightly less trivial; but does
> anybody actually want it?

Gaaak!  Let me again plead for supporting a post-clone hook rather than
inventing some crazy config-file syntax that is becoming ever more
complicated.  A post-clone hook would make all of these things that have
been suggested pretty easy, and would also open lots of other
possibilities, all without further changes in git.core, like (I'm just
brainstorming here):

    #! /bin/sh

    remote="$1"

    ln -s $(HOME)/.githooks/* .git/hooks

    case "$(git --version)" in
    *.1.[78].*)
        git config include.path "$(HOME)/.gitinclude
        ;;
    esac

    echo "(cd $(pwd) && git gc)" >>"$(HOME)/cron.weekly/git-gc"

    case "$remote" in
    *.work.com/*)
        git config user.email me@work.com
        ;;
    *.github.com/*)
        git config user.email me@debian.org
        ;;
    *)
        echo '### Remember to set user.email ###'
        ;;
    esac

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: git should not use a default user.email config value

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> I guess we could do something like:
>>
>>   [include "repo:...your regex here..."]
>>     path = .gitconfig-only-for-some-repos
>>   [include "env:USE_MY_MAGIC_CONFIG"]
>>     path = .gitconfig-only-when-magic-env-set
>
> I am not sure if "env" is very useful, but there certainly are other
> possibilities (e.g. apply this only on this host, only for members
> of this UNIX group, etc.)

I have already wished I had "git version >= XXX" here (but that's tricky
to implement).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git should not use a default user.email config value

[Jeff King]

On Wed, Aug 14, 2013 at 09:09:05AM +0200, Michael Haggerty wrote:

> Gaaak!  Let me again plead for supporting a post-clone hook rather than
> inventing some crazy config-file syntax that is becoming ever more
> complicated.  A post-clone hook would make all of these things that have
> been suggested pretty easy, and would also open lots of other
> possibilities, all without further changes in git.core, like (I'm just
> brainstorming here):

My problem with a post-clone hook is that it only runs once, and then
potentially goes stale.  For example:

>     ln -s $(HOME)/.githooks/* .git/hooks

Because of the symlink, this tracks hooks as they are updated, but what
happens when you add a new hook (or delete one)? You have to manually
hunt down each repository using it and update the links. You can get it
around it by replacing and symlinking the whole hook directory, though.

>     case "$(git --version)" in
>     *.1.[78].*)
>         git config include.path "$(HOME)/.gitinclude
>         ;;
>     esac

What happens when you upgrade (or downgrade) your git, or even use
multiple versions interleaved? You need to revisit this version check.

>     echo "(cd $(pwd) && git gc)" >>"$(HOME)/cron.weekly/git-gc"

What happens when you move your repository to a different directory? You
have to manually fix up the generated cron script.

>     case "$remote" in
>     *.work.com/*)
>         git config user.email me@work.com
>         ;;
>     *.github.com/*)
>         git config user.email me@debian.org
>         ;;
>     *)
>         echo '### Remember to set user.email ###'
>         ;;
>     esac

What happens when you update your address? You have to manually fix up
each repository.

I agree that running arbitrarily shell code is the most flexible thing,
but I think in many cases users would prefer to have something that
makes decisions at runtime, rather than having to remember to update
existing repositories with changes. That can be shell code, too, though
there are complications (performance and security come to mind).

I do not see the two features as necessarily an either-or; they can
accomplish the same thing, but with different tradeoffs in complexity
for the user.

-Peff

Re: git should not use a default user.email config value

[Jeff King]

On Wed, Aug 14, 2013 at 09:28:24AM +0200, Matthieu Moy wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Jeff King <peff@peff.net> writes:
> >
> >> I guess we could do something like:
> >>
> >>   [include "repo:...your regex here..."]
> >>     path = .gitconfig-only-for-some-repos
> >>   [include "env:USE_MY_MAGIC_CONFIG"]
> >>     path = .gitconfig-only-when-magic-env-set
> >
> > I am not sure if "env" is very useful, but there certainly are other
> > possibilities (e.g. apply this only on this host, only for members
> > of this UNIX group, etc.)
> 
> I have already wished I had "git version >= XXX" here (but that's tricky
> to implement).

I assume it is "because version XXX understands config option Y, but
older versions do not"[1]. Rather than ask for version XXX, then, you
could ask for

  [include "option:Y"]
    path = ...

and versions which understand Y (which happens to be XXX or greater)
would internally know that and consider the conditional true.

This whole discussion is basically implementing conditional config. In
my patch, the conditional is limited only to including other config. But
if you have many such conditions (and especially if each one only has
one varying config key), the result can be unwieldy. Another way of
doing this would be to introduce a conditional syntax to ignore or
respect some part of the file. The problem is that it would be tricky to
do in a backwards-compatible way.

-Peff

[1] I used to run into this with pager.*, which originally could only be
    a bool, but later learned to take custom pagers. I solved it with:

      git config --file .gitconfig-pager pager.diff ...
      git config --global include.path .gitconfig-pager

    which does not need a version or option conditional, because the
    option was added _before_ the include feature. IOW, older versions
    of git ignore it, and any which actually respect the include will
    know how to handle custom pagers. But that does not work with
    changes that came after the include feature was added. :)

Re: git should not use a default user.email config value

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> This whole discussion is basically implementing conditional config.
> [...] The problem is that it would be tricky to do in a
> backwards-compatible way.

That could be done with "conditional comments" like

# if <some-condition> then
[core]
        pager = less
# endif

That's rather ugly, and the implementation would be even more ugly, but
backward-compatible.

> [1] I used to run into this with pager.*, which originally could only be
>     a bool, but later learned to take custom pagers. I solved it with:
>
>       git config --file .gitconfig-pager pager.diff ...
>       git config --global include.path .gitconfig-pager

Same here, with push.default = upstream, which breaks old versions of
Git ;-).

(I have a recent Git on my desktop, and my $HOME is shared with a server
running Debian oldstable)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: git should not use a default user.email config value

[Junio C Hamano]

On Wed, Aug 14, 2013 at 1:37 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> > This whole discussion is basically implementing conditional config.
> > [...] The problem is that it would be tricky to do in a
> > backwards-compatible way.
>
> That could be done with "conditional comments" like
>
> # if <some-condition> then
> [core]
>         pager = less
> # endif
>
> That's rather ugly, and the implementation would be even more ugly, but
> backward-compatible.


I highly doubt that you would want to be "backward compatible" in this
case, though.
The section of the configuration you are enclosing the new if/endif
syntax may be
understood only by newer Git (e.g. imagine core.pager is still
bool-only today), and
older Git that do not understand if/endif syntax will happily read
that section and
choke on it, no?

Re: git should not use a default user.email config value

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> On Wed, Aug 14, 2013 at 1:37 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>
>> Jeff King <peff@peff.net> writes:
>>
>> > This whole discussion is basically implementing conditional config.
>> > [...] The problem is that it would be tricky to do in a
>> > backwards-compatible way.
>>
>> That could be done with "conditional comments" like
>>
>> # if <some-condition> then
>> [core]
>>         pager = less
>> # endif
>>
>> That's rather ugly, and the implementation would be even more ugly, but
>> backward-compatible.
>
>
> I highly doubt that you would want to be "backward compatible" in this
> case, though.
> The section of the configuration you are enclosing the new if/endif
> syntax may be
> understood only by newer Git (e.g. imagine core.pager is still
> bool-only today), and
> older Git that do not understand if/endif syntax will happily read
> that section and
> choke on it, no?

Indeed. That would be more

# if <some-condition> then
# [core]
#       pager = less
# endif

which is even more ugly ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

conditional config syntax

[Jeff King]

[updated subject, as we are very far off the original topic]

On Wed, Aug 14, 2013 at 07:00:58AM -0700, Junio C Hamano wrote:

> > > This whole discussion is basically implementing conditional config.
> > > [...] The problem is that it would be tricky to do in a
> > > backwards-compatible way.
> >
> > That could be done with "conditional comments" like
> >
> > # if <some-condition> then
> > [core]
> >         pager = less
> > # endif
> >
> > That's rather ugly, and the implementation would be even more ugly, but
> > backward-compatible.
> 
> I highly doubt that you would want to be "backward compatible" in this
> case, though.  The section of the configuration you are enclosing the
> new if/endif syntax may be understood only by newer Git (e.g. imagine
> core.pager is still bool-only today), and older Git that do not
> understand if/endif syntax will happily read that section and choke on
> it, no?

I would think the ideal behavior would be for existing implementations
to just not include the conditional section.

If we take the conditional by default in existing versions of git (i.e.,
the behavior of Matthieu's proposal), then any "do this only if version
X or greater" conditional is going to be inconsistent (it will be true
for old versions, not true for versions which understand conditionals
but pre-date X, and then true again for the actual versions you want).

Likewise, if we introduce some new non-backwards-compatible syntax that
existing Git chokes on, then you have created a new compatibility
problem. You cannot use older versions of git, which is the exact
problem a version conditional is trying to solve.

That is one of the reasons that include.path is designed as it is; old
versions accept it and do nothing (unless you specifically ask for it as
a value). And likewise, include.*.path will do nothing for existing
versions of git.

Or hmm. Maybe that is what you mean by "choke on it". Choke on the
invalid config, not on the new syntax.

-Peff

Re: conditional config syntax

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Or hmm. Maybe that is what you mean by "choke on it". Choke on the
> invalid config, not on the new syntax.

Yes ;-)