{"thread":{"id":"54924","subject":"[PATCH v2 1/5] mailmap: add a function to inspect the number of entries","startedAt":"2021-01-03T21:19:53Z","lastAt":"2021-01-12T14:09:33Z","messageCount":19,"participants":["brian m. carlson","Ævar Arnfjörð Bjarmason","René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"413343","messageId":"20210103211849.2691287-2-sandals@crustytoothpaste.net","threadId":"54924","inReplyTo":"20210103211849.2691287-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 1/5] mailmap: add a function to inspect the number of entries","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-03T21:18:45Z","receivedAt":"2021-01-03T21:19:53Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We're soon going to change the type of our mailmap into an opaque struct\nso we can add features and improve performance. When we do so, it won't\nbe possible for users to inspect its internals to determine how many\nitems are present, so let's introduce a function that lets users inquire\nhow many objects are in the mailmap and use it where we want this\ninformation.\n\nSigned-off-by: brian m. carlson \n---\n mailmap.c | 5 +++++\n mailmap.h | 1 +\n pretty.c | 2 +-\n 3 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/mailmap.c b/mailmap.c\nindex 962fd86d6d..c9a538f4e2 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -361,3 +361,8 @@ int map_user(struct string_list *map,\n \tdebug_mm(\"map_user: --\\n\");\n \treturn 0;\n }\n+\n+int mailmap_entries(struct string_list *map)\n+{\n+\treturn map->nr;\n+}\ndiff --git a/mailmap.h b/mailmap.h\nindex d0e65646cb..ff57b05a15 100644\n--- a/mailmap.h\n+++ b/mailmap.h\n@@ -5,6 +5,7 @@ struct string_list;\n \n int read_mailmap(struct string_list *map, char **repo_abbrev);\n void clear_mailmap(struct string_list *map);\n+int mailmap_entries(struct string_list *map);\n \n int map_user(struct string_list *map,\n \t\t\t const char **email, size_t *emaillen, const char **name, size_t *namelen);\ndiff --git a/pretty.c b/pretty.c\nindex 7a7708a0ea..43a0039870 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -681,7 +681,7 @@ static int mailmap_name(const char **email, size_t *email_len,\n \t\tmail_map = xcalloc(1, sizeof(*mail_map));\n \t\tread_mailmap(mail_map, NULL);\n \t}\n-\treturn mail_map->nr && map_user(mail_map, email, email_len, name, name_len);\n+\treturn mailmap_entries(mail_map) && map_user(mail_map, email, email_len, name, name_len);\n }\n \n static size_t format_person_part(struct strbuf *sb, char part,\n"},{"id":"413344","messageId":"20210103211849.2691287-1-sandals@crustytoothpaste.net","threadId":"54924","inReplyTo":null,"subject":"[PATCH v2 0/5] Hashed mailmap","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-03T21:18:44Z","receivedAt":"2021-01-03T21:20:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Many people, through the course of their lives, will change either a\nname or an email address. For this reason, we have the mailmap, to map\nfrom a user's former name or email address to their current, canonical\nforms. Normally, this works well as it is.\n\nHowever, sometimes people change a name (or an email) and want to\ncompletely cease use of the former name or email. This could be because\na transgender person has transitioned, because a person has left an\nabusive partner or broken ties with an abusive family member, or for any\nother number of good and valuable reasons. In these cases, placing the\nformer name in the .mailmap may be undesirable.\n\nFor those situations, let's introduce a hashed mailmap, where the user's\nformer name or email address can be in the form @sha256:. This\nobscures the former name or email.\n\nIn the course of experimenting with some solutions for v2, I noticed\nthat our mailmap support has a bunch of problems with case sensitivity.\nNotably, it treats local-parts of email addresses in a case-insensitive\nway, when the RFC specifically says that they are case sensitive, and we\nalso treat names case insensitively, but only for ASCII characters.\nBoth of those have been fixed here, and the commit messages explain in\nlurid detail why, while incompatible, this is the correct behavior.\n\nI've also added some performance numbers and explained some alternate\nsolutions in the commit message for the final patch. That's in addition\nto the performance improvements I've done so that the feature is both\ncheaper for users and nearly invisible for non-users. That isn't quite\nthe same as adding a perf test, which I haven't done, but I think this\nexplains the situation quite well. If folks are still dying for a perf\ntest, I can add one in v3.\n\nI will point out that fully hashing a mailmap isn't necessarily cheap,\nbut how expensive it is depends on the weighting of current and former\nmembers of the project. As mentioned in the original thread, I think a\nhash rather than an encoding is the right choice here. It is likely\nthat in a few iterations of hardware, all users will have accelerated\nSHA-256 and the cost will end up being a handful of cycles per name\noverall.\n\nChanges from v1:\n* Fix case-sensitivity problems in the mailmap.\n* Add documentation.\n* Add explanation of how to compute the value.\n* Add some optimizations to improve performance.\n* Improve commit message to discuss performance numbers and explain\n rationale better.\n\nbrian m. carlson (5):\n mailmap: add a function to inspect the number of entries\n mailmap: switch to opaque struct\n t4203: add failing test for case-sensitive local-parts and names\n mailmap: use case-sensitive comparisons for local-parts and names\n mailmap: support hashed entries in mailmaps\n\n Documentation/mailmap.txt | 28 ++++++++\n builtin/blame.c | 2 +-\n builtin/check-mailmap.c | 4 +-\n builtin/commit.c | 2 +-\n mailmap.c | 139 +++++++++++++++++++++++++++++++++-----\n mailmap.h | 15 ++--\n pretty.c | 4 +-\n pretty.h | 2 +-\n revision.c | 2 +-\n revision.h | 3 +-\n shortlog.h | 3 +-\n t/t4203-mailmap.sh | 64 +++++++++++++++++-\n 12 files changed, 236 insertions(+), 32 deletions(-)\n\n"},{"id":"413345","messageId":"20210103211849.2691287-3-sandals@crustytoothpaste.net","threadId":"54924","inReplyTo":"20210103211849.2691287-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 2/5] mailmap: switch to opaque struct","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-03T21:18:46Z","receivedAt":"2021-01-03T21:20:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Currently a mailmap is a simple sorted string list. However, we'll soon\nwant to add additional data to our mailmap, and in order to do so\neffectively, we'll need a struct of our own. To do that, let's create a\nstruct mailmap and use that everywhere we're creating a mailmap.\n\nNote that we no longer explicitly initialize our mailmaps, since\nread_mailmap will do that for us. We also don't need to explicitly set\nthe flag to strdup strings, since we've passed that argument when we\ninitialize the string list.\n\nSigned-off-by: brian m. carlson \n---\n builtin/blame.c | 2 +-\n builtin/check-mailmap.c | 4 ++--\n builtin/commit.c | 2 +-\n mailmap.c | 20 +++++++++++++-------\n mailmap.h | 14 +++++++++-----\n pretty.c | 2 +-\n pretty.h | 2 +-\n revision.c | 2 +-\n revision.h | 3 ++-\n shortlog.h | 3 ++-\n 10 files changed, 33 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 6f7e32411a..d48dbbd005 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -60,7 +60,7 @@ static int mark_ignored_lines;\n static struct date_mode blame_date_mode = { DATE_ISO8601 };\n static size_t blame_date_width;\n \n-static struct string_list mailmap = STRING_LIST_INIT_NODUP;\n+static struct mailmap mailmap;\n \n #ifndef DEBUG_BLAME\n #define DEBUG_BLAME 0\ndiff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c\nindex cdce144f3b..ad155e9092 100644\n--- a/builtin/check-mailmap.c\n+++ b/builtin/check-mailmap.c\n@@ -15,7 +15,7 @@ static const struct option check_mailmap_options[] = {\n \tOPT_END()\n };\n \n-static void check_mailmap(struct string_list *mailmap, const char *contact)\n+static void check_mailmap(struct mailmap *mailmap, const char *contact)\n {\n \tconst char *name, *mail;\n \tsize_t namelen, maillen;\n@@ -39,7 +39,7 @@ static void check_mailmap(struct string_list *mailmap, const char *contact)\n int cmd_check_mailmap(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n-\tstruct string_list mailmap = STRING_LIST_INIT_NODUP;\n+\tstruct mailmap mailmap;\n \n \tgit_config(git_default_config, NULL);\n \targc = parse_options(argc, argv, prefix, check_mailmap_options,\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 505fe60956..2d69847c49 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1027,7 +1027,7 @@ static const char *find_author_by_nickname(const char *name)\n \tstruct rev_info revs;\n \tstruct commit *commit;\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct string_list mailmap = STRING_LIST_INIT_NODUP;\n+\tstruct mailmap mailmap;\n \tconst char *av[20];\n \tint ac = 0;\n \ndiff --git a/mailmap.c b/mailmap.c\nindex c9a538f4e2..d3287b409a 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -237,11 +237,14 @@ static int read_mailmap_blob(struct string_list *map,\n \treturn 0;\n }\n \n-int read_mailmap(struct string_list *map, char **repo_abbrev)\n+int read_mailmap(struct mailmap *mailmap, char **repo_abbrev)\n {\n \tint err = 0;\n+\tstruct string_list *map;\n+\n+\tmap = mailmap->mailmap = malloc(sizeof(*mailmap->mailmap));\n+\tstring_list_init(map, 1);\n \n-\tmap->strdup_strings = 1;\n \tmap->cmp = namemap_cmp;\n \n \tif (!git_mailmap_blob && is_bare_repository())\n@@ -254,11 +257,14 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)\n \treturn err;\n }\n \n-void clear_mailmap(struct string_list *map)\n+void clear_mailmap(struct mailmap *mailmap)\n {\n+\tstruct string_list *map = mailmap->mailmap;\n \tdebug_mm(\"mailmap: clearing %d entries...\\n\", map->nr);\n \tmap->strdup_strings = 1;\n \tstring_list_clear_func(map, free_mailmap_entry);\n+\tstring_list_clear(map, 1);\n+\tfree(map);\n \tdebug_mm(\"mailmap: cleared\\n\");\n }\n \n@@ -313,7 +319,7 @@ static struct string_list_item *lookup_prefix(struct string_list *map,\n \treturn NULL;\n }\n \n-int map_user(struct string_list *map,\n+int map_user(struct mailmap *map,\n \t const char **email, size_t *emaillen,\n \t const char **name, size_t *namelen)\n {\n@@ -324,7 +330,7 @@ int map_user(struct string_list *map,\n \t\t (int)*namelen, debug_str(*name),\n \t\t (int)*emaillen, debug_str(*email));\n \n-\titem = lookup_prefix(map, *email, *emaillen);\n+\titem = lookup_prefix(map->mailmap, *email, *emaillen);\n \tif (item != NULL) {\n \t\tme = (struct mailmap_entry *)item->util;\n \t\tif (me->namemap.nr) {\n@@ -362,7 +368,7 @@ int map_user(struct string_list *map,\n \treturn 0;\n }\n \n-int mailmap_entries(struct string_list *map)\n+int mailmap_entries(struct mailmap *map)\n {\n-\treturn map->nr;\n+\treturn map->mailmap->nr;\n }\ndiff --git a/mailmap.h b/mailmap.h\nindex ff57b05a15..4cdce3b064 100644\n--- a/mailmap.h\n+++ b/mailmap.h\n@@ -1,13 +1,17 @@\n #ifndef MAILMAP_H\n #define MAILMAP_H\n \n-struct string_list;\n+#include \"string-list.h\"\n \n-int read_mailmap(struct string_list *map, char **repo_abbrev);\n-void clear_mailmap(struct string_list *map);\n-int mailmap_entries(struct string_list *map);\n+struct mailmap {\n+\tstruct string_list *mailmap;\n+};\n \n-int map_user(struct string_list *map,\n+int read_mailmap(struct mailmap *map, char **repo_abbrev);\n+void clear_mailmap(struct mailmap *map);\n+int mailmap_entries(struct mailmap *map);\n+\n+int map_user(struct mailmap *map,\n \t\t\t const char **email, size_t *emaillen, const char **name, size_t *namelen);\n \n #endif\ndiff --git a/pretty.c b/pretty.c\nindex 43a0039870..0dc2c98e4a 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -676,7 +676,7 @@ const char *repo_logmsg_reencode(struct repository *r,\n static int mailmap_name(const char **email, size_t *email_len,\n \t\t\tconst char **name, size_t *name_len)\n {\n-\tstatic struct string_list *mail_map;\n+\tstatic struct mailmap *mail_map;\n \tif (!mail_map) {\n \t\tmail_map = xcalloc(1, sizeof(*mail_map));\n \t\tread_mailmap(mail_map, NULL);\ndiff --git a/pretty.h b/pretty.h\nindex 7ce6c0b437..15735a4c51 100644\n--- a/pretty.h\n+++ b/pretty.h\n@@ -40,7 +40,7 @@ struct pretty_print_context {\n \tstruct reflog_walk_info *reflog_info;\n \tstruct rev_info *rev;\n \tconst char *output_encoding;\n-\tstruct string_list *mailmap;\n+\tstruct mailmap *mailmap;\n \tint color;\n \tstruct ident_split *from_ident;\n \tunsigned encode_email_headers:1;\ndiff --git a/revision.c b/revision.c\nindex 9dff845bed..848f43d88b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3659,7 +3659,7 @@ int rewrite_parents(struct rev_info *revs, struct commit *commit,\n \treturn 0;\n }\n \n-static int commit_rewrite_person(struct strbuf *buf, const char *what, struct string_list *mailmap)\n+static int commit_rewrite_person(struct strbuf *buf, const char *what, struct mailmap *mailmap)\n {\n \tchar *person, *endp;\n \tsize_t len, namelen, maillen;\ndiff --git a/revision.h b/revision.h\nindex 086ff10280..06bc127e90 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -7,6 +7,7 @@\n #include \"notes.h\"\n #include \"pretty.h\"\n #include \"diff.h\"\n+#include \"mailmap.h\"\n #include \"commit-slab-decl.h\"\n \n /**\n@@ -241,7 +242,7 @@ struct rev_info {\n \tint\t\tpatch_name_max;\n \tint\t\tno_inline;\n \tint\t\tshow_log_size;\n-\tstruct string_list *mailmap;\n+\tstruct mailmap *mailmap;\n \n \t/* Filter by commit log message */\n \tstruct grep_opt\tgrep_filter;\ndiff --git a/shortlog.h b/shortlog.h\nindex 64be879b24..9ebf7bbb9c 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -2,6 +2,7 @@\n #define SHORTLOG_H\n \n #include \"string-list.h\"\n+#include \"mailmap.h\"\n \n struct commit;\n \n@@ -25,7 +26,7 @@ struct shortlog {\n \n \tchar *common_repo_prefix;\n \tint email;\n-\tstruct string_list mailmap;\n+\tstruct mailmap mailmap;\n \tFILE *file;\n };\n \n"},{"id":"413346","messageId":"20210103211849.2691287-4-sandals@crustytoothpaste.net","threadId":"54924","inReplyTo":"20210103211849.2691287-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 3/5] t4203: add failing test for case-sensitive local-parts and names","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-03T21:18:47Z","receivedAt":"2021-01-03T21:20:36Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Currently, Git always looks up entries in the mailmap in a\ncase-insensitive way, both for names and addresses, which is, as\nexplained below, suboptimal.\n\nFirst, for email addresses, RFC 5321 is clear that only domains are case\ninsensitive; local-parts (the portion before the at sign) are not. It\nstates this:\n\n The local-part of a mailbox MUST BE treated as case sensitive.\n Therefore, SMTP implementations MUST take care to preserve the case of\n mailbox local-parts.\n\nThere exist systems today where local-parts remain case sensitive (and\nthis author has one), and as such, it's incorrect for us to case fold\nthem in any way. Let's add a failing test that indicates this is a\nproblem, while still keeping the test for case-insensitive domains.\n\nNote that it's also incorrect for us to case-fold names because we don't\nguarantee that we're using the locale of the author, and it's impossible\nto case-fold names in a locale-insensitive way. Turkish and Azeri\ncontain both a dotted and dotless I, and the uppercase ASCII I folds not\nto the lowercase ASCII I, but to a dotless version, and vice versa with\nthe lowercase I. There are many words in Turkish which differ only in\nthe dottedness of the I, so it is likely that there are also personal\nnames which differ in the same way.\n\nThat would be a problem even if our implementation were perfect, which\nit is not. We currently fold only ASCII characters, so this feature has\nnever worked correctly for the vast majority of the users on the planet,\nregardless of the locale. For example, on Linux, even in a Spanish\nlocale, we don't handle \"Simón\" properly. Even if we did handle that,\nwe'd probably also want to implement Unicode normalization, which we\ndon't.\n\nIn general, case-folding text is extremely language- and locale-specific\nand requires intimacy with the spelling and grammar of the language in\nquestion and careful attention to the Unicode details in order to\nproduce a result that is meaningful to humans and conforms with\nlinguistic and societal norms.\n\nBecause we do not have any of the required context with a plain personal\nname, we cannot hope to possibly case-fold personal names correctly. We\nshould stop trying to do so and just treat them as a series of bytes, so\nlet's add a test that we don't case-fold personal names as well.\n\nSigned-off-by: brian m. carlson \n---\n t/t4203-mailmap.sh | 29 +++++++++++++++++++++++++++--\n 1 file changed, 27 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 586c3a86b1..32e849504c 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -170,10 +170,35 @@ Repo Guy (1):\n \n EOF\n \n-test_expect_success 'name entry after email entry, case-insensitive' '\n+test_expect_success 'name entry after email entry, case-insensitive domain' '\n \tmkdir -p internal_mailmap &&\n \techo \" \" >internal_mailmap/.mailmap &&\n-\techo \"Internal Guy \" >>internal_mailmap/.mailmap &&\n+\techo \"Internal Guy \" >>internal_mailmap/.mailmap &&\n+\tgit shortlog HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<\\EOF\n+Repo Guy (1):\n+ initial\n+\n+nick1 (1):\n+ second\n+\n+EOF\n+\n+test_expect_failure 'name entry after email entry, case-sensitive local-part' '\n+\tmkdir -p internal_mailmap &&\n+\techo \" \" >internal_mailmap/.mailmap &&\n+\techo \"Internal Guy \" >>internal_mailmap/.mailmap &&\n+\tgit shortlog HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_failure 'name entry after email entry, case-sensitive personal name' '\n+\tmkdir -p internal_mailmap &&\n+\techo \" \" >internal_mailmap/.mailmap &&\n+\techo \"Nick1 NICK1 \" >internal_mailmap/.mailmap &&\n \tgit shortlog HEAD >actual &&\n \ttest_cmp expect actual\n '\n"},{"id":"413347","messageId":"20210103211849.2691287-5-sandals@crustytoothpaste.net","threadId":"54924","inReplyTo":"20210103211849.2691287-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 4/5] mailmap: use case-sensitive comparisons for local-parts and names","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-03T21:18:48Z","receivedAt":"2021-01-03T21:20:36Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"RFC 5321 is clear that the local-part of an email address (the part\nbefore the at sign) is case sensitive, and this has been the case since\nthe original RFC 821. It directs us that \"the local-part MUST be\ninterpreted and assigned semantics only by the host specified in the\ndomain part of the address.\"\n\nSince we are not that party, it's not correct for us to compare them\ncase insensitively. However, we do still want to compare the domain\nparts case insensitively, so let's add a helper that downcases the\ndomain and then compare byte by byte.\n\nSimilarly, it's not possible for us to correctly case-fold text in a\nlocale-insensitive way, so our handling of personal names has also been\nsubject to bugs. Additionally, we've never handled non-ASCII characters\ncorrectly, which means that our previous comparisons really only worked\nwell for a fraction of the people on the planet. Since our code wasn't\nright and it's basically impossible to compare personal names without\nregard to case, let's also switch our matching of names to be byte by\nbyte.\n\nSigned-off-by: brian m. carlson \n---\n mailmap.c | 27 ++++++++++++++++++++++++---\n t/t4203-mailmap.sh | 4 ++--\n 2 files changed, 26 insertions(+), 5 deletions(-)\n\ndiff --git a/mailmap.c b/mailmap.c\nindex d3287b409a..5c52dbb7e0 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -64,7 +64,22 @@ static void free_mailmap_entry(void *p, const char *s)\n */\n static int namemap_cmp(const char *a, const char *b)\n {\n-\treturn strcasecmp(a, b);\n+\treturn strcmp(a, b);\n+}\n+\n+/*\n+ * Lowercases the domain (and only the domain) part of an email address. The\n+ * local-part, which is defined by RFC 5321 to be case sensitive, is not\n+ * affected.\n+ */\n+static char *lowercase_email(char *s)\n+{\n+\tchar *end = strchrnul(s, '@');\n+\twhile (*end) {\n+\t\t*end = tolower(*end);\n+\t\tend++;\n+\t}\n+\treturn s;\n }\n \n static void add_mapping(struct string_list *map,\n@@ -74,9 +89,13 @@ static void add_mapping(struct string_list *map,\n \tstruct mailmap_entry *me;\n \tstruct string_list_item *item;\n \n+\tlowercase_email(new_email);\n+\n \tif (old_email == NULL) {\n \t\told_email = new_email;\n \t\tnew_email = NULL;\n+\t} else {\n+\t\tlowercase_email(old_email);\n \t}\n \n \titem = string_list_insert(map, old_email);\n@@ -300,7 +319,7 @@ static struct string_list_item *lookup_prefix(struct string_list *map,\n \t * real location of the key if one exists.\n \t */\n \twhile (0 <= --i && i < map->nr) {\n-\t\tint cmp = strncasecmp(map->items[i].string, string, len);\n+\t\tint cmp = strncmp(map->items[i].string, string, len);\n \t\tif (cmp < 0)\n \t\t\t/*\n \t\t\t * \"i\" points at a key definitely below the prefix;\n@@ -323,6 +342,7 @@ int map_user(struct mailmap *map,\n \t const char **email, size_t *emaillen,\n \t const char **name, size_t *namelen)\n {\n+\tchar *searchable_email = xstrndup(*email, *emaillen);\n \tstruct string_list_item *item;\n \tstruct mailmap_entry *me;\n \n@@ -330,7 +350,8 @@ int map_user(struct mailmap *map,\n \t\t (int)*namelen, debug_str(*name),\n \t\t (int)*emaillen, debug_str(*email));\n \n-\titem = lookup_prefix(map->mailmap, *email, *emaillen);\n+\titem = lookup_prefix(map->mailmap, searchable_email, *emaillen);\n+\tfree(searchable_email);\n \tif (item != NULL) {\n \t\tme = (struct mailmap_entry *)item->util;\n \t\tif (me->namemap.nr) {\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 32e849504c..df4a0e03cc 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -187,7 +187,7 @@ nick1 (1):\n \n EOF\n \n-test_expect_failure 'name entry after email entry, case-sensitive local-part' '\n+test_expect_success 'name entry after email entry, case-sensitive local-part' '\n \tmkdir -p internal_mailmap &&\n \techo \" \" >internal_mailmap/.mailmap &&\n \techo \"Internal Guy \" >>internal_mailmap/.mailmap &&\n@@ -195,7 +195,7 @@ test_expect_failure 'name entry after email entry, case-sensitive local-part' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'name entry after email entry, case-sensitive personal name' '\n+test_expect_success 'name entry after email entry, case-sensitive personal name' '\n \tmkdir -p internal_mailmap &&\n \techo \" \" >internal_mailmap/.mailmap &&\n \techo \"Nick1 NICK1 \" >internal_mailmap/.mailmap &&\n"},{"id":"413348","messageId":"20210103211849.2691287-6-sandals@crustytoothpaste.net","threadId":"54924","inReplyTo":"20210103211849.2691287-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 5/5] mailmap: support hashed entries in mailmaps","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-03T21:18:49Z","receivedAt":"2021-01-03T21:20:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Many people, through the course of their lives, will change either a\nname or an email address. For this reason, we have the mailmap, to map\nfrom a user's former name or email address to their current, canonical\nforms. Normally, this works well as it is.\n\nHowever, sometimes people change a name or an email address and wish to\nwholly disassociate themselves from that former name or email address.\nFor example, a person may transition from one gender to another,\nchanging their name, or they may have changed their name to disassociate\nthemselves from an abusive family or partner. In such a case, using the\nformer name or address in any way may be undesirable and the person may\nwish to replace it as completely as possible.\n\nFor projects which wish to support this, introduce hashed forms into the\nmailmap. These forms, which start with \"@sha256:\" followed by a SHA-256\nhash of the entry, can be used in place of the form used in the commit\nfield. This form is intentionally designed to be unlikely to conflict\nwith legitimate use cases. For example, this is not a valid email\naddress according to RFC 5322. In the unlikely event that a user has\nput such a form into the actual commit as their name, we will accept it.\n\nWhile the form of the data is designed to accept multiple hash\nalgorithms, we intentionally do not support SHA-1. There is little\nreason to support such a weak algorithm in new use cases and no\nbackwards compatibility to consider. Moreover, SHA-256 is faster than\nthe SHA1DC implementation we use, so this not only improves performance,\nbut simplifies the current implementation somewhat as well.\n\nNote that it is, of course, possible to perform a lookup on all commit\nobjects to determine the actual entry which matches the hashed form of\nthe data. However, this is an improvement over the status quo.\n\nThe performance of this patch with no hashed entries is very similar to\nthe performance without this patch. Considering a git log command to\nlook up author and committer information on 981,680 commits in the Linux\nkernel history, either with an unhashed mailmap or a mailmap with all\nold values hashed:\n\n Shortest Longest Average Change\n Git 2.30 7.876 8.297 8.143\n This patch, unhashed 7.923 8.484 8.237 + 1.15%\n This patch, hashed 14.510 14.783 14.672 +80.17%\n This patch, hashed, unoptimized 15.425 16.318 15.901 +95.27%\n\nThus, the average performance after this patch is within normal\nvariation of the pre-patch performance. It's unlikely that users will\nnotice the difference in practice, even on much larger\nrepositories, unless they're using the new feature.\n\nTo minimize the performance impact of the hashing process, we maintain a\nreference count of each mailmap entry and when we encounter an entry we\nmust hash, we insert the same object under the unhashed key as well. We\nalso keep a count of the number of hashed entries. This means we must\nhash an object at most once and once we've seen all the hashed objects,\nwe won't hash any more objects. Times without this optimization are\nlisted above in the unoptimized entry.\n\nThis has the potential to cause a performance problem as we insert items\ninto a sorted list, but changing the implementation to use a khash map\ninstead does not result in a significantly faster implementation,\ndespite the improved insertion speed. Performance in the unhashed case\nis slightly worse, so this approach was not adopted since it provides\nfew benefits.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/mailmap.txt | 28 +++++++++++\n mailmap.c | 99 ++++++++++++++++++++++++++++++++++-----\n mailmap.h | 2 +\n t/t4203-mailmap.sh | 35 ++++++++++++++\n 4 files changed, 152 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/mailmap.txt b/Documentation/mailmap.txt\nindex 4a8c276529..b21194bf3e 100644\n--- a/Documentation/mailmap.txt\n+++ b/Documentation/mailmap.txt\n@@ -73,3 +73,31 @@ Santa Claus \n \n Use hash '#' for comments that are either on their own line, or after\n the email address.\n+\n+In addition to specifying a former name or email literally, it is also possible\n+to specify it in a hashed form, which consists of the string `@sha256:`,\n+followed by an all-lowercase SHA-256 hash of the entry in hexadecimal. For\n+example, to take the example above, instead of specifying the replacement for\n+\"Some Dude\" as such, you could specify one of these lines:\n+\n+------------\n+Some Dude nick1 <@sha256:bee4fdd8c5e2e85009c8ae231d5a395adb24d5a597f2b75489926460680b8ce1>\n+Some Dude @sha256:56030827e2765e8878c94c4cc43f5410b22f3b8c2b1ef8f631ac3953f8299279 \n+Some Dude @sha256:56030827e2765e8878c94c4cc43f5410b22f3b8c2b1ef8f631ac3953f8299279 <@sha256:bee4fdd8c5e2e85009c8ae231d5a395adb24d5a597f2b75489926460680b8ce1>\n+------------\n+\n+These hash is a hash of the literal name or email without any trailing newlines.\n+For example, you can compute the values above like so, using the Perl `shasum`\n+command (or a similar command of your choice):\n+\n+------------\n+$ printf '%s' bugs@company.xx | shasum -a 256\n+bee4fdd8c5e2e85009c8ae231d5a395adb24d5a597f2b75489926460680b8ce1 -\n+------------\n+\n+SHA-1 is not accepted as a hash algorithm in mailmaps.\n+\n+Using the hashed form may be desirable to obscure one's former name or email,\n+but be aware that it is just obfuscation: it's still possible for someone with\n+access to the repository to iterate through all authors and committers and map\n+the hashed values to unhashed ones.\ndiff --git a/mailmap.c b/mailmap.c\nindex 5c52dbb7e0..ed401bb1e4 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -18,6 +18,8 @@ const char *git_mailmap_blob;\n struct mailmap_info {\n \tchar *name;\n \tchar *email;\n+\n+\tunsigned refcount;\n };\n \n struct mailmap_entry {\n@@ -25,6 +27,10 @@ struct mailmap_entry {\n \tchar *name;\n \tchar *email;\n \n+\tunsigned refcount;\n+\tunsigned hashed_count;\n+\tunsigned hashed_seen;\n+\n \t/* name and email for the complex mail and name matching case */\n \tstruct string_list namemap;\n };\n@@ -32,6 +38,9 @@ struct mailmap_entry {\n static void free_mailmap_info(void *p, const char *s)\n {\n \tstruct mailmap_info *mi = (struct mailmap_info *)p;\n+\tif (--mi->refcount)\n+\t\treturn;\n+\n \tdebug_mm(\"mailmap: -- complex: '%s' -> '%s' <%s>\\n\",\n \t\t s, debug_str(mi->name), debug_str(mi->email));\n \tfree(mi->name);\n@@ -41,6 +50,9 @@ static void free_mailmap_info(void *p, const char *s)\n static void free_mailmap_entry(void *p, const char *s)\n {\n \tstruct mailmap_entry *me = (struct mailmap_entry *)p;\n+\tif (--me->refcount)\n+\t\treturn;\n+\n \tdebug_mm(\"mailmap: removing entries for <%s>, with %d sub-entries\\n\",\n \t\t s, me->namemap.nr);\n \tdebug_mm(\"mailmap: - simple: '%s' <%s>\\n\",\n@@ -82,10 +94,17 @@ static char *lowercase_email(char *s)\n \treturn s;\n }\n \n-static void add_mapping(struct string_list *map,\n+static int is_hashed(const char *s)\n+{\n+\tconst char *prefix = \"@sha256:\";\n+\treturn strncmp(s, prefix, strlen(prefix)) == 0;\n+}\n+\n+static void add_mapping(struct mailmap *mailmap,\n \t\t\tchar *new_name, char *new_email,\n \t\t\tchar *old_name, char *old_email)\n {\n+\tstruct string_list *map = mailmap->mailmap;\n \tstruct mailmap_entry *me;\n \tstruct string_list_item *item;\n \n@@ -95,7 +114,10 @@ static void add_mapping(struct string_list *map,\n \t\told_email = new_email;\n \t\tnew_email = NULL;\n \t} else {\n-\t\tlowercase_email(old_email);\n+\t\tif (is_hashed(old_email))\n+\t\t\tmailmap->hashed_count++;\n+\t\telse\n+\t\t\tlowercase_email(old_email);\n \t}\n \n \titem = string_list_insert(map, old_email);\n@@ -105,6 +127,7 @@ static void add_mapping(struct string_list *map,\n \t\tme = xcalloc(1, sizeof(struct mailmap_entry));\n \t\tme->namemap.strdup_strings = 1;\n \t\tme->namemap.cmp = namemap_cmp;\n+\t\tme->refcount = 1;\n \t\titem->util = me;\n \t}\n \n@@ -125,6 +148,9 @@ static void add_mapping(struct string_list *map,\n \t\tdebug_mm(\"mailmap: adding (complex) entry for '%s'\\n\", old_email);\n \t\tmi->name = xstrdup_or_null(new_name);\n \t\tmi->email = xstrdup_or_null(new_email);\n+\t\tmi->refcount = 1;\n+\t\tif (is_hashed(old_name))\n+\t\t\tme->hashed_count++;\n \t\tstring_list_insert(&me->namemap, old_name)->util = mi;\n \t}\n \n@@ -162,7 +188,7 @@ static char *parse_name_and_email(char *buffer, char **name,\n \treturn (*right == '\\0' ? NULL : right);\n }\n \n-static void read_mailmap_line(struct string_list *map, char *buffer,\n+static void read_mailmap_line(struct mailmap *map, char *buffer,\n \t\t\t char **repo_abbrev)\n {\n \tchar *name1 = NULL, *email1 = NULL, *name2 = NULL, *email2 = NULL;\n@@ -194,7 +220,7 @@ static void read_mailmap_line(struct string_list *map, char *buffer,\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+static int read_mailmap_file(struct mailmap *map, const char *filename,\n \t\t\t char **repo_abbrev)\n {\n \tchar buffer[1024];\n@@ -216,7 +242,7 @@ static int read_mailmap_file(struct string_list *map, const char *filename,\n \treturn 0;\n }\n \n-static void read_mailmap_string(struct string_list *map, char *buf,\n+static void read_mailmap_string(struct mailmap *map, char *buf,\n \t\t\t\tchar **repo_abbrev)\n {\n \twhile (*buf) {\n@@ -230,7 +256,7 @@ static void read_mailmap_string(struct string_list *map, char *buf,\n \t}\n }\n \n-static int read_mailmap_blob(struct string_list *map,\n+static int read_mailmap_blob(struct mailmap *map,\n \t\t\t const char *name,\n \t\t\t char **repo_abbrev)\n {\n@@ -269,10 +295,10 @@ int read_mailmap(struct mailmap *mailmap, char **repo_abbrev)\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_file(mailmap, \".mailmap\", repo_abbrev);\n \tif (startup_info->have_repository)\n-\t\terr |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);\n-\terr |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);\n+\t\terr |= read_mailmap_blob(mailmap, git_mailmap_blob, repo_abbrev);\n+\terr |= read_mailmap_file(mailmap, git_mailmap_file, repo_abbrev);\n \treturn err;\n }\n \n@@ -282,7 +308,7 @@ void clear_mailmap(struct mailmap *mailmap)\n \tdebug_mm(\"mailmap: clearing %d entries...\\n\", map->nr);\n \tmap->strdup_strings = 1;\n \tstring_list_clear_func(map, free_mailmap_entry);\n-\tstring_list_clear(map, 1);\n+\tstring_list_clear(map, 0);\n \tfree(map);\n \tdebug_mm(\"mailmap: cleared\\n\");\n }\n@@ -338,6 +364,55 @@ static struct string_list_item *lookup_prefix(struct string_list *map,\n \treturn NULL;\n }\n \n+/*\n+ * Convert an email or name into a hashed form for comparison. The hashed form\n+ * will be created in the form\n+ * @sha256:c68b7a430ac8dee9676ec77a387194e23f234d024e03d844050cf6c01775c8f6,\n+ * which would be the hashed form for \"doe@example.com\".\n+ */\n+static char *hashed_form(struct strbuf *buf, const struct git_hash_algo *algop, const char *key, size_t keylen)\n+{\n+\tgit_hash_ctx ctx;\n+\tunsigned char hashbuf[GIT_MAX_RAWSZ];\n+\tchar hexbuf[GIT_MAX_HEXSZ + 1];\n+\n+\talgop->init_fn(&ctx);\n+\talgop->update_fn(&ctx, key, keylen);\n+\talgop->final_fn(hashbuf, &ctx);\n+\thash_to_hex_algop_r(hexbuf, hashbuf, algop);\n+\n+\tstrbuf_addf(buf, \"@%s:%s\", algop->name, hexbuf);\n+\treturn buf->buf;\n+}\n+\n+static struct string_list_item *lookup_one(struct string_list *map,\n+\t\t\t\t\t const char *string, size_t len,\n+\t\t\t\t\t unsigned hashed_count,\n+\t\t\t\t\t unsigned *hashed_seen)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct string_list_item *item = lookup_prefix(map, string, len);\n+\tif (item || !hashed_count || hashed_count == *hashed_seen)\n+\t\treturn item;\n+\n+\thashed_form(&buf, &hash_algos[GIT_HASH_SHA256], string, len);\n+\titem = lookup_prefix(map, buf.buf, buf.len);\n+\tif (item) {\n+\t\tstruct mailmap_info *mi = (struct mailmap_info *)item->util;\n+\t\tchar *s = xstrndup(string, len);\n+\t\tmap->strdup_strings = 0;\n+\t\titem = string_list_insert(map, s);\n+\t\tmap->strdup_strings = 1;\n+\t\tif (!item->util) {\n+\t\t\titem->util = mi;\n+\t\t\tmi->refcount++;\n+\t\t\t(*hashed_seen)++;\n+\t\t}\n+\t}\n+\tstrbuf_release(&buf);\n+\treturn item;\n+}\n+\n int map_user(struct mailmap *map,\n \t const char **email, size_t *emaillen,\n \t const char **name, size_t *namelen)\n@@ -350,7 +425,7 @@ int map_user(struct mailmap *map,\n \t\t (int)*namelen, debug_str(*name),\n \t\t (int)*emaillen, debug_str(*email));\n \n-\titem = lookup_prefix(map->mailmap, searchable_email, *emaillen);\n+\titem = lookup_one(map->mailmap, searchable_email, *emaillen, map->hashed_count, &map->hashed_seen);\n \tfree(searchable_email);\n \tif (item != NULL) {\n \t\tme = (struct mailmap_entry *)item->util;\n@@ -361,7 +436,7 @@ int map_user(struct mailmap *map,\n \t\t\t * simple entry.\n \t\t\t */\n \t\t\tstruct string_list_item *subitem;\n-\t\t\tsubitem = lookup_prefix(&me->namemap, *name, *namelen);\n+\t\t\tsubitem = lookup_one(&me->namemap, *name, *namelen, me->hashed_count, &me->hashed_seen);\n \t\t\tif (subitem)\n \t\t\t\titem = subitem;\n \t\t}\ndiff --git a/mailmap.h b/mailmap.h\nindex 4cdce3b064..69f8be5705 100644\n--- a/mailmap.h\n+++ b/mailmap.h\n@@ -5,6 +5,8 @@\n \n struct mailmap {\n \tstruct string_list *mailmap;\n+\tunsigned hashed_count;\n+\tunsigned hashed_seen;\n };\n \n int read_mailmap(struct mailmap *map, char **repo_abbrev);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex df4a0e03cc..004b4a3d40 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -62,6 +62,41 @@ test_expect_success 'check-mailmap --stdin arguments' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'hashed mailmap' '\n+\ttest_config mailmap.file ./hashed &&\n+\thashed_author_name=\"@sha256:$(printf \"$GIT_AUTHOR_NAME\" | test-tool sha256)\" &&\n+\thashed_author_email=\"@sha256:$(printf \"$GIT_AUTHOR_EMAIL\" | test-tool sha256)\" &&\n+\tcat >expect <<-EOF &&\n+\t$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\tEOF\n+\n+\tcat >hashed <<-EOF &&\n+\t$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $hashed_author_name <$GIT_AUTHOR_EMAIL>\n+\tEOF\n+\tgit check-mailmap \"$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>\" >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tcat >hashed <<-EOF &&\n+\tWrong $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>\n+\t$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $hashed_author_name <$GIT_AUTHOR_EMAIL>\n+\tEOF\n+\t# Check that we prefer literal matches over hashed names.\n+\tgit check-mailmap \"$hashed_author_name <$GIT_AUTHOR_EMAIL>\" >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tcat >hashed <<-EOF &&\n+\t$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $hashed_author_name <$hashed_author_email>\n+\tEOF\n+\tgit check-mailmap \"$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>\" >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tcat >hashed <<-EOF &&\n+\t$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> <$hashed_author_email>\n+\tEOF\n+\tgit check-mailmap \"$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'check-mailmap bogus contact' '\n \ttest_must_fail git check-mailmap bogus\n '\n"},{"id":"413373","messageId":"87im8cvinn.fsf@evledraar.gmail.com","threadId":"54924","inReplyTo":"20210103211849.2691287-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 1/5] mailmap: add a function to inspect the number of entries","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-04T15:14:20Z","receivedAt":"2021-01-04T15:15:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jan 03 2021, brian m. carlson wrote:\n\n> We're soon going to change the type of our mailmap into an opaque struct\n> so we can add features and improve performance. When we do so, it won't\n> be possible for users to inspect its internals to determine how many\n> items are present, so let's introduce a function that lets users inquire\n> how many objects are in the mailmap and use it where we want this\n> information.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> mailmap.c | 5 +++++\n> mailmap.h | 1 +\n> pretty.c | 2 +-\n> 3 files changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/mailmap.c b/mailmap.c\n> index 962fd86d6d..c9a538f4e2 100644\n> --- a/mailmap.c\n> +++ b/mailmap.c\n> @@ -361,3 +361,8 @@ int map_user(struct string_list *map,\n> \tdebug_mm(\"map_user: --\\n\");\n> \treturn 0;\n> }\n> +\n> +int mailmap_entries(struct string_list *map)\n\nWe're returning ->nr here from string_list, whose \"nr\" is unsigned\nint. Wouldn't it make more sense to do the same here?...\n\n> +{\n> +\treturn map->nr;\n> +}\n> diff --git a/mailmap.h b/mailmap.h\n> index d0e65646cb..ff57b05a15 100644\n> --- a/mailmap.h\n> +++ b/mailmap.h\n> @@ -5,6 +5,7 @@ struct string_list;\n> \n> int read_mailmap(struct string_list *map, char **repo_abbrev);\n> void clear_mailmap(struct string_list *map);\n> +int mailmap_entries(struct string_list *map);\n> \n> int map_user(struct string_list *map,\n> \t\t\t const char **email, size_t *emaillen, const char **name, size_t *namelen);\n> diff --git a/pretty.c b/pretty.c\n> index 7a7708a0ea..43a0039870 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -681,7 +681,7 @@ static int mailmap_name(const char **email, size_t *email_len,\n> \t\tmail_map = xcalloc(1, sizeof(*mail_map));\n> \t\tread_mailmap(mail_map, NULL);\n> \t}\n> -\treturn mail_map->nr && map_user(mail_map, email, email_len, name, name_len);\n> +\treturn mailmap_entries(mail_map) && map_user(mail_map, email, email_len, name, name_len);\n\n...and isn't this introducing a bug where the number of entries is\nbeyond \"signed int\" but not \"unsigned int\" by overflowing?\n\nBikeshedding: For both hashmap and strmap we have a\n{hashmap,strmap}_get_size() function. I think a \"static inline unsigned\nint mailmap_get_size()\" here would be more consistent & obvious.\n\n> }\n> \n> static size_t format_person_part(struct strbuf *sb, char part,\n\n"},{"id":"413374","messageId":"87ft3gviir.fsf@evledraar.gmail.com","threadId":"54924","inReplyTo":"20210103211849.2691287-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 2/5] mailmap: switch to opaque struct","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-04T15:17:16Z","receivedAt":"2021-01-04T15:18:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jan 03 2021, brian m. carlson wrote:\n\n> -void clear_mailmap(struct string_list *map)\n> +void clear_mailmap(struct mailmap *mailmap)\n> {\n> +\tstruct string_list *map = mailmap->mailmap;\n> \tdebug_mm(\"mailmap: clearing %d entries...\\n\", map->nr);\n\nRe my comment on 1/5 wouldn't it make more sense to use the new API\nhere:\n\n - debug_mm(\"mailmap: clearing %d entries...\\n\", map->nr);\n + debug_mm(\"mailmap: clearing %u entries...\\n\", mailmap_entries(mailmap));\n\nI changed %d to %u here, maybe we'd also want to do that earlier while\nwe're at it (not code you're changing):\n\n - debug_mm(\"mailmap: removing entries for <%s>, with %d sub-entries\\n\",\n + debug_mm(\"mailmap: removing entries for <%s>, with %u sub-entries\\n\",\n"},{"id":"413375","messageId":"87czykvg19.fsf@evledraar.gmail.com","threadId":"54924","inReplyTo":"20210103211849.2691287-5-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 4/5] mailmap: use case-sensitive comparisons for local-parts and names","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-04T16:10:58Z","receivedAt":"2021-01-04T16:11:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jan 03 2021, brian m. carlson wrote:\n\n[For the purposes of this reply I'm also replying to your commit message\nin 3/5, which I think makes things less confusing than two E-Mails]\n\n> Currently, Git always looks up entries in the mailmap in a\n> case-insensitive way, both for names and addresses, which is, as\n> explained below, suboptimal.\n>\n> First, for email addresses, RFC 5321 is clear that only domains are case\n> insensitive; local-parts (the portion before the at sign) are not. It\n> states this:\n> \n> The local-part of a mailbox MUST BE treated as case sensitive.\n> Therefore, SMTP implementations MUST take care to preserve the case of\n> mailbox local-parts.\n\nIt seems better to quote the part of this where it goes on to say:\n\n However, exploiting the case sensitivity of mailbox local-parts\n impedes interoperability and is discouraged. Mailbox domains follow\n normal DNS rules and are hence not case sensitive.\n\nAnd also that RFC 5321 says earlier\n\n \"a host that expects to receive mail SHOULD avoid defining mailboxes\n where the Local-part requires (or uses) the Quoted-string form or\n where the Local-part is case- sensitive. For any purposes that\n require generating or comparing Local-parts (e.g., to specific\n mailbox names), all quoted forms MUST be treated as equivalent,\".\n\nSo if we're taking RFC 5321 MUST requirements as something we've got to\ndo we're still not following it. But as I'll go on to note I think\nrationale by RFC 5321 is perhaps not the best thing to do here...\n\n> There exist systems today where local-parts remain case sensitive (and\n> this author has one), and as such, it's incorrect for us to case fold\n> them in any way. Let's add a failing test that indicates this is a\n> problem, while still keeping the test for case-insensitive domains.\n\nI don't really care much about this feature of mailmap, but I'm\nstruggling a bit to understand this part.\n\nWe're not an SMTP server or SMTP server library, so we don't have to\nworry about mail mis-routing or whatever.\n\nWe just have to worry about cases where you're not all of these people\nin one project's commit metadata and/or .mailmap, and thus mailmap rules\nwould match too generously:\n\n \"brian m. carlson\" \n \"brian m. carlson\" \n \"BRIAN M. CARLSON\" \n \"BRIAN M. CARLSON\" \n\nIs that really plausible? In any case, neither of these two patches make\nreference to us already having changed this in the past in 1.6.2 and &\nthere being reports on the ML about the bug & us changing it back. See\nhttps://lore.kernel.org/git/f182fb1700e8dea15459fd02ced2a6e5797bec99.1238458535u.git.johannes.schindelin@gmx.de/\n\nMaybe we should still do this, but I think for a v3 it makes sense to\nsummarize that discussion etc.\n\n> Note that it's also incorrect for us to case-fold names because we don't\n> guarantee that we're using the locale of the author, and it's impossible\n> to case-fold names in a locale-insensitive way. Turkish and Azeri\n> contain both a dotted and dotless I, and the uppercase ASCII I folds not\n> to the lowercase ASCII I, but to a dotless version, and vice versa with\n> the lowercase I. There are many words in Turkish which differ only in\n> the dottedness of the I, so it is likely that there are also personal\n> names which differ in the same way.\n> \n> That would be a problem even if our implementation were perfect, which\n> it is not. We currently fold only ASCII characters, so this feature has\n> never worked correctly for the vast majority of the users on the planet,\n> regardless of the locale. For example, on Linux, even in a Spanish\n> locale, we don't handle \"Simón\" properly. Even if we did handle that,\n> we'd probably also want to implement Unicode normalization, which we\n> don't.\n\nAs one of those people, aren't you confusing two things here. I daresay\nthat almost everyone with a non-ASCII name doesn't have a non-ASCII\nE-Mail address, sure some do, but treating those as one and the same\ndoesn't seem to make sense.\n\nYes you can have non-ASCII on the LHS of @ in an E-Mail address, it just\nseems to me that's very rare.\n\nDo we have known cases where we're making use of this case-insensitive\nmatching of the *name* part? Showing some of those non-ASCII cases in\nthe tests in 3/5 would be nice.\n\n> IN general, case-folding text is extremely language- and locale-specific\n> and requires intimacy with the spelling and grammar of the language in\n> question and careful attention to the Unicode details in order to\n> produce a result that is meaningful to humans and conforms with\n> linguistic and societal norms.\n> \n> Because we do not have any of the required context with a plain personal\n> name, we cannot hope to possibly case-fold personal names correctly. We\n> should stop trying to do so and just treat them as a series of bytes, so\n> let's add a test that we don't case-fold personal names as well.\n\n[... end of commit 3/5 ...]\n\n> RFC 5321 is clear that the local-part of an email address (the part\n> before the at sign) is case sensitive, and this has been the case since\n> the original RFC 821. It directs us that \"the local-part MUST be\n> interpreted and assigned semantics only by the host specified in the\n> domain part of the address.\"\n>\n> Since we are not that party, it's not correct for us to compare them\n> case insensitively. However, we do still want to compare the domain\n> parts case insensitively, so let's add a helper that downcases the\n> domain and then compare byte by byte.\n>\n> Similarly, it's not possible for us to correctly case-fold text in a\n> locale-insensitive way, so our handling of personal names has also been\n> subject to bugs. Additionally, we've never handled non-ASCII characters\n> correctly, which means that our previous comparisons really only worked\n> well for a fraction of the people on the planet. Since our code wasn't\n> right and it's basically impossible to compare personal names without\n> regard to case, let's also switch our matching of names to be byte by\n> byte.\n\nI'm still undecided about whether we should be calling strcasecmp() to\nbegin with, but I don't really find this convincing. Just because we\nonly support brian@ and BRIAN@ being the same but not ævar@ and ÆVAR@ we\nshould break the existing behavior for everyone?\n\n> Signed-off-by: brian m. carlson \n> ---\n> mailmap.c | 27 ++++++++++++++++++++++++---\n> t/t4203-mailmap.sh | 4 ++--\n> 2 files changed, 26 insertions(+), 5 deletions(-)\n>\n> diff --git a/mailmap.c b/mailmap.c\n> index d3287b409a..5c52dbb7e0 100644\n> --- a/mailmap.c\n> +++ b/mailmap.c\n> @@ -64,7 +64,22 @@ static void free_mailmap_entry(void *p, const char *s)\n> */\n> static int namemap_cmp(const char *a, const char *b)\n> {\n> -\treturn strcasecmp(a, b);\n> +\treturn strcmp(a, b);\n> +}\n\nIt seems to me if we're not going to use strcasecmp() we can get rid of\nthis whole namemap_cmp() function. See the comment just above it &\nde2f95ebed2 (mailmap: work around implementations with pure inline\nstrcasecmp, 2013-09-12). I.e. we had a wrapper function here just\nbecause we were using strcasecmp().\n\n> +/*\n> + * Lowercases the domain (and only the domain) part of an email address. The\n> + * local-part, which is defined by RFC 5321 to be case sensitive, is not\n> + * affected.\n> + */\n> +static char *lowercase_email(char *s)\n> +{\n> +\tchar *end = strchrnul(s, '@');\n\nSpeaking of pedantic readings of RFC 5321, aren't we closing the door\nhere to proper DQUOTE handling? I.e. the local-part containing a @\nwithin quotes :)\n\n"},{"id":"413384","messageId":"abe4aab0-8c95-2ba4-fe21-81e020869418@web.de","threadId":"54924","inReplyTo":"20210103211849.2691287-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 1/5] mailmap: add a function to inspect the number of entries","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-01-04T17:04:33Z","receivedAt":"2021-01-04T17:06:58Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 03.01.21 um 22:18 schrieb brian m. carlson:\n> We're soon going to change the type of our mailmap into an opaque struct\n> so we can add features and improve performance. When we do so, it won't\n> be possible for users to inspect its internals to determine how many\n> items are present, so let's introduce a function that lets users inquire\n> how many objects are in the mailmap and use it where we want this\n> information.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> mailmap.c | 5 +++++\n> mailmap.h | 1 +\n> pretty.c | 2 +-\n> 3 files changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/mailmap.c b/mailmap.c\n> index 962fd86d6d..c9a538f4e2 100644\n> --- a/mailmap.c\n> +++ b/mailmap.c\n> @@ -361,3 +361,8 @@ int map_user(struct string_list *map,\n> \tdebug_mm(\"map_user: --\\n\");\n> \treturn 0;\n> }\n> +\n> +int mailmap_entries(struct string_list *map)\n> +{\n> +\treturn map->nr;\n> +}\n> diff --git a/mailmap.h b/mailmap.h\n> index d0e65646cb..ff57b05a15 100644\n> --- a/mailmap.h\n> +++ b/mailmap.h\n> @@ -5,6 +5,7 @@ struct string_list;\n>\n> int read_mailmap(struct string_list *map, char **repo_abbrev);\n> void clear_mailmap(struct string_list *map);\n> +int mailmap_entries(struct string_list *map);\n>\n> int map_user(struct string_list *map,\n> \t\t\t const char **email, size_t *emaillen, const char **name, size_t *namelen);\n> diff --git a/pretty.c b/pretty.c\n> index 7a7708a0ea..43a0039870 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -681,7 +681,7 @@ static int mailmap_name(const char **email, size_t *email_len,\n> \t\tmail_map = xcalloc(1, sizeof(*mail_map));\n> \t\tread_mailmap(mail_map, NULL);\n> \t}\n> -\treturn mail_map->nr && map_user(mail_map, email, email_len, name, name_len);\n> +\treturn mailmap_entries(mail_map) && map_user(mail_map, email, email_len, name, name_len);\n\nThis seems to be the only caller of the new function. It uses it to\navoid calling map_user() in case the mailmap is empty. Since you're\ndoing performance measurements: How bad would it be to simply remove\nthat check?\n\nRené\n"},{"id":"413456","messageId":"875z4bsbuz.fsf@evledraar.gmail.com","threadId":"54924","inReplyTo":"20210103211849.2691287-6-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 5/5] mailmap: support hashed entries in mailmaps","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-05T14:21:40Z","receivedAt":"2021-01-05T14:22:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jan 03 2021, brian m. carlson wrote:\n\nI think it makes sense to split up 1-4/5 here from 5/5 in this series\nsince they're really unrelated changes, although due to the changes in\n1-4 they'll conflict.\n\n> Many people, through the course of their lives, will change either a\n> name or an email address. For this reason, we have the mailmap, to map\n> from a user's former name or email address to their current, canonical\n> forms. Normally, this works well as it is.\n>\n> However, sometimes people change a name or an email address and wish to\n> wholly disassociate themselves from that former name or email address.\n> For example, a person may transition from one gender to another,\n> changing their name, or they may have changed their name to disassociate\n> themselves from an abusive family or partner. In such a case, using the\n> former name or address in any way may be undesirable and the person may\n> wish to replace it as completely as possible.\n\nThe cover letter noted \"As mentioned in the original thread, I think a\nhash rather than an encoding is the right choice here.\". Reading the v1\nI think you're referring to\nhttps://lore.kernel.org/git/X9wUGaR3IXcpV0nT@camp.crustytoothpaste.net/\n\nIn v1 I pointed out you needed to read some combination of the cover\nletter & the patch to see what this was intended for (see [1]). I think\nfor v3 the commit itself should summarize the trade-offs & design\nchoices.\n\n> For projects which wish to support this, introduce hashed forms into the\n> mailmap. These forms, which start with \"@sha256:\" followed by a SHA-256\n> hash of the entry, can be used in place of the form used in the commit\n> field. This form is intentionally designed to be unlikely to conflict\n> with legitimate use cases. For example, this is not a valid email\n> address according to RFC 5322. In the unlikely event that a user has\n> put such a form into the actual commit as their name, we will accept it.\n\nWe'll emit the commit author information as-is in that case under \"git\nshow\", or run the mapping and map it via mailmap? Anyway, it seems\nthere's a test for this. Probably better to just point to it.\n\n> While the form of the data is designed to accept multiple hash\n> algorithms, we intentionally do not support SHA-1. There is little\n> reason to support such a weak algorithm in new use cases and no\n> backwards compatibility to consider. Moreover, SHA-256 is faster than\n> the SHA1DC implementation we use, so this not only improves performance,\n> but simplifies the current implementation somewhat as well.\n\nI agree with most of this aside from the \"weak algorithm\" part. That\nseems like an irrelevant aside for this specific use of a hashing\nalgorithm, no? We could even use MD5 here, so SHA256-only is just\nsetting is up for not needing to deal with SHA1 forever in this one\nplace in some SHA256 future repo.\n\n> Note that it is, of course, possible to perform a lookup on all commit\n> objects to determine the actual entry which matches the hashed form of\n> the data. However, this is an improvement over the status quo.\n>\n> The performance of this patch with no hashed entries is very similar to\n> the performance without this patch. Considering a git log command to\n> look up author and committer information on 981,680 commits in the Linux\n> kernel history, either with an unhashed mailmap or a mailmap with all\n> old values hashed:\n>\n> Shortest Longest Average Change\n> Git 2.30 7.876 8.297 8.143\n> This patch, unhashed 7.923 8.484 8.237 + 1.15%\n> This patch, hashed 14.510 14.783 14.672 +80.17%\n> This patch, hashed, unoptimized 15.425 16.318 15.901 +95.27%\n>\n> Thus, the average performance after this patch is within normal\n> variation of the pre-patch performance. It's unlikely that users will\n> notice the difference in practice, even on much larger\n> repositories, unless they're using the new feature.\n\nAm I reading this right that if there's a single hashed entry in\n.mailmap anything using %aE or %aN is around 2x as slow?\n\nYour v1 mentioned that a project might \"insert entries for many\ncontributors in order to make discovery of \"interesting\" entries\nsignificantly less convenient.\" which is gone in the v2 patch. As noted\nin [1] I don't see how it helps the obscurity much, but if that's still\nthe intended use we'd expect to get more slowdowns in the wild if users\nintend to convert their whole mailmap to this form if they want a single\nentry to use the form.\n\nAnyway, as you might have guessed I'm still not a fan of this direction.\n\nBut most of it is because I honestly don't get why this specific\napproach is required to achieve the stated aims, there's a few of them,\nso here's an attempt to break them down:\n\n1. User changed their name and doesn't want themselves or others to see\n their old name\n\nFor the case where Joe Developer is now known as Jane Doe in most cases\nyou don't need to put the old name at all into the .mailmap. E.g. for\ngit.git this patch to our .mailmap produces the same output for `log\n--all --pretty=\"%h %an%ae%aN%aE\"`:\n \n brian m. carlson \n -brian m. carlson \n -brian m. carlson \n + \n + \n\nSo the new->name/email mapping (as opposed to new->email) is really only\nneeded for some really obscure cases where two people shared an E-Mail\nor something.\n\nSo we're talking about hiding the old E-Mail, presumably because it was\njoe@ intsead of jane@, so in that case we could just support URI\nencoding:\n\n Jane Doe \n <%6A%6F%65@%64%65%76%65%6C%6F%70%65%72.%63%6F%6D>\n\nMade via:\n\n $ perl -MURI::Escape=uri_escape -wE 'say uri_escape q[joe@developer.com], \"^@.\"'\n %6A%6F%65@%64%65%76%65%6C%6F%70%65%72.%63%6F%6D\n\nWhich also has the nice attribute that people can make it obvious what\npart they want to hide, since this is really a feature to enable social\npoliteness & consideration:\n\n Jane Doe \n # I don't want to be known by my old name, thanks\n <%6A%6F%65@developer.com>\n\n2. Hiding from your enemies\n\nFor the other use-case of \"abusive family or partner\" I had the comment\nin v1 of \"but not so much that you'd still take the risk of submitting a\npatch to .mailmap?\".\n\nNow that's obviously phrased in an off-the-cuff manner, but I'm\nserious.\n\nI think it is important that the non-security of this feature obviously\nlooks like some trivial encoding, because that's what it is. People get\nlulled into a false sense of security with these things all the time\n(e.g. thinking their \"Authorization\" HTTP header is safe to post on a\npublic pastebin). So we should as much as possible make this look like\nthe non-security it is.\n\n3. Enabling people not to treat .mailmap as binary or a multi-encoding\nfile.\n\nI mentioned this in my [1]. Your implementation doesn't do this, but\ne.g. it would be very nice for a project that switched from latin-1 to\nutf-8 to be able to do, in some cases:\n\n # Made with: perl -MURI::Escape=uri_escape -wE 'say uri_escape \"@ARGV\", \"^a-z@. \"' $(echo Ævar Arnfjörð Bjarmason | iconv -f utf-8 -t iso-8859-1)\n #\n\n Ævar Arnfjörð Bjarmason %C6var %41rnfj%F6r%F0 %42jarmason \n\nOr some combination thereof, so e.g. previously Big5/latin1 who migrated\nto UTF-8 don't need to have non-valid UTF-8 in .mailmap\n\n4. Spam\n\nYou mentioned this in your [2] (but not as a use-case in the v2\nre-rolled commit message):\n\n And we know that spammers and recruiters (which, in this case, are\n also spammers) do indeed scrape repositories via the repository web\n interfaces.\n\nSurely these people are most interested in the current E-Mail addresses,\nwhich if they're scraping the common web interfaces (e.g. Github,\nGitLab) are easily accessible there. It doesn't seem very plausible that\nsomeone would care enough to scrape .mailmap for old addresses but not\njust update their scraper to clone & run \"git log\" for the purposes of\ne.g. their recruitment E-Mails.\n\n5. Interaction with other systems\n\nSomething I mentioned in the last 3 paragraphs of my [1]. I think you're\nonly considering the cases where git itself does the mailmap\ntranslation, but we have 3rd party systems that make use of the format\nin good ways (also doing the Joe->Jane mapping). Making it a hassle for\nthose systems makes it more likely that Jane doesn't get the mapping she\nwants.\n\n\n1. https://lore.kernel.org/git/87eejswql6.fsf@evledraar.gmail.com/\n2. https://lore.kernel.org/git/X9wUGaR3IXcpV0nT@camp.crustytoothpaste.net/\n"},{"id":"413508","messageId":"xmqqft3fywsd.fsf@gitster.c.googlers.com","threadId":"54924","inReplyTo":"20210103211849.2691287-6-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 5/5] mailmap: support hashed entries in mailmaps","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-05T20:05:22Z","receivedAt":"2021-01-05T20:06:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> For example, a person may transition from one gender to another,\n> changing their name, or they may have changed their name to disassociate\n> themselves from an abusive family or partner. In such a case, using the\n> former name or address in any way may be undesirable and the person may\n> wish to replace it as completely as possible.\n\nI am not sure if we want to even mention the \"for example\" here.\n\nThese are certainly all legitimate reasons to want this feature, but\nafter reading the \"for example\", lack of a corresponding negative\nstatement (e.g. sometimes people also change their name or address\nto hide their bad behaviour in the past that is associated with\nthese names) needlessly stood out and made me wonder if we need to\nsomehow defend the feature with \"...but we do not mean to abet\npeople in hiding their past bad behaviour with this mechanism\". I'd\nprefer us not forced to defend the mechanism if we did not have to.\n\n> Note that it is, of course, possible to perform a lookup on all commit\n> objects to determine the actual entry which matches the hashed form of\n> the data. However, this is an improvement over the status quo.\n\nThere were suggestions to use reversible encoding, IIRC, just for\nobscurity. I do not have a strong preference either way myself, but\nbecause such an approach would give the same improvement over the\nstatus quo, would be simpler, more performant and most importantly,\nit makes it clear that this is not serious security but casual\nobscurity, I'd want to be convinced why we want to use a hash here\na bit more strongly.\n\n> +In addition to specifying a former name or email literally, it is also possible\n> +to specify it in a hashed form, which consists of the string `@sha256:`,\n> +followed by an all-lowercase SHA-256 hash of the entry in hexadecimal. For\n> +example, to take the example above, instead of specifying the replacement for\n> +\"Some Dude\" as such, you could specify one of these lines:\n> ...\n\n> +SHA-1 is not accepted as a hash algorithm in mailmaps.\n\nIs this needed to be said? After all, we won't take @md5: or\n@blake2: or anything other than @sha256: in this version (and\nprobably any forseeable versions). Unless we offer a way to plug-in\nalgos of projects' choice, that is, and at that point, \"SHA-1 is not\naccepted\" is a statement too strong for us to make.\n"},{"id":"413517","messageId":"X/UDT/oLKNQmxBXR@camp.crustytoothpaste.net","threadId":"54924","inReplyTo":"875z4bsbuz.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 5/5] mailmap: support hashed entries in mailmaps","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-06T00:24:47Z","receivedAt":"2021-01-06T00:26:06Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-01-05 at 14:21:40, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Jan 03 2021, brian m. carlson wrote:\n> \n> I think it makes sense to split up 1-4/5 here from 5/5 in this series\n> since they're really unrelated changes, although due to the changes in\n> 1-4 they'll conflict.\n\nOkay, I'll drop them.\n\n> In v1 I pointed out you needed to read some combination of the cover\n> letter & the patch to see what this was intended for (see [1]). I think\n> for v3 the commit itself should summarize the trade-offs & design\n> choices.\n\nI can do that. It's a very long commit message anyway, but if you think\nit would be better in the commit message, I can add it.\n\n> > For projects which wish to support this, introduce hashed forms into the\n> > mailmap. These forms, which start with \"@sha256:\" followed by a SHA-256\n> > hash of the entry, can be used in place of the form used in the commit\n> > field. This form is intentionally designed to be unlikely to conflict\n> > with legitimate use cases. For example, this is not a valid email\n> > address according to RFC 5322. In the unlikely event that a user has\n> > put such a form into the actual commit as their name, we will accept it.\n> \n> We'll emit the commit author information as-is in that case under \"git\n> show\", or run the mapping and map it via mailmap? Anyway, it seems\n> there's a test for this. Probably better to just point to it.\n\nIt will be handled correctly via the mailmap code, in which case we'll\nmake a no-op transformation. If the user is not using the mailmap, then\nit will be handled trivially.\n\n> > While the form of the data is designed to accept multiple hash\n> > algorithms, we intentionally do not support SHA-1. There is little\n> > reason to support such a weak algorithm in new use cases and no\n> > backwards compatibility to consider. Moreover, SHA-256 is faster than\n> > the SHA1DC implementation we use, so this not only improves performance,\n> > but simplifies the current implementation somewhat as well.\n> \n> I agree with most of this aside from the \"weak algorithm\" part. That\n> seems like an irrelevant aside for this specific use of a hashing\n> algorithm, no? We could even use MD5 here, so SHA256-only is just\n> setting is up for not needing to deal with SHA1 forever in this one\n> place in some SHA256 future repo.\n\nOne should avoid the use of weak algorithms when possible even if they\nare not being used in a way that makes them weak because it incentivizes\nothers to use them, often in a way that is insecure. I had a\nconversation with a junior candidate during an interview who said they\nused SHA-1 in a particular case \"because Git uses it.\" That's why I\nmentioned it.\n\n> > Note that it is, of course, possible to perform a lookup on all commit\n> > objects to determine the actual entry which matches the hashed form of\n> > the data. However, this is an improvement over the status quo.\n> >\n> > The performance of this patch with no hashed entries is very similar to\n> > the performance without this patch. Considering a git log command to\n> > look up author and committer information on 981,680 commits in the Linux\n> > kernel history, either with an unhashed mailmap or a mailmap with all\n> > old values hashed:\n> >\n> > Shortest Longest Average Change\n> > Git 2.30 7.876 8.297 8.143\n> > This patch, unhashed 7.923 8.484 8.237 + 1.15%\n> > This patch, hashed 14.510 14.783 14.672 +80.17%\n> > This patch, hashed, unoptimized 15.425 16.318 15.901 +95.27%\n> >\n> > Thus, the average performance after this patch is within normal\n> > variation of the pre-patch performance. It's unlikely that users will\n> > notice the difference in practice, even on much larger\n> > repositories, unless they're using the new feature.\n> \n> Am I reading this right that if there's a single hashed entry in\n> .mailmap anything using %aE or %aN is around 2x as slow?\n\nNo, that's not the case. As soon as we see every hashed entry, we will\nstop hashing new entries. Linux is not necessarily the best case for\nthis because it has a long history with many one-off contributors long\nago in the history.\n\nI'll explain that further in the commit message and add some more\nmetrics.\n\n> Your v1 mentioned that a project might \"insert entries for many\n> contributors in order to make discovery of \"interesting\" entries\n> significantly less convenient.\" which is gone in the v2 patch. As noted\n> in [1] I don't see how it helps the obscurity much, but if that's still\n> the intended use we'd expect to get more slowdowns in the wild if users\n> intend to convert their whole mailmap to this form if they want a single\n> entry to use the form.\n\nPeff objected to that text, so I removed it.\n\nAs mentioned above, it depends on who you put in the mailmap. If\nthey're the most recent 50 contributors, it'll probably be pretty cheap.\nIf you put the oldest contributors in there and they've not sent any\nrecent commits, it will be more expensive.\n\n> Anyway, as you might have guessed I'm still not a fan of this direction.\n\nI've got that impression pretty strongly.\n\nI do want to point out that generally I'm pretty willing to change\napproaches and do things differently. I've completely redone a decent\nnumber of patches in the past in response to feedback on the list.\n\nI'm not changing the approach here because, as mentioned below, I don't\nthink that just encoding meets the use cases I'm targeting here. So I\nhave heard your suggestions and to be clear, I do value your input on\nthis (and on other topics), it's just that I disagree that such a change\nis one I should make.\n\n> So the new->name/email mapping (as opposed to new->email) is really only\n> needed for some really obscure cases where two people shared an E-Mail\n> or something.\n\nThat's unlikely, but it does happen. That's why we have it.\n\n> So we're talking about hiding the old E-Mail, presumably because it was\n> joe@ intsead of jane@, so in that case we could just support URI\n> encoding:\n> \n> Jane Doe \n> <%6A%6F%65@%64%65%76%65%6C%6F%70%65%72.%63%6F%6D>\n> \n> Made via:\n> \n> $ perl -MURI::Escape=uri_escape -wE 'say uri_escape q[joe@developer.com], \"^@.\"'\n> %6A%6F%65@%64%65%76%65%6C%6F%70%65%72.%63%6F%6D\n> \n> Which also has the nice attribute that people can make it obvious what\n> part they want to hide, since this is really a feature to enable social\n> politeness & consideration:\n> \n> Jane Doe \n> # I don't want to be known by my old name, thanks\n> <%6A%6F%65@developer.com>\n\nI don't think this feature is going to get used if we just encode names\nor email addresses. In the United States, when someone transitions,\nthey get a court order to change their name. I don't think a lot of\ncorporate environments are going to want to just encode an old name or\nemail address in a trivially invertible way given that. This is\ntypically a topic handled with some sensitivity in most companies.\n\nI will tell you that I would not just use an encoded version if I were\nchanging my name for any of the reasons I've mentioned. That wouldn't\ncut it for me, and I wouldn't use such a feature. The feature I'm\nimplementing is a feature I've talked with trans folks about, and that's\nwhy I'm implementing this as it is. The response I got was essentially,\n\"It's not everything I want, but it's an improvement.\"\n\nIf the decision is that we want to go with encoding instead of hashing,\nthen I'll drop this patch. I'm not going to put my name or sign-off on\nthat because I don't think it meets the need I'm addressing here.\n\nThe entire problem, of course, is that we bake a human's personal name\nand email address immutably into a Merkle tree. We know full well that\npeople do change their names and email addresses all the time (e.g.,\nmarriage, job changes), and yet we have this design. In retrospect, we\nshould have done something different, but hindsight is 20/20 and I'm\njust trying to do the best we can with what we've got.\n\n> 2. Hiding from your enemies\n> \n> For the other use-case of \"abusive family or partner\" I had the comment\n> in v1 of \"but not so much that you'd still take the risk of submitting a\n> patch to .mailmap?\".\n\nNo, my use case isn't \"hiding from an abusive family or partner\". It's\n\"I'm finally free of that **** and I never want to hear their name\nagain.\" (I've known people in this situation.) Also, the similar use\ncase of, \"my family member, with whom I share an uncommon name, murdered\nsomeone, which I obviously found abhorrent, and I would like to not be\nassociated with them when my name is Googled.\" And yes, I knew an\nacquaintance many years ago whose family member murdered someone.\n\nIn other words, the person changed their name to disassociate\nthemselves, not to hide from their abuser.\n\n> 4. Spam\n> \n> You mentioned this in your [2] (but not as a use-case in the v2\n> re-rolled commit message):\n> \n> And we know that spammers and recruiters (which, in this case, are\n> also spammers) do indeed scrape repositories via the repository web\n> interfaces.\n> \n> Surely these people are most interested in the current E-Mail addresses,\n> which if they're scraping the common web interfaces (e.g. Github,\n> GitLab) are easily accessible there. It doesn't seem very plausible that\n> someone would care enough to scrape .mailmap for old addresses but not\n> just update their scraper to clone & run \"git log\" for the purposes of\n> e.g. their recruitment E-Mails.\n\nUnless the user is using the GitHub-provided noreply address or a\nsimilar address, which is common. This allows people to map all of\ntheir old addresses to such an address, which, judging from\nStackOverflow, is a thing people want to do.\n\nI can tell you from dealing with abuse that raising the bar even the\ntiniest bit is very significant to stopping it. Most recruiters are not\ndevelopers and they and spammers don't have Git installed. They're\ngoing to rely on Googling or other public search functionality, and this\nmakes that harder.\n\nGreylisting is exactly raising the bar the tiniest amount and it's\nextraordinarily effective.\n\n> 5. Interaction with other systems\n> \n> Something I mentioned in the last 3 paragraphs of my [1]. I think you're\n> only considering the cases where git itself does the mailmap\n> translation, but we have 3rd party systems that make use of the format\n> in good ways (also doing the Joe->Jane mapping). Making it a hassle for\n> those systems makes it more likely that Jane doesn't get the mapping she\n> wants.\n\nThis is an argument for never changing the format. Sometimes things\nchange, and I don't want to avoid making a change because other\nimplementations haven't implemented it yet. Under that approach, we'd\nnever have the SHA-256 work.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"413518","messageId":"X/UENS0EXyFjjjHA@camp.crustytoothpaste.net","threadId":"54924","inReplyTo":"xmqqft3fywsd.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 5/5] mailmap: support hashed entries in mailmaps","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-06T00:28:37Z","receivedAt":"2021-01-06T00:29:24Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-01-05 at 20:05:22, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> \n> > For example, a person may transition from one gender to another,\n> > changing their name, or they may have changed their name to disassociate\n> > themselves from an abusive family or partner. In such a case, using the\n> > former name or address in any way may be undesirable and the person may\n> > wish to replace it as completely as possible.\n> \n> I am not sure if we want to even mention the \"for example\" here.\n> \n> These are certainly all legitimate reasons to want this feature, but\n> after reading the \"for example\", lack of a corresponding negative\n> statement (e.g. sometimes people also change their name or address\n> to hide their bad behaviour in the past that is associated with\n> these names) needlessly stood out and made me wonder if we need to\n> somehow defend the feature with \"...but we do not mean to abet\n> people in hiding their past bad behaviour with this mechanism\". I'd\n> prefer us not forced to defend the mechanism if we did not have to.\n\nI added it because I imagine the use cases for this feature aren't\nimmediately obvious to a lot of people and the general rule is that\ncommit messages explain why we would implement such a feature. If you'd\nprefer I drop it and leave it up to the imagination (or to the list\narchives), I can do that.\n\n> > +SHA-1 is not accepted as a hash algorithm in mailmaps.\n> \n> Is this needed to be said? After all, we won't take @md5: or\n> @blake2: or anything other than @sha256: in this version (and\n> probably any forseeable versions). Unless we offer a way to plug-in\n> algos of projects' choice, that is, and at that point, \"SHA-1 is not\n> accepted\" is a statement too strong for us to make.\n\nI'll drop that line.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"413519","messageId":"xmqq8s96yjs7.fsf@gitster.c.googlers.com","threadId":"54924","inReplyTo":"87czykvg19.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 4/5] mailmap: use case-sensitive comparisons for local-parts and names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-06T00:46:16Z","receivedAt":"2021-01-06T00:47:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Sun, Jan 03 2021, brian m. carlson wrote:\n>\n> We just have to worry about cases where you're not all of these people\n> in one project's commit metadata and/or .mailmap, and thus mailmap rules\n> would match too generously:\n>\n> \"brian m. carlson\" \n> \"brian m. carlson\" \n> \"BRIAN M. CARLSON\" \n> \"BRIAN M. CARLSON\" \n>\n> Is that really plausible? In any case, neither of these two patches make\n> reference to us already having changed this in the past in 1.6.2 and &\n> there being reports on the ML about the bug & us changing it back. See\n> https://lore.kernel.org/git/f182fb1700e8dea15459fd02ced2a6e5797bec99.1238458535u.git.johannes.schindelin@gmx.de/\n>\n> Maybe we should still do this, but I think for a v3 it makes sense to\n> summarize that discussion etc.\n\nAfter reading the old discussion again, I am not sure if this is\nworth doing. To many people, it is a promise we've made and kept\nthat we treat addresses including the local part case-insensitively\nwhen summarising commits by ident strings.\n\nI'd really wish that this series were structured to have 5/5 early\nand 3&4/5 squashed into a single final patch.\n"},{"id":"413521","messageId":"xmqqzh1mx291.fsf@gitster.c.googlers.com","threadId":"54924","inReplyTo":"X/UENS0EXyFjjjHA@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 5/5] mailmap: support hashed entries in mailmaps","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-06T01:50:18Z","receivedAt":"2021-01-06T01:51:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> I added it because I imagine the use cases for this feature aren't\n> immediately obvious to a lot of people and the general rule is that\n> commit messages explain why we would implement such a feature.\n\nYeah, I understand that.\n\n>> > +SHA-1 is not accepted as a hash algorithm in mailmaps.\n>> ...\n> I'll drop that line.\n\nThanks.\n"},{"id":"413983","messageId":"87sg7861e5.fsf@evledraar.gmail.com","threadId":"54924","inReplyTo":"X/UDT/oLKNQmxBXR@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 5/5] mailmap: support hashed entries in mailmaps","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-10T19:24:34Z","receivedAt":"2021-01-10T19:25:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jan 06 2021, brian m. carlson wrote:\n\n> On 2021-01-05 at 14:21:40, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Sun, Jan 03 2021, brian m. carlson wrote:\n>> \n>> I think it makes sense to split up 1-4/5 here from 5/5 in this series\n>> since they're really unrelated changes, although due to the changes in\n>> 1-4 they'll conflict.\n>\n> Okay, I'll drop them.\n\nNot replying to most of this E-Mail because I think there's nothing left\nto add / you clarified things for me in those cases / we respectfully\ndisagree / any outstanding points we can pick up in your re-roll /\nwhatever :)\n\n>> So we're talking about hiding the old E-Mail, presumably because it was\n>> joe@ intsead of jane@, so in that case we could just support URI\n>> encoding:\n>> \n>> Jane Doe \n>> <%6A%6F%65@%64%65%76%65%6C%6F%70%65%72.%63%6F%6D>\n>> \n>> Made via:\n>> \n>> $ perl -MURI::Escape=uri_escape -wE 'say uri_escape q[joe@developer.com], \"^@.\"'\n>> %6A%6F%65@%64%65%76%65%6C%6F%70%65%72.%63%6F%6D\n>> \n>> Which also has the nice attribute that people can make it obvious what\n>> part they want to hide, since this is really a feature to enable social\n>> politeness & consideration:\n>> \n>> Jane Doe \n>> # I don't want to be known by my old name, thanks\n>> <%6A%6F%65@developer.com>\n>\n> I don't think this feature is going to get used if we just encode names\n> or email addresses. In the United States, when someone transitions,\n> they get a court order to change their name. I don't think a lot of\n> corporate environments are going to want to just encode an old name or\n> email address in a trivially invertible way given that. This is\n> typically a topic handled with some sensitivity in most companies.\n>\n> I will tell you that I would not just use an encoded version if I were\n> changing my name for any of the reasons I've mentioned. That wouldn't\n> cut it for me, and I wouldn't use such a feature. The feature I'm\n> implementing is a feature I've talked with trans folks about, and that's\n> why I'm implementing this as it is. The response I got was essentially,\n> \"It's not everything I want, but it's an improvement.\"\n>\n> If the decision is that we want to go with encoding instead of hashing,\n> then I'll drop this patch. I'm not going to put my name or sign-off on\n> that because I don't think it meets the need I'm addressing here.\n>\n> The entire problem, of course, is that we bake a human's personal name\n> and email address immutably into a Merkle tree. We know full well that\n> people do change their names and email addresses all the time (e.g.,\n> marriage, job changes), and yet we have this design. In retrospect, we\n> should have done something different, but hindsight is 20/20 and I'm\n> just trying to do the best we can with what we've got.\n\nDoesn't the difference in some sense boil down to either an implicit\npromise or an implicit assumption that the hashed version is forever\ngoing to be protected by some security-through-obscurity/inconvenience\nwhen it comes to git.git & its default tooling?\n\nAnd would those users be as comfortable with the difference between\nencoded v.s. hashed if e.g. \"git check-mailmap\" learned to read the\n.mailmap and search-replace all the hashed versions with their\nmaterialized values, or if popular tools like Emacs learned to via a Git\n.mailmap in a \"need translation\" similar to *.gpg and *.gz. How about if\npopular web views of Git served up that materialized \"check-mailmap\"\noutput by default?\n\nNone of which I think is implausible that we'll get as follow-up\npatches, I might even submit some at some point, not out of some spite.\nJust because I don't want to maintain out-of-tree code for an\nout-of-tree program that understands a Git .mailmap today, but where I'd\nneed to search-replace the hashed versions.\n\nDitto it being very likely that popular editors or web viewers will gain\nsupport for this, just because it's tedious to manually hash &\ncopy/paste & validate values.\n\nIn looking at some of the fsck code recently & having some\nyet-unsubmitted patches I thought of trying to compine it with\nmailmap. I.e. it seems like a natural feature for fsck to gain to warn\nyou about unused mailmap entries, just like it can warn about\nunreachable/dangling objects. After all these are really just sort-of\npointers into our Merkle tree. Spewing out all the mappings seems like\nan obvious addition to that, e.g. in spewing out an\n\"optimized/non-redundant\" (plain or hashed) mailmap to re-commit.\n\nThat's the main reason I'm uncomfortable with this approach, because it\nseems to me to implicitly rely on things that are tedious now, but which\nthe march of history all but inevitably should make trivial if we were\nto integrate it. Unless we're *also* promising to forever intentionally\n(and artificially) keep it inconvenient.\n\nE.g. the example of how long it takes to clone & extract this info from\nchromium.git in the v1 thread.\n\nIt seems like a fair assumption that we'll have some future version of\ngit where you can ask a remote server about that sort of thing in\nmilliseconds.\n\nNot because of this hashed .mailmap thing in particular, just as an\nemergent effect that it's happy to serve up things it knows about the\nDAG from having walked & cached it in general. E.g. info from the\ncommit-graph, what hash is contained in what ref, or how one value (such\nas a .mailmap entry) maps to another etc.\n"},{"id":"413990","messageId":"X/txB8b3/qqbwbmC@camp.crustytoothpaste.net","threadId":"54924","inReplyTo":"87sg7861e5.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 5/5] mailmap: support hashed entries in mailmaps","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-01-10T21:26:31Z","receivedAt":"2021-01-10T21:28:04Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-01-10 at 19:24:34, Ævar Arnfjörð Bjarmason wrote:\n> Doesn't the difference in some sense boil down to either an implicit\n> promise or an implicit assumption that the hashed version is forever\n> going to be protected by some security-through-obscurity/inconvenience\n> when it comes to git.git & its default tooling?\n> \n> And would those users be as comfortable with the difference between\n> encoded v.s. hashed if e.g. \"git check-mailmap\" learned to read the\n> .mailmap and search-replace all the hashed versions with their\n> materialized values, or if popular tools like Emacs learned to via a Git\n> .mailmap in a \"need translation\" similar to *.gpg and *.gz. How about if\n> popular web views of Git served up that materialized \"check-mailmap\"\n> output by default?\n> \n> None of which I think is implausible that we'll get as follow-up\n> patches, I might even submit some at some point, not out of some spite.\n> Just because I don't want to maintain out-of-tree code for an\n> out-of-tree program that understands a Git .mailmap today, but where I'd\n> need to search-replace the hashed versions.\n\nYes, I think we do rely on this being inconvenient. If you plan to\nsubmit such a patch, I'm going to let this series drop.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"414152","messageId":"87h7nm5jtb.fsf@evledraar.gmail.com","threadId":"54924","inReplyTo":"xmqq8s96yjs7.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 4/5] mailmap: use case-sensitive comparisons for local-parts and names","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-12T14:08:48Z","receivedAt":"2021-01-12T14:09:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jan 06 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> On Sun, Jan 03 2021, brian m. carlson wrote:\n>>\n>> We just have to worry about cases where you're not all of these people\n>> in one project's commit metadata and/or .mailmap, and thus mailmap rules\n>> would match too generously:\n>>\n>> \"brian m. carlson\" \n>> \"brian m. carlson\" \n>> \"BRIAN M. CARLSON\" \n>> \"BRIAN M. CARLSON\" \n>>\n>> Is that really plausible? In any case, neither of these two patches make\n>> reference to us already having changed this in the past in 1.6.2 and &\n>> there being reports on the ML about the bug & us changing it back. See\n>> https://lore.kernel.org/git/f182fb1700e8dea15459fd02ced2a6e5797bec99.1238458535u.git.johannes.schindelin@gmx.de/\n>>\n>> Maybe we should still do this, but I think for a v3 it makes sense to\n>> summarize that discussion etc.\n>\n> After reading the old discussion again, I am not sure if this is\n> worth doing. To many people, it is a promise we've made and kept\n> that we treat addresses including the local part case-insensitively\n> when summarising commits by ident strings.\n>\n> I'd really wish that this series were structured to have 5/5 early\n> and 3&4/5 squashed into a single final patch.\n\nSomething that I only realized after I sent\n<87czykvg19.fsf@evledraar.gmail.com>: Any problems .mailmap has with\nTurkish \"dotless I\" we have already with \"git log --author= -i\".\n\nNot to say that there isn't some problem to solve here, just that if we\ndo it's a more general issue than mailmap.\n\nAs can be inferred from my upthread reply I thought that was ASCII-only,\nbut it turns out we do set LC_CTYPE based on the user's locale, and it\nalso applies for English-speakers. E.g. in LANG=en_US.UTF-8\n\"--author=ævar -i\" will work.\n\nOf course that doesn't address the point of whether we should be\nDWYM-ing E-Mail addresses, just the sub-claim that one reason we\nshouldn't is because of impossible-to-solve Unicode edge cases.\n"}]}