[PATCH] builtin/commit.c: prevent bad commits

[Dan Aloni]

These commits which have hashes starting with the hex string 'bad',
always give me the chills. Why should a perfectly good commit be
jinxed?

Statistically, one of 4096 commits may be 'bad'. This change adds a
'--prevent-bad' switch to the commit command in order to prevent such
commit hashes from being generated. Internally, the commit is retried
with a slight commit meta-data modification - a newline is added to the
end of the commit message. The meta-data change results in a different
hash, that if we are lucky enough (4095/4096 chance) may not be 'bad'.

Note that this change does not affect actual software quality maintained
using Git. Thus, it is recommended keep testing all generated versions
regardless of commit hash jinxes.

Signed-off-by: Dan Aloni <alo...@gmail.com>
---
 builtin/commit.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 37fcb55ab0a0..afaa7cefaedf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -89,6 +89,7 @@ static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message;
+static int prevent_bad;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, 
*ignored_arg;
 static char *sign_commit;
 
@@ -1449,6 +1450,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
OPT_BOOL('z', "null", _termination,
 N_("terminate entries with NUL")),
OPT_BOOL(0, "amend", , N_("amend previous commit")),
+   OPT_BOOL(0, "prevent-bad", _bad, N_("prevent a bad 
commit")),
OPT_BOOL(0, "no-post-rewrite", _post_rewrite, N_("bypass 
post-rewrite hook")),
{ OPTION_STRING, 'u', "untracked-files", _files_arg, 
N_("mode"), N_("show untracked files, optional modes: all, normal, no. 
(Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
/* end commit contents options */
@@ -1583,12 +1585,34 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
append_merge_tag_headers(parents, );
}
 
-   if (commit_tree_extended(sb.buf, sb.len, _cache_tree->oid,
-parents, , author_ident.buf, sign_commit,
-extra)) {
-   rollback_index_files();
-   die(_("failed to write commit object"));
+   for (;;) {
+   char *oid_hex;
+   struct commit_list *copy_parents;
+
+   copy_parents = copy_commit_list(parents);
+
+   if (commit_tree_extended(sb.buf, sb.len, 
_cache_tree->oid,
+parents, , author_ident.buf, 
sign_commit,
+extra)) {
+   rollback_index_files();
+   die(_("failed to write commit object"));
+   }
+
+   oid_hex = oid_to_hex();
+   if (prevent_bad &&
+   oid_hex[0] == 'b' &&
+   oid_hex[1] == 'a' &&
+   oid_hex[2] == 'd' )
+   {
+   parents = copy_parents;
+   strbuf_add(, "\n", 1);
+   continue;
+   }
+
+   free_commit_list(copy_parents);
+   break;
}
+
strbuf_release(_ident);
free_commit_extra_headers(extra);
 
-- 
2.14.3

[PATCH v8 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

It used to be that:

   git config --global user.email "(none)"

was a viable way for people to force themselves to set user.email in
each repository.  This was helpful for people with more than one
email address, targeting different email addresses for different
clones, as it barred git from creating a commit unless the user.email
config was set in the per-repo config to the correct email address.

A recent change, 19ce497c (ident: keep a flag for bogus
default_email, 2015-12-10), however, declared that an explicitly
configured user.email is not bogus, no matter what its value is, so
this hack no longer works.

Provide the same functionality by adding a new configuration
variable user.useConfigOnly; when this variable is set, the
user must explicitly set user.email configuration.

Signed-off-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Jeff King <p...@peff.net>
Signed-off-by: Dan Aloni <alo...@gmail.com>
---
 Documentation/config.txt  | 10 ++
 ident.c   | 16 
 t/t7517-per-repo-email.sh | 39 +++
 3 files changed, 65 insertions(+)
 create mode 100755 t/t7517-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..e153c7c1ec35 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,16 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.useConfigOnly::
+   Instruct Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name', and instead retrieve the values only from the
+   configuration. For example, if you have multiple email addresses
+   and would like to use a different one for each repository, then
+   with this configuration option set to `true` in the global config
+   along with a name, Git will prompt you to set up an email upon
+   making new commits in a newly cloned repository.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..6e125821f056 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
 static int default_name_is_bogus;
 
+static int ident_use_config_only;
+
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_config_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')", 
name);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_NAME_GIVEN))
+   die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')", 
email);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_MAIL_GIVEN))
+   die("user.useConfigOnly set but no mail given");
}
 
strbuf_reset();
@@ -446,6 +455,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, "user.useconfigonly")) {
+   ident_use_config_only = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +467,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   ident_config_given |= IDENT_NAME_GIVEN;
return 0;
}
 
@@ -463,6 +478,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_email, value);
committ

[PATCH v8 1/2] fmt_ident: refactor strictness checks

[Dan Aloni]

From: Jeff King 

This function has evolved quite a bit over time, and as a
result, the logic for "is this an OK ident" has been
sprinkled throughout. This ends up with a lot of redundant
conditionals, like checking want_name repeatedly. Worse,
we want to know in many cases whether we are using the
"default" ident, and we do so by comparing directly to the
global strbuf, which violates the abstraction of the
ident_default_* functions.

Let's reorganize the function into a hierarchy of
conditionals to handle similar cases together. The only
case that doesn't just work naturally for this is that of an
empty name, where our advice is different based on whether
we came from ident_default_name() or not. We can use a
simple flag to cover this case.

Signed-off-by: Jeff King 
---
 ident.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ident.c b/ident.c
index 3da555634290..f3a431f738cc 100644
--- a/ident.c
+++ b/ident.c
@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
 
-   if (want_name && !name)
-   name = ident_default_name();
-   if (!email)
-   email = ident_default_email();
-
-   if (want_name && !*name) {
-   struct passwd *pw;
-
-   if (strict) {
-   if (name == git_default_name.buf)
+   if (want_name) {
+   int using_default = 0;
+   if (!name) {
+   name = ident_default_name();
+   using_default = 1;
+   if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
-   die("empty ident name (for <%s>) not allowed", email);
+   die("unable to auto-detect name (got '%s')", 
name);
+   }
+   }
+   if (!*name) {
+   struct passwd *pw;
+   if (strict) {
+   if (using_default)
+   fputs(env_hint, stderr);
+   die("empty ident name (for <%s>) not allowed", 
email);
+   }
+   pw = xgetpwuid_self(NULL);
+   name = pw->pw_name;
}
-   pw = xgetpwuid_self(NULL);
-   name = pw->pw_name;
-   }
-
-   if (want_name && strict &&
-   name == git_default_name.buf && default_name_is_bogus) {
-   fputs(env_hint, stderr);
-   die("unable to auto-detect name (got '%s')", name);
}
 
-   if (strict && email == git_default_email.buf && default_email_is_bogus) 
{
-   fputs(env_hint, stderr);
-   die("unable to auto-detect email address (got '%s')", email);
+   if (!email) {
+   email = ident_default_email();
+   if (strict && default_email_is_bogus) {
+   fputs(env_hint, stderr);
+   die("unable to auto-detect email address (got '%s')", 
email);
+   }
}
 
strbuf_reset();
-- 
2.5.0

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

[PATCH v8] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

Changes between v7 -> v8:

* Proofing fixes suggestions by Eric.
* Test script cleanup by Jeff.
* Renumbered test script.

v7: http://article.gmane.org/gmane.comp.version-control.git/285636
--
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

[PATCH v9] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

Changes between v8 -> v9:

* Rebased and tested on v2.7.1.
* Made a small correction suggested by Junio in the documentation for
  the new option: s/upon/before

v8: http://article.gmane.org/gmane.comp.version-control.git/285646
--
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

[PATCH v9 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

It used to be that:

   git config --global user.email "(none)"

was a viable way for people to force themselves to set user.email in
each repository.  This was helpful for people with more than one
email address, targeting different email addresses for different
clones, as it barred git from creating a commit unless the user.email
config was set in the per-repo config to the correct email address.

A recent change, 19ce497c (ident: keep a flag for bogus
default_email, 2015-12-10), however, declared that an explicitly
configured user.email is not bogus, no matter what its value is, so
this hack no longer works.

Provide the same functionality by adding a new configuration
variable user.useConfigOnly; when this variable is set, the
user must explicitly set user.email configuration.

Signed-off-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Jeff King <p...@peff.net>
Signed-off-by: Dan Aloni <alo...@gmail.com>
---
 Documentation/config.txt  | 10 ++
 ident.c   | 16 
 t/t7517-per-repo-email.sh | 39 +++
 3 files changed, 65 insertions(+)
 create mode 100755 t/t7517-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..cfb7d0e7652d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,16 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.useConfigOnly::
+   Instruct Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name', and instead retrieve the values only from the
+   configuration. For example, if you have multiple email addresses
+   and would like to use a different one for each repository, then
+   with this configuration option set to `true` in the global config
+   along with a name, Git will prompt you to set up an email before
+   making new commits in a newly cloned repository.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..6e125821f056 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
 static int default_name_is_bogus;
 
+static int ident_use_config_only;
+
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_config_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')", 
name);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_NAME_GIVEN))
+   die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')", 
email);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_MAIL_GIVEN))
+   die("user.useConfigOnly set but no mail given");
}
 
