{"thread":{"id":"41346","subject":"[PATCH v7] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","startedAt":"2016-02-05T21:29:04Z","lastAt":"2016-02-05T22:20:49Z","messageCount":8,"participants":["Dan Aloni","Jeff King","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":7,"patchTotal":null},"messages":[{"id":"277600","messageId":"1454707746-18672-1-git-send-email-alonid@gmail.com","threadId":"41346","inReplyTo":null,"subject":"[PATCH v7] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T21:29:04Z","receivedAt":"2016-02-05T21:29:04Z","isPatch":true,"sender":{"key":"alonid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/321273?v=4"},"body":"\nChanges between v6 -> v7:\n\n* Dropped patch: ident: cleanup wrt ident's source\n* Revised the documentation of the feature according to comments.\n* Revised the test according to comments.\n* Styling fix.\n\nv6: http://article.gmane.org/gmane.comp.version-control.git/285550\n"},{"id":"277601","messageId":"1454707746-18672-2-git-send-email-alonid@gmail.com","threadId":"41346","inReplyTo":"1454707746-18672-1-git-send-email-alonid@gmail.com","subject":"[PATCH v7 1/2] fmt_ident: refactor strictness checks","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T21:29:05Z","receivedAt":"2016-02-05T21:29:05Z","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":"277602","messageId":"1454707746-18672-3-git-send-email-alonid@gmail.com","threadId":"41346","inReplyTo":"1454707746-18672-1-git-send-email-alonid@gmail.com","subject":"[PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T21:29:06Z","receivedAt":"2016-02-05T21:29:06Z","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  | 10 +++++++++\n ident.c                   | 16 ++++++++++++++\n t/t9904-per-repo-email.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 81 insertions(+)\n create mode 100755 t/t9904-per-repo-email.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 02bcde6bb596..0d168d92fd79 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2821,6 +2821,16 @@ 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 instructs Git to avoid trying to guess defaults for 'user.email'\n+\tand 'user.name' other than strictly from config. For example, if\n+\tyou have multiple email addresses and would like to use a different\n+\tone for each repository, then with this configuration option set\n+\tto `true` in the global config along with a name, Git would prompt\n+\tfor you for setting up an email upon making new commits in a newly\n+\tcloned repository.\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..6e125821f056 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..f2b33881e46b\n--- /dev/null\n+++ b/t/t9904-per-repo-email.sh\n@@ -0,0 +1,55 @@\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+reprepare () {\n+\tgit reset --hard initial\n+}\n+\n+test_expect_success setup '\n+\t# Initial repo state\n+\techo \"Initial\" >foo &&\n+\tgit add foo &&\n+\tgit commit -m foo &&\n+\tgit tag initial &&\n+\n+\t# Setup a likely user.useConfigOnly use case\n+\tsane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&\n+\tsane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&\n+\tgit config user.name \"test\" &&\n+\tgit config --global user.useConfigOnly true &&\n+\n+\treprepare\n+'\n+\n+test_expect_success 'fails committing if clone email is not set' '\n+\ttest_when_finished reprepare &&\n+\n+\ttest_must_fail git commit --allow-empty -m msg\n+'\n+\n+test_expect_success 'fails committing if clone email is not set, but EMAIL set' '\n+\ttest_when_finished reprepare &&\n+\n+\ttest_must_fail env EMAIL=test@fail.com git commit --allow-empty -m msg\n+'\n+\n+test_expect_success 'succeeds committing if clone email is set' '\n+\ttest_when_finished reprepare &&\n+\n+\ttest_config user.email \"test@ok.com\" &&\n+\tgit commit --allow-empty -m msg\n+'\n+\n+test_expect_success 'succeeds cloning if global email is not set' '\n+\ttest_when_finished reprepare &&\n+\n+\tgit clone . clone\n+'\n+\n+test_done\n-- \n2.5.0\n"},{"id":"277603","messageId":"20160205214832.GA10052@sigill.intra.peff.net","threadId":"41346","inReplyTo":"1454707746-18672-3-git-send-email-alonid@gmail.com","subject":"Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-05T21:48:33Z","receivedAt":"2016-02-05T21:48:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:\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..f2b33881e46b\n> --- /dev/null\n> +++ b/t/t9904-per-repo-email.sh\n\nIs t9904 the right place for this? Usually t99xx is for very separate\ncomponents.\n\nThis is sort-of about \"commit\", which would put it in the t75xx range.\nBut in some ways, it is even more fundamental than that. We don't seem\nto have a lot of tests for ident stuff. The closest is the strict ident\nstuff in t0007.\n\n> +reprepare () {\n> +\tgit reset --hard initial\n> +}\n\nDo we need this reprepare stuff at all now? The tests don't care which\ncommit we're at when they start.\n\n> +test_expect_success setup '\n> +\t# Initial repo state\n> +\techo \"Initial\" >foo &&\n> +\tgit add foo &&\n> +\tgit commit -m foo &&\n> +\tgit tag initial &&\n\nA shorter way of saying this is \"test_commit foo\".\n\nI almost thought we could get rid of this part entirely; the commit\ntests don't care. But we do still need _a_ commit for the clone test,\nsince we want to make sure a reflog is written. It would be nice to push\nit down there, but our test environment doesn't allow creating commits,\nbecause of of useConfigOnly. So it's probably fine to leave it here.\n\nTechnically, the final \"commit\" test does make a commit for us to push,\nbut we do generally try to avoid unnecessary dependencies between the\nindividual tests.\n\nSo all together, maybe:\n\ndiff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh\nindex f2b3388..5694b84 100755\n--- a/t/t9904-per-repo-email.sh\n+++ b/t/t9904-per-repo-email.sh\n@@ -7,48 +7,31 @@ test_description='per-repo forced setting of email address'\n \n . ./test-lib.sh\n \n-reprepare () {\n-\tgit reset --hard initial\n-}\n-\n-test_expect_success setup '\n-\t# Initial repo state\n-\techo \"Initial\" >foo &&\n-\tgit add foo &&\n-\tgit commit -m foo &&\n-\tgit tag initial &&\n-\n-\t# Setup a likely user.useConfigOnly use case\n+test_expect_success 'setup a likely user.useConfigOnly use case' '\n+\t# we want to make sure a reflog is written, since that needs\n+\t# a non-strict ident. So be sure we have an actual commit.\n+\ttest_commit foo &&\n+\n \tsane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&\n \tsane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&\n \tgit config user.name \"test\" &&\n-\tgit config --global user.useConfigOnly true &&\n-\n-\treprepare\n+\tgit config --global user.useConfigOnly true\n '\n \n test_expect_success 'fails committing if clone email is not set' '\n-\ttest_when_finished reprepare &&\n-\n \ttest_must_fail git commit --allow-empty -m msg\n '\n \n test_expect_success 'fails committing if clone email is not set, but EMAIL set' '\n-\ttest_when_finished reprepare &&\n-\n \ttest_must_fail env EMAIL=test@fail.com git commit --allow-empty -m msg\n '\n \n test_expect_success 'succeeds committing if clone email is set' '\n-\ttest_when_finished reprepare &&\n-\n \ttest_config user.email \"test@ok.com\" &&\n \tgit commit --allow-empty -m msg\n '\n \n test_expect_success 'succeeds cloning if global email is not set' '\n-\ttest_when_finished reprepare &&\n-\n \tgit clone . clone\n '\n \n"},{"id":"277605","messageId":"20160205215952.GA4901@flurp.local","threadId":"41346","inReplyTo":"1454707746-18672-3-git-send-email-alonid@gmail.com","subject":"Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-05T21:59:52Z","receivedAt":"2016-02-05T21:59:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:\n> It used to be that:\n> \n>    git config --global user.email \"(none)\"\n> \n> was a viable way for people to force themselves to set user.email in\n> each repository.  This was helpful for people with more than one\n> email address, targeting different email addresses for different\n> clones, as it barred git from creating commit unless the user.email\n\nEither: s/commit/a commit/ or s/commit/commits/\n\n> config was set in the per-repo config to the correct email address.\n> \n> A recent change, 19ce497c (ident: keep a flag for bogus\n> default_email, 2015-12-10), however declared that an explicitly\n\ns/however/&,/\n\n> configured user.email is not bogus, no matter what its value is, so\n> this hack no longer works.\n> \n> Provide the same functionality by adding a new configuration\n> variable user.useConfigOnly; when this variable is set, the\n> user must explicitly set user.email configuration.\n> \n> Signed-off-by: Dan Aloni <alonid@gmail.com>\n> Helped-by: Jeff King <peff@peff.net>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> Cc: Eric Sunshine <sunshine@sunshineco.com>\n\nYou'd generally place your sign-off last.\n\n> ---\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> @@ -2821,6 +2821,16 @@ user.name::\n> +user.useConfigOnly::\n> +\tThis instructs Git to avoid trying to guess defaults for 'user.email'\n\nPerhaps: s/This instructs/Instruct/\n\n> +\tand 'user.name' other than strictly from config. For example, if\n\nThe way this is phrased, it sounds almost as if Git also \"guesses\"\nthe value from config. Perhaps rephrase like this:\n\n    Instruct Git to avoid trying to guess defaults for 'user.email'\n    and 'user.name', and instead retrieve the values only from\n    configuration.\n\n> +\tyou have multiple email addresses and would like to use a different\n> +\tone for each repository, then with this configuration option set\n> +\tto `true` in the global config along with a name, Git would prompt\n\ns/would/will/\n\n> +\tfor you for setting up an email upon making new commits in a newly\n\ns/for you for setting/you to set/\n\nMore below...\n\n> +\tcloned repository.\n> +\tDefaults to `false`.\n> diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh\n> @@ -0,0 +1,55 @@\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> +reprepare () {\n> +\tgit reset --hard initial\n> +}\n> +\n> +test_expect_success setup '\n> +\t# Initial repo state\n> +\techo \"Initial\" >foo &&\n> +\tgit add foo &&\n> +\tgit commit -m foo &&\n> +\tgit tag initial &&\n> +\n> +\t# Setup a likely user.useConfigOnly use case\n> +\tsane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&\n> +\tsane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&\n> +\tgit config user.name \"test\" &&\n> +\tgit config --global user.useConfigOnly true &&\n> +\n> +\treprepare\n> +'\n> +\n> +test_expect_success 'fails committing if clone email is not set' '\n> +\ttest_when_finished reprepare &&\n> +\n> +\ttest_must_fail git commit --allow-empty -m msg\n> +'\n> +\n> +test_expect_success 'fails committing if clone email is not set, but EMAIL set' '\n> +\ttest_when_finished reprepare &&\n> +\n> +\ttest_must_fail env EMAIL=test@fail.com git commit --allow-empty -m msg\n> +'\n> +\n> +test_expect_success 'succeeds committing if clone email is set' '\n> +\ttest_when_finished reprepare &&\n> +\n> +\ttest_config user.email \"test@ok.com\" &&\n> +\tgit commit --allow-empty -m msg\n> +'\n> +\n> +test_expect_success 'succeeds cloning if global email is not set' '\n> +\ttest_when_finished reprepare &&\n> +\n> +\tgit clone . clone\n> +'\n> +\n> +test_done\n\nThis test script still has a fair amount of unnecessary cruft in it\nwhich obscures the important bits showing what you are really\ntesting. Below is a more concise version with the unnecessary stuff\nremoved:\n\n--- 8< ---\n#!/bin/sh\n#\n# Copyright (c) 2016 Dan Aloni\n#\n\ntest_description='per-repo forced setting of email address'\n\n. ./test-lib.sh\n\ntest_expect_success setup '\n\tsane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&\n\tsane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&\n\tgit config user.name \"test\" &&\n\tgit config --global user.useConfigOnly true\n'\n\ntest_expect_success 'fails committing if clone email is not set' '\n\ttest_must_fail git commit --allow-empty -m msg\n'\n\ntest_expect_success 'fails committing if clone email is not set, but EMAIL set' '\n\ttest_must_fail env EMAIL=test@fail.com git commit --allow-empty -m msg\n'\n\ntest_expect_success 'succeeds committing if clone email is set' '\n\tgit -c user.email=test@ok.com commit --allow-empty -m msg\n'\n\ntest_expect_success 'succeeds cloning if global email is not set' '\n\tgit clone . clone\n'\n\ntest_done\n--- 8< ---\n"},{"id":"277606","messageId":"20160205220325.GA19465@gmail.com","threadId":"41346","inReplyTo":"20160205214832.GA10052@sigill.intra.peff.net","subject":"Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T22:03:25Z","receivedAt":"2016-02-05T22:03:25Z","isPatch":true,"sender":{"key":"alonid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/321273?v=4"},"body":"On Fri, Feb 05, 2016 at 04:48:33PM -0500, Jeff King wrote:\n> On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:\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..f2b33881e46b\n> > --- /dev/null\n> > +++ b/t/t9904-per-repo-email.sh\n> \n> Is t9904 the right place for this? Usually t99xx is for very separate\n> components.\n> \n> This is sort-of about \"commit\", which would put it in the t75xx range.\n> But in some ways, it is even more fundamental than that. We don't seem\n> to have a lot of tests for ident stuff. The closest is the strict ident\n> stuff in t0007.\n\nWill move to t7517. IMHO it's better to verify the commit operation\nitself before running further tests that rely on its proper function.\n\n>[..]\n> > +reprepare () {\n> > +\tgit reset --hard initial\n> > +}\n> \n> Do we need this reprepare stuff at all now? The tests don't care which\n> commit we're at when they start.\n> \n> > +test_expect_success setup '\n> > +\t# Initial repo state\n> > +\techo \"Initial\" >foo &&\n> > +\tgit add foo &&\n> > +\tgit commit -m foo &&\n> > +\tgit tag initial &&\n> \n> A shorter way of saying this is \"test_commit foo\".\n> \n> I almost thought we could get rid of this part entirely; the commit\n> tests don't care. But we do still need _a_ commit for the clone test,\n> since we want to make sure a reflog is written. It would be nice to push\n> it down there, but our test environment doesn't allow creating commits,\n> because of of useConfigOnly. So it's probably fine to leave it here.\n> \n> Technically, the final \"commit\" test does make a commit for us to push,\n> but we do generally try to avoid unnecessary dependencies between the\n> individual tests.\n> \n> So all together, maybe:\n>[..]\n\nYes, shorted is better.\n\nI'm squashing in these changes and adding you as Signed-off for v8.\n\n-- \nDan Aloni\n"},{"id":"277608","messageId":"20160205221811.GB19465@gmail.com","threadId":"41346","inReplyTo":"20160205215952.GA4901@flurp.local","subject":"Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Dan Aloni","fromEmail":"alonid@gmail.com","sentAt":"2016-02-05T22:18:11Z","receivedAt":"2016-02-05T22:18:11Z","isPatch":true,"sender":{"key":"alonid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/321273?v=4"},"body":"On Fri, Feb 05, 2016 at 04:59:52PM -0500, Eric Sunshine wrote:\n> On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:\n> > It used to be that:\n> > \n> >    git config --global user.email \"(none)\"\n> > \n> > was a viable way for people to force themselves to set user.email in\n> > each repository.  This was helpful for people with more than one\n> > email address, targeting different email addresses for different\n> > clones, as it barred git from creating commit unless the user.email\n> \n> Either: s/commit/a commit/ or s/commit/commits/\n\nThanks for all the proofing in your reply.\n\n>[..]\n> > config was set in the per-repo config to the correct email address.\n> > \n> > A recent change, 19ce497c (ident: keep a flag for bogus\n> > default_email, 2015-12-10), however declared that an explicitly\n> \n> s/however/&,/\n> \n> > configured user.email is not bogus, no matter what its value is, so\n> > this hack no longer works.\n> > \n> > Provide the same functionality by adding a new configuration\n> > variable user.useConfigOnly; when this variable is set, the\n> > user must explicitly set user.email configuration.\n> > \n> > Signed-off-by: Dan Aloni <alonid@gmail.com>\n> > Helped-by: Jeff King <peff@peff.net>\n> > Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> > Cc: Eric Sunshine <sunshine@sunshineco.com>\n> \n> You'd generally place your sign-off last.\n> [..]\n\nGood to know :)\n\n> This test script still has a fair amount of unnecessary cruft in it\n> which obscures the important bits showing what you are really\n> testing. Below is a more concise version with the unnecessary stuff\n> removed:\n\nThanks, though I'll stick to what Jeff suggested. Also, perhaps better\nto keep 'test_config' as it is instead of using '-c', to better mimick\nthe tested use case.\n\n-- \nDan Aloni\n"},{"id":"277609","messageId":"xmqqwpqibxz2.fsf@gitster.mtv.corp.google.com","threadId":"41346","inReplyTo":"20160205214832.GA10052@sigill.intra.peff.net","subject":"Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-05T22:20:49Z","receivedAt":"2016-02-05T22:20:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This is sort-of about \"commit\", which would put it in the t75xx range.\n> But in some ways, it is even more fundamental than that. We don't seem\n> to have a lot of tests for ident stuff. The closest is the strict ident\n> stuff in t0007.\n\nGood point.\n\n>> +reprepare () {\n>> +\tgit reset --hard initial\n>> +}\n>\n> Do we need this reprepare stuff at all now? The tests don't care which\n> commit we're at when they start.\n\nAh, I thought the function was needed to make sure we have something\nto commit, but I agree that you do not need it, if you are going to\nuse \"commit --allow-empty\" in the tests.\n\n> So all together, maybe:\n\nSounds sensible.\n\nThanks.\n"}]}