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

2013-08-14 Thread 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 m...@work.com
;;
*.github.com/*)
git config user.email m...@debian.org
;;
*)
echo '### Remember to set user.email ###'
;;
esac

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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: git should not use a default user.email config value

2013-08-14 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 Jeff King p...@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/
--
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: git should not use a default user.email config value

2013-08-14 Thread 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 m...@work.com
 ;;
 *.github.com/*)
 git config user.email m...@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
--
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: git should not use a default user.email config value

2013-08-14 Thread Jeff King
On Wed, Aug 14, 2013 at 09:28:24AM +0200, Matthieu Moy wrote:

 Junio C Hamano gits...@pobox.com writes:
 
  Jeff King p...@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. :)
--
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: git should not use a default user.email config value

2013-08-14 Thread Matthieu Moy
Jeff King p...@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/
--
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: git should not use a default user.email config value

2013-08-14 Thread Junio C Hamano
On Wed, Aug 14, 2013 at 1:37 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:

 Jeff King p...@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?
--
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: git should not use a default user.email config value

2013-08-14 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 On Wed, Aug 14, 2013 at 1:37 AM, Matthieu Moy
 matthieu@grenoble-inp.fr wrote:

 Jeff King p...@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/
--
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: git should not use a default user.email config value

2013-08-13 Thread Matthieu Moy
Junio C Hamano gits...@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/
--
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: git should not use a default user.email config value

2013-08-13 Thread Matthieu Moy
Jonathan Nieder jrnie...@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/
--
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: git should not use a default user.email config value

2013-08-13 Thread Matthieu Moy
Felipe Contreras felipe.contre...@gmail.com writes:

 This is how to implement that:

 From f1feaa05ce3772d8006078c4aeabcbd55b52d58e Mon Sep 17 00:00:00 2001
 From: Felipe Contreras 2nd felipe.contrera...@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.contre...@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/
--
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: git should not use a default user.email config value

2013-08-13 Thread 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). ;)
Hallo, ich bin der Holger (Hallo Holger!), und ich bin ebenfalls
... pine-User, und das auch noch gewohnheitsmäßig (Oooohhh).  [aus dasr]
--
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: git should not use a default user.email config value

2013-08-13 Thread Andrew Ardill
On Mon, Aug 12, 2013 at 11:01:03PM +1000, Andrew Ardill wrote:
On 12 August 2013 22:39, Jeff King p...@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
--
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: git should not use a default user.email config value

2013-08-13 Thread 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
--
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: git should not use a default user.email config value

2013-08-13 Thread 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))
--
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: git should not use a default user.email config value

2013-08-13 Thread Andrew Ardill
On 13 August 2013 21:46, Jeff King p...@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
--
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: git should not use a default user.email config value

2013-08-13 Thread 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
--
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: git should not use a default user.email config value

2013-08-13 Thread Junio C Hamano
Jeff King p...@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.
--
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: git should not use a default user.email config value

2013-08-13 Thread Junio C Hamano
Jeff King p...@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.
--
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: git should not use a default user.email config value

2013-08-12 Thread Andrew Ardill
On 11 August 2013 02:58, Junio C Hamano gits...@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
--
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: git should not use a default user.email config value

2013-08-12 Thread Jeff King
On Mon, Aug 12, 2013 at 09:52:45PM +1000, Andrew Ardill wrote:

 On 11 August 2013 02:58, Junio C Hamano gits...@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))
--
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: git should not use a default user.email config value

2013-08-12 Thread Greg Troxel

Jeff King p...@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.



pgpGtvyDdMd2C.pgp
Description: PGP signature


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

2013-08-12 Thread 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 gits...@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
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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: git should not use a default user.email config value

2013-08-12 Thread Andrew Ardill
On 12 August 2013 22:39, Jeff King p...@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
--
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: git should not use a default user.email config value

2013-08-12 Thread Jeff King
On Mon, Aug 12, 2013 at 11:01:03PM +1000, Andrew Ardill wrote:

 On 12 August 2013 22:39, Jeff King p...@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
--
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: git should not use a default user.email config value

2013-08-12 Thread 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
--
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: git should not use a default user.email config value

2013-08-10 Thread Jeff King
On Fri, Aug 09, 2013 at 04:06:16PM -0700, Junio C Hamano wrote:

 Jeff King p...@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
--
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: git should not use a default user.email config value

2013-08-10 Thread 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
u...@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
--
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: git should not use a default user.email config value

2013-08-10 Thread 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
--
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: git should not use a default user.email config value

2013-08-10 Thread 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
--
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: git should not use a default user.email config value

2013-08-10 Thread 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
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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: git should not use a default user.email config value

2013-08-10 Thread 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
--
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: git should not use a default user.email config value

2013-08-10 Thread 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
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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: git should not use a default user.email config value

2013-08-10 Thread Andreas Schwab
Jeff King p...@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, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
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: git should not use a default user.email config value

2013-08-10 Thread 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
--
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: git should not use a default user.email config value

2013-08-09 Thread 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 y...@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 t...@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
--
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: git should not use a default user.email config value

2013-08-09 Thread 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 'de...@pc-bn-041.lan.tarent.de'
instead of 'd.e...@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
--
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: git should not use a default user.email config value

2013-08-09 Thread Felipe Contreras
On Fri, Aug 9, 2013 at 3:00 PM, Thorsten Glaser t...@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 'de...@pc-bn-041.lan.tarent.de'
 instead of 'd.e...@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.contrera...@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.contre...@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
--
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: git should not use a default user.email config value

2013-08-09 Thread 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 t...@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 p...@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 y...@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
--
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: git should not use a default user.email config value

2013-08-09 Thread Junio C Hamano
Jeff King p...@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
--
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: git should not use a default user.email config value

2013-08-09 Thread 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
--
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