strbuf_reset();
@@ -446,6 +455,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, "user.useconfigonly")) {
+   ident_use_config_only = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +467,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   ident_config_given |= IDENT_NAME_GIVEN;
return 0;
}
 
@@ -463,6 +478,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_email, value);
committ

[PATCH v9 1/2] fmt_ident: refactor strictness checks

[Dan Aloni]

From: Jeff King 

This function has evolved quite a bit over time, and as a
result, the logic for "is this an OK ident" has been
sprinkled throughout. This ends up with a lot of redundant
conditionals, like checking want_name repeatedly. Worse,
we want to know in many cases whether we are using the
"default" ident, and we do so by comparing directly to the
global strbuf, which violates the abstraction of the
ident_default_* functions.

Let's reorganize the function into a hierarchy of
conditionals to handle similar cases together. The only
case that doesn't just work naturally for this is that of an
empty name, where our advice is different based on whether
we came from ident_default_name() or not. We can use a
simple flag to cover this case.

Signed-off-by: Jeff King 
---
 ident.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ident.c b/ident.c
index 3da555634290..f3a431f738cc 100644
--- a/ident.c
+++ b/ident.c
@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
 
-   if (want_name && !name)
-   name = ident_default_name();
-   if (!email)
-   email = ident_default_email();
-
-   if (want_name && !*name) {
-   struct passwd *pw;
-
-   if (strict) {
-   if (name == git_default_name.buf)
+   if (want_name) {
+   int using_default = 0;
+   if (!name) {
+   name = ident_default_name();
+   using_default = 1;
+   if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
-   die("empty ident name (for <%s>) not allowed", email);
+   die("unable to auto-detect name (got '%s')", 
name);
+   }
+   }
+   if (!*name) {
+   struct passwd *pw;
+   if (strict) {
+   if (using_default)
+   fputs(env_hint, stderr);
+   die("empty ident name (for <%s>) not allowed", 
email);
+   }
+   pw = xgetpwuid_self(NULL);
+   name = pw->pw_name;
}
-   pw = xgetpwuid_self(NULL);
-   name = pw->pw_name;
-   }
-
-   if (want_name && strict &&
-   name == git_default_name.buf && default_name_is_bogus) {
-   fputs(env_hint, stderr);
-   die("unable to auto-detect name (got '%s')", name);
}
 
-   if (strict && email == git_default_email.buf && default_email_is_bogus) 
{
-   fputs(env_hint, stderr);
-   die("unable to auto-detect email address (got '%s')", email);
+   if (!email) {
+   email = ident_default_email();
+   if (strict && default_email_is_bogus) {
+   fputs(env_hint, stderr);
+   die("unable to auto-detect email address (got '%s')", 
email);
+   }
}
 
strbuf_reset();
-- 
2.5.0

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

Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

On Fri, Feb 05, 2016 at 04:59:52PM -0500, Eric Sunshine wrote:
> On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:
> > It used to be that:
> > 
> >git config --global user.email "(none)"
> > 
> > was a viable way for people to force themselves to set user.email in
> > each repository.  This was helpful for people with more than one
> > email address, targeting different email addresses for different
> > clones, as it barred git from creating commit unless the user.email
> 
> Either: s/commit/a commit/ or s/commit/commits/

Thanks for all the proofing in your reply.

>[..]
> > config was set in the per-repo config to the correct email address.
> > 
> > A recent change, 19ce497c (ident: keep a flag for bogus
> > default_email, 2015-12-10), however declared that an explicitly
> 
> s/however/&,/
> 
> > configured user.email is not bogus, no matter what its value is, so
> > this hack no longer works.
> > 
> > Provide the same functionality by adding a new configuration
> > variable user.useConfigOnly; when this variable is set, the
> > user must explicitly set user.email configuration.
> > 
> > Signed-off-by: Dan Aloni <alo...@gmail.com>
> > Helped-by: Jeff King <p...@peff.net>
> > Signed-off-by: Junio C Hamano <gits...@pobox.com>
> > Cc: Eric Sunshine <sunsh...@sunshineco.com>
> 
> You'd generally place your sign-off last.
> [..]

Good to know :)

> This test script still has a fair amount of unnecessary cruft in it
> which obscures the important bits showing what you are really
> testing. Below is a more concise version with the unnecessary stuff
> removed:

Thanks, though I'll stick to what Jeff suggested. Also, perhaps better
to keep 'test_config' as it is instead of using '-c', to better mimick
the tested use case.

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

Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

On Fri, Feb 05, 2016 at 04:48:33PM -0500, Jeff King wrote:
> On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:
> 
> > diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
> > new file mode 100755
> > index ..f2b33881e46b
> > --- /dev/null
> > +++ b/t/t9904-per-repo-email.sh
> 
> Is t9904 the right place for this? Usually t99xx is for very separate
> components.
> 
> This is sort-of about "commit", which would put it in the t75xx range.
> But in some ways, it is even more fundamental than that. We don't seem
> to have a lot of tests for ident stuff. The closest is the strict ident
> stuff in t0007.

Will move to t7517. IMHO it's better to verify the commit operation
itself before running further tests that rely on its proper function.

>[..]
> > +reprepare () {
> > +   git reset --hard initial
> > +}
> 
> Do we need this reprepare stuff at all now? The tests don't care which
> commit we're at when they start.
> 
> > +test_expect_success setup '
> > +   # Initial repo state
> > +   echo "Initial" >foo &&
> > +   git add foo &&
> > +   git commit -m foo &&
> > +   git tag initial &&
> 
> A shorter way of saying this is "test_commit foo".
> 
> I almost thought we could get rid of this part entirely; the commit
> tests don't care. But we do still need _a_ commit for the clone test,
> since we want to make sure a reflog is written. It would be nice to push
> it down there, but our test environment doesn't allow creating commits,
> because of of useConfigOnly. So it's probably fine to leave it here.
> 
> Technically, the final "commit" test does make a commit for us to push,
> but we do generally try to avoid unnecessary dependencies between the
> individual tests.
> 
> So all together, maybe:
>[..]

Yes, shorted is better.

I'm squashing in these changes and adding you as Signed-off for v8.

-- 
Dan Aloni
--
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

[PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

It used to be that:

   git config --global user.email "(none)"

was a viable way for people to force themselves to set user.email in
each repository.  This was helpful for people with more than one
email address, targeting different email addresses for different
clones, as it barred git from creating commit unless the user.email
config was set in the per-repo config to the correct email address.

A recent change, 19ce497c (ident: keep a flag for bogus
default_email, 2015-12-10), however declared that an explicitly
configured user.email is not bogus, no matter what its value is, so
this hack no longer works.

Provide the same functionality by adding a new configuration
variable user.useConfigOnly; when this variable is set, the
user must explicitly set user.email configuration.

Signed-off-by: Dan Aloni <alo...@gmail.com>
Helped-by: Jeff King <p...@peff.net>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
Cc: Eric Sunshine <sunsh...@sunshineco.com>
---
 Documentation/config.txt  | 10 +
 ident.c   | 16 ++
 t/t9904-per-repo-email.sh | 55 +++
 3 files changed, 81 insertions(+)
 create mode 100755 t/t9904-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..0d168d92fd79 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,16 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.useConfigOnly::
+   This instructs Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name' other than strictly from config. For example, if
+   you have multiple email addresses and would like to use a different
+   one for each repository, then with this configuration option set
+   to `true` in the global config along with a name, Git would prompt
+   for you for setting up an email upon making new commits in a newly
+   cloned repository.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..6e125821f056 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
 static int default_name_is_bogus;
 
+static int ident_use_config_only;
+
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_config_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')", 
name);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_NAME_GIVEN))
+   die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')", 
email);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_MAIL_GIVEN))
+   die("user.useConfigOnly set but no mail given");
}
 
strbuf_reset();
@@ -446,6 +455,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, "user.useconfigonly")) {
+   ident_use_config_only = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +467,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   ident_config_given |= IDENT_NAME_GIVEN;
return 0;
}
 
@@ -463,6 +478,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_email, value);
committer_ident_explicitly_

[PATCH v7] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

Changes between v6 -> v7:

* Dropped patch: ident: cleanup wrt ident's source
* Revised the documentation of the feature according to comments.
* Revised the test according to comments.
* Styling fix.

v6: http://article.gmane.org/gmane.comp.version-control.git/285550
--
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

[PATCH v7 1/2] fmt_ident: refactor strictness checks

[Dan Aloni]

From: Jeff King 

