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

From: Dan Aloni <[email-masked]>
Date: Wed, 3 Feb 2016 21:22:27 +0200

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 <[email-masked]>
---
 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_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(&ident);
 	if (want_name) {
 		strbuf_addstr_without_crud(&ident, name);
@@ -405,6 +421,18 @@ 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_explicit) {
+		if (!(author_ident_explicitly_given & IDENT_NAME_GIVEN))
+			die("requested explicitly given ident in config, "
+			    "but user.name is not set, or GIT_AUTHOR_NAME "
+			    "is not set");
+		if (!(author_ident_explicitly_given & IDENT_MAIL_GIVEN))
+			die("requested explicitly given ident in config, "
+			    "but user.email is not set, or GIT_AUTHOR_EMAIL "
+			    "is not set");
+	}
+
 	return fmt_ident(getenv("GIT_AUTHOR_NAME"),
 			 getenv("GIT_AUTHOR_EMAIL"),
 			 getenv("GIT_AUTHOR_DATE"),
@@ -417,6 +445,18 @@ const char *git_committer_info(int flag)
 		committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
 	if (getenv("GIT_COMMITTER_EMAIL"))
 		committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+
+	if (ident_explicit) {
+		if (!(committer_ident_explicitly_given & IDENT_NAME_GIVEN))
+			die("requested explicitly given ident in config, "
+			    "but user.name is not set, or GIT_COMMITTER_NAME "
+			    "is not set");
+		if (!(committer_ident_explicitly_given & IDENT_MAIL_GIVEN))
+			die("requested explicitly given ident in config, "
+			    "but user.email is not set, or GIT_COMMITTER_EMAIL "
+			    "is not set");
+	}
+
 	return fmt_ident(getenv("GIT_COMMITTER_NAME"),
 			 getenv("GIT_COMMITTER_EMAIL"),
 			 getenv("GIT_COMMITTER_DATE"),
@@ -444,6 +484,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+	if (!strcmp(var, "user.explicit")) {
+		ident_explicit = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (!strcmp(var, "user.name")) {
 		if (!value)
 			return config_error_nonbool(var);
diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
new file mode 100755
index 000000000000..f6f42288a10c
--- /dev/null
+++ b/t/t9904-per-repo-email.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Dan Aloni
+#
+
+test_description='per-repo forced setting of email address'
+
+. ./test-lib.sh
+
+prepare () {
+	echo "Initial" >foo &&
+	git add foo &&
+	unset GIT_AUTHOR_NAME &&
+	unset GIT_AUTHOR_EMAIL &&
+	test_unconfig --global user.name &&
+	test_unconfig --global user.email &&
+	test_unconfig user.name &&
+	test_unconfig user.email &&
+	test_config_global user.explicit true
+}
+
+test_expect_success 'fails commiting if clone email is not set' '
+        prepare &&
+
+	EDITOR=: VISUAL=: test_must_fail git commit -m msg
+
+'
+
+test_expect_success 'succeeds commiting if clone email is set' '
+        prepare &&
+
+	test_config user.name "test" &&
+	test_config user.email "[email-masked]" &&
+	EDITOR=: VISUAL=: git commit -m msg
+'
+
+test_done
-- 
2.5.0

-- 
Dan Aloni