On Wed, Dec 12, 2012 at 12:59:00PM -0500, Jeff King wrote:
> > Have you considered defaulting to read from HEAD:.mailmap even when
> > this new configuration is not there if core.bare is set? I would
> > imagine that it would be the most convenient and match people's
> > expectations.
>
> Yeah, I almost suggested that, but I figured it could wait for the
> feature to prove itself in the real world before turning it on by
> default. It _should_ be pretty harmless, though, so I don't mind turning
> it on by default.
That patch would look like this:
-- >8 --
Subject: [PATCH] mailmap: default mailmap.blob in bare repositories
The motivation for mailmap.blob is to let users of bare
repositories use the mailmap feature, as they would not have
a checkout containing the .mailmap file. We can make it even
easier for them by just looking in HEAD:.mailmap by default.
We can't know for sure that this is where they would keep a
mailmap, of course, but it is the best guess (and it matches
the non-bare behavior, which reads from HEAD:.mailmap in the
working tree). If it's missing, git will silently ignore the
setting.
We do not do the same magic in the non-bare case, because:
1. In the common case, HEAD:.mailmap will be the same as
the .mailmap in the working tree, which is a no-op.
2. In the uncommon case, the user has modified .mailmap
but not yet committed it, and would expect the working
tree version to take precedence.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
I went with defaulting mailmap.blob, because it provides an easy path
for turning off the feature (you just override mailmap.blob).
Another way of thinking about this would be that it is the bare analog
to "read .mailmap from the working tree", and the logic should be:
if (is_bare_repository())
read_mailmap_blob(map, "HEAD:.mailmap");
else
read_mailmap_file(map, ".mailmap");
read_mailmap_blob(map, git_mailmap_blob);
read_mailmap_file(map, git_mailmap_file);
However, the current is not exactly "read from the root of the working
tree". It is "read from the current directory", and it works because all
of the callers run from the toplevel of the working tree (when one
exists). That means that bare repositories have always read from
$GIT_DIR/.mailmap. I doubt that was intentional, but people with bare
repositories may be depending on it. So I think that falls into the
"how I might do it from scratch" bin.
We could still do:
if (is_bare_repository())
read_mailmap_blob(map, "HEAD:.mailmap");
read_mailmap_file(map, ".mailmap");
read_mailmap_blob(map, git_mailmap_blob);
read_mailmap_file(map, git_mailmap_file);
but IMHO that is just making the rules more complicated to explain for
little benefit (and in fact, you lose the ability to suppress the HEAD
mailmap, which you might care about if you are hosting multiple bits of
unrelated history in the same repo).
Documentation/config.txt | 8 +++++---
mailmap.c | 5 +++++
t/t4203-mailmap.sh | 25 +++++++++++++++++++++++++
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3760077..1a3c554 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1519,9 +1519,11 @@ mailmap.blob::
mailmap.blob::
Like `mailmap.file`, but consider the value as a reference to a
- blob in the repository (e.g., `HEAD:.mailmap`). If both
- `mailmap.file` and `mailmap.blob` are given, both are parsed,
- with entries from `mailmap.file` taking precedence.
+ blob in the repository. If both `mailmap.file` and
+ `mailmap.blob` are given, both are parsed, with entries from
+ `mailmap.file` taking precedence. In a bare repository, this
+ defaults to `HEAD:.mailmap`. In a non-bare repository, it
+ defaults to empty.
man.viewer::
Specify the programs that may be used to display help in the
diff --git a/mailmap.c b/mailmap.c
index 5ffe48a..b16542f 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -233,7 +233,12 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)
int read_mailmap(struct string_list *map, char **repo_abbrev)
{
int err = 0;
+
map->strdup_strings = 1;
+
+ if (!git_mailmap_blob && is_bare_repository())
+ git_mailmap_blob = "HEAD:.mailmap";
+
err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);
err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index e7ea40c..aae30d9 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -218,6 +218,31 @@ test_expect_success 'mailmap.blob can be missing' '
test_cmp expect actual
'
+test_expect_success 'mailmap.blob defaults to off in non-bare repo' '
+ git init non-bare &&
+ (
+ cd non-bare &&
+ test_commit one .mailmap "Fake Name <author@xxxxxxxxxxx>" &&
+ echo " 1 Fake Name" >expect &&
+ git shortlog -ns HEAD >actual &&
+ test_cmp expect actual &&
+ rm .mailmap &&
+ echo " 1 A U Thor" >expect &&
+ git shortlog -ns HEAD >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '
+ git clone --bare non-bare bare &&
+ (
+ cd bare &&
+ echo " 1 Fake Name" >expect &&
+ git shortlog -ns HEAD >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'cleanup after mailmap.blob tests' '
rm -f .mailmap
'
--
1.8.0.2.4.g59402aa
--
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