This function has evolved quite a bit over time, and as a
result, the logic for "is this an OK ident" has been
sprinkled throughout. This ends up with a lot of redundant
conditionals, like checking want_name repeatedly. Worse,
we want to know in many cases whether we are using the
"default" ident, and we do so by comparing directly to the
global strbuf, which violates the abstraction of the
ident_default_* functions.

Let's reorganize the function into a hierarchy of
conditionals to handle similar cases together. The only
case that doesn't just work naturally for this is that of an
empty name, where our advice is different based on whether
we came from ident_default_name() or not. We can use a
simple flag to cover this case.

Signed-off-by: Jeff King 
---
 ident.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ident.c b/ident.c
index 3da555634290..f3a431f738cc 100644
--- a/ident.c
+++ b/ident.c
@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
 
-   if (want_name && !name)
-   name = ident_default_name();
-   if (!email)
-   email = ident_default_email();
-
-   if (want_name && !*name) {
-   struct passwd *pw;
-
-   if (strict) {
-   if (name == git_default_name.buf)
+   if (want_name) {
+   int using_default = 0;
+   if (!name) {
+   name = ident_default_name();
+   using_default = 1;
+   if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
-   die("empty ident name (for <%s>) not allowed", email);
+   die("unable to auto-detect name (got '%s')", 
name);
+   }
+   }
+   if (!*name) {
+   struct passwd *pw;
+   if (strict) {
+   if (using_default)
+   fputs(env_hint, stderr);
+   die("empty ident name (for <%s>) not allowed", 
email);
+   }
+   pw = xgetpwuid_self(NULL);
+   name = pw->pw_name;
}
-   pw = xgetpwuid_self(NULL);
-   name = pw->pw_name;
-   }
-
-   if (want_name && strict &&
-   name == git_default_name.buf && default_name_is_bogus) {
-   fputs(env_hint, stderr);
-   die("unable to auto-detect name (got '%s')", name);
}
 
-   if (strict && email == git_default_email.buf && default_email_is_bogus) 
{
-   fputs(env_hint, stderr);
-   die("unable to auto-detect email address (got '%s')", email);
+   if (!email) {
+   email = ident_default_email();
+   if (strict && default_email_is_bogus) {
+   fputs(env_hint, stderr);
+   die("unable to auto-detect email address (got '%s')", 
email);
+   }
}
 
strbuf_reset();
-- 
2.5.0

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

Re: [PATCH v6 3/3] ident: cleanup wrt ident's source

[Dan Aloni]

On Fri, Feb 05, 2016 at 02:24:13PM -0500, Jeff King wrote:
> On Fri, Feb 05, 2016 at 11:05:19AM -0800, Junio C Hamano wrote:
> 
> > Dan Aloni <alo...@gmail.com> writes:
> > 
> > > This change condenses the variables that tells where we got the user's
> > > ident into single enum, instead of a collection of booleans.
> > >
> > > In addtion, also have {committer,author}_ident_sufficiently_given
> > > directly probe the environment and the afformentioned enum instead of
> > > relying on git_{committer,author}_info to do so.
> > >
> > > Signed-off-by: Dan Aloni <alo...@gmail.com>
> > > Helped-by: Jeff King <p...@peff.net>
> > > Helped-by: Junio C Hamano <gits...@pobox.com>
> > > ---
> > >  ident.c | 126 
> > > 
> > >  1 file changed, 80 insertions(+), 46 deletions(-)
> > 
> > Peff what do you think?  I am asking you because personally I do not
> > find this particularly easier to read than the original, but since
> > you stared at the code around here recently much longer than I did
> > when doing the 1/3, I thought you may be a better judge than I am.
> 
> I'm not sure it is really worth it unless we are going to expose this to
> the user, and let them say "I am OK with IDENT_SOURCE_GUESSED, but not
> IDENT_SOURCE_GUESSED_BOGUS" or similar.
> 
> Without that, I think it is probably just making things a bit more
> brittle.

Okay, will drop it now.

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

Re: [PATCH v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

On Fri, Feb 05, 2016 at 11:31:34AM -0800, Junio C Hamano wrote:
> > +   If you have multiple email addresses that you would like to set
> > +   up per repository, you may want to set this to 'true' in the global
> > +   config, and then Git would prompt you to set user.email separately,
> > +   in each of the cloned repositories.
> 
> The first sentence mentioned both name and email, but here the
> example is only about email.  A first time reader might be led into
> thinking this is only about email and not name, but I am assuming
> that is not the intention (i.e. this is merely showing just one use
> case).
>[..]

Going to revise per yours and Jeff's suggestions.

>[..]
> I can read the split expression either with && hanging at the end of
> line or && leading the next line just fine, but you'd want to be
> consistent especially when you are writing two almost identical
> things.

Sure.

>[..]
>   test_expect_success 'suceed with config' '
>   test_when_finished reprepare &&
>   test_config user.email t...@ok.com &&
> test_must_fail git commit -m msg
>   '
> 
> Note that you do not need "test_unconfig user.email" in reprepare,
> as the variable is set in one test with test_config, which uses
> test_when_finished to arrange the variable to be removed after
> running the test.

Alright. It was worth to understand the differing behavior between
'test_config' and 'git config'.

-- 
Dan Aloni
--
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

[PATCH v5 3/3] ident: cleanup wrt ident's source

[Dan Aloni]

This change condenses the variables that tells where we got the user's
ident into single enum, instead of a collection of booleans.

In addtion, also have {committer,author}_ident_sufficiently_given
directly probe the environment and the afformentioned enum instead of
relying on git_{committer,author}_info to do so.

Signed-off-by: Dan Aloni <alo...@gmail.com>
Helped-by: Jeff King <p...@peff.net>
Helped-by: Junio C Hamano <gits...@pobox.com>
---
 ident.c | 126 
 1 file changed, 80 insertions(+), 46 deletions(-)

diff --git a/ident.c b/ident.c
index 9bd6ac69bfe8..725d0aeb7da4 100644
--- a/ident.c
+++ b/ident.c
@@ -10,17 +10,19 @@
 static struct strbuf git_default_name = STRBUF_INIT;
 static struct strbuf git_default_email = STRBUF_INIT;
 static struct strbuf git_default_date = STRBUF_INIT;
-static int default_email_is_bogus;
-static int default_name_is_bogus;
+
+enum ident_source {
+   IDENT_SOURCE_UNKNOWN = 0,
+   IDENT_SOURCE_CONFIG,
+   IDENT_SOURCE_ENVIRONMENT,
+   IDENT_SOURCE_GUESSED,
+   IDENT_SOURCE_GUESSED_BOGUS
+};
 
 static int ident_use_config_only;
 
-#define IDENT_NAME_GIVEN 01
-#define IDENT_MAIL_GIVEN 02
-#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
-static int committer_ident_explicitly_given;
-static int author_ident_explicitly_given;
-static int ident_config_given;
+static enum ident_source source_of_default_email = IDENT_SOURCE_UNKNOWN;
+static enum ident_source source_of_default_name = IDENT_SOURCE_UNKNOWN;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -28,7 +30,7 @@ static int ident_config_given;
 #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)
 #endif
 
-static struct passwd *xgetpwuid_self(int *is_bogus)
+static struct passwd *xgetpwuid_self(enum ident_source *source)
 {
struct passwd *pw;
 
@@ -41,9 +43,13 @@ static struct passwd *xgetpwuid_self(int *is_bogus)
fallback.pw_gecos = "Unknown";
 #endif
pw = 
-   if (is_bogus)
-   *is_bogus = 1;
+   if (source)
+   *source = IDENT_SOURCE_GUESSED_BOGUS;
+   } else {
+   if (source)
+   *source = IDENT_SOURCE_GUESSED;
}
+
return pw;
 }
 
@@ -120,26 +126,26 @@ static int canonical_name(const char *host, struct strbuf 
*out)
return status;
 }
 
-static void add_domainname(struct strbuf *out, int *is_bogus)
+static void add_domainname(struct strbuf *out, enum ident_source *source)
 {
char buf[1024];
 
if (gethostname(buf, sizeof(buf))) {
warning("cannot get host name: %s", strerror(errno));
strbuf_addstr(out, "(none)");
-   *is_bogus = 1;
+   *source = IDENT_SOURCE_GUESSED_BOGUS;
return;
}
if (strchr(buf, '.'))
strbuf_addstr(out, buf);
else if (canonical_name(buf, out) < 0) {
strbuf_addf(out, "%s.(none)", buf);
-   *is_bogus = 1;
+   *source = IDENT_SOURCE_GUESSED_BOGUS;
}
 }
 
 static void copy_email(const struct passwd *pw, struct strbuf *email,
-  int *is_bogus)
+  enum ident_source *source)
 {
/*
 * Make up a fake email address
@@ -150,13 +156,13 @@ static void copy_email(const struct passwd *pw, struct 
strbuf *email,
 
if (!add_mailname_host(email))
return; /* read from "/etc/mailname" (Debian) */
-   add_domainname(email, is_bogus);
+   add_domainname(email, source);
 }
 
 const char *ident_default_name(void)
 {
if (!git_default_name.len) {
-   copy_gecos(xgetpwuid_self(_name_is_bogus), 
_default_name);
+   copy_gecos(xgetpwuid_self(_of_default_name), 
_default_name);
strbuf_trim(_default_name);
}
return git_default_name.buf;
@@ -169,11 +175,12 @@ const char *ident_default_email(void)
 
if (email && email[0]) {
strbuf_addstr(_default_email, email);
-   committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-   author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-   } else
-   copy_email(xgetpwuid_self(_email_is_bogus),
-  _default_email, _email_is_bogus);
+   source_of_default_email = IDENT_SOURCE_ENVIRONMENT;
+   } else {
+   copy_email(xgetpwuid_self(_of_default_email),
+  _default_email, 
_of_default_email);
+   }
+
strbuf_trim(_default_email);
}
return git_default_email.buf;
@@ -353,12 +360,13 @@ const char *fmt_ident(const char *name, c

[PATCH v5 1/3] fmt_ident: refactor strictness checks

[Dan Aloni]

From: Jeff King 

This function has evolved quite a bit over time, and as a
result, the logic for "is this an OK ident" has been
sprinkled throughout. This ends up with a lot of redundant
conditionals, like checking want_name repeatedly. Worse,
we want to know in many cases whether we are using the
"default" ident, and we do so by comparing directly to the
global strbuf, which violates the abstraction of the
ident_default_* functions.

Let's reorganize the function into a hierarchy of
conditionals to handle similar cases together. The only
case that doesn't just work naturally for this is that of an
empty name, where our advice is different based on whether
we came from ident_default_name() or not. We can use a
simple flag to cover this case.

Signed-off-by: Jeff King 
---
 ident.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ident.c b/ident.c
index 3da555634290..f3a431f738cc 100644
--- a/ident.c
+++ b/ident.c
@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
 
-   if (want_name && !name)
-   name = ident_default_name();
-   if (!email)
-   email = ident_default_email();
-
-   if (want_name && !*name) {
-   struct passwd *pw;
-
-   if (strict) {
-   if (name == git_default_name.buf)
+   if (want_name) {
+   int using_default = 0;
+   if (!name) {
+   name = ident_default_name();
+   using_default = 1;
+   if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
-   die("empty ident name (for <%s>) not allowed", email);
+   die("unable to auto-detect name (got '%s')", 
name);
+   }
+   }
+   if (!*name) {
+   struct passwd *pw;
+   if (strict) {
+   if (using_default)
+   fputs(env_hint, stderr);
+   die("empty ident name (for <%s>) not allowed", 
email);
+   }
+   pw = xgetpwuid_self(NULL);
+   name = pw->pw_name;
}
-   pw = xgetpwuid_self(NULL);
-   name = pw->pw_name;
-   }
-
-   if (want_name && strict &&
-   name == git_default_name.buf && default_name_is_bogus) {
-   fputs(env_hint, stderr);
-   die("unable to auto-detect name (got '%s')", name);
}
 
-   if (strict && email == git_default_email.buf && default_email_is_bogus) 
{
-   fputs(env_hint, stderr);
-   die("unable to auto-detect email address (got '%s')", email);
+   if (!email) {
+   email = ident_default_email();
+   if (strict && default_email_is_bogus) {
+   fputs(env_hint, stderr);
+   die("unable to auto-detect email address (got '%s')", 
email);
+   }
}
 
strbuf_reset();
-- 
2.5.0

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

[PATCH v5] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

Changes between v4 -> v5:

 * Fixes for compiler warnings under standard compilation.
 * Revised commit messages according to remarks.
 * Cleanups and fixes of the added test script.
 * Indentation and styling fixes.

v4: http://thread.gmane.org/gmane.comp.version-control.git/285441
--
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

[PATCH v5 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

It used to be that:

   git config --global user.email "(none)"

was a viable way for people to force themselves to set user.email in
each repository.  This was helpful for people with more than one
email address, targeting different email addresses for different
clones, as it barred git from creating commit unless the user.email
config was set in the per-repo config to the correct email address.

A recent change, 19ce497c (ident: keep a flag for bogus
default_email, 2015-12-10), however declared that an explicitly
configured user.email is not bogus, no matter what its value is, so
this hack no longer works.

Provide the same functionality by adding a new configuration
variable user.useConfigOnly; when this variable is set, the
user must explicitly set user.email configuration.

Signed-off-by: Dan Aloni <alo...@gmail.com>
Helped-by: Jeff King <p...@peff.net>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
Cc: Eric Sunshine <sunsh...@sunshineco.com>
---
 Documentation/config.txt  |  9 
 ident.c   | 16 +
 t/t9904-per-repo-email.sh | 58 +++
 3 files changed, 83 insertions(+)
 create mode 100755 t/t9904-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..25cf7ce4e83a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,15 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.useConfigOnly::
+   This instruct Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name' other than strictly from environment or config.
+   If you have multiple email addresses that you would like to set
+   up per repository, you may want to set this to 'true' in the global
+   config, and then Git would prompt you to set user.email separately,
+   in each of the cloned repositories.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..9bd6ac69bfe8 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
 static int default_name_is_bogus;
 
+static int ident_use_config_only;
+
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_config_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')", 
name);
}
+   if (strict && ident_use_config_only &&
+   !(ident_config_given & IDENT_NAME_GIVEN))
+   die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')", 
email);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_MAIL_GIVEN))
+   die("user.useConfigOnly set but no mail given");
}
 
