{"thread":{"id":"41336","subject":"[PATCH v6 1/3] fmt_ident: refactor strictness checks","startedAt":"2016-02-05T07:42:25Z","lastAt":"2016-02-05T21:14:35Z","messageCount":11,"participants":["Dan Aloni","Junio C Hamano","Jeff King","Eric Sunshine"],"isPatch":true,"patchVersion":6,"patchTotal":3},"messages":[{"id":"277516","messageId":"1454658148-3031-1-git-send-email-alonid@gmail.com","threadId":"41336","inReplyTo":null,"subject":"[PATCH v6] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T07:42:25Z","receivedAt":"2016-02-05T07:42:25Z","isPatch":true,"sender":{"key":"alonid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/321273?v=4"},"body":"\nChange between v5 -> v6:\n\n* Removed trailing comma in 'enum ident_person'.\n\nv5: http://article.gmane.org/gmane.comp.version-control.git/285544\n"},{"id":"277515","messageId":"1454658148-3031-2-git-send-email-alonid@gmail.com","threadId":"41336","inReplyTo":"1454658148-3031-1-git-send-email-alonid@gmail.com","subject":"[PATCH v6 1/3] fmt_ident: refactor strictness checks","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T07:42:26Z","receivedAt":"2016-02-05T07:42:26Z","isPatch":true,"sender":{"key":"alonid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/321273?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nThis function has evolved quite a bit over time, and as a\nresult, the logic for \"is this an OK ident\" has been\nsprinkled throughout. This ends up with a lot of redundant\nconditionals, like checking want_name repeatedly. Worse,\nwe want to know in many cases whether we are using the\n\"default\" ident, and we do so by comparing directly to the\nglobal strbuf, which violates the abstraction of the\nident_default_* functions.\n\nLet's reorganize the function into a hierarchy of\nconditionals to handle similar cases together. The only\ncase that doesn't just work naturally for this is that of an\nempty name, where our advice is different based on whether\nwe came from ident_default_name() or not. We can use a\nsimple flag to cover this case.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n ident.c | 46 ++++++++++++++++++++++++----------------------\n 1 file changed, 24 insertions(+), 22 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 3da555634290..f3a431f738cc 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,\n \tint want_date = !(flag & IDENT_NO_DATE);\n \tint want_name = !(flag & IDENT_NO_NAME);\n \n-\tif (want_name && !name)\n-\t\tname = ident_default_name();\n-\tif (!email)\n-\t\temail = ident_default_email();\n-\n-\tif (want_name && !*name) {\n-\t\tstruct passwd *pw;\n-\n-\t\tif (strict) {\n-\t\t\tif (name == git_default_name.buf)\n+\tif (want_name) {\n+\t\tint using_default = 0;\n+\t\tif (!name) {\n+\t\t\tname = ident_default_name();\n+\t\t\tusing_default = 1;\n+\t\t\tif (strict && default_name_is_bogus) {\n \t\t\t\tfputs(env_hint, stderr);\n-\t\t\tdie(\"empty ident name (for <%s>) not allowed\", email);\n+\t\t\t\tdie(\"unable to auto-detect name (got '%s')\", name);\n+\t\t\t}\n+\t\t}\n+\t\tif (!*name) {\n+\t\t\tstruct passwd *pw;\n+\t\t\tif (strict) {\n+\t\t\t\tif (using_default)\n+\t\t\t\t\tfputs(env_hint, stderr);\n+\t\t\t\tdie(\"empty ident name (for <%s>) not allowed\", email);\n+\t\t\t}\n+\t\t\tpw = xgetpwuid_self(NULL);\n+\t\t\tname = pw->pw_name;\n \t\t}\n-\t\tpw = xgetpwuid_self(NULL);\n-\t\tname = pw->pw_name;\n-\t}\n-\n-\tif (want_name && strict &&\n-\t    name == git_default_name.buf && default_name_is_bogus) {\n-\t\tfputs(env_hint, stderr);\n-\t\tdie(\"unable to auto-detect name (got '%s')\", name);\n \t}\n \n-\tif (strict && email == git_default_email.buf && default_email_is_bogus) {\n-\t\tfputs(env_hint, stderr);\n-\t\tdie(\"unable to auto-detect email address (got '%s')\", email);\n+\tif (!email) {\n+\t\temail = ident_default_email();\n+\t\tif (strict && default_email_is_bogus) {\n+\t\t\tfputs(env_hint, stderr);\n+\t\t\tdie(\"unable to auto-detect email address (got '%s')\", email);\n+\t\t}\n \t}\n \n \tstrbuf_reset(&ident);\n-- \n2.5.0\n"},{"id":"277518","messageId":"1454658148-3031-3-git-send-email-alonid@gmail.com","threadId":"41336","inReplyTo":"1454658148-3031-1-git-send-email-alonid@gmail.com","subject":"[PATCH v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T07:42:27Z","receivedAt":"2016-02-05T07:42:27Z","isPatch":true,"sender":{"key":"alonid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/321273?v=4"},"body":"It used to be that:\n\n   git config --global user.email \"(none)\"\n\nwas a viable way for people to force themselves to set user.email in\neach repository.  This was helpful for people with more than one\nemail address, targeting different email addresses for different\nclones, as it barred git from creating commit unless the user.email\nconfig was set in the per-repo config to the correct email address.\n\nA recent change, 19ce497c (ident: keep a flag for bogus\ndefault_email, 2015-12-10), however declared that an explicitly\nconfigured user.email is not bogus, no matter what its value is, so\nthis hack no longer works.\n\nProvide the same functionality by adding a new configuration\nvariable user.useConfigOnly; when this variable is set, the\nuser must explicitly set user.email configuration.\n\nSigned-off-by: Dan Aloni <alonid@gmail.com>\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nCc: Eric Sunshine <sunshine@sunshineco.com>\n---\n Documentation/config.txt  |  9 ++++++++\n ident.c                   | 16 +++++++++++++\n t/t9904-per-repo-email.sh | 58 +++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 83 insertions(+)\n create mode 100755 t/t9904-per-repo-email.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 02bcde6bb596..25cf7ce4e83a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2821,6 +2821,15 @@ user.name::\n \tCan be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'\n \tenvironment variables.  See linkgit:git-commit-tree[1].\n \n+user.useConfigOnly::\n+\tThis instruct Git to avoid trying to guess defaults for 'user.email'\n+\tand 'user.name' other than strictly from environment or config.\n+\tIf you have multiple email addresses that you would like to set\n+\tup per repository, you may want to set this to 'true' in the global\n+\tconfig, and then Git would prompt you to set user.email separately,\n+\tin each of the cloned repositories.\n+\tDefaults to `false`.\n+\n user.signingKey::\n \tIf linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the\n \tkey you want it to automatically when creating a signed tag or\ndiff --git a/ident.c b/ident.c\nindex f3a431f738cc..9bd6ac69bfe8 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;\n static int default_email_is_bogus;\n static int default_name_is_bogus;\n \n+static int ident_use_config_only;\n+\n #define IDENT_NAME_GIVEN 01\n #define IDENT_MAIL_GIVEN 02\n #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)\n static int committer_ident_explicitly_given;\n static int author_ident_explicitly_given;\n+static int ident_config_given;\n \n #ifdef NO_GECOS_IN_PWENT\n #define get_gecos(ignored) \"&\"\n@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,\n \t\t\t\tfputs(env_hint, stderr);\n \t\t\t\tdie(\"unable to auto-detect name (got '%s')\", name);\n \t\t\t}\n+\t\t\tif (strict && ident_use_config_only &&\n+\t\t\t    !(ident_config_given & IDENT_NAME_GIVEN))\n+\t\t\t\tdie(\"user.useConfigOnly set but no name given\");\n \t\t}\n \t\tif (!*name) {\n \t\t\tstruct passwd *pw;\n@@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,\n \t\t\tfputs(env_hint, stderr);\n \t\t\tdie(\"unable to auto-detect email address (got '%s')\", email);\n \t\t}\n+\t\tif (strict && ident_use_config_only\n+\t\t    && !(ident_config_given & IDENT_MAIL_GIVEN))\n+\t\t\tdie(\"user.useConfigOnly set but no mail given\");\n \t}\n \n \tstrbuf_reset(&ident);\n@@ -446,6 +455,11 @@ int author_ident_sufficiently_given(void)\n \n int git_ident_config(const char *var, const char *value, void *data)\n {\n+\tif (!strcmp(var, \"user.useconfigonly\")) {\n+\t\tident_use_config_only = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"user.name\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -453,6 +467,7 @@ int git_ident_config(const char *var, const char *value, void *data)\n \t\tstrbuf_addstr(&git_default_name, value);\n \t\tcommitter_ident_explicitly_given |= IDENT_NAME_GIVEN;\n \t\tauthor_ident_explicitly_given |= IDENT_NAME_GIVEN;\n+\t\tident_config_given |= IDENT_NAME_GIVEN;\n \t\treturn 0;\n \t}\n \n@@ -463,6 +478,7 @@ int git_ident_config(const char *var, const char *value, void *data)\n \t\tstrbuf_addstr(&git_default_email, value);\n \t\tcommitter_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\tauthor_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n+\t\tident_config_given |= IDENT_MAIL_GIVEN;\n \t\treturn 0;\n \t}\n \ndiff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh\nnew file mode 100755\nindex 000000000000..0430f2e38434\n--- /dev/null\n+++ b/t/t9904-per-repo-email.sh\n@@ -0,0 +1,58 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2016 Dan Aloni\n+#\n+\n+test_description='per-repo forced setting of email address'\n+\n+. ./test-lib.sh\n+\n+prepare () {\n+\t# Have a non-empty repository\n+\trm -fr .git\n+\tgit init\n+\techo \"Initial\" >foo &&\n+\tgit add foo &&\n+\tgit commit -m foo &&\n+\n+\t# Setup a likely user.useConfigOnly use case\n+\tsane_unset GIT_AUTHOR_NAME &&\n+\tsane_unset GIT_AUTHOR_EMAIL &&\n+\ttest_unconfig --global user.name &&\n+\ttest_unconfig --global user.email &&\n+\ttest_config user.name \"test\" &&\n+\ttest_unconfig user.email &&\n+\ttest_config_global user.useConfigOnly true\n+}\n+\n+about_to_commit () {\n+\techo \"Second\" >>foo &&\n+\tgit add foo\n+}\n+\n+test_expect_success 'fails committing if clone email is not set' '\n+\tprepare && about_to_commit &&\n+\n+\ttest_must_fail git commit -m msg\n+'\n+\n+test_expect_success 'fails committing if clone email is not set, but EMAIL set' '\n+\tprepare && about_to_commit &&\n+\n+\ttest_must_fail env EMAIL=test@fail.com git commit -m msg\n+'\n+\n+test_expect_success 'succeeds committing if clone email is set' '\n+\tprepare && about_to_commit &&\n+\n+\ttest_config user.email \"test@ok.com\" &&\n+\tgit commit -m msg\n+'\n+\n+test_expect_success 'succeeds cloning if global email is not set' '\n+\tprepare &&\n+\n+\tgit clone . clone\n+'\n+\n+test_done\n-- \n2.5.0\n"},{"id":"277517","messageId":"1454658148-3031-4-git-send-email-alonid@gmail.com","threadId":"41336","inReplyTo":"1454658148-3031-1-git-send-email-alonid@gmail.com","subject":"[PATCH v6 3/3] ident: cleanup wrt ident's source","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T07:42:28Z","receivedAt":"2016-02-05T07:42:28Z","isPatch":true,"sender":{"key":"alonid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/321273?v=4"},"body":"This change condenses the variables that tells where we got the user's\nident into single enum, instead of a collection of booleans.\n\nIn addtion, also have {committer,author}_ident_sufficiently_given\ndirectly probe the environment and the afformentioned enum instead of\nrelying on git_{committer,author}_info to do so.\n\nSigned-off-by: Dan Aloni <alonid@gmail.com>\nHelped-by: Jeff King <peff@peff.net>\nHelped-by: Junio C Hamano <gitster@pobox.com>\n---\n ident.c | 126 ++++++++++++++++++++++++++++++++++++++++------------------------\n 1 file changed, 80 insertions(+), 46 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 9bd6ac69bfe8..9bb05912b59a 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -10,17 +10,19 @@\n static struct strbuf git_default_name = STRBUF_INIT;\n static struct strbuf git_default_email = STRBUF_INIT;\n static struct strbuf git_default_date = STRBUF_INIT;\n-static int default_email_is_bogus;\n-static int default_name_is_bogus;\n+\n+enum ident_source {\n+\tIDENT_SOURCE_UNKNOWN = 0,\n+\tIDENT_SOURCE_CONFIG,\n+\tIDENT_SOURCE_ENVIRONMENT,\n+\tIDENT_SOURCE_GUESSED,\n+\tIDENT_SOURCE_GUESSED_BOGUS\n+};\n \n static int ident_use_config_only;\n \n-#define IDENT_NAME_GIVEN 01\n-#define IDENT_MAIL_GIVEN 02\n-#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)\n-static int committer_ident_explicitly_given;\n-static int author_ident_explicitly_given;\n-static int ident_config_given;\n+static enum ident_source source_of_default_email = IDENT_SOURCE_UNKNOWN;\n+static enum ident_source source_of_default_name = IDENT_SOURCE_UNKNOWN;\n \n #ifdef NO_GECOS_IN_PWENT\n #define get_gecos(ignored) \"&\"\n@@ -28,7 +30,7 @@ static int ident_config_given;\n #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)\n #endif\n \n-static struct passwd *xgetpwuid_self(int *is_bogus)\n+static struct passwd *xgetpwuid_self(enum ident_source *source)\n {\n \tstruct passwd *pw;\n \n@@ -41,9 +43,13 @@ static struct passwd *xgetpwuid_self(int *is_bogus)\n \t\tfallback.pw_gecos = \"Unknown\";\n #endif\n \t\tpw = &fallback;\n-\t\tif (is_bogus)\n-\t\t\t*is_bogus = 1;\n+\t\tif (source)\n+\t\t\t*source = IDENT_SOURCE_GUESSED_BOGUS;\n+\t} else {\n+\t\tif (source)\n+\t\t\t*source = IDENT_SOURCE_GUESSED;\n \t}\n+\n \treturn pw;\n }\n \n@@ -120,26 +126,26 @@ static int canonical_name(const char *host, struct strbuf *out)\n \treturn status;\n }\n \n-static void add_domainname(struct strbuf *out, int *is_bogus)\n+static void add_domainname(struct strbuf *out, enum ident_source *source)\n {\n \tchar buf[1024];\n \n \tif (gethostname(buf, sizeof(buf))) {\n \t\twarning(\"cannot get host name: %s\", strerror(errno));\n \t\tstrbuf_addstr(out, \"(none)\");\n-\t\t*is_bogus = 1;\n+\t\t*source = IDENT_SOURCE_GUESSED_BOGUS;\n \t\treturn;\n \t}\n \tif (strchr(buf, '.'))\n \t\tstrbuf_addstr(out, buf);\n \telse if (canonical_name(buf, out) < 0) {\n \t\tstrbuf_addf(out, \"%s.(none)\", buf);\n-\t\t*is_bogus = 1;\n+\t\t*source = IDENT_SOURCE_GUESSED_BOGUS;\n \t}\n }\n \n static void copy_email(const struct passwd *pw, struct strbuf *email,\n-\t\t       int *is_bogus)\n+\t\t       enum ident_source *source)\n {\n \t/*\n \t * Make up a fake email address\n@@ -150,13 +156,13 @@ static void copy_email(const struct passwd *pw, struct strbuf *email,\n \n \tif (!add_mailname_host(email))\n \t\treturn;\t/* read from \"/etc/mailname\" (Debian) */\n-\tadd_domainname(email, is_bogus);\n+\tadd_domainname(email, source);\n }\n \n const char *ident_default_name(void)\n {\n \tif (!git_default_name.len) {\n-\t\tcopy_gecos(xgetpwuid_self(&default_name_is_bogus), &git_default_name);\n+\t\tcopy_gecos(xgetpwuid_self(&source_of_default_name), &git_default_name);\n \t\tstrbuf_trim(&git_default_name);\n \t}\n \treturn git_default_name.buf;\n@@ -169,11 +175,12 @@ const char *ident_default_email(void)\n \n \t\tif (email && email[0]) {\n \t\t\tstrbuf_addstr(&git_default_email, email);\n-\t\t\tcommitter_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n-\t\t\tauthor_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n-\t\t} else\n-\t\t\tcopy_email(xgetpwuid_self(&default_email_is_bogus),\n-\t\t\t\t   &git_default_email, &default_email_is_bogus);\n+\t\t\tsource_of_default_email = IDENT_SOURCE_ENVIRONMENT;\n+\t\t} else {\n+\t\t\tcopy_email(xgetpwuid_self(&source_of_default_email),\n+\t\t\t\t   &git_default_email, &source_of_default_email);\n+\t\t}\n+\n \t\tstrbuf_trim(&git_default_email);\n \t}\n \treturn git_default_email.buf;\n@@ -353,12 +360,13 @@ const char *fmt_ident(const char *name, const char *email,\n \t\tif (!name) {\n \t\t\tname = ident_default_name();\n \t\t\tusing_default = 1;\n-\t\t\tif (strict && default_name_is_bogus) {\n+\t\t\tif (strict &&\n+\t\t\t    source_of_default_name == IDENT_SOURCE_GUESSED_BOGUS) {\n \t\t\t\tfputs(env_hint, stderr);\n \t\t\t\tdie(\"unable to auto-detect name (got '%s')\", name);\n \t\t\t}\n \t\t\tif (strict && ident_use_config_only &&\n-\t\t\t    !(ident_config_given & IDENT_NAME_GIVEN))\n+\t\t\t    source_of_default_name != IDENT_SOURCE_CONFIG)\n \t\t\t\tdie(\"user.useConfigOnly set but no name given\");\n \t\t}\n \t\tif (!*name) {\n@@ -375,12 +383,12 @@ const char *fmt_ident(const char *name, const char *email,\n \n \tif (!email) {\n \t\temail = ident_default_email();\n-\t\tif (strict && default_email_is_bogus) {\n+\t\tif (strict && source_of_default_email == IDENT_SOURCE_GUESSED_BOGUS) {\n \t\t\tfputs(env_hint, stderr);\n \t\t\tdie(\"unable to auto-detect email address (got '%s')\", email);\n \t\t}\n-\t\tif (strict && ident_use_config_only\n-\t\t    && !(ident_config_given & IDENT_MAIL_GIVEN))\n+\t\tif (strict && ident_use_config_only &&\n+\t\t    source_of_default_email != IDENT_SOURCE_CONFIG)\n \t\t\tdie(\"user.useConfigOnly set but no mail given\");\n \t}\n \n@@ -412,10 +420,6 @@ const char *fmt_name(const char *name, const char *email)\n \n const char *git_author_info(int flag)\n {\n-\tif (getenv(\"GIT_AUTHOR_NAME\"))\n-\t\tauthor_ident_explicitly_given |= IDENT_NAME_GIVEN;\n-\tif (getenv(\"GIT_AUTHOR_EMAIL\"))\n-\t\tauthor_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \treturn fmt_ident(getenv(\"GIT_AUTHOR_NAME\"),\n \t\t\t getenv(\"GIT_AUTHOR_EMAIL\"),\n \t\t\t getenv(\"GIT_AUTHOR_DATE\"),\n@@ -424,33 +428,67 @@ const char *git_author_info(int flag)\n \n const char *git_committer_info(int flag)\n {\n-\tif (getenv(\"GIT_COMMITTER_NAME\"))\n-\t\tcommitter_ident_explicitly_given |= IDENT_NAME_GIVEN;\n-\tif (getenv(\"GIT_COMMITTER_EMAIL\"))\n-\t\tcommitter_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \treturn fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n \t\t\t getenv(\"GIT_COMMITTER_EMAIL\"),\n \t\t\t getenv(\"GIT_COMMITTER_DATE\"),\n \t\t\t flag);\n }\n \n-static int ident_is_sufficient(int user_ident_explicitly_given)\n+enum ident_person {\n+\tIDENT_PERSON_COMMITTER,\n+\tIDENT_PERSON_AUTHOR\n+};\n+\n+static int ident_source_is_sufficient(enum ident_source source)\n+{\n+\tswitch (source) {\n+\tcase IDENT_SOURCE_CONFIG:\n+\tcase IDENT_SOURCE_ENVIRONMENT:\n+\t\treturn 1;\n+\tdefault:\n+\t\tbreak;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int ident_is_sufficient(enum ident_person person)\n {\n+\tconst char *str_name, *str_email;\n+\tint name, email;\n+\n+\tswitch (person) {\n+\tcase IDENT_PERSON_COMMITTER:\n+\t\tstr_name = getenv(\"GIT_COMMITTER_NAME\");\n+\t\tstr_email = getenv(\"GIT_COMMITTER_EMAIL\");\n+\t\tbreak;\n+\tcase IDENT_PERSON_AUTHOR:\n+\t\tstr_name = getenv(\"GIT_AUTHOR_NAME\");\n+\t\tstr_email = getenv(\"GIT_AUTHOR_EMAIL\");\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"invalid parameter to ident_is_sufficient()\");\n+\t}\n+\n+\tname = !!str_name || ident_source_is_sufficient(source_of_default_name);\n+\temail = !!str_email || ident_source_is_sufficient(source_of_default_email);\n+\n #ifndef WINDOWS\n-\treturn (user_ident_explicitly_given & IDENT_MAIL_GIVEN);\n+\t(void)name;\n+\treturn email;\n #else\n-\treturn (user_ident_explicitly_given == IDENT_ALL_GIVEN);\n+\treturn name && email;\n #endif\n }\n \n int committer_ident_sufficiently_given(void)\n {\n-\treturn ident_is_sufficient(committer_ident_explicitly_given);\n+\treturn ident_is_sufficient(IDENT_PERSON_COMMITTER);\n }\n \n int author_ident_sufficiently_given(void)\n {\n-\treturn ident_is_sufficient(author_ident_explicitly_given);\n+\treturn ident_is_sufficient(IDENT_PERSON_AUTHOR);\n }\n \n int git_ident_config(const char *var, const char *value, void *data)\n@@ -465,9 +503,7 @@ int git_ident_config(const char *var, const char *value, void *data)\n \t\t\treturn config_error_nonbool(var);\n \t\tstrbuf_reset(&git_default_name);\n \t\tstrbuf_addstr(&git_default_name, value);\n-\t\tcommitter_ident_explicitly_given |= IDENT_NAME_GIVEN;\n-\t\tauthor_ident_explicitly_given |= IDENT_NAME_GIVEN;\n-\t\tident_config_given |= IDENT_NAME_GIVEN;\n+\t\tsource_of_default_name = IDENT_SOURCE_CONFIG;\n \t\treturn 0;\n \t}\n \n@@ -476,9 +512,7 @@ int git_ident_config(const char *var, const char *value, void *data)\n \t\t\treturn config_error_nonbool(var);\n \t\tstrbuf_reset(&git_default_email);\n \t\tstrbuf_addstr(&git_default_email, value);\n-\t\tcommitter_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n-\t\tauthor_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n-\t\tident_config_given |= IDENT_MAIL_GIVEN;\n+\t\tsource_of_default_email = IDENT_SOURCE_CONFIG;\n \t\treturn 0;\n \t}\n \n-- \n2.5.0\n"},{"id":"277558","messageId":"xmqqsi17c70w.fsf@gitster.mtv.corp.google.com","threadId":"41336","inReplyTo":"1454658148-3031-4-git-send-email-alonid@gmail.com","subject":"Re: [PATCH v6 3/3] ident: cleanup wrt ident's source","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-05T19:05:19Z","receivedAt":"2016-02-05T19:05:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dan Aloni <alonid@gmail.com> writes:\n\n> This change condenses the variables that tells where we got the user's\n> ident into single enum, instead of a collection of booleans.\n>\n> In addtion, also have {committer,author}_ident_sufficiently_given\n> directly probe the environment and the afformentioned enum instead of\n> relying on git_{committer,author}_info to do so.\n>\n> Signed-off-by: Dan Aloni <alonid@gmail.com>\n> Helped-by: Jeff King <peff@peff.net>\n> Helped-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  ident.c | 126 ++++++++++++++++++++++++++++++++++++++++------------------------\n>  1 file changed, 80 insertions(+), 46 deletions(-)\n\nPeff what do you think?  I am asking you because personally I do not\nfind this particularly easier to read than the original, but since\nyou stared at the code around here recently much longer than I did\nwhen doing the 1/3, I thought you may be a better judge than I am.\n\n> diff --git a/ident.c b/ident.c\n> index 9bd6ac69bfe8..9bb05912b59a 100644\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -10,17 +10,19 @@\n>  static struct strbuf git_default_name = STRBUF_INIT;\n>  static struct strbuf git_default_email = STRBUF_INIT;\n>  static struct strbuf git_default_date = STRBUF_INIT;\n> -static int default_email_is_bogus;\n> -static int default_name_is_bogus;\n> +\n> +enum ident_source {\n> +\tIDENT_SOURCE_UNKNOWN = 0,\n> +\tIDENT_SOURCE_CONFIG,\n> +\tIDENT_SOURCE_ENVIRONMENT,\n> +\tIDENT_SOURCE_GUESSED,\n> +\tIDENT_SOURCE_GUESSED_BOGUS\n> +};\n>  \n>  static int ident_use_config_only;\n>  \n> -#define IDENT_NAME_GIVEN 01\n> -#define IDENT_MAIL_GIVEN 02\n> -#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)\n> -static int committer_ident_explicitly_given;\n> -static int author_ident_explicitly_given;\n> -static int ident_config_given;\n> +static enum ident_source source_of_default_email = IDENT_SOURCE_UNKNOWN;\n> +static enum ident_source source_of_default_name = IDENT_SOURCE_UNKNOWN;\n>  \n>  #ifdef NO_GECOS_IN_PWENT\n>  #define get_gecos(ignored) \"&\"\n> @@ -28,7 +30,7 @@ static int ident_config_given;\n>  #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)\n>  #endif\n>  \n> -static struct passwd *xgetpwuid_self(int *is_bogus)\n> +static struct passwd *xgetpwuid_self(enum ident_source *source)\n>  {\n>  \tstruct passwd *pw;\n>  \n> @@ -41,9 +43,13 @@ static struct passwd *xgetpwuid_self(int *is_bogus)\n>  \t\tfallback.pw_gecos = \"Unknown\";\n>  #endif\n>  \t\tpw = &fallback;\n> -\t\tif (is_bogus)\n> -\t\t\t*is_bogus = 1;\n> +\t\tif (source)\n> +\t\t\t*source = IDENT_SOURCE_GUESSED_BOGUS;\n> +\t} else {\n> +\t\tif (source)\n> +\t\t\t*source = IDENT_SOURCE_GUESSED;\n>  \t}\n> +\n>  \treturn pw;\n>  }\n>  \n> @@ -120,26 +126,26 @@ static int canonical_name(const char *host, struct strbuf *out)\n>  \treturn status;\n>  }\n>  \n> -static void add_domainname(struct strbuf *out, int *is_bogus)\n> +static void add_domainname(struct strbuf *out, enum ident_source *source)\n>  {\n>  \tchar buf[1024];\n>  \n>  \tif (gethostname(buf, sizeof(buf))) {\n>  \t\twarning(\"cannot get host name: %s\", strerror(errno));\n>  \t\tstrbuf_addstr(out, \"(none)\");\n> -\t\t*is_bogus = 1;\n> +\t\t*source = IDENT_SOURCE_GUESSED_BOGUS;\n>  \t\treturn;\n>  \t}\n>  \tif (strchr(buf, '.'))\n>  \t\tstrbuf_addstr(out, buf);\n>  \telse if (canonical_name(buf, out) < 0) {\n>  \t\tstrbuf_addf(out, \"%s.(none)\", buf);\n> -\t\t*is_bogus = 1;\n> +\t\t*source = IDENT_SOURCE_GUESSED_BOGUS;\n>  \t}\n>  }\n>  \n>  static void copy_email(const struct passwd *pw, struct strbuf *email,\n> -\t\t       int *is_bogus)\n> +\t\t       enum ident_source *source)\n>  {\n>  \t/*\n>  \t * Make up a fake email address\n> @@ -150,13 +156,13 @@ static void copy_email(const struct passwd *pw, struct strbuf *email,\n>  \n>  \tif (!add_mailname_host(email))\n>  \t\treturn;\t/* read from \"/etc/mailname\" (Debian) */\n> -\tadd_domainname(email, is_bogus);\n> +\tadd_domainname(email, source);\n>  }\n>  \n>  const char *ident_default_name(void)\n>  {\n>  \tif (!git_default_name.len) {\n> -\t\tcopy_gecos(xgetpwuid_self(&default_name_is_bogus), &git_default_name);\n> +\t\tcopy_gecos(xgetpwuid_self(&source_of_default_name), &git_default_name);\n>  \t\tstrbuf_trim(&git_default_name);\n>  \t}\n>  \treturn git_default_name.buf;\n> @@ -169,11 +175,12 @@ const char *ident_default_email(void)\n>  \n>  \t\tif (email && email[0]) {\n>  \t\t\tstrbuf_addstr(&git_default_email, email);\n> -\t\t\tcommitter_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n> -\t\t\tauthor_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n> -\t\t} else\n> -\t\t\tcopy_email(xgetpwuid_self(&default_email_is_bogus),\n> -\t\t\t\t   &git_default_email, &default_email_is_bogus);\n> +\t\t\tsource_of_default_email = IDENT_SOURCE_ENVIRONMENT;\n> +\t\t} else {\n> +\t\t\tcopy_email(xgetpwuid_self(&source_of_default_email),\n> +\t\t\t\t   &git_default_email, &source_of_default_email);\n> +\t\t}\n> +\n>  \t\tstrbuf_trim(&git_default_email);\n>  \t}\n>  \treturn git_default_email.buf;\n> @@ -353,12 +360,13 @@ const char *fmt_ident(const char *name, const char *email,\n>  \t\tif (!name) {\n>  \t\t\tname = ident_default_name();\n>  \t\t\tusing_default = 1;\n> -\t\t\tif (strict && default_name_is_bogus) {\n> +\t\t\tif (strict &&\n> +\t\t\t    source_of_default_name == IDENT_SOURCE_GUESSED_BOGUS) {\n>  \t\t\t\tfputs(env_hint, stderr);\n>  \t\t\t\tdie(\"unable to auto-detect name (got '%s')\", name);\n>  \t\t\t}\n>  \t\t\tif (strict && ident_use_config_only &&\n> -\t\t\t    !(ident_config_given & IDENT_NAME_GIVEN))\n> +\t\t\t    source_of_default_name != IDENT_SOURCE_CONFIG)\n>  \t\t\t\tdie(\"user.useConfigOnly set but no name given\");\n>  \t\t}\n>  \t\tif (!*name) {\n> @@ -375,12 +383,12 @@ const char *fmt_ident(const char *name, const char *email,\n>  \n>  \tif (!email) {\n>  \t\temail = ident_default_email();\n> -\t\tif (strict && default_email_is_bogus) {\n> +\t\tif (strict && source_of_default_email == IDENT_SOURCE_GUESSED_BOGUS) {\n>  \t\t\tfputs(env_hint, stderr);\n>  \t\t\tdie(\"unable to auto-detect email address (got '%s')\", email);\n>  \t\t}\n> -\t\tif (strict && ident_use_config_only\n> -\t\t    && !(ident_config_given & IDENT_MAIL_GIVEN))\n> +\t\tif (strict && ident_use_config_only &&\n> +\t\t    source_of_default_email != IDENT_SOURCE_CONFIG)\n>  \t\t\tdie(\"user.useConfigOnly set but no mail given\");\n>  \t}\n>  \n> @@ -412,10 +420,6 @@ const char *fmt_name(const char *name, const char *email)\n>  \n>  const char *git_author_info(int flag)\n>  {\n> -\tif (getenv(\"GIT_AUTHOR_NAME\"))\n> -\t\tauthor_ident_explicitly_given |= IDENT_NAME_GIVEN;\n> -\tif (getenv(\"GIT_AUTHOR_EMAIL\"))\n> -\t\tauthor_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n>  \treturn fmt_ident(getenv(\"GIT_AUTHOR_NAME\"),\n>  \t\t\t getenv(\"GIT_AUTHOR_EMAIL\"),\n>  \t\t\t getenv(\"GIT_AUTHOR_DATE\"),\n> @@ -424,33 +428,67 @@ const char *git_author_info(int flag)\n>  \n>  const char *git_committer_info(int flag)\n>  {\n> -\tif (getenv(\"GIT_COMMITTER_NAME\"))\n> -\t\tcommitter_ident_explicitly_given |= IDENT_NAME_GIVEN;\n> -\tif (getenv(\"GIT_COMMITTER_EMAIL\"))\n> -\t\tcommitter_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n>  \treturn fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n>  \t\t\t getenv(\"GIT_COMMITTER_EMAIL\"),\n>  \t\t\t getenv(\"GIT_COMMITTER_DATE\"),\n>  \t\t\t flag);\n>  }\n>  \n> -static int ident_is_sufficient(int user_ident_explicitly_given)\n> +enum ident_person {\n> +\tIDENT_PERSON_COMMITTER,\n> +\tIDENT_PERSON_AUTHOR\n> +};\n> +\n> +static int ident_source_is_sufficient(enum ident_source source)\n> +{\n> +\tswitch (source) {\n> +\tcase IDENT_SOURCE_CONFIG:\n> +\tcase IDENT_SOURCE_ENVIRONMENT:\n> +\t\treturn 1;\n> +\tdefault:\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int ident_is_sufficient(enum ident_person person)\n>  {\n> +\tconst char *str_name, *str_email;\n> +\tint name, email;\n\nCan we call this \"name_ok\" or something?\n\nAlso it feels strange to do the all name-related things in this\nfunction even though we immediately discard.  The code in this patch\nis still better than sprinkling more #ifdef to conditionally grab\nnames only on platforms that require names, but it may be an\nindication of the design flaw of trying to squeeze this entire logic\ninto this single function.  Perhaps another little helper function\ncan clean it up?  e.g.\n\n\tident_is_sufficient(committer_or_author?)\n        {\n\t#ifdef WINDOWS\n\t\tif (!explicitly_given(committer_or_author?, NAME))\n\t\t\treturn 0;\n\t#endif\n        \treturn explicitly_given(committer_or_author?, EMAIL))\n\t}\n\nor something?  I dunno.\n\n> +\n> +\tswitch (person) {\n> +\tcase IDENT_PERSON_COMMITTER:\n> +\t\tstr_name = getenv(\"GIT_COMMITTER_NAME\");\n> +\t\tstr_email = getenv(\"GIT_COMMITTER_EMAIL\");\n> +\t\tbreak;\n> +\tcase IDENT_PERSON_AUTHOR:\n> +\t\tstr_name = getenv(\"GIT_AUTHOR_NAME\");\n> +\t\tstr_email = getenv(\"GIT_AUTHOR_EMAIL\");\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tdie(\"invalid parameter to ident_is_sufficient()\");\n> +\t}\n> +\n> +\tname = !!str_name || ident_source_is_sufficient(source_of_default_name);\n> +\temail = !!str_email || ident_source_is_sufficient(source_of_default_email);\n>  #ifndef WINDOWS\n> -\treturn (user_ident_explicitly_given & IDENT_MAIL_GIVEN);\n> +\t(void)name;\n> +\treturn email;\n>  #else\n> -\treturn (user_ident_explicitly_given == IDENT_ALL_GIVEN);\n> +\treturn name && email;\n>  #endif\n>  }\n"},{"id":"277560","messageId":"20160205191805.GA7245@sigill.intra.peff.net","threadId":"41336","inReplyTo":"1454658148-3031-3-git-send-email-alonid@gmail.com","subject":"Re: [PATCH v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-05T19:18:05Z","receivedAt":"2016-02-05T19:18:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 05, 2016 at 09:42:27AM +0200, Dan Aloni wrote:\n\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 02bcde6bb596..25cf7ce4e83a 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2821,6 +2821,15 @@ user.name::\n>  \tCan be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'\n>  \tenvironment variables.  See linkgit:git-commit-tree[1].\n>  \n> +user.useConfigOnly::\n> +\tThis instruct Git to avoid trying to guess defaults for 'user.email'\n\ns/instruct/instructs/\n\n> +\tand 'user.name' other than strictly from environment or config.\n\nI find mention of the environment a bit ambiguous. Given our discussion,\nI'm sure you mean $GIT_AUTHOR_EMAIL, etc, and not $EMAIL. But I don't\nthink that is clear to somebody who has not been looking at this patch\nseries.\n\nI actually think we could simply say \"other than strictly from the\nconfig\", as people don't generally use $GIT_* themselves (rather, they\nget used mostly for inter-process communication, so at most script\nauthors need to know about them).\n\n> +\tIf you have multiple email addresses that you would like to set\n> +\tup per repository, you may want to set this to 'true' in the global\n\nI parsed this sentence as \"multiple addresses per repository\". Maybe:\n\n  If you have multiple email addresses and would like to use a different\n  one for each repository, you may...\n\nwould be more clear?\n\n> +test_description='per-repo forced setting of email address'\n> +\n> +. ./test-lib.sh\n> +\n> +prepare () {\n> +\t# Have a non-empty repository\n> +\trm -fr .git\n> +\tgit init\n> +\techo \"Initial\" >foo &&\n> +\tgit add foo &&\n> +\tgit commit -m foo &&\n> +\n> +\t# Setup a likely user.useConfigOnly use case\n> +\tsane_unset GIT_AUTHOR_NAME &&\n> +\tsane_unset GIT_AUTHOR_EMAIL &&\n> +\ttest_unconfig --global user.name &&\n> +\ttest_unconfig --global user.email &&\n> +\ttest_config user.name \"test\" &&\n> +\ttest_unconfig user.email &&\n> +\ttest_config_global user.useConfigOnly true\n> +}\n> +\n> +about_to_commit () {\n> +\techo \"Second\" >>foo &&\n> +\tgit add foo\n> +}\n> +\n> +test_expect_success 'fails committing if clone email is not set' '\n> +\tprepare && about_to_commit &&\n> +\n> +\ttest_must_fail git commit -m msg\n> +'\n\nThe flow of this test script is a bit different than what we usually\nwrite. Typically we have some early test_expect_success blocks do setup\nfor the whole script, and then progress through a sequence (and we rely\non the test harness to do things like \"git init\").\n\nIOW, most of your \"prepare\" would go in the first block, and then the\nrest of the tests rely on it.\n\nThe only thing I really see that needs to be repeated for each test is\nsetting up the \"about to commit\" scenario. But you can simply use\n\"commit --allow-empty\" so that the tests work no matter what state the\nprevious test left us in. We care about the ident, not what gets\ncommitted.\n\n-Peff\n"},{"id":"277561","messageId":"20160205192413.GB7245@sigill.intra.peff.net","threadId":"41336","inReplyTo":"xmqqsi17c70w.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v6 3/3] ident: cleanup wrt ident's source","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-05T19:24:13Z","receivedAt":"2016-02-05T19:24:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 05, 2016 at 11:05:19AM -0800, Junio C Hamano wrote:\n\n> Dan Aloni <alonid@gmail.com> writes:\n> \n> > This change condenses the variables that tells where we got the user's\n> > ident into single enum, instead of a collection of booleans.\n> >\n> > In addtion, also have {committer,author}_ident_sufficiently_given\n> > directly probe the environment and the afformentioned enum instead of\n> > relying on git_{committer,author}_info to do so.\n> >\n> > Signed-off-by: Dan Aloni <alonid@gmail.com>\n> > Helped-by: Jeff King <peff@peff.net>\n> > Helped-by: Junio C Hamano <gitster@pobox.com>\n> > ---\n> >  ident.c | 126 ++++++++++++++++++++++++++++++++++++++++------------------------\n> >  1 file changed, 80 insertions(+), 46 deletions(-)\n> \n> Peff what do you think?  I am asking you because personally I do not\n> find this particularly easier to read than the original, but since\n> you stared at the code around here recently much longer than I did\n> when doing the 1/3, I thought you may be a better judge than I am.\n\nI'm not sure it is really worth it unless we are going to expose this to\nthe user, and let them say \"I am OK with IDENT_SOURCE_GUESSED, but not\nIDENT_SOURCE_GUESSED_BOGUS\" or similar.\n\nWithout that, I think it is probably just making things a bit more\nbrittle.\n\n-Peff\n"},{"id":"277563","messageId":"CAPig+cRuRiL1_44tVFSP8k64aMhf6Tu0PeXfN8HUV7wX8=vLzA@mail.gmail.com","threadId":"41336","inReplyTo":"20160205191805.GA7245@sigill.intra.peff.net","subject":"Re: [PATCH v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-05T19:30:46Z","receivedAt":"2016-02-05T19:30:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Feb 5, 2016 at 2:18 PM, Jeff King <peff@peff.net> wrote:\n> On Fri, Feb 05, 2016 at 09:42:27AM +0200, Dan Aloni wrote:\n>> +prepare () {\n>> +     # Have a non-empty repository\n>> +     rm -fr .git\n>> +     git init\n>> +     echo \"Initial\" >foo &&\n>> +     git add foo &&\n>> +     git commit -m foo &&\n>> +\n>> +     # Setup a likely user.useConfigOnly use case\n>> +     sane_unset GIT_AUTHOR_NAME &&\n>> +     sane_unset GIT_AUTHOR_EMAIL &&\n>> +     test_unconfig --global user.name &&\n>> +     test_unconfig --global user.email &&\n>> +     test_config user.name \"test\" &&\n>> +     test_unconfig user.email &&\n>> +     test_config_global user.useConfigOnly true\n>> +}\n>\n> The flow of this test script is a bit different than what we usually\n> write. Typically we have some early test_expect_success blocks do setup\n> for the whole script, and then progress through a sequence (and we rely\n> on the test harness to do things like \"git init\").\n>\n> IOW, most of your \"prepare\" would go in the first block, and then the\n> rest of the tests rely on it.\n>\n> The only thing I really see that needs to be repeated for each test is\n> setting up the \"about to commit\" scenario. But you can simply use\n> \"commit --allow-empty\" so that the tests work no matter what state the\n> previous test left us in. We care about the ident, not what gets\n> committed.\n\nI was going to make all the same suggestions, so thanks. One thing to\nadd is that the &&-chain is broken in the prepare() function.\n"},{"id":"277564","messageId":"xmqqio23c5t5.fsf@gitster.mtv.corp.google.com","threadId":"41336","inReplyTo":"1454658148-3031-3-git-send-email-alonid@gmail.com","subject":"Re: [PATCH v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-05T19:31:34Z","receivedAt":"2016-02-05T19:31:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dan Aloni <alonid@gmail.com> writes:\n\n> +user.useConfigOnly::\n> +\tThis instruct Git to avoid trying to guess defaults for 'user.email'\n> +\tand 'user.name' other than strictly from environment or config.\n\nOK.\n\n> +\tIf you have multiple email addresses that you would like to set\n> +\tup per repository, you may want to set this to 'true' in the global\n> +\tconfig, and then Git would prompt you to set user.email separately,\n> +\tin each of the cloned repositories.\n\nThe first sentence mentioned both name and email, but here the\nexample is only about email.  A first time reader might be led into\nthinking this is only about email and not name, but I am assuming\nthat is not the intention (i.e. this is merely showing just one use\ncase).\n\n> +\tDefaults to `false`.\n> +\n>  user.signingKey::\n>  \tIf linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the\n>  \tkey you want it to automatically when creating a signed tag or\n> diff --git a/ident.c b/ident.c\n> index f3a431f738cc..9bd6ac69bfe8 100644\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;\n>  static int default_email_is_bogus;\n>  static int default_name_is_bogus;\n>  \n> +static int ident_use_config_only;\n> +\n>  #define IDENT_NAME_GIVEN 01\n>  #define IDENT_MAIL_GIVEN 02\n>  #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)\n>  static int committer_ident_explicitly_given;\n>  static int author_ident_explicitly_given;\n> +static int ident_config_given;\n>  \n>  #ifdef NO_GECOS_IN_PWENT\n>  #define get_gecos(ignored) \"&\"\n> @@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,\n>  \t\t\t\tfputs(env_hint, stderr);\n>  \t\t\t\tdie(\"unable to auto-detect name (got '%s')\", name);\n>  \t\t\t}\n> +\t\t\tif (strict && ident_use_config_only &&\n> +\t\t\t    !(ident_config_given & IDENT_NAME_GIVEN))\n> +\t\t\t\tdie(\"user.useConfigOnly set but no name given\");\n>  \t\t}\n>  \t\tif (!*name) {\n>  \t\t\tstruct passwd *pw;\n> @@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,\n>  \t\t\tfputs(env_hint, stderr);\n>  \t\t\tdie(\"unable to auto-detect email address (got '%s')\", email);\n>  \t\t}\n> +\t\tif (strict && ident_use_config_only\n> +\t\t    && !(ident_config_given & IDENT_MAIL_GIVEN))\n> +\t\t\tdie(\"user.useConfigOnly set but no mail given\");\n\nI can read the split expression either with && hanging at the end of\nline or && leading the next line just fine, but you'd want to be\nconsistent especially when you are writing two almost identical\nthings.\n\n> diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh\n> new file mode 100755\n> index 000000000000..0430f2e38434\n> --- /dev/null\n> +++ b/t/t9904-per-repo-email.sh\n> @@ -0,0 +1,58 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2016 Dan Aloni\n> +#\n> +\n> +test_description='per-repo forced setting of email address'\n> +\n> +. ./test-lib.sh\n> +\n> +prepare () {\n> +\t# Have a non-empty repository\n> +\trm -fr .git\n> +\tgit init\n\nHmm, do we do something drastic like this in any of our existing\ntests?\n\nWhen your test script starts by dot-sourcing test-lib.sh, you will\nbe given an initial repository with an empty history, so I'd expect\nthat you would be able to use that without calling \"prepare\" over\nand over again.  The usual convention is to do this kind of setup\nto establish a reasonable baseline in the first test titled 'setup'.\nI think the part up to where you unset the environment variables (by\nthe way, don't you need to unset GIT_COMMITTER_* variables, too?)\nbelongs to the 'setup' that is done only once at the beginning of\nthis script.\n\nEach of your tests will make changes to the state by setting or\nunsetting configuration variables and possibly making commits, that\nwould affect the state of the repository that will be used by the\nnext and subsequent tests.  test_when_finished helper can register\nthe clean-up procedure to revert these possible state changes, and\nyou can further avoid code duplication by calling that same clean-up\nprocedure at the end of the setup test.\n\nSo if this followed the style of a typical existing test, we would\nprobably do something along these lines:\n\n\treprepare () {\n\t\tgit reset --hard initial &&\n                echo Second >foo &&\n                git add foo\n\t}\n\n\ttest_expect_success setup '\n\t\techo Initial >foo &&\n                git add foo &&\n                git commit -m foo &&\n                git tag initial &&\n\n\t\tsane_unset GIT_AUTHOR_NAME GIT_COMMITTER_NAME ... &&\n                git config --global user.name test &&\n                git config --global user.useConfigOnly true &&\n\t\treprepare\n        '\n\n\ttest_expect_success 'fail without email anywhere' '\n        \ttest_when_finished reprepare &&\n                test_must_fail git commit -m msg\n\t'\n\n\ttest_expect_success 'suceed with config' '\n        \ttest_when_finished reprepare &&\n\t\ttest_config user.email test@ok.com &&\n                test_must_fail git commit -m msg\n\t'\n\nNote that you do not need \"test_unconfig user.email\" in reprepare,\nas the variable is set in one test with test_config, which uses\ntest_when_finished to arrange the variable to be removed after\nrunning the test.\n\n> +\techo \"Initial\" >foo &&\n> +\tgit add foo &&\n> +\tgit commit -m foo &&\n> +\n> +\t# Setup a likely user.useConfigOnly use case\n> +\tsane_unset GIT_AUTHOR_NAME &&\n> +\tsane_unset GIT_AUTHOR_EMAIL &&\n> +\ttest_unconfig --global user.name &&\n> +\ttest_unconfig --global user.email &&\n> +\ttest_config user.name \"test\" &&\n> +\ttest_unconfig user.email &&\n> +\ttest_config_global user.useConfigOnly true\n> +}\n> +\n> +about_to_commit () {\n> +\techo \"Second\" >>foo &&\n> +\tgit add foo\n> +}\n\n> +test_expect_success 'fails committing if clone email is not set' '\n> +\tprepare && about_to_commit &&\n> +\n> +\ttest_must_fail git commit -m msg\n> +'\n\n> +test_expect_success 'fails committing if clone email is not set, but EMAIL set' '\n> +\tprepare && about_to_commit &&\n> +\n> +\ttest_must_fail env EMAIL=test@fail.com git commit -m msg\n> +'\n> +\n> +test_expect_success 'succeeds committing if clone email is set' '\n> +\tprepare && about_to_commit &&\n> +\n> +\ttest_config user.email \"test@ok.com\" &&\n> +\tgit commit -m msg\n> +'\n> +\n> +test_expect_success 'succeeds cloning if global email is not set' '\n> +\tprepare &&\n> +\n> +\tgit clone . clone\n> +'\n> +\n> +test_done\n"},{"id":"277596","messageId":"20160205210330.GA7245@gmail.com","threadId":"41336","inReplyTo":"20160205192413.GB7245@sigill.intra.peff.net","subject":"Re: [PATCH v6 3/3] ident: cleanup wrt ident's source","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T21:03:30Z","receivedAt":"2016-02-05T21:03:30Z","isPatch":true,"sender":{"key":"alonid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/321273?v=4"},"body":"On Fri, Feb 05, 2016 at 02:24:13PM -0500, Jeff King wrote:\n> On Fri, Feb 05, 2016 at 11:05:19AM -0800, Junio C Hamano wrote:\n> \n> > Dan Aloni <alonid@gmail.com> writes:\n> > \n> > > This change condenses the variables that tells where we got the user's\n> > > ident into single enum, instead of a collection of booleans.\n> > >\n> > > In addtion, also have {committer,author}_ident_sufficiently_given\n> > > directly probe the environment and the afformentioned enum instead of\n> > > relying on git_{committer,author}_info to do so.\n> > >\n> > > Signed-off-by: Dan Aloni <alonid@gmail.com>\n> > > Helped-by: Jeff King <peff@peff.net>\n> > > Helped-by: Junio C Hamano <gitster@pobox.com>\n> > > ---\n> > >  ident.c | 126 ++++++++++++++++++++++++++++++++++++++++------------------------\n> > >  1 file changed, 80 insertions(+), 46 deletions(-)\n> > \n> > Peff what do you think?  I am asking you because personally I do not\n> > find this particularly easier to read than the original, but since\n> > you stared at the code around here recently much longer than I did\n> > when doing the 1/3, I thought you may be a better judge than I am.\n> \n> I'm not sure it is really worth it unless we are going to expose this to\n> the user, and let them say \"I am OK with IDENT_SOURCE_GUESSED, but not\n> IDENT_SOURCE_GUESSED_BOGUS\" or similar.\n> \n> Without that, I think it is probably just making things a bit more\n> brittle.\n\nOkay, will drop it now.\n\n-- \nDan Aloni\n"},{"id":"277598","messageId":"20160205211435.GB7245@gmail.com","threadId":"41336","inReplyTo":"xmqqio23c5t5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T21:14:35Z","receivedAt":"2016-02-05T21:14:35Z","isPatch":true,"sender":{"key":"alonid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/321273?v=4"},"body":"On Fri, Feb 05, 2016 at 11:31:34AM -0800, Junio C Hamano wrote:\n> > +\tIf you have multiple email addresses that you would like to set\n> > +\tup per repository, you may want to set this to 'true' in the global\n> > +\tconfig, and then Git would prompt you to set user.email separately,\n> > +\tin each of the cloned repositories.\n> \n> The first sentence mentioned both name and email, but here the\n> example is only about email.  A first time reader might be led into\n> thinking this is only about email and not name, but I am assuming\n> that is not the intention (i.e. this is merely showing just one use\n> case).\n>[..]\n\nGoing to revise per yours and Jeff's suggestions.\n\n>[..]\n> I can read the split expression either with && hanging at the end of\n> line or && leading the next line just fine, but you'd want to be\n> consistent especially when you are writing two almost identical\n> things.\n\nSure.\n\n>[..]\n> \ttest_expect_success 'suceed with config' '\n>         \ttest_when_finished reprepare &&\n> \t\ttest_config user.email test@ok.com &&\n>                 test_must_fail git commit -m msg\n> \t'\n> \n> Note that you do not need \"test_unconfig user.email\" in reprepare,\n> as the variable is set in one test with test_config, which uses\n> test_when_finished to arrange the variable to be removed after\n> running the test.\n\nAlright. It was worth to understand the differing behavior between\n'test_config' and 'git config'.\n\n-- \nDan Aloni\n"}]}