Dan Aloni <alonid@xxxxxxxxx> 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 <alonid@xxxxxxxxx>
Helped-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
-- 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 000000000000..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 <msg>" 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 'VAR1=VAL2 VAR2=VAL2 thing', $VAR1 and $VAR2 get temporary
values only during the execution of the thing". If you really need
to, you would need to do this like so
test_must_fail env EDITOR=: git commit -m msg
But again, I do not think you need to override EDITOR/VISUAL here,
so...
> +test_expect_success 'fails committing if clone email is not set, but EMAIL set' '
> + prepare && about_to_commit &&
> +
> + EMAIL=test@xxxxxxxx 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=test@xxxxxxxx git commit -m msg
I would think.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html