strbuf_reset();
@@ -446,6 +455,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, "user.useconfigonly")) {
+   ident_use_config_only = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +467,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   ident_config_given |= IDENT_NAME_GIVEN;
return 0;
}
 
@@ -463,6 +478,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_email, value);
committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
author_ident_explicitly_gi

Re: [PATCH v4 3/3] ident.c: cleanup wrt ident's source

[Dan Aloni]

On Thu, Feb 04, 2016 at 02:42:55PM -0800, Junio C Hamano wrote:
> Dan Aloni <alo...@gmail.com> writes:
> 
> > +static int ident_source_is_sufficient(enum ident_source source)
> > +{
> > +   switch (source) {
> > +   case IDENT_SOURCE_CONFIG:
> > +   case IDENT_SOURCE_ENVIRONMENT:
> > +   return 1;
> 
> Without adding these two lines here:
> 
>   default:
>   break;
> 
> I get this build failure:
> 
> ident.c: In function 'ident_source_is_sufficient':
> ident.c:444:2: error: enumeration value 'IDENT_SOURCE_UNKNOWN' not handled in 
> switch [-Werror=switch]
>   switch (source) {
>   ^
> ident.c:444:2: error: enumeration value 'IDENT_SOURCE_GUESSED' not handled in 
> switch [-Werror=switch]
> ident.c:444:2: error: enumeration value 'IDENT_SOURCE_GUESSED_BOGUS' not 
> handled in switch [-Werror=switch]

My fault - must have left over CFLAGS="-O0 -g" while debugging, so it
got missed. Will fix.

>[..]
> ident.c: In function 'ident_is_sufficient':
> ident.c:456:6: error: variable 'name' set but not used 
> [-Werror=unused-but-set-variable]
>   int name, email;
>   ^
> cc1: all warnings being treated as errors
> make: *** [ident.o] Error 1

Ditto.

-- 
Dan Aloni
--
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

[PATCH v6 3/3] ident: cleanup wrt ident's source

[Dan Aloni]

This change condenses the variables that tells where we got the user's
ident into single enum, instead of a collection of booleans.

In addtion, also have {committer,author}_ident_sufficiently_given
directly probe the environment and the afformentioned enum instead of
relying on git_{committer,author}_info to do so.

Signed-off-by: Dan Aloni <alo...@gmail.com>
Helped-by: Jeff King <p...@peff.net>
Helped-by: Junio C Hamano <gits...@pobox.com>
---
 ident.c | 126 
 1 file changed, 80 insertions(+), 46 deletions(-)

diff --git a/ident.c b/ident.c
index 9bd6ac69bfe8..9bb05912b59a 100644
--- a/ident.c
+++ b/ident.c
@@ -10,17 +10,19 @@
 static struct strbuf git_default_name = STRBUF_INIT;
 static struct strbuf git_default_email = STRBUF_INIT;
 static struct strbuf git_default_date = STRBUF_INIT;
-static int default_email_is_bogus;
-static int default_name_is_bogus;
+
+enum ident_source {
+   IDENT_SOURCE_UNKNOWN = 0,
+   IDENT_SOURCE_CONFIG,
+   IDENT_SOURCE_ENVIRONMENT,
+   IDENT_SOURCE_GUESSED,
+   IDENT_SOURCE_GUESSED_BOGUS
+};
 
 static int ident_use_config_only;
 
-#define IDENT_NAME_GIVEN 01
-#define IDENT_MAIL_GIVEN 02
-#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
-static int committer_ident_explicitly_given;
-static int author_ident_explicitly_given;
-static int ident_config_given;
+static enum ident_source source_of_default_email = IDENT_SOURCE_UNKNOWN;
+static enum ident_source source_of_default_name = IDENT_SOURCE_UNKNOWN;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -28,7 +30,7 @@ static int ident_config_given;
 #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)
 #endif
 
-static struct passwd *xgetpwuid_self(int *is_bogus)
+static struct passwd *xgetpwuid_self(enum ident_source *source)
 {
struct passwd *pw;
 
@@ -41,9 +43,13 @@ static struct passwd *xgetpwuid_self(int *is_bogus)
fallback.pw_gecos = "Unknown";
 #endif
pw = 
-   if (is_bogus)
-   *is_bogus = 1;
+   if (source)
+   *source = IDENT_SOURCE_GUESSED_BOGUS;
+   } else {
+   if (source)
+   *source = IDENT_SOURCE_GUESSED;
}
+
return pw;
 }
 
@@ -120,26 +126,26 @@ static int canonical_name(const char *host, struct strbuf 
*out)
return status;
 }
 
-static void add_domainname(struct strbuf *out, int *is_bogus)
+static void add_domainname(struct strbuf *out, enum ident_source *source)
 {
char buf[1024];
 
if (gethostname(buf, sizeof(buf))) {
warning("cannot get host name: %s", strerror(errno));
strbuf_addstr(out, "(none)");
-   *is_bogus = 1;
+   *source = IDENT_SOURCE_GUESSED_BOGUS;
return;
}
if (strchr(buf, '.'))
strbuf_addstr(out, buf);
else if (canonical_name(buf, out) < 0) {
strbuf_addf(out, "%s.(none)", buf);
-   *is_bogus = 1;
+   *source = IDENT_SOURCE_GUESSED_BOGUS;
}
 }
 
 static void copy_email(const struct passwd *pw, struct strbuf *email,
-  int *is_bogus)
+  enum ident_source *source)
 {
/*
 * Make up a fake email address
@@ -150,13 +156,13 @@ static void copy_email(const struct passwd *pw, struct 
strbuf *email,
 
if (!add_mailname_host(email))
return; /* read from "/etc/mailname" (Debian) */
-   add_domainname(email, is_bogus);
+   add_domainname(email, source);
 }
 
 const char *ident_default_name(void)
 {
if (!git_default_name.len) {
-   copy_gecos(xgetpwuid_self(_name_is_bogus), 
_default_name);
+   copy_gecos(xgetpwuid_self(_of_default_name), 
_default_name);
strbuf_trim(_default_name);
}
return git_default_name.buf;
@@ -169,11 +175,12 @@ const char *ident_default_email(void)
 
if (email && email[0]) {
strbuf_addstr(_default_email, email);
-   committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-   author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-   } else
-   copy_email(xgetpwuid_self(_email_is_bogus),
-  _default_email, _email_is_bogus);
+   source_of_default_email = IDENT_SOURCE_ENVIRONMENT;
+   } else {
+   copy_email(xgetpwuid_self(_of_default_email),
+  _default_email, 
_of_default_email);
+   }
+
strbuf_trim(_default_email);
}
return git_default_email.buf;
@@ -353,12 +360,13 @@ const char *fmt_ident(const char *name, c

[PATCH v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

It used to be that:

   git config --global user.email "(none)"

was a viable way for people to force themselves to set user.email in
each repository.  This was helpful for people with more than one
email address, targeting different email addresses for different
clones, as it barred git from creating commit unless the user.email
config was set in the per-repo config to the correct email address.

A recent change, 19ce497c (ident: keep a flag for bogus
default_email, 2015-12-10), however declared that an explicitly
configured user.email is not bogus, no matter what its value is, so
this hack no longer works.

Provide the same functionality by adding a new configuration
variable user.useConfigOnly; when this variable is set, the
user must explicitly set user.email configuration.

Signed-off-by: Dan Aloni <alo...@gmail.com>
Helped-by: Jeff King <p...@peff.net>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
Cc: Eric Sunshine <sunsh...@sunshineco.com>
---
 Documentation/config.txt  |  9 
 ident.c   | 16 +
 t/t9904-per-repo-email.sh | 58 +++
 3 files changed, 83 insertions(+)
 create mode 100755 t/t9904-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..25cf7ce4e83a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,15 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.useConfigOnly::
+   This instruct Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name' other than strictly from environment or config.
+   If you have multiple email addresses that you would like to set
+   up per repository, you may want to set this to 'true' in the global
+   config, and then Git would prompt you to set user.email separately,
+   in each of the cloned repositories.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..9bd6ac69bfe8 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
 static int default_name_is_bogus;
 
+static int ident_use_config_only;
+
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_config_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')", 
name);
}
+   if (strict && ident_use_config_only &&
+   !(ident_config_given & IDENT_NAME_GIVEN))
+   die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')", 
email);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_MAIL_GIVEN))
+   die("user.useConfigOnly set but no mail given");
}
 
strbuf_reset();
@@ -446,6 +455,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, "user.useconfigonly")) {
+   ident_use_config_only = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +467,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   ident_config_given |= IDENT_NAME_GIVEN;
return 0;
}
 
@@ -463,6 +478,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_email, value);
committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
author_ident_explicitly_gi

[PATCH v6 1/3] fmt_ident: refactor strictness checks

[Dan Aloni]

From: Jeff King 

This function has evolved quite a bit over time, and as a
result, the logic for "is this an OK ident" has been
sprinkled throughout. This ends up with a lot of redundant
conditionals, like checking want_name repeatedly. Worse,
we want to know in many cases whether we are using the
"default" ident, and we do so by comparing directly to the
global strbuf, which violates the abstraction of the
ident_default_* functions.

Let's reorganize the function into a hierarchy of
conditionals to handle similar cases together. The only
case that doesn't just work naturally for this is that of an
empty name, where our advice is different based on whether
we came from ident_default_name() or not. We can use a
simple flag to cover this case.

Signed-off-by: Jeff King 
---
 ident.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ident.c b/ident.c
index 3da555634290..f3a431f738cc 100644
--- a/ident.c
+++ b/ident.c
@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
 
-   if (want_name && !name)
-   name = ident_default_name();
-   if (!email)
-   email = ident_default_email();
-
-   if (want_name && !*name) {
-   struct passwd *pw;
-
-   if (strict) {
-   if (name == git_default_name.buf)
+   if (want_name) {
+   int using_default = 0;
+   if (!name) {
+   name = ident_default_name();
+   using_default = 1;
+   if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
-   die("empty ident name (for <%s>) not allowed", email);
+   die("unable to auto-detect name (got '%s')", 
name);
+   }
+   }
+   if (!*name) {
+   struct passwd *pw;
+   if (strict) {
+   if (using_default)
+   fputs(env_hint, stderr);
+   die("empty ident name (for <%s>) not allowed", 
email);
+   }
+   pw = xgetpwuid_self(NULL);
+   name = pw->pw_name;
}
-   pw = xgetpwuid_self(NULL);
-   name = pw->pw_name;
-   }
-
-   if (want_name && strict &&
-   name == git_default_name.buf && default_name_is_bogus) {
-   fputs(env_hint, stderr);
-   die("unable to auto-detect name (got '%s')", name);
}
 
-   if (strict && email == git_default_email.buf && default_email_is_bogus) 
{
-   fputs(env_hint, stderr);
-   die("unable to auto-detect email address (got '%s')", email);
+   if (!email) {
+   email = ident_default_email();
+   if (strict && default_email_is_bogus) {
+   fputs(env_hint, stderr);
+   die("unable to auto-detect email address (got '%s')", 
email);
+   }
}
 
strbuf_reset();
-- 
2.5.0

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

[PATCH v6] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

Change between v5 -> v6:

* Removed trailing comma in 'enum ident_person'.

v5: http://article.gmane.org/gmane.comp.version-control.git/285544
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/3] Add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

On Thu, Feb 04, 2016 at 01:53:25PM -0800, Junio C Hamano wrote:
>[..]
> "by adding a new configuration variable" is a bit weak.  Help the
> reader by mentioning what it is called and what it does in the same
> sentence.
> 
> Perhaps like this?
> 
> -- >8 --
>[..]
>

Looks good, I'll just take that :)

> ident: add user.useConfigOnly boolean for when ident shouldn't be guessed
>[..]
> > }
> > +   if (strict && ident_use_config_only && !(ident_config_given & 
> > IDENT_MAIL_GIVEN))
> > +   die("user.useConfigOnly set but no mail given");
> > }
> 
> By folding the line just like you did for "name" above, you do not
> have to worry about an overlong line here.

Consistency is important. Will fix here too, though it got fixed later
in the cleanup.

