[PATCH v4 2/3] Add user.useConfigOnly boolean for when ident shouldn't be guessed
Previously, before 5498c57cdd63, many people did the following:
git config --global user.email "(none)"
This was helpful for people with more than one email address,
targeting different email addresses for different clones.
as it barred git from creating commit unless the user.email
config was set in the per-repo config to the correct email
address.
This commit provides the same functionality by adding a new
configuration variable.
Signed-off-by: Dan Aloni
Cc: Eric Sunshine
Cc: Jeff King
Cc: Git List
---
Documentation/config.txt | 9
ident.c | 15
t/t9904-per-repo-email.sh | 58 +++
3 files changed, 82 insertions(+)
create mode 100755 t/t9904-per-repo-email.sh
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..25cf7ce4e83a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,15 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables. See linkgit:git-commit-tree[1].
+user.useConfigOnly::
+ This instruct Git to avoid trying to guess defaults for 'user.email'
+ and 'user.name' other than strictly from environment or config.
+ If you have multiple email addresses that you would like to set
+ up per repository, you may want to set this to 'true' in the global
+ config, and then Git would prompt you to set user.email separately,
+ in each of the cloned repositories.
+ Defaults to `false`.
+
user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..1216079d0b0d 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
static int default_email_is_bogus;
static int default_name_is_bogus;
+static int ident_use_config_only;
+
#define IDENT_NAME_GIVEN 01
#define IDENT_MAIL_GIVEN 02
#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
static int committer_ident_explicitly_given;
static int author_ident_explicitly_given;
+static int ident_config_given;
#ifdef NO_GECOS_IN_PWENT
#define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')",
name);
}
+ if (strict && ident_use_config_only &&
+ !(ident_config_given & IDENT_NAME_GIVEN))
+ die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,8 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')",
email);
}
+ if (strict && ident_use_config_only && !(ident_config_given &
IDENT_MAIL_GIVEN))
+ die("user.useConfigOnly set but no mail given");
}
strbuf_reset(&ident);
@@ -446,6 +454,11 @@ int author_ident_sufficiently_given(void)
int git_ident_config(const char *var, const char *value, void *data)
{
+ if (!strcmp(var, "user.useconfigonly")) {
+ ident_use_config_only = git_config_bool(var, value);
+ return 0;
+ }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +466,7 @@ int git_ident_config(const char *var, const char *value,
void *data)
strbuf_addstr(&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 +477,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 ..9522a640951b
--- /dev/null
+++ b/t/t9904-per-repo-email.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Dan Aloni
+#
+
+test_description='per-repo forced setting of email address'
+
+. ./test-lib.sh
+
+prepare () {
+# Have a non-empty repository
+rm -fr .git
+git init
+ echo "Initial" >foo &&
+git add foo &&
+EDITOR=: VISUAL=:
Re: [PATCH v4 2/3] Add user.useConfigOnly boolean for when ident shouldn't be guessed
Dan Aloni writes:
> Previously, before 5498c57cdd63, many people did the following:
>
>git config --global user.email "(none)"
>
> This was helpful for people with more than one email address,
> targeting different email addresses for different clones.
> as it barred git from creating commit unless the user.email
> config was set in the per-repo config to the correct email
> address.
>
> This commit provides the same functionality by adding a new
> configuration variable.
Thanks. I'd rather cite an individual commit, not a merge of a
topic, without forcing people to run "git show" only to see the
title of the commit. Also I'd avoid "was it that really that many
people did so?" discussion by not saying "many people" altogether.
"by adding a new configuration variable" is a bit weak. Help the
reader by mentioning what it is called and what it does in the same
sentence.
Perhaps like this?
-- >8 --
ident: add user.useConfigOnly boolean for when ident shouldn't be guessed
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
Helped-by: Jeff King
Signed-off-by: Junio C Hamano
-- 8< --
The logic in the patch is very cleanly written, partly thanks to the
previous step that was a real clean-up.
> @@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
> fputs(env_hint, stderr);
> die("unable to auto-detect name (got '%s')",
> name);
> }
> + if (strict && ident_use_config_only &&
> + !(ident_config_given & IDENT_NAME_GIVEN))
> + die("user.useConfigOnly set but no name given");
> }
> if (!*name) {
> struct passwd *pw;
> @@ -373,6 +379,8 @@ const char *fmt_ident(const char *name, const char *email,
> fputs(env_hint, stderr);
> die("unable to auto-detect email address (got '%s')",
> email);
> }
> + if (strict && ident_use_config_only && !(ident_config_given &
> IDENT_MAIL_GIVEN))
> + die("user.useConfigOnly set but no mail given");
> }
By folding the line just like you did for "name" above, you do not
have to worry about an overlong line here.
> diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
> new file mode 100755
> index ..9522a640951b
> --- /dev/null
> +++ b/t/t9904-per-repo-email.sh
> @@ -0,0 +1,58 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2016 Dan Aloni
> +#
> +
> +test_description='per-repo forced setting of email address'
> +
> +. ./test-lib.sh
> +
> +prepare () {
> +# Have a non-empty repository
> +rm -fr .git
> +git init
> + echo "Initial" >foo &&
By attempting to reply to a patch, one discovers that the patch has
mixed indentation style ;-) Indent with tabs.
> +git add foo &&
> +EDITOR=: VISUAL=: git commit -m foo &&
What is the point of these one-shot assignments to the environment
variables?
"git commit -m " does not invoke the editor unless given "-e",
and EDITOR=: is done early in test-lib.sh already, so I am puzzled.
Besides, if you are worried about some stray environment variable,
overriding EDITOR and VISUAL would not guard you against a stray
GIT_EDITOR, which takes the precedence, I think.
> + # Setup a likely user.useConfigOnly use case
> + unset GIT_AUTHOR_NAME &&
> + unset GIT_AUTHOR_EMAIL &&
Doesn't unset fail when the variable is not set (we have sane_unset
helper for that)?
> + 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 &&
> +
> + EDITOR=: VISUAL=: test_must_fail git commit -m msg
test_must_fail, being a shell function, does not follow the usual
"With '
Re: [PATCH v4 2/3] Add user.useConfigOnly boolean for when ident shouldn't be guessed
On Thu, Feb 04, 2016 at 01:53:25PM -0800, Junio C Hamano wrote:
>[..]
> "by adding a new configuration variable" is a bit weak. Help the
> reader by mentioning what it is called and what it does in the same
> sentence.
>
> Perhaps like this?
>
> -- >8 --
>[..]
>
Looks good, I'll just take that :)
> ident: add user.useConfigOnly boolean for when ident shouldn't be guessed
>[..]
> > }
> > + if (strict && ident_use_config_only && !(ident_config_given &
> > IDENT_MAIL_GIVEN))
> > + die("user.useConfigOnly set but no mail given");
> > }
>
> By folding the line just like you did for "name" above, you do not
> have to worry about an overlong line here.
Consistency is important. Will fix here too, though it got fixed later
in the cleanup.
>[..]
> > +git add foo &&
> > +EDITOR=: VISUAL=: git commit -m foo &&
>
> What is the point of these one-shot assignments to the environment
> variables?
>
> "git commit -m " does not invoke the editor unless given "-e",
> and EDITOR=: is done early in test-lib.sh already, so I am puzzled.
>
> Besides, if you are worried about some stray environment variable,
> overriding EDITOR and VISUAL would not guard you against a stray
> GIT_EDITOR, which takes the precedence, I think.
Being new to this testing framework, I tried learning the trade from
other tests. Maybe I goofed, or the other tests need cleaning?
>
> > + # Setup a likely user.useConfigOnly use case
> > + unset GIT_AUTHOR_NAME &&
> > + unset GIT_AUTHOR_EMAIL &&
>
> Doesn't unset fail when the variable is not set (we have sane_unset
> helper for that)?
Sure.
>[..]
> > +test_expect_success 'fails committing if clone email is not set, but EMAIL
> > set' '
> > +prepare && about_to_commit &&
> > +
> > + EMAIL=t...@fail.com EDITOR=: VISUAL=: test_must_fail git commit -m msg
>
> This is a good place to use the "test_must_fail env" pattern, i.e.
>
> test_must_fail env EMAIL=t...@fail.com git commit -m msg
>
> I would think.
Yes, and the fixed test still passes. Will resubmit the patches.
--
Dan Aloni
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
