[PATCH] Trick to force setup of a specific configured E-Mail per repo
Previously, before 5498c57cdd63, many people did the following:
git config --global user.email "(none)"
This was helpful for people with more than one E-Mail address,
targeting different E-Mail addresses for different clones.
as it barred git from creating commit unless the user.email
config was set in the per-clone config to the correct E-Mail
address.
Now, since the original 'bug' was fixed, and practically every
string is acceptable for user.email and user.name, it is best
to reimplement the feature not as an exploit of a bug, but as
an actual feature.
Signed-off-by: Dan Aloni
---
Documentation/config.txt | 3 +++
ident.c | 5 +
t/t9904-per-repo-email.sh | 26 ++
3 files changed, 34 insertions(+)
create mode 100755 t/t9904-per-repo-email.sh
diff --git a/Documentation/config.txt b/Documentation/config.txt
index f61788668e89..f9712e7c7752 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2769,6 +2769,9 @@ user.email::
Your email address to be recorded in any newly created commits.
Can be overridden by the 'GIT_AUTHOR_EMAIL', 'GIT_COMMITTER_EMAIL', and
'EMAIL' environment variables. See linkgit:git-commit-tree[1].
+ For people who seek setting different E-Mail addresses depending
+ on the clone, set to '(per-repo)' on the global configuration,
+ and Git will prompt you to set the E-Mail address in the clone.
user.name::
Your full name to be recorded in any newly created commits.
diff --git a/ident.c b/ident.c
index daf7e1ea8370..0e07d45f8ff3 100644
--- a/ident.c
+++ b/ident.c
@@ -373,6 +373,11 @@ const char *fmt_ident(const char *name, const char *email,
die("unable to auto-detect email address (got '%s')", email);
}
+ if (strict && email && !strcmp(email, "(per-repo)")) {
+ die("email is '(per-repo)', suggesting to set specific email "
+ "for the current repo");
+ }
+
strbuf_reset(&ident);
if (want_name) {
strbuf_addstr_without_crud(&ident, name);
diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
new file mode 100755
index ..c085ba671b85
--- /dev/null
+++ b/t/t9904-per-repo-email.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Dan Aloni
+#
+
+test_description='per-repo forced setting of E-Mail address'
+
+. ./test-lib.sh
+
+test_expect_failure 'fails commiting if clone email is not set' '
+ echo "Initial" >foo &&
+ git add foo &&
+ unset GIT_AUTHOR_EMAIL &&
+ git config --global user.email "(per-repo)" &&
+ EDITOR=: VISUAL=: git commit -a -m x
+'
+
+test_expect_success 'succeeds commiting if clone email is set' '
+ echo "Initial" >foo &&
+ git add foo &&
+ git config --global user.email "(per-repo)" &&
+ git config user.email "t...@ok.com" &&
+ EDITOR=: VISUAL=: git commit -a -m x
+'
+
+test_done
--
2.5.0
--
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
Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo
On Tue, Feb 02, 2016 at 09:54:21PM +0200, Dan Aloni wrote:
> Previously, before 5498c57cdd63, many people did the following:
>
>git config --global user.email "(none)"
>
> This was helpful for people with more than one E-Mail address,
> targeting different E-Mail addresses for different clones.
> as it barred git from creating commit unless the user.email
> config was set in the per-clone config to the correct E-Mail
> address.
>
> Now, since the original 'bug' was fixed, and practically every
> string is acceptable for user.email and user.name, it is best
> to reimplement the feature not as an exploit of a bug, but as
> an actual feature.
Just when I dare to think "somebody cannot possibly be relying on this
arcane behavior", I am proven wrong. :)
The motivating case for your patch makes sense to me. In the
implementation, though:
> + if (strict && email && !strcmp(email, "(per-repo)")) {
> + die("email is '(per-repo)', suggesting to set specific email "
> + "for the current repo");
> + }
I find it disappointing that we go back to looking for magic sequences
in the string. Could we perhaps do this more cleanly with a new config
option? Like a "user.guessIdent" which defaults to true, but people can
set to false. And without that, we do not do any automagic at all; we
get the values from the GIT_COMMITTER_* environment or the
user.{name,email} config variables, or we die().
I think that should allow your use case (and extend the same feature to
user.name). It wouldn't work on older versions of git, but nor would
your fix here (the only way to do that is to re-instate "(none)" as
magical).
-Peff
--
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
Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo
On Wed, Feb 3, 2016 at 10:56 AM, Jeff King wrote:
> I find it disappointing that we go back to looking for magic sequences
> in the string. Could we perhaps do this more cleanly with a new config
> option? Like a "user.guessIdent" which defaults to true, but people can
> set to false. And without that, we do not do any automagic at all; we
> get the values from the GIT_COMMITTER_* environment or the
> user.{name,email} config variables, or we die().
>
> I think that should allow your use case (and extend the same feature to
> user.name). It wouldn't work on older versions of git, but nor would
> your fix here (the only way to do that is to re-instate "(none)" as
> magical).
Should we generalize this use case, i.e. define a list of
configuration variables that must be (re-)defined per-repo? Maybe not
worth it, I don't know. I can't think of any other variable that
should behave this way off the top of my head.
--
Duy
--
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
Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo
On Wed, Feb 03, 2016 at 12:19:20PM +0700, Duy Nguyen wrote:
> On Wed, Feb 3, 2016 at 10:56 AM, Jeff King wrote:
> > I find it disappointing that we go back to looking for magic sequences
> > in the string. Could we perhaps do this more cleanly with a new config
> > option? Like a "user.guessIdent" which defaults to true, but people can
> > set to false. And without that, we do not do any automagic at all; we
> > get the values from the GIT_COMMITTER_* environment or the
> > user.{name,email} config variables, or we die().
> >
> > I think that should allow your use case (and extend the same feature to
> > user.name). It wouldn't work on older versions of git, but nor would
> > your fix here (the only way to do that is to re-instate "(none)" as
> > magical).
>
> Should we generalize this use case, i.e. define a list of
> configuration variables that must be (re-)defined per-repo? Maybe not
> worth it, I don't know. I can't think of any other variable that
> should behave this way off the top of my head.
That's an interesting thought, but I'm not sure how it would work. The
ident variables are special in that people are often unhappy with the
fallback. What would it mean for somebody to say "do not proceed if
diff.renameLimit is not set", and where would we enforce that?
-Peff
--
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
Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo
On Wed, Feb 3, 2016 at 12:22 PM, Jeff King wrote:
> On Wed, Feb 03, 2016 at 12:19:20PM +0700, Duy Nguyen wrote:
>
>> On Wed, Feb 3, 2016 at 10:56 AM, Jeff King wrote:
>> > I find it disappointing that we go back to looking for magic sequences
>> > in the string. Could we perhaps do this more cleanly with a new config
>> > option? Like a "user.guessIdent" which defaults to true, but people can
>> > set to false. And without that, we do not do any automagic at all; we
>> > get the values from the GIT_COMMITTER_* environment or the
>> > user.{name,email} config variables, or we die().
>> >
>> > I think that should allow your use case (and extend the same feature to
>> > user.name). It wouldn't work on older versions of git, but nor would
>> > your fix here (the only way to do that is to re-instate "(none)" as
>> > magical).
>>
>> Should we generalize this use case, i.e. define a list of
>> configuration variables that must be (re-)defined per-repo? Maybe not
>> worth it, I don't know. I can't think of any other variable that
>> should behave this way off the top of my head.
>
> That's an interesting thought, but I'm not sure how it would work. The
> ident variables are special in that people are often unhappy with the
> fallback. What would it mean for somebody to say "do not proceed if
> diff.renameLimit is not set", and where would we enforce that?
Enforcing it at git-init and git-clone, interactively asking the
values of these variables, may be too much. If only we can catch if a
variable is used, then we probably can catch it there. But that may
require some more changes in config api.
--
Duy
--
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
Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo
On Wed, Feb 03, 2016 at 12:26:36PM +0700, Duy Nguyen wrote: > >> Should we generalize this use case, i.e. define a list of > >> configuration variables that must be (re-)defined per-repo? Maybe not > >> worth it, I don't know. I can't think of any other variable that > >> should behave this way off the top of my head. > > > > That's an interesting thought, but I'm not sure how it would work. The > > ident variables are special in that people are often unhappy with the > > fallback. What would it mean for somebody to say "do not proceed if > > diff.renameLimit is not set", and where would we enforce that? > > Enforcing it at git-init and git-clone, interactively asking the > values of these variables, may be too much. If only we can catch if a > variable is used, then we probably can catch it there. But that may > require some more changes in config api. Yeah, maybe. I do not want to say "definitely not", but I do think this is sufficiently more complicated than something like user.guessIdent that we should not make it a dependency. -Peff -- 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
Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo
Jeff King writes:
> Just when I dare to think "somebody cannot possibly be relying on this
> arcane behavior", I am proven wrong. :)
My thoughts, exactly. This is the kind of thing that makes this
project uncomfortably "interesting"; we cannot make improvements
without risking breaking existing users.
> Could we perhaps do this more cleanly with a new config
> option? Like a "user.guessIdent" which defaults to true, but people can
> set to false. And without that, we do not do any automagic at all; we
> get the values from the GIT_COMMITTER_* environment or the
> user.{name,email} config variables, or we die().
That sounds like an excellent and clean way to do this.
--
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
Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo
(resend - my mailer was misconfigured)
On Tue, Feb 02, 2016 at 10:56:48PM -0500, Jeff King wrote:
> On Tue, Feb 02, 2016 at 09:54:21PM +0200, Dan Aloni wrote:
>[..]
> > + if (strict && email && !strcmp(email, "(per-repo)")) {
> > + die("email is '(per-repo)', suggesting to set specific email "
> > + "for the current repo");
> > + }
>
> I find it disappointing that we go back to looking for magic sequences
> in the string. Could we perhaps do this more cleanly with a new config
> option? Like a "user.guessIdent" which defaults to true, but people can
> set to false. And without that, we do not do any automagic at all; we
> get the values from the GIT_COMMITTER_* environment or the
> user.{name,email} config variables, or we die().
>[..]
Agreed. New patch attached, feel free to amend.
--
Dan Aloni
>From 35d94d4a00af70eeaf6b291ec951f555b0bc99d3 Mon Sep 17 00:00:00 2001
From: Dan Aloni
Date: Wed, 3 Feb 2016 10:09:40 +0200
Subject: [PATCH] Add user.explicit 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 E-Mail address,
targeting different E-Mail 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 E-Mail
address.
Now, since the original 'bug' was fixed, and practically every
string is acceptable for user.email and user.name, it is best
to reimplement the feature not as an exploit of a bug, but as
an actual feature.
Signed-off-by: Dan Aloni
---
Documentation/config.txt | 8
ident.c | 51 +++
t/t9904-per-repo-email.sh | 36 +
3 files changed, 95 insertions(+)
create mode 100755 t/t9904-per-repo-email.sh
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 877cbc875ec3..4c99f8f1de4f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2791,6 +2791,14 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables. See linkgit:git-commit-tree[1].
+user.explicit::
+ 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 multiply E-Mail 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.
+
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 9dd3ae345255..305dc32a8eaf 100644
--- a/ident.c
+++ b/ident.c
@@ -18,6 +18,7 @@ static int default_name_is_bogus;
#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_explicit = 0;
#ifdef NO_GECOS_IN_PWENT
#define get_gecos(ignored) "&"
@@ -373,6 +374,23 @@ const char *fmt_ident(const char *name, const char *email,
die("unable to auto-detect email address (got '%s')", email);
}
+ if (ident_explicit) {
+ if (name == git_default_name.buf &&
+ !(committer_ident_explicitly_given & IDENT_NAME_GIVEN) &&
+ !(author_ident_explicitly_given & IDENT_NAME_GIVEN)) {
+ die("requested explicitly given ident in config, "
+ "but user.name is not set, or environment is "
+ "not set");
+ }
+ if (email == git_default_email.buf &&
+ !(committer_ident_explicitly_given & IDENT_MAIL_GIVEN) &&
+ !(author_ident_explicitly_given & IDENT_MAIL_GIVEN)) {
+ die("requested explicitly given ident in config, "
+ "but user.email is not set, or environment is "
+ "not set");
+ }
+ }
+
strbuf_reset(&ident);
if (want_name) {
strbuf_addstr_without_crud(&ident, name);
@@ -405,6 +423,20 @@ const char *git_author_info(int flag)
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
if (getenv("GIT_AUTHOR_EMAIL"))
author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+
+ if (ident_explicit) {
+ if (!(author_ident_explicitly_given & IDENT_NAME_GIVEN)) {
+ die("requested explicitly given ident in config, "
+ "but user.name is not set, or GIT_AUTHOR_NAME "
+ "is not set");
+ }
+ if (!(author_ident_ex
Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo
On Wed, Feb 3, 2016 at 3:21 AM, Dan Aloni wrote:
> On Tue, Feb 02, 2016 at 10:56:48PM -0500, Jeff King wrote:
>> On Tue, Feb 02, 2016 at 09:54:21PM +0200, Dan Aloni wrote:
>> > + if (strict && email && !strcmp(email, "(per-repo)")) {
>> > + die("email is '(per-repo)', suggesting to set specific email "
>> > + "for the current repo");
>> > + }
>>
>> I find it disappointing that we go back to looking for magic sequences
>> in the string. Could we perhaps do this more cleanly with a new config
>> option? Like a "user.guessIdent" which defaults to true, but people can
>> set to false. And without that, we do not do any automagic at all; we
>> get the values from the GIT_COMMITTER_* environment or the
>> user.{name,email} config variables, or we die().
>
> Agreed. New patch attached, feel free to amend.
Please re-send with the patch inline since reviewers will want to
comment on on specific bits of code inline as well. When the patch is
an attachment, this process becomes too onerous for reviewers, and
most will simply ignore the patch. Thanks.
Before sending v3 (inline), perhaps take note of the few issues below
which I noticed while quickly scanning the attachment:
* The final paragraph of the commit message appears to be outdated
since it still seems to be describing the approach taken by v1.
* Elsewhere, in the project, the spelling "email" is preferred over
"E-Mail"; that's true even in the files your patch is touching.
* In the documentation:s/mutiply/multiple/.
* The documentation doesn't seem to mention the default value of the
new config variable.
* The new config variable "user.explicit" has a more nebulous name
than Peff's suggestion of "user.guessIdent", which may make its intent
harder to determine without consulting documentation.
* Don't initialize static variables to 0 (let the .bss section do that
automatically).
* One space before && operator; not two.
* Drop unnecessary braces.
* Perhaps use test_config(), test_unconfig(), test_config_global() in
the test script rather than the manual git-config invocations in
subshells.
* test_expect_failure() is for indicating that a test will fail
because some feature is known to be broken, not for when you expect a
git command to fail in a controlled fashion. Instead, use
test_expect_success, and then use test_must_fail() within the body of
the test.
test_expect_success '...' '
... &&
test_must_fail git commit -m msg
'
* Do these new tests really deserve a new test script, or would they
fit into an existing script? (Genuine question.)
It's also reviewer-friendly to indicate the patch revision in the
subject "[PATCH v3]", and to describe what changed since the previous
version of the patch. Providing a gmane link to the previous version
is also very helpful.
--
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