>[..]
> > +git add foo &&
> > +EDITOR=: VISUAL=: git commit -m foo &&
> 
> What is the point of these one-shot assignments to the environment
> variables?
> 
> "git commit -m " does not invoke the editor unless given "-e",
> and EDITOR=: is done early in test-lib.sh already, so I am puzzled.
> 
> Besides, if you are worried about some stray environment variable,
> overriding EDITOR and VISUAL would not guard you against a stray
> GIT_EDITOR, which takes the precedence, I think.

Being new to this testing framework, I tried learning the trade from
other tests. Maybe I goofed, or the other tests need cleaning?

> 
> > +   # Setup a likely user.useConfigOnly use case
> > +   unset GIT_AUTHOR_NAME &&
> > +   unset GIT_AUTHOR_EMAIL &&
> 
> Doesn't unset fail when the variable is not set (we have sane_unset
> helper for that)?

Sure.

>[..] 
> > +test_expect_success 'fails committing if clone email is not set, but EMAIL 
> > set' '
> > +prepare && about_to_commit &&
> > +
> > +   EMAIL=t...@fail.com EDITOR=: VISUAL=: test_must_fail git commit -m msg
> 
> This is a good place to use the "test_must_fail env" pattern, i.e.
> 
>   test_must_fail env EMAIL=t...@fail.com git commit -m msg
> 
> I would think.

Yes, and the fixed test still passes. Will resubmit the patches.

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

Re: [PATCH v3] Add user.explicit boolean for when ident shouldn't be guessed

[Dan Aloni]

On Thu, Feb 04, 2016 at 12:50:35AM -0500, Jeff King wrote:
> On Thu, Feb 04, 2016 at 07:36:46AM +0200, Dan Aloni wrote:
>[..]
> > The code should be cleaned up anyway. I only delved into that code for
> > the first time two days ago, so it would take me more time to come up
> > with a new one (though reading your overview here of the cases is going
> > to be helpful, thanks).
> 
> Feel free to look into this direction, but having pushed a little
> further towards the "simple" approach (with the 2 patches I just sent),
> I think that does what you want without too much complication. I'd be
> fine, too, if you wanted to pick those up[1] and put the finishing
> touches on the second one.
> 
> -Peff
> 
> [1] To clarify, since you are new to the git.git workflow: I'd expect
> you to use `git am` to pick up my two patches. Leave me as the
> author of the first cleanup patch. Squash your additions onto the
> second one using `cherry-pick`, `commit --amend`, or whatever, and
> make sure to `commit --reset-author` so that you're the author. Post
> both as part of the v4 re-roll.
> 
> But that's just "here is what I meant", not "what you have to do". :)

Thanks. Being familiar with Linux kernel patch submission process, good to
be focused about git.git's idiosyncrasies too.

The cleanup was easy thanks to your patches. Going to post v4.

-- 
Dan Aloni
--
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

[PATCH v4] Add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

v3->v4 changes:

 * Added cleanup patch for ident.c
 * Added Jeff's refactoring to fmt_indent
 * Renamed configuration option to useConfigOnly.
 * Extended tests to include a clone, EMAIL env.

Also, ran the entire Git test suite to make sure nothing's broken.
--
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

[PATCH v4 1/3] fmt_ident: refactor strictness checks

[Dan Aloni]

From: Jeff King 

This function has evolved quite a bit over time, and as a
result, the logic for "is this an OK ident" has been
sprinkled throughout. This ends up with a lot of redundant
conditionals, like checking want_name repeatedly. Worse,
we want to know in many cases whether we are using the
"default" ident, and we do so by comparing directly to the
global strbuf, which violates the abstraction of the
ident_default_* functions.

Let's reorganize the function into a hierarchy of
conditionals to handle similar cases together. The only
case that doesn't just work naturally for this is that of an
empty name, where our advice is different based on whether
we came from ident_default_name() or not. We can use a
simple flag to cover this case.

Signed-off-by: Jeff King 
---
 ident.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ident.c b/ident.c
index 3da555634290..f3a431f738cc 100644
--- a/ident.c
+++ b/ident.c
@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
 
-   if (want_name && !name)
-   name = ident_default_name();
-   if (!email)
-   email = ident_default_email();
-
-   if (want_name && !*name) {
-   struct passwd *pw;
-
-   if (strict) {
-   if (name == git_default_name.buf)
+   if (want_name) {
+   int using_default = 0;
+   if (!name) {
+   name = ident_default_name();
+   using_default = 1;
+   if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
-   die("empty ident name (for <%s>) not allowed", email);
+   die("unable to auto-detect name (got '%s')", 
name);
+   }
+   }
+   if (!*name) {
+   struct passwd *pw;
+   if (strict) {
+   if (using_default)
+   fputs(env_hint, stderr);
+   die("empty ident name (for <%s>) not allowed", 
email);
+   }
+   pw = xgetpwuid_self(NULL);
+   name = pw->pw_name;
}
-   pw = xgetpwuid_self(NULL);
-   name = pw->pw_name;
-   }
-
-   if (want_name && strict &&
-   name == git_default_name.buf && default_name_is_bogus) {
-   fputs(env_hint, stderr);
-   die("unable to auto-detect name (got '%s')", name);
}
 
-   if (strict && email == git_default_email.buf && default_email_is_bogus) 
{
-   fputs(env_hint, stderr);
-   die("unable to auto-detect email address (got '%s')", email);
+   if (!email) {
+   email = ident_default_email();
+   if (strict && default_email_is_bogus) {
+   fputs(env_hint, stderr);
+   die("unable to auto-detect email address (got '%s')", 
email);
+   }
}
 
strbuf_reset();
-- 
2.5.0

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

[PATCH v4 2/3] Add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

Previously, before 5498c57cdd63, many people did the following:

   git config --global user.email "(none)"

This was helpful for people with more than one email address,
targeting different email addresses for different clones.
as it barred git from creating commit unless the user.email
config was set in the per-repo config to the correct email
address.

This commit provides the same functionality by adding a new
configuration variable.

Signed-off-by: Dan Aloni <alo...@gmail.com>
Cc: Eric Sunshine <sunsh...@sunshineco.com>
Cc: Jeff King <p...@peff.net>
Cc: Git List <git@vger.kernel.org>
---
 Documentation/config.txt  |  9 
 ident.c   | 15 
 t/t9904-per-repo-email.sh | 58 +++
 3 files changed, 82 insertions(+)
 create mode 100755 t/t9904-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..25cf7ce4e83a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,15 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.useConfigOnly::
+   This instruct Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name' other than strictly from environment or config.
+   If you have multiple email addresses that you would like to set
+   up per repository, you may want to set this to 'true' in the global
+   config, and then Git would prompt you to set user.email separately,
+   in each of the cloned repositories.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..1216079d0b0d 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
 static int default_name_is_bogus;
 
+static int ident_use_config_only;
+
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_config_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')", 
name);
}
+   if (strict && ident_use_config_only &&
+   !(ident_config_given & IDENT_NAME_GIVEN))
+   die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,8 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')", 
email);
}
+   if (strict && ident_use_config_only && !(ident_config_given & 
IDENT_MAIL_GIVEN))
+   die("user.useConfigOnly set but no mail given");
}
 
strbuf_reset();
@@ -446,6 +454,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, "user.useconfigonly")) {
+   ident_use_config_only = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +466,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   ident_config_given |= IDENT_NAME_GIVEN;
return 0;
}
 
@@ -463,6 +477,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_email, value);
committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+   ident_config_given |= IDENT_MAIL_GIVEN;
return 0;
}
 
diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
new file mode 100755
index ..9522a640951b
--- /dev/null
+++ b/t/t9904-per-repo-email.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Dan Aloni
+#
+
+test_description='per-repo forced set

[PATCH v4 3/3] ident.c: cleanup wrt ident's source

[Dan Aloni]

 * Condense the variables that tells where we got the user's
   ident into single enum, instead of a collection of booleans.
 * Have {committer,author}_ident_sufficiently_given directly
   probe the environment and the afformentioned enum instead of
   relying on git_{committer,author}_info to do so.

Signed-off-by: Dan Aloni <alo...@gmail.com>
---
 ident.c | 122 
 1 file changed, 77 insertions(+), 45 deletions(-)

diff --git a/ident.c b/ident.c
index 1216079d0b0d..b9aad38e0621 100644
--- a/ident.c
+++ b/ident.c
@@ -10,17 +10,19 @@
 static struct strbuf git_default_name = STRBUF_INIT;
 static struct strbuf git_default_email = STRBUF_INIT;
 static struct strbuf git_default_date = STRBUF_INIT;
-static int default_email_is_bogus;
-static int default_name_is_bogus;
+
+enum ident_source {
+   IDENT_SOURCE_UNKNOWN = 0,
+   IDENT_SOURCE_CONFIG,
+   IDENT_SOURCE_ENVIRONMENT,
+   IDENT_SOURCE_GUESSED,
+   IDENT_SOURCE_GUESSED_BOGUS,
+};
 
 static int ident_use_config_only;
 
-#define IDENT_NAME_GIVEN 01
-#define IDENT_MAIL_GIVEN 02
-#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
-static int committer_ident_explicitly_given;
-static int author_ident_explicitly_given;
-static int ident_config_given;
+static enum ident_source source_of_default_email = IDENT_SOURCE_UNKNOWN;
+static enum ident_source source_of_default_name = IDENT_SOURCE_UNKNOWN;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -28,7 +30,7 @@ static int ident_config_given;
 #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)
 #endif
 
-static struct passwd *xgetpwuid_self(int *is_bogus)
+static struct passwd *xgetpwuid_self(enum ident_source *source)
 {
struct passwd *pw;
 
@@ -41,9 +43,13 @@ static struct passwd *xgetpwuid_self(int *is_bogus)
fallback.pw_gecos = "Unknown";
 #endif
pw = 
-   if (is_bogus)
-   *is_bogus = 1;
+   if (source)
+   *source = IDENT_SOURCE_GUESSED_BOGUS;
+   } else {
+   if (source)
+   *source = IDENT_SOURCE_GUESSED;
}
+
return pw;
 }
 
@@ -120,26 +126,26 @@ static int canonical_name(const char *host, struct strbuf 
*out)
return status;
 }
 
-static void add_domainname(struct strbuf *out, int *is_bogus)
+static void add_domainname(struct strbuf *out, enum ident_source *source)
 {
char buf[1024];
 
if (gethostname(buf, sizeof(buf))) {
warning("cannot get host name: %s", strerror(errno));
strbuf_addstr(out, "(none)");
-   *is_bogus = 1;
+   *source = IDENT_SOURCE_GUESSED_BOGUS;
return;
}
if (strchr(buf, '.'))
strbuf_addstr(out, buf);
else if (canonical_name(buf, out) < 0) {
strbuf_addf(out, "%s.(none)", buf);
-   *is_bogus = 1;
+   *source = IDENT_SOURCE_GUESSED_BOGUS;
}
 }
 
 static void copy_email(const struct passwd *pw, struct strbuf *email,
-  int *is_bogus)
+  enum ident_source *source)
 {
/*
 * Make up a fake email address
@@ -150,13 +156,13 @@ static void copy_email(const struct passwd *pw, struct 
strbuf *email,
 
if (!add_mailname_host(email))
return; /* read from "/etc/mailname" (Debian) */
-   add_domainname(email, is_bogus);
+   add_domainname(email, source);
 }
 
 const char *ident_default_name(void)
 {
if (!git_default_name.len) {
-   copy_gecos(xgetpwuid_self(_name_is_bogus), 
_default_name);
+   copy_gecos(xgetpwuid_self(_of_default_name), 
_default_name);
strbuf_trim(_default_name);
}
return git_default_name.buf;
@@ -169,11 +175,12 @@ const char *ident_default_email(void)
 
if (email && email[0]) {
strbuf_addstr(_default_email, email);
-   committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-   author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-   } else
-   copy_email(xgetpwuid_self(_email_is_bogus),
-  _default_email, _email_is_bogus);
+   source_of_default_email = IDENT_SOURCE_ENVIRONMENT;
+   } else {
+   copy_email(xgetpwuid_self(_of_default_email),
+  _default_email, 
_of_default_email);
+   }
+
strbuf_trim(_default_email);
}
return git_default_email.buf;
@@ -353,12 +360,13 @@ const char *fmt_ident(const char *name, const char *email,
if (!name) {
name = ident_default_name();
  

Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo

[Dan Aloni]

(resend - my mailer was misconfigured)

On Tue, Feb 02, 2016 at 10:56:48PM -0500, Jeff King wrote:
> On Tue, Feb 02, 2016 at 09:54:21PM +0200, Dan Aloni wrote:
>[..]
> > +   if (strict && email && !strcmp(email, "(per-repo)")) {
> > +   die("email is '(per-repo)', suggesting to set specific email "
> > +   "for the current repo");
> > +   }
> 
> I find it disappointing that we go back to looking for magic sequences
> in the string. Could we perhaps do this more cleanly with a new config
> option? Like a "user.guessIdent" which defaults to true, but people can
> set to false. And without that, we do not do any automagic at all; we
> get the values from the GIT_COMMITTER_* environment or the
> user.{name,email} config variables, or we die().
>[..]

Agreed. New patch attached, feel free to amend.

-- 
Dan Aloni

>From 35d94d4a00af70eeaf6b291ec951f555b0bc99d3 Mon Sep 17 00:00:00 2001
From: Dan Aloni <alo...@gmail.com>
Date: Wed, 3 Feb 2016 10:09:40 +0200
Subject: [PATCH] Add user.explicit boolean for when ident shouldn't be guessed

Previously, before 5498c57cdd63, many people did the following:

   git config --global user.email "(none)"

This was helpful for people with more than one E-Mail address,
targeting different E-Mail addresses for different clones.
as it barred git from creating commit unless the user.email
config was set in the per-repo config to the correct E-Mail
address.

Now, since the original 'bug' was fixed, and practically every
string is acceptable for user.email and user.name, it is best
to reimplement the feature not as an exploit of a bug, but as
an actual feature.

Signed-off-by: Dan Aloni <alo...@gmail.com>
---
 Documentation/config.txt  |  8 
 ident.c   | 51 +++
 t/t9904-per-repo-email.sh | 36 +
 3 files changed, 95 insertions(+)
 create mode 100755 t/t9904-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 877cbc875ec3..4c99f8f1de4f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2791,6 +2791,14 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.explicit::
+   This instruct Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name' other than strictly from environment or config.
+   If you have multiply E-Mail addresses that you would like to set
+   up per repository, you may want to set this to 'true' in the global
+   config, and then Git would prompt you to set user.email separately,
+   in each of the cloned repositories.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index 9dd3ae345255..305dc32a8eaf 100644
--- a/ident.c
+++ b/ident.c
@@ -18,6 +18,7 @@ static int default_name_is_bogus;
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_explicit = 0;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -373,6 +374,23 @@ const char *fmt_ident(const char *name, const char *email,
die("unable to auto-detect email address (got '%s')", email);
}
 
+   if (ident_explicit) {
+   if (name == git_default_name.buf  &&
+   !(committer_ident_explicitly_given & IDENT_NAME_GIVEN) &&
+   !(author_ident_explicitly_given & IDENT_NAME_GIVEN)) {
+   die("requested explicitly given ident in config, "
+   "but user.name is not set, or environment is "
+   "not set");
+   }
+   if (email == git_default_email.buf  &&
+   !(committer_ident_explicitly_given & IDENT_MAIL_GIVEN) &&
+   !(author_ident_explicitly_given & IDENT_MAIL_GIVEN)) {
+   die("requested explicitly given ident in config, "
+   "but user.email is not set, or environment is "
+   "not set");
+   }
+   }
+
strbuf_reset();
if (want_name) {
strbuf_addstr_without_crud(, name);
@@ -405,6 +423,20 @@ const char *git_author_info(int flag)
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
if (getenv("GIT_AUTHOR_EMAIL"))
author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+
+   if (ident_

Re: [PATCH v3] Add user.explicit boolean for when ident shouldn't be guessed

[Dan Aloni]

On Wed, Feb 03, 2016 at 11:01:11PM -0500, Jeff King wrote:
>[..]
> > > * Do these new tests really deserve a new test script, or would they
> > > fit into an existing script? (Genuine question.)
> > 
> > I am not sure. IMHO it makes sense to have a new test script for a new
> > feature.
> 
> I hope we never have more than  features, then. :)
> 
> On to the patch itself...
> 
> > @@ -373,6 +374,21 @@ const char *fmt_ident(const char *name, const char 
> > *email,
> > die("unable to auto-detect email address (got '%s')", email);
> > }
> >  
> > +   if (ident_explicit) {
> > +   if (name == git_default_name.buf &&
> > +   !(committer_ident_explicitly_given & IDENT_NAME_GIVEN) &&
> > +   !(author_ident_explicitly_given & IDENT_NAME_GIVEN))
> > +   die("requested explicitly given ident in config, "
> > +   "but user.name is not set, or environment is "
> > +   "not set");
> > +   if (email == git_default_email.buf &&
> > +   !(committer_ident_explicitly_given & IDENT_MAIL_GIVEN) &&
> > +   !(author_ident_explicitly_given & IDENT_MAIL_GIVEN))
> > +   die("requested explicitly given ident in config, "
> > +   "but user.email is not set, or environment is "
> > +   "not set");
> > +   }
> > +
> 
> I'm not sure why this block is effectively repeated here, in
> git_author_info(), and in git_committer_info(). Don't the latter two
> just call fmt_ident?

Suspected that. Originally I started changing git_committer_info() and
git_author_info(), and only changed fmt_ident() later when I saw it
covers more cases. But the flow was confusing enough so I wasn't sure
whether to keep it.

> To be honest, I had expected something much simpler, like this (I
> omitted the config parsing for brevity):
>[..]
> In a sense, that encourages a nice workflow for your intended feature.
> You have to do:
> 
>   git clone -c user.name=... -c user.email=... clone ...
> 
> to set up your ident in the newly-cloned repository, or else clone will
> yell at you. But it's a little unfriendly. If you are just cloning to
> view and not make commits, you don't need your ident set up. And worse,
> if you forget to add your "-c" ident, clone will go through the trouble
> to copy all of the objects, and only then complain about your ident.

I think that forcing to give the configuration in 'git clone' could be
problematic for automated tools (e.g. o build) that invoke 'git clone'
just for building purposes (i.e. read-only) to a tool-managed directory.
And what about sub-modules clones? It would be hard to distinguish manual
clones and automatic clones anyway.

> So I'd argue that this should only kick in for the strict case. Which
> means the check _has_ to go into fmt_ident, and we have to somehow
> inform fmt_ident of the four cases:
> 
>   1. this ident came from git config
> 
>   2. this ident came from the environment, but from explicit variables
> 
>   3. this ident came from the environment; we guessed but the results
>  look pretty good
> 
>   4. this ident came from the environment; it's probably bogus
> 
> Right now we can identify type 4, with the *_is_bogus flags. We can
> identify 3, because EXPLICITLY_GIVEN flags won't be set. But we can't
> tell the difference between types 1 and 2.
> 
> I suppose there is also a type 0: "this ident was from GIT_COMMITTER_*
> and we did not bother to look up ident at all". But I think we agree
> that strictness and explicitness don't even come into play there.

Looks like an enum type would be better here instead of a set of booleans.

> So I think we can make this work by adding another variable to
> communicate the difference between types 1 and 2, and then act on it in
> fmt_ident only when "strict" is set. And I think "user.explicit" is not
> quite the right config name there. "user.guessIdent" is perhaps closer,
> but what we are really saying is "the ident must come from git config".
> So maybe "user.useConfigOnly" or something?

Yes, seems to me that useConfigOnly is better than both so far.

> I also wonder if we could simply expose the 4 levels of above in a
> variable, and default it to type-3. That would let people loosen or
> tighten as they see fit. But it would be a more complicated patch, so if
> nobody really cares about it beyond this use case, it may be overkill.

I get the impression from th

[PATCH v3] Add user.explicit boolean for when ident shouldn't be guessed

[Dan Aloni]

On Wed, Feb 03, 2016 at 12:47:56PM -0500, Eric Sunshine wrote:
>[..]
> * The final paragraph of the commit message appears to be outdated
> since it still seems to be describing the approach taken by v1.

Revised.

> * Elsewhere, in the project, the spelling "email" is preferred over
> "E-Mail"; that's true even in the files your patch is touching.

Done.

> * In the documentation:s/mutiply/multiple/.

Done.

> * The documentation doesn't seem to mention the default value of the
> new config variable.

Done.

> * The new config variable "user.explicit" has a more nebulous name
> than Peff's suggestion of "user.guessIdent", which may make its intent
> harder to determine without consulting documentation.

I wasn't sure about that, was waiting for input from Jeff. Kept it as
it is.

> * Don't initialize static variables to 0 (let the .bss section do that
> automatically).

Done.

> * One space before && operator; not two.

Done.

> * Drop unnecessary braces.

Done.

> * Perhaps use test_config(), test_unconfig(), test_config_global() in
> the test script rather than the manual git-config invocations in
> subshells.

Done, although test_unconfig_global was missing, so I revised.

> * test_expect_failure() is for indicating that a test will fail
> because some feature is known to be broken, not for when you expect a
> git command to fail in a controlled fashion. Instead, use
> test_expect_success, and then use test_must_fail() within the body of
> the test.
> 
> test_expect_success '...' '
> ... &&
> test_must_fail git commit -m msg
> '

Done. Also refactored both aspects of the test to a function.

> * Do these new tests really deserve a new test script, or would they
> fit into an existing script? (Genuine question.)

I am not sure. IMHO it makes sense to have a new test script for a new
feature.

> It's also reviewer-friendly to indicate the patch revision in the
> subject "[PATCH v3]", and to describe what changed since the previous
> version of the patch. Providing a gmane link to the previous version
> is also very helpful.

All changes from v2 to v3 listed above.

http://article.gmane.org/gmane.comp.version-control.git/285340

-- >8 --
Subject: Add user.explicit boolean for when ident shouldn't be guessed

Previously, before 5498c57cdd63, many people did the following:

   git config --global user.email "(none)"

This was helpful for people with more than one email address,
targeting different email addresses for different clones.
as it barred git from creating commit unless the user.email
config was set in the per-repo config to the correct email
address.

This commit provides the same functionality by adding a new
configuration variable.

Signed-off-by: Dan Aloni <alo...@gmail.com>
---
 Documentation/config.txt  |  9 +
 ident.c   | 45 +
 t/t9904-per-repo-email.sh | 37 +
 3 files changed, 91 insertions(+)
 create mode 100755 t/t9904-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 877cbc875ec3..d329ec9ac8d1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2791,6 +2791,15 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.explicit::
+   This instruct Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name' other than strictly from environment or config.
+   If you have multiple email addresses that you would like to set
+   up per repository, you may want to set this to 'true' in the global
+   config, and then Git would prompt you to set user.email separately,
+   in each of the cloned repositories.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index 9dd3ae345255..c950b5e3490f 100644
--- a/ident.c
+++ b/ident.c
@@ -18,6 +18,7 @@ static int default_name_is_bogus;
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_explicit;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -373,6 +374,21 @@ const char *fmt_ident(const char *name, const char *email,
die("unable to auto-detect email address (got '%s')", email);
}
 
+   if (ident_explicit) {
+   if (name == git_default_name.buf &&
+   !(committer_ident_explicitly_given & IDENT_NAME_GIVEN) &&
+   !(author

[PATCH] Trick to force setup of a specific configured E-Mail per repo

[Dan Aloni]

Previously, before 5498c57cdd63, many people did the following:

   git config --global user.email "(none)"

This was helpful for people with more than one E-Mail address,
targeting different E-Mail addresses for different clones.
as it barred git from creating commit unless the user.email
config was set in the per-clone config to the correct E-Mail
address.

Now, since the original 'bug' was fixed, and practically every
string is acceptable for user.email and user.name, it is best
to reimplement the feature not as an exploit of a bug, but as
an actual feature.

Signed-off-by: Dan Aloni <alo...@gmail.com>
---
 Documentation/config.txt  |  3 +++
 ident.c   |  5 +
 t/t9904-per-repo-email.sh | 26 ++
 3 files changed, 34 insertions(+)
 create mode 100755 t/t9904-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f61788668e89..f9712e7c7752 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2769,6 +2769,9 @@ user.email::
Your email address to be recorded in any newly created commits.
Can be overridden by the 'GIT_AUTHOR_EMAIL', 'GIT_COMMITTER_EMAIL', and
'EMAIL' environment variables.  See linkgit:git-commit-tree[1].
+   For people who seek setting different E-Mail addresses depending
+   on the clone, set to '(per-repo)' on the global configuration,
+   and Git will prompt you to set the E-Mail address in the clone.
 
 user.name::
Your full name to be recorded in any newly created commits.
diff --git a/ident.c b/ident.c
index daf7e1ea8370..0e07d45f8ff3 100644
--- a/ident.c
+++ b/ident.c
@@ -373,6 +373,11 @@ const char *fmt_ident(const char *name, const char *email,
die("unable to auto-detect email address (got '%s')", email);
}
 
+   if (strict && email && !strcmp(email, "(per-repo)")) {
+   die("email is '(per-repo)', suggesting to set specific email "
+   "for the current repo");
+   }
+
strbuf_reset();
if (want_name) {
strbuf_addstr_without_crud(, name);
diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
new file mode 100755
index ..c085ba671b85
--- /dev/null
+++ b/t/t9904-per-repo-email.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Dan Aloni
+#
+
+test_description='per-repo forced setting of E-Mail address'
+
+. ./test-lib.sh
+
+test_expect_failure 'fails commiting if clone email is not set' '
+   echo "Initial" >foo &&
+   git add foo &&
+   unset GIT_AUTHOR_EMAIL &&
+   git config --global user.email "(per-repo)" &&
+   EDITOR=: VISUAL=: git commit -a -m x
+'
+
+test_expect_success 'succeeds commiting if clone email is set' '
+   echo "Initial" >foo &&
+   git add foo &&
+   git config --global user.email "(per-repo)" &&
+   git config user.email "t...@ok.com" &&
+   EDITOR=: VISUAL=: git commit -a -m x
+'
+
+test_done
-- 
2.5.0

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