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

[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

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

[Dan Aloni]

From: Jeff King <peff@peff.net>

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 <peff@peff.net>
---
 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(&ident);
-- 
2.5.0

[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 <alonid@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Cc: Eric Sunshine <sunshine@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(&ident);
@@ -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(&git_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(&git_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 000000000000..0430f2e38434
--- /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 setting of email address'
+
+. ./test-lib.sh
+
+prepare () {
+	# Have a non-empty repository
+	rm -fr .git
+	git init
+	echo "Initial" >foo &&
+	git add foo &&
+	git commit -m foo &&
+
+	# Setup a likely user.useConfigOnly use case
+	sane_unset GIT_AUTHOR_NAME &&
+	sane_unset GIT_AUTHOR_EMAIL &&
+	test_unconfig --global user.name &&
+	test_unconfig --global user.email &&
+	test_config user.name "test" &&
+	test_unconfig user.email &&
+	test_config_global user.useConfigOnly true
+}
+
+about_to_commit () {
+	echo "Second" >>foo &&
+	git add foo
+}
+
+test_expect_success 'fails committing if clone email is not set' '
+	prepare && about_to_commit &&
+
+	test_must_fail git commit -m msg
+'
+
+test_expect_success 'fails committing if clone email is not set, but EMAIL set' '
+	prepare && about_to_commit &&
+
+	test_must_fail env EMAIL=test@fail.com git commit -m msg
+'
+
+test_expect_success 'succeeds committing if clone email is set' '
+	prepare && about_to_commit &&
+
+	test_config user.email "test@ok.com" &&
+	git commit -m msg
+'
+
+test_expect_success 'succeeds cloning if global email is not set' '
+	prepare &&
+
+	git clone . clone
+'
+
+test_done
-- 
2.5.0

[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 <alonid@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@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 = &fallback;
-		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(&default_name_is_bogus), &git_default_name);
+		copy_gecos(xgetpwuid_self(&source_of_default_name), &git_default_name);
 		strbuf_trim(&git_default_name);
 	}
 	return git_default_name.buf;
@@ -169,11 +175,12 @@ const char *ident_default_email(void)
 
 		if (email && email[0]) {
 			strbuf_addstr(&git_default_email, email);
-			committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-			author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-		} else
-			copy_email(xgetpwuid_self(&default_email_is_bogus),
-				   &git_default_email, &default_email_is_bogus);
+			source_of_default_email = IDENT_SOURCE_ENVIRONMENT;
+		} else {
+			copy_email(xgetpwuid_self(&source_of_default_email),
+				   &git_default_email, &source_of_default_email);
+		}
+
 		strbuf_trim(&git_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();
 			using_default = 1;
-			if (strict && default_name_is_bogus) {
+			if (strict &&
+			    source_of_default_name == IDENT_SOURCE_GUESSED_BOGUS) {
 				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))
+			    source_of_default_name != IDENT_SOURCE_CONFIG)
 				die("user.useConfigOnly set but no name given");
 		}
 		if (!*name) {
@@ -375,12 +383,12 @@ const char *fmt_ident(const char *name, const char *email,
 
 	if (!email) {
 		email = ident_default_email();
-		if (strict && default_email_is_bogus) {
+		if (strict && source_of_default_email == IDENT_SOURCE_GUESSED_BOGUS) {
 			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))
+		if (strict && ident_use_config_only &&
+		    source_of_default_email != IDENT_SOURCE_CONFIG)
 			die("user.useConfigOnly set but no mail given");
 	}
 
@@ -412,10 +420,6 @@ const char *fmt_name(const char *name, const char *email)
 
 const char *git_author_info(int flag)
 {
-	if (getenv("GIT_AUTHOR_NAME"))
-		author_ident_explicitly_given |= IDENT_NAME_GIVEN;
-	if (getenv("GIT_AUTHOR_EMAIL"))
-		author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
 	return fmt_ident(getenv("GIT_AUTHOR_NAME"),
 			 getenv("GIT_AUTHOR_EMAIL"),
 			 getenv("GIT_AUTHOR_DATE"),
@@ -424,33 +428,67 @@ const char *git_author_info(int flag)
 
 const char *git_committer_info(int flag)
 {
-	if (getenv("GIT_COMMITTER_NAME"))
-		committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
-	if (getenv("GIT_COMMITTER_EMAIL"))
-		committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
 	return fmt_ident(getenv("GIT_COMMITTER_NAME"),
 			 getenv("GIT_COMMITTER_EMAIL"),
 			 getenv("GIT_COMMITTER_DATE"),
 			 flag);
 }
 
-static int ident_is_sufficient(int user_ident_explicitly_given)
+enum ident_person {
+	IDENT_PERSON_COMMITTER,
+	IDENT_PERSON_AUTHOR,
+};
+
+static int ident_source_is_sufficient(enum ident_source source)
+{
+	switch (source) {
+	case IDENT_SOURCE_CONFIG:
+	case IDENT_SOURCE_ENVIRONMENT:
+		return 1;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int ident_is_sufficient(enum ident_person person)
 {
+	const char *str_name, *str_email;
+	int name, email;
+
+	switch (person) {
+	case IDENT_PERSON_COMMITTER:
+		str_name = getenv("GIT_COMMITTER_NAME");
+		str_email = getenv("GIT_COMMITTER_EMAIL");
+		break;
+	case IDENT_PERSON_AUTHOR:
+		str_name = getenv("GIT_AUTHOR_NAME");
+		str_email = getenv("GIT_AUTHOR_EMAIL");
+		break;
+	default:
+		die("invalid parameter to ident_is_sufficient()");
+	}
+
+	name = !!str_name || ident_source_is_sufficient(source_of_default_name);
+	email = !!str_email || ident_source_is_sufficient(source_of_default_email);
+
 #ifndef WINDOWS
-	return (user_ident_explicitly_given & IDENT_MAIL_GIVEN);
+	(void)name;
+	return email;
 #else
-	return (user_ident_explicitly_given == IDENT_ALL_GIVEN);
+	return name && email;
 #endif
 }
 
 int committer_ident_sufficiently_given(void)
 {
-	return ident_is_sufficient(committer_ident_explicitly_given);
+	return ident_is_sufficient(IDENT_PERSON_COMMITTER);
 }
 
 int author_ident_sufficiently_given(void)
 {
-	return ident_is_sufficient(author_ident_explicitly_given);
+	return ident_is_sufficient(IDENT_PERSON_AUTHOR);
 }
 
 int git_ident_config(const char *var, const char *value, void *data)
@@ -465,9 +503,7 @@ int git_ident_config(const char *var, const char *value, void *data)
 			return config_error_nonbool(var);
 		strbuf_reset(&git_default_name);
 		strbuf_addstr(&git_default_name, value);
-		committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
-		author_ident_explicitly_given |= IDENT_NAME_GIVEN;
-		ident_config_given |= IDENT_NAME_GIVEN;
+		source_of_default_name = IDENT_SOURCE_CONFIG;
 		return 0;
 	}
 
@@ -476,9 +512,7 @@ int git_ident_config(const char *var, const char *value, void *data)
 			return config_error_nonbool(var);
 		strbuf_reset(&git_default_email);
 		strbuf_addstr(&git_default_email, value);
-		committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-		author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-		ident_config_given |= IDENT_MAIL_GIVEN;
+		source_of_default_email = IDENT_SOURCE_CONFIG;
 		return 0;
 	}
 
-- 
2.5.0