{"thread":{"id":"32319","subject":"[PATCH 0/2] mailmap from blobs","startedAt":"2012-12-12T10:58:23Z","lastAt":"2012-12-13T18:23:39Z","messageCount":9,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"204725","messageId":"20121212105822.GA15842@sigill.intra.peff.net","threadId":"32319","inReplyTo":null,"subject":"[PATCH 0/2] mailmap from blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-12T10:58:23Z","receivedAt":"2012-12-12T10:58:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I noticed recently that the GitHub contributions page for git.git did\nnot seem very accurate. The problem is that while it uses shortlog, it\ndoes not respect .mailmap, because we do not have a working tree from\nwhich to read the .mailmap.\n\nThis series adds a config option analogous to mailmap.file, but which\nreads from a blob in the repository (so the obvious thing to set it to\nis \"HEAD:.mailmap\" in a bare repo, and probably \"master:.mailmap\" if you\nfrequently want to traverse while on unrelated branches). The obvious\nalternative is to checkout a temporary file of .mailmap and point\nmailmap.file at it, but this is a bit more convenient.\n\nA config option is perhaps not the most flexible way to access this. For\nexample, one could in theory want to pull the mailmap from the tip of\nthe history being traversed (e.g., because you have multiple unrelated\nDAGs in a single repo). But that could also produce the _wrong_ results,\nif you are looking at the shortlog of older history (e.g., when doing\n\"git shortlog v1.5.0..v1.5.5\", you would still want to be using the\nmodern mailmap from \"master\").\n\nBy making it a config option, the simple, common case does the right\nthing, and people with complex cases can use \"git -c mailmap.blob=...\"\nto feed the appropriate map for the history they are traversing. If\nsomebody wants to do something fancier (like --mailmap-from-tip or\nsomething), it would be easy to build on top later.\n\n [1/2]: mailmap: refactor mailmap parsing for non-file sources\n [2/2]: mailmap: support reading mailmap from blobs\n\n-Peff\n"},{"id":"204726","messageId":"20121212105945.GA19653@sigill.intra.peff.net","threadId":"32319","inReplyTo":"20121212105822.GA15842@sigill.intra.peff.net","subject":"[PATCH 1/2] mailmap: refactor mailmap parsing for non-file sources","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-12T10:59:45Z","receivedAt":"2012-12-12T10:59:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The read_single_mailmap function opens a mailmap file and\nparses each line. In preparation for having non-file\nmailmaps, let's pull out the line-parsing logic into its own\nfunction (read_mailmap_line), and rename the file-parsing\nfunction to match (read_mailmap_file).\n\nSigned-off-by: Jeff King \n---\nCleanup for the next patch. It's mostly indentation changes, so \"diff\n-w\" is much easier to review.\n\n mailmap.c | 74 ++++++++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 40 insertions(+), 34 deletions(-)\n\ndiff --git a/mailmap.c b/mailmap.c\nindex ea4b471..89bc318 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -129,44 +129,50 @@ static int read_single_mailmap(struct string_list *map, const char *filename, ch\n \treturn (*right == '\\0' ? NULL : right);\n }\n \n-static int read_single_mailmap(struct string_list *map, const char *filename, char **repo_abbrev)\n+static void read_mailmap_line(struct string_list *map, char *buffer,\n+\t\t\t char **repo_abbrev)\n+{\n+\tchar *name1 = NULL, *email1 = NULL, *name2 = NULL, *email2 = NULL;\n+\tif (buffer[0] == '#') {\n+\t\tstatic const char abbrev[] = \"# repo-abbrev:\";\n+\t\tint abblen = sizeof(abbrev) - 1;\n+\t\tint len = strlen(buffer);\n+\n+\t\tif (!repo_abbrev)\n+\t\t\treturn;\n+\n+\t\tif (len && buffer[len - 1] == '\\n')\n+\t\t\tbuffer[--len] = 0;\n+\t\tif (!strncmp(buffer, abbrev, abblen)) {\n+\t\t\tchar *cp;\n+\n+\t\t\tif (repo_abbrev)\n+\t\t\t\tfree(*repo_abbrev);\n+\t\t\t*repo_abbrev = xmalloc(len);\n+\n+\t\t\tfor (cp = buffer + abblen; isspace(*cp); cp++)\n+\t\t\t\t; /* nothing */\n+\t\t\tstrcpy(*repo_abbrev, cp);\n+\t\t}\n+\t\treturn;\n+\t}\n+\tif ((name2 = parse_name_and_email(buffer, &name1, &email1, 0)) != NULL)\n+\t\tparse_name_and_email(name2, &name2, &email2, 1);\n+\n+\tif (email1)\n+\t\tadd_mapping(map, name1, email1, name2, email2);\n+}\n+\n+static int read_mailmap_file(struct string_list *map, const char *filename,\n+\t\t\t char **repo_abbrev)\n {\n \tchar buffer[1024];\n \tFILE *f = (filename == NULL ? NULL : fopen(filename, \"r\"));\n \n \tif (f == NULL)\n \t\treturn 1;\n-\twhile (fgets(buffer, sizeof(buffer), f) != NULL) {\n-\t\tchar *name1 = NULL, *email1 = NULL, *name2 = NULL, *email2 = NULL;\n-\t\tif (buffer[0] == '#') {\n-\t\t\tstatic const char abbrev[] = \"# repo-abbrev:\";\n-\t\t\tint abblen = sizeof(abbrev) - 1;\n-\t\t\tint len = strlen(buffer);\n-\n-\t\t\tif (!repo_abbrev)\n-\t\t\t\tcontinue;\n-\n-\t\t\tif (len && buffer[len - 1] == '\\n')\n-\t\t\t\tbuffer[--len] = 0;\n-\t\t\tif (!strncmp(buffer, abbrev, abblen)) {\n-\t\t\t\tchar *cp;\n-\n-\t\t\t\tif (repo_abbrev)\n-\t\t\t\t\tfree(*repo_abbrev);\n-\t\t\t\t*repo_abbrev = xmalloc(len);\n-\n-\t\t\t\tfor (cp = buffer + abblen; isspace(*cp); cp++)\n-\t\t\t\t\t; /* nothing */\n-\t\t\t\tstrcpy(*repo_abbrev, cp);\n-\t\t\t}\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif ((name2 = parse_name_and_email(buffer, &name1, &email1, 0)) != NULL)\n-\t\t\tparse_name_and_email(name2, &name2, &email2, 1);\n-\n-\t\tif (email1)\n-\t\t\tadd_mapping(map, name1, email1, name2, email2);\n-\t}\n+\twhile (fgets(buffer, sizeof(buffer), f) != NULL)\n+\t\tread_mailmap_line(map, buffer, repo_abbrev);\n \tfclose(f);\n \treturn 0;\n }\n@@ -175,8 +181,8 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)\n {\n \tmap->strdup_strings = 1;\n \t/* each failure returns 1, so >1 means both calls failed */\n-\treturn read_single_mailmap(map, \".mailmap\", repo_abbrev) +\n-\t read_single_mailmap(map, git_mailmap_file, repo_abbrev) > 1;\n+\treturn read_mailmap_file(map, \".mailmap\", repo_abbrev) +\n+\t read_mailmap_file(map, git_mailmap_file, repo_abbrev) > 1;\n }\n \n void clear_mailmap(struct string_list *map)\n-- \n1.8.0.2.4.g59402aa\n"},{"id":"204727","messageId":"20121212110404.GB19653@sigill.intra.peff.net","threadId":"32319","inReplyTo":"20121212105822.GA15842@sigill.intra.peff.net","subject":"[PATCH 2/2] mailmap: support reading mailmap from blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-12T11:04:04Z","receivedAt":"2012-12-12T11:04:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In a bare repository, there isn't a simple way to respect an\nin-tree mailmap without extracting it to a temporary file.\nThis patch provides a config variable, similar to\nmailmap.file, which reads the mailmap from a blob in the\nrepository.\n\nSigned-off-by: Jeff King \n---\nThe error-return convention from read_mailmap is really wonky, but I\ndidn't change it here. It will return \"1\" for error, and will do so only\nif no mailmap sources could be read (including if they simply don't\nexist). But it's perfectly OK not to have a mailmap at all. However,\nnobody actually seems to check the return code, so nobody has cared.\n\nA more sane convention would probably be:\n\n 1. If ENOENT (or no such blob), silently return success.\n\n 2. Otherwise, return -1 and print a message to stderr indicating that\n there is a mailmap file, but it is broken or otherwise could not be\n opened.\n\n Documentation/config.txt | 6 ++++\n cache.h | 1 +\n config.c | 2 ++\n mailmap.c | 49 ++++++++++++++++++++++++++++++--\n t/t4203-mailmap.sh | 73 ++++++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 129 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bf8f911..3760077 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1517,6 +1517,12 @@ mailmap.file::\n \tsubdirectory, or somewhere outside of the repository itself.\n \tSee linkgit:git-shortlog[1] and linkgit:git-blame[1].\n \n+mailmap.blob::\n+\tLike `mailmap.file`, but consider the value as a reference to a\n+\tblob in the repository (e.g., `HEAD:.mailmap`). If both\n+\t`mailmap.file` and `mailmap.blob` are given, both are parsed,\n+\twith entries from `mailmap.file` taking precedence.\n+\n man.viewer::\n \tSpecify the programs that may be used to display help in the\n \t'man' format. See linkgit:git-help[1].\ndiff --git a/cache.h b/cache.h\nindex 18fdd18..a65f6d1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1155,6 +1155,7 @@ extern const char *git_mailmap_file;\n extern const char *git_commit_encoding;\n extern const char *git_log_output_encoding;\n extern const char *git_mailmap_file;\n+extern const char *git_mailmap_blob;\n \n /* IO helper functions */\n extern void maybe_flush_or_die(FILE *, const char *);\ndiff --git a/config.c b/config.c\nindex fb3f868..97364c0 100644\n--- a/config.c\n+++ b/config.c\n@@ -839,6 +839,8 @@ static int git_default_mailmap_config(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"mailmap.file\"))\n \t\treturn git_config_string(&git_mailmap_file, var, value);\n+\tif (!strcmp(var, \"mailmap.blob\"))\n+\t\treturn git_config_string(&git_mailmap_blob, var, value);\n \n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\ndiff --git a/mailmap.c b/mailmap.c\nindex 89bc318..2f9c691 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -10,6 +10,7 @@ const char *git_mailmap_file;\n #endif\n \n const char *git_mailmap_file;\n+const char *git_mailmap_blob;\n \n struct mailmap_info {\n \tchar *name;\n@@ -177,12 +178,56 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)\n \treturn 0;\n }\n \n+static void read_mailmap_buf(struct string_list *map,\n+\t\t\t const char *buf, unsigned long len,\n+\t\t\t char **repo_abbrev)\n+{\n+\twhile (len) {\n+\t\tconst char *end = strchrnul(buf, '\\n');\n+\t\tunsigned long linelen = end - buf + 1;\n+\t\tchar *line = xmemdupz(buf, linelen);\n+\n+\t\tread_mailmap_line(map, line, repo_abbrev);\n+\n+\t\tfree(line);\n+\t\tbuf += linelen;\n+\t\tlen -= linelen;\n+\t}\n+}\n+\n+static int read_mailmap_blob(struct string_list *map,\n+\t\t\t const char *name,\n+\t\t\t char **repo_abbrev)\n+{\n+\tunsigned char sha1[20];\n+\tchar *buf;\n+\tunsigned long size;\n+\tenum object_type type;\n+\n+\tif (!name)\n+\t\treturn 1;\n+\tif (get_sha1(name, sha1) < 0)\n+\t\treturn 1;\n+\n+\tbuf = read_sha1_file(sha1, &type, &size);\n+\tif (!buf)\n+\t\treturn 1;\n+\tif (type != OBJ_BLOB)\n+\t\treturn 1;\n+\n+\tread_mailmap_buf(map, buf, size, repo_abbrev);\n+\n+\tfree(buf);\n+\treturn 0;\n+}\n+\n int read_mailmap(struct string_list *map, char **repo_abbrev)\n {\n \tmap->strdup_strings = 1;\n-\t/* each failure returns 1, so >1 means both calls failed */\n+\t/* each failure returns 1, so >2 means all calls failed */\n \treturn read_mailmap_file(map, \".mailmap\", repo_abbrev) +\n-\t read_mailmap_file(map, git_mailmap_file, repo_abbrev) > 1;\n+\t read_mailmap_blob(map, git_mailmap_blob, repo_abbrev) +\n+\t read_mailmap_file(map, git_mailmap_file, repo_abbrev) > 2;\n }\n \n void clear_mailmap(struct string_list *map)\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 1f182f6..e7ea40c 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -149,6 +149,79 @@ test_expect_success 'No mailmap files, but configured' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup mailmap blob tests' '\n+\tgit checkout -b map &&\n+\ttest_when_finished \"git checkout master\" &&\n+\tcat >just-bugs <<-\\EOF &&\n+\tBlob Guy \n+\tEOF\n+\tcat >both <<-\\EOF &&\n+\tBlob Guy \n+\tBlob Guy \n+\tEOF\n+\tgit add just-bugs both &&\n+\tgit commit -m \"my mailmaps\" &&\n+\techo \"Repo Guy \" >.mailmap &&\n+\techo \"Internal Guy \" >internal.map\n+'\n+\n+test_expect_success 'mailmap.blob set' '\n+\tcat >expect <<-\\EOF &&\n+\tBlob Guy (1):\n+\t second\n+\n+\tRepo Guy (1):\n+\t initial\n+\n+\tEOF\n+\tgit -c mailmap.blob=map:just-bugs shortlog HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'mailmap.blob overrides .mailmap' '\n+\tcat >expect <<-\\EOF &&\n+\tBlob Guy (2):\n+\t initial\n+\t second\n+\n+\tEOF\n+\tgit -c mailmap.blob=map:both shortlog HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'mailmap.file overrides mailmap.blob' '\n+\tcat >expect <<-\\EOF &&\n+\tBlob Guy (1):\n+\t second\n+\n+\tInternal Guy (1):\n+\t initial\n+\n+\tEOF\n+\tgit \\\n+\t -c mailmap.blob=map:both \\\n+\t -c mailmap.file=internal.map \\\n+\t shortlog HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'mailmap.blob can be missing' '\n+\tcat >expect <<-\\EOF &&\n+\tRepo Guy (1):\n+\t initial\n+\n+\tnick1 (1):\n+\t second\n+\n+\tEOF\n+\tgit -c mailmap.blob=map:nonexistent shortlog HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'cleanup after mailmap.blob tests' '\n+\trm -f .mailmap\n+'\n+\n # Extended mailmap configurations should give us the following output for shortlog\n cat >expect <<\\EOF\n A U Thor (1):\n-- \n1.8.0.2.4.g59402aa\n"},{"id":"204728","messageId":"20121212111802.GA19625@sigill.intra.peff.net","threadId":"32319","inReplyTo":"20121212110404.GB19653@sigill.intra.peff.net","subject":"[PATCH 3/2] mailmap: clean up read_mailmap error handling","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-12T11:18:02Z","receivedAt":"2012-12-12T11:18:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 12, 2012 at 06:04:04AM -0500, Jeff King wrote:\n\n> The error-return convention from read_mailmap is really wonky, but I\n> didn't change it here. It will return \"1\" for error, and will do so only\n> if no mailmap sources could be read (including if they simply don't\n> exist). But it's perfectly OK not to have a mailmap at all. However,\n> nobody actually seems to check the return code, so nobody has cared.\n> \n> A more sane convention would probably be:\n> \n> 1. If ENOENT (or no such blob), silently return success.\n> \n> 2. Otherwise, return -1 and print a message to stderr indicating that\n> there is a mailmap file, but it is broken or otherwise could not be\n> opened.\n\nMaybe like this:\n\n-- >8 --\nSubject: [PATCH] mailmap: clean up read_mailmap error handling\n\nThe error handling for the read_mailmap function is odd. It\nreturns 1 on error, rather than -1. And it treats a\nnon-existent mailmap as an error, even though there is no\nreason that one needs to exist. Unless some other mailmap\nsource loads successfully, in which case the original error\nis completely masked.\n\nThis does not cause any bugs, however, because no caller\nbothers to check the return value, anyway. Let's make this a\nlittle more robust to real errors and less surprising for\nfuture callers that do check the error code:\n\n 1. Return -1 on errors.\n\n 2. Treat a missing entry (e.g., no mailmap.file given),\n ENOENT, or a non-existent blob (for mailmap.blob) as\n \"no error\".\n\n 3. Complain loudly when a real error (e.g., a transient\n I/O error, no permission to open the mailmap file,\n missing or corrupted blob object, etc) occurs.\n\nSigned-off-by: Jeff King \n---\n mailmap.c | 32 +++++++++++++++++++++-----------\n 1 file changed, 21 insertions(+), 11 deletions(-)\n\ndiff --git a/mailmap.c b/mailmap.c\nindex 2f9c691..5ffe48a 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -168,10 +168,19 @@ static int read_mailmap_file(struct string_list *map, const char *filename,\n \t\t\t char **repo_abbrev)\n {\n \tchar buffer[1024];\n-\tFILE *f = (filename == NULL ? NULL : fopen(filename, \"r\"));\n+\tFILE *f;\n+\n+\tif (!filename)\n+\t\treturn 0;\n+\n+\tf = fopen(filename, \"r\");\n+\tif (!f) {\n+\t\tif (errno == ENOENT)\n+\t\t\treturn 0;\n+\t\treturn error(\"unable to open mailmap at %s: %s\",\n+\t\t\t filename, strerror(errno));\n+\t}\n \n-\tif (f == NULL)\n-\t\treturn 1;\n \twhile (fgets(buffer, sizeof(buffer), f) != NULL)\n \t\tread_mailmap_line(map, buffer, repo_abbrev);\n \tfclose(f);\n@@ -205,15 +214,15 @@ static int read_mailmap_blob(struct string_list *map,\n \tenum object_type type;\n \n \tif (!name)\n-\t\treturn 1;\n+\t\treturn 0;\n \tif (get_sha1(name, sha1) < 0)\n-\t\treturn 1;\n+\t\treturn 0;\n \n \tbuf = read_sha1_file(sha1, &type, &size);\n \tif (!buf)\n-\t\treturn 1;\n+\t\treturn error(\"unable to read mailmap object at %s\", name);\n \tif (type != OBJ_BLOB)\n-\t\treturn 1;\n+\t\treturn error(\"mailmap is not a blob: %s\", name);\n \n \tread_mailmap_buf(map, buf, size, repo_abbrev);\n \n@@ -223,11 +232,12 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)\n \n int read_mailmap(struct string_list *map, char **repo_abbrev)\n {\n+\tint err = 0;\n \tmap->strdup_strings = 1;\n-\t/* each failure returns 1, so >2 means all calls failed */\n-\treturn read_mailmap_file(map, \".mailmap\", repo_abbrev) +\n-\t read_mailmap_blob(map, git_mailmap_blob, repo_abbrev) +\n-\t read_mailmap_file(map, git_mailmap_file, repo_abbrev) > 2;\n+\terr |= read_mailmap_file(map, \".mailmap\", repo_abbrev);\n+\terr |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);\n+\terr |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);\n+\treturn err;\n }\n \n void clear_mailmap(struct string_list *map)\n-- \n1.8.0.2.4.g59402aa\n"},{"id":"204763","messageId":"7vhanr5f74.fsf@alter.siamese.dyndns.org","threadId":"32319","inReplyTo":"20121212105822.GA15842@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] mailmap from blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-12T17:54:23Z","receivedAt":"2012-12-12T17:54:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I noticed recently that the GitHub contributions page for git.git did\n> not seem very accurate. The problem is that while it uses shortlog, it\n> does not respect .mailmap, because we do not have a working tree from\n> which to read the .mailmap.\n>\n> This series adds a config option analogous to mailmap.file, but which\n> reads from a blob in the repository (so the obvious thing to set it to\n> is \"HEAD:.mailmap\" in a bare repo, and probably \"master:.mailmap\" if you\n> frequently want to traverse while on unrelated branches). The obvious\n> alternative is to checkout a temporary file of .mailmap and point\n> mailmap.file at it, but this is a bit more convenient.\n\nYeah, I think this is sane.\n\nHave you considered defaulting to read from HEAD:.mailmap even when\nthis new configuration is not there if core.bare is set? I would\nimagine that it would be the most convenient and match people's\nexpectations.\n"},{"id":"204765","messageId":"20121212175900.GA32767@sigill.intra.peff.net","threadId":"32319","inReplyTo":"7vhanr5f74.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/2] mailmap from blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-12T17:59:00Z","receivedAt":"2012-12-12T17:59:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 12, 2012 at 09:54:23AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I noticed recently that the GitHub contributions page for git.git did\n> > not seem very accurate. The problem is that while it uses shortlog, it\n> > does not respect .mailmap, because we do not have a working tree from\n> > which to read the .mailmap.\n> >\n> > This series adds a config option analogous to mailmap.file, but which\n> > reads from a blob in the repository (so the obvious thing to set it to\n> > is \"HEAD:.mailmap\" in a bare repo, and probably \"master:.mailmap\" if you\n> > frequently want to traverse while on unrelated branches). The obvious\n> > alternative is to checkout a temporary file of .mailmap and point\n> > mailmap.file at it, but this is a bit more convenient.\n> \n> Yeah, I think this is sane.\n> \n> Have you considered defaulting to read from HEAD:.mailmap even when\n> this new configuration is not there if core.bare is set? I would\n> imagine that it would be the most convenient and match people's\n> expectations.\n\nYeah, I almost suggested that, but I figured it could wait for the\nfeature to prove itself in the real world before turning it on by\ndefault. It _should_ be pretty harmless, though, so I don't mind turning\nit on by default.\n\n-Peff\n"},{"id":"204830","messageId":"20121213130447.GA4353@sigill.intra.peff.net","threadId":"32319","inReplyTo":"20121212175900.GA32767@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] mailmap from blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-13T13:04:47Z","receivedAt":"2012-12-13T13:04:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 12, 2012 at 12:59:00PM -0500, Jeff King wrote:\n\n> > Have you considered defaulting to read from HEAD:.mailmap even when\n> > this new configuration is not there if core.bare is set? I would\n> > imagine that it would be the most convenient and match people's\n> > expectations.\n> \n> Yeah, I almost suggested that, but I figured it could wait for the\n> feature to prove itself in the real world before turning it on by\n> default. It _should_ be pretty harmless, though, so I don't mind turning\n> it on by default.\n\nThat patch would look like this:\n\n-- >8 --\nSubject: [PATCH] mailmap: default mailmap.blob in bare repositories\n\nThe motivation for mailmap.blob is to let users of bare\nrepositories use the mailmap feature, as they would not have\na checkout containing the .mailmap file. We can make it even\neasier for them by just looking in HEAD:.mailmap by default.\n\nWe can't know for sure that this is where they would keep a\nmailmap, of course, but it is the best guess (and it matches\nthe non-bare behavior, which reads from HEAD:.mailmap in the\nworking tree). If it's missing, git will silently ignore the\nsetting.\n\nWe do not do the same magic in the non-bare case, because:\n\n 1. In the common case, HEAD:.mailmap will be the same as\n the .mailmap in the working tree, which is a no-op.\n\n 2. In the uncommon case, the user has modified .mailmap\n but not yet committed it, and would expect the working\n tree version to take precedence.\n\nSigned-off-by: Jeff King \n---\nI went with defaulting mailmap.blob, because it provides an easy path\nfor turning off the feature (you just override mailmap.blob).\n\nAnother way of thinking about this would be that it is the bare analog\nto \"read .mailmap from the working tree\", and the logic should be:\n\n if (is_bare_repository())\n read_mailmap_blob(map, \"HEAD:.mailmap\");\n else\n read_mailmap_file(map, \".mailmap\");\n read_mailmap_blob(map, git_mailmap_blob);\n read_mailmap_file(map, git_mailmap_file);\n\nHowever, the current is not exactly \"read from the root of the working\ntree\". It is \"read from the current directory\", and it works because all\nof the callers run from the toplevel of the working tree (when one\nexists). That means that bare repositories have always read from\n$GIT_DIR/.mailmap. I doubt that was intentional, but people with bare\nrepositories may be depending on it. So I think that falls into the\n\"how I might do it from scratch\" bin.\n\nWe could still do:\n\n if (is_bare_repository())\n read_mailmap_blob(map, \"HEAD:.mailmap\");\n read_mailmap_file(map, \".mailmap\");\n read_mailmap_blob(map, git_mailmap_blob);\n read_mailmap_file(map, git_mailmap_file);\n\nbut IMHO that is just making the rules more complicated to explain for\nlittle benefit (and in fact, you lose the ability to suppress the HEAD\nmailmap, which you might care about if you are hosting multiple bits of\nunrelated history in the same repo).\n\n Documentation/config.txt | 8 +++++---\n mailmap.c | 5 +++++\n t/t4203-mailmap.sh | 25 +++++++++++++++++++++++++\n 3 files changed, 35 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 3760077..1a3c554 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1519,9 +1519,11 @@ mailmap.blob::\n \n mailmap.blob::\n \tLike `mailmap.file`, but consider the value as a reference to a\n-\tblob in the repository (e.g., `HEAD:.mailmap`). If both\n-\t`mailmap.file` and `mailmap.blob` are given, both are parsed,\n-\twith entries from `mailmap.file` taking precedence.\n+\tblob in the repository. If both `mailmap.file` and\n+\t`mailmap.blob` are given, both are parsed, with entries from\n+\t`mailmap.file` taking precedence. In a bare repository, this\n+\tdefaults to `HEAD:.mailmap`. In a non-bare repository, it\n+\tdefaults to empty.\n \n man.viewer::\n \tSpecify the programs that may be used to display help in the\ndiff --git a/mailmap.c b/mailmap.c\nindex 5ffe48a..b16542f 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -233,7 +233,12 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)\n int read_mailmap(struct string_list *map, char **repo_abbrev)\n {\n \tint err = 0;\n+\n \tmap->strdup_strings = 1;\n+\n+\tif (!git_mailmap_blob && is_bare_repository())\n+\t\tgit_mailmap_blob = \"HEAD:.mailmap\";\n+\n \terr |= read_mailmap_file(map, \".mailmap\", repo_abbrev);\n \terr |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);\n \terr |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex e7ea40c..aae30d9 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -218,6 +218,31 @@ test_expect_success 'mailmap.blob can be missing' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'mailmap.blob defaults to off in non-bare repo' '\n+\tgit init non-bare &&\n+\t(\n+\t\tcd non-bare &&\n+\t\ttest_commit one .mailmap \"Fake Name \" &&\n+\t\techo \" 1\tFake Name\" >expect &&\n+\t\tgit shortlog -ns HEAD >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\trm .mailmap &&\n+\t\techo \" 1\tA U Thor\" >expect &&\n+\t\tgit shortlog -ns HEAD >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '\n+\tgit clone --bare non-bare bare &&\n+\t(\n+\t\tcd bare &&\n+\t\techo \" 1\tFake Name\" >expect &&\n+\t\tgit shortlog -ns HEAD >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_expect_success 'cleanup after mailmap.blob tests' '\n \trm -f .mailmap\n '\n-- \n1.8.0.2.4.g59402aa\n"},{"id":"204831","messageId":"20121213130812.GA4900@sigill.intra.peff.net","threadId":"32319","inReplyTo":"20121212110404.GB19653@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] mailmap: support reading mailmap from blobs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-13T13:08:12Z","receivedAt":"2012-12-13T13:08:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 12, 2012 at 06:04:04AM -0500, Jeff King wrote:\n\n> In a bare repository, there isn't a simple way to respect an\n> in-tree mailmap without extracting it to a temporary file.\n> This patch provides a config variable, similar to\n> mailmap.file, which reads the mailmap from a blob in the\n> repository.\n\nWhile preparing the \"default mailmap.blob\" patch, I noticed a few loose\nends in the documentation. They can be squashed into this 2/2\n(jk/mailmap-from-blob~1), or can go on top of the series:\n\n-- >8 --\nSubject: [PATCH] fix some documentation loose-ends for mailmap.blob\n\nAnywhere we mention mailmap.file, it is probably worth\nmentioning mailmap.blob, as well.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-log.txt | 2 +-\n Documentation/mailmap.txt | 3 ++-\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 585dac4..08a185d 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -167,7 +167,7 @@ log.showroot::\n \t`git log -p` output would be shown without a diff attached.\n \tThe default is `true`.\n \n-mailmap.file::\n+mailmap.*::\n \tSee linkgit:git-shortlog[1].\n \n notes.displayRef::\ndiff --git a/Documentation/mailmap.txt b/Documentation/mailmap.txt\nindex 288f04e..bb349c2 100644\n--- a/Documentation/mailmap.txt\n+++ b/Documentation/mailmap.txt\n@@ -1,5 +1,6 @@ If the file `.mailmap` exists at the toplevel of the repository, or at\n If the file `.mailmap` exists at the toplevel of the repository, or at\n-the location pointed to by the mailmap.file configuration option, it\n+the location pointed to by the mailmap.file or mailmap.blob\n+configuration options, it\n is used to map author and committer names and email addresses to\n canonical real names and email addresses.\n \n-- \n1.8.0.2.4.g59402aa\n"},{"id":"204842","messageId":"7vfw39zu8k.fsf@alter.siamese.dyndns.org","threadId":"32319","inReplyTo":"20121213130447.GA4353@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] mailmap from blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-13T18:23:39Z","receivedAt":"2012-12-13T18:23:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Subject: [PATCH] mailmap: default mailmap.blob in bare repositories\n>\n> The motivation for mailmap.blob is to let users of bare\n> repositories use the mailmap feature, as they would not have\n> a checkout containing the .mailmap file. We can make it even\n> easier for them by just looking in HEAD:.mailmap by default.\n>\n> We can't know for sure that this is where they would keep a\n> mailmap, of course, but it is the best guess (and it matches\n> the non-bare behavior, which reads from HEAD:.mailmap in the\n> working tree). If it's missing, git will silently ignore the\n> setting.\n>\n> We do not do the same magic in the non-bare case, because:\n>\n> 1. In the common case, HEAD:.mailmap will be the same as\n> the .mailmap in the working tree, which is a no-op.\n>\n> 2. In the uncommon case, the user has modified .mailmap\n> but not yet committed it, and would expect the working\n> tree version to take precedence.\n>\n> Signed-off-by: Jeff King \n> ---\n> I went with defaulting mailmap.blob, because it provides an easy path\n> for turning off the feature (you just override mailmap.blob).\n\nVery sensibly explained. I like it when people clearly explain the\nthinking behind the change in the log message.\n\nThanks, will queue.\n\n\n> Documentation/config.txt | 8 +++++---\n> mailmap.c | 5 +++++\n> t/t4203-mailmap.sh | 25 +++++++++++++++++++++++++\n> 3 files changed, 35 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 3760077..1a3c554 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1519,9 +1519,11 @@ mailmap.blob::\n> \n> mailmap.blob::\n> \tLike `mailmap.file`, but consider the value as a reference to a\n> -\tblob in the repository (e.g., `HEAD:.mailmap`). If both\n> -\t`mailmap.file` and `mailmap.blob` are given, both are parsed,\n> -\twith entries from `mailmap.file` taking precedence.\n> +\tblob in the repository. If both `mailmap.file` and\n> +\t`mailmap.blob` are given, both are parsed, with entries from\n> +\t`mailmap.file` taking precedence. In a bare repository, this\n> +\tdefaults to `HEAD:.mailmap`. In a non-bare repository, it\n> +\tdefaults to empty.\n> \n> man.viewer::\n> \tSpecify the programs that may be used to display help in the\n> diff --git a/mailmap.c b/mailmap.c\n> index 5ffe48a..b16542f 100644\n> --- a/mailmap.c\n> +++ b/mailmap.c\n> @@ -233,7 +233,12 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)\n> int read_mailmap(struct string_list *map, char **repo_abbrev)\n> {\n> \tint err = 0;\n> +\n> \tmap->strdup_strings = 1;\n> +\n> +\tif (!git_mailmap_blob && is_bare_repository())\n> +\t\tgit_mailmap_blob = \"HEAD:.mailmap\";\n> +\n> \terr |= read_mailmap_file(map, \".mailmap\", repo_abbrev);\n> \terr |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);\n> \terr |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);\n> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\n> index e7ea40c..aae30d9 100755\n> --- a/t/t4203-mailmap.sh\n> +++ b/t/t4203-mailmap.sh\n> @@ -218,6 +218,31 @@ test_expect_success 'mailmap.blob can be missing' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'mailmap.blob defaults to off in non-bare repo' '\n> +\tgit init non-bare &&\n> +\t(\n> +\t\tcd non-bare &&\n> +\t\ttest_commit one .mailmap \"Fake Name \" &&\n> +\t\techo \" 1\tFake Name\" >expect &&\n> +\t\tgit shortlog -ns HEAD >actual &&\n> +\t\ttest_cmp expect actual &&\n> +\t\trm .mailmap &&\n> +\t\techo \" 1\tA U Thor\" >expect &&\n> +\t\tgit shortlog -ns HEAD >actual &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n> +test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '\n> +\tgit clone --bare non-bare bare &&\n> +\t(\n> +\t\tcd bare &&\n> +\t\techo \" 1\tFake Name\" >expect &&\n> +\t\tgit shortlog -ns HEAD >actual &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n> test_expect_success 'cleanup after mailmap.blob tests' '\n> \trm -f .mailmap\n> '\n"}]}