v2, 5 partsmailmap: add a function to inspect the number of entries

Many people, through the course of their lives, will change either a name or an email address. For this reason, we have the mailmap, to map from a user's former name or email address to their current, canonical forms. Normally, this works well as it is.

However, sometimes people change a name (or an email) and want to completely cease use of the former name or email. This could be because a transgender person has transitioned, because a person has left an abusive partner or broken ties with an abusive family member, or for any other number of good and valuable reasons. In these cases, placing the former name in the .mailmap may be undesirable.

For those situations, let's introduce a hashed mailmap, where the user's former name or email address can be in the form @sha256:<hash>. This obscures the former name or email.

In the course of experimenting with some solutions for v2, I noticed that our mailmap support has a bunch of problems with case sensitivity. Notably, it treats local-parts of email addresses in a case-insensitive way, when the RFC specifically says that they are case sensitive, and we also treat names case insensitively, but only for ASCII characters. Both of those have been fixed here, and the commit messages explain in lurid detail why, while incompatible, this is the correct behavior.

I've also added some performance numbers and explained some alternate solutions in the commit message for the final patch. That's in addition to the performance improvements I've done so that the feature is both cheaper for users and nearly invisible for non-users. That isn't quite the same as adding a perf test, which I haven't done, but I think this explains the situation quite well. If folks are still dying for a perf test, I can add one in v3.

I will point out that fully hashing a mailmap isn't necessarily cheap, but how expensive it is depends on the weighting of current and former members of the project. As mentioned in the original thread, I think a hash rather than an encoding is the right choice here. It is likely that in a few iterations of hardware, all users will have accelerated SHA-256 and the cost will end up being a handful of cycles per name overall.

Changes from v1:
* Fix case-sensitivity problems in the mailmap.
* Add documentation.
* Add explanation of how to compute the value.
* Add some optimizations to improve performance.
* Improve commit message to discuss performance numbers and explain
  rationale better.

brian m. carlson (5):
  mailmap: add a function to inspect the number of entries
  mailmap: switch to opaque struct
  t4203: add failing test for case-sensitive local-parts and names
  mailmap: use case-sensitive comparisons for local-parts and names
  mailmap: support hashed entries in mailmaps

 Documentation/mailmap.txt |  28 ++++++++
 builtin/blame.c           |   2 +-
 builtin/check-mailmap.c   |   4 +-
 builtin/commit.c          |   2 +-
 mailmap.c                 | 139 +++++++++++++++++++++++++++++++++-----
 mailmap.h                 |  15 ++--
 pretty.c                  |   4 +-
 pretty.h                  |   2 +-
 revision.c                |   2 +-
 revision.h                |   3 +-
 shortlog.h                |   3 +-
 t/t4203-mailmap.sh        |  64 +++++++++++++++++-
 12 files changed, 236 insertions(+), 32 deletions(-)

We're soon going to change the type of our mailmap into an opaque struct so we can add features and improve performance. When we do so, it won't be possible for users to inspect its internals to determine how many items are present, so let's introduce a function that lets users inquire how many objects are in the mailmap and use it where we want this information.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 mailmap.c | 5 +++++
 mailmap.h | 1 +
 pretty.c  | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

On Sun, Jan 03 2021, brian m. carlson wrote:

We're returning ->nr here from string_list, whose "nr" is unsigned int. Wouldn't it make more sense to do the same here?...

...and isn't this introducing a bug where the number of entries is beyond "signed int" but not "unsigned int" by overflowing?

Bikeshedding: For both hashmap and strmap we have a
{hashmap,strmap}_get_size() function. I think a "static inline unsigned
int mailmap_get_size()" here would be more consistent & obvious.

Am 03.01.21 um 22:18 schrieb brian m. carlson:

This seems to be the only caller of the new function. It uses it to avoid calling map_user() in case the mailmap is empty. Since you're doing performance measurements: How bad would it be to simply remove that check?

René

Currently a mailmap is a simple sorted string list. However, we'll soon want to add additional data to our mailmap, and in order to do so effectively, we'll need a struct of our own. To do that, let's create a struct mailmap and use that everywhere we're creating a mailmap.

Note that we no longer explicitly initialize our mailmaps, since read_mailmap will do that for us. We also don't need to explicitly set the flag to strdup strings, since we've passed that argument when we initialize the string list.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/blame.c         |  2 +-
 builtin/check-mailmap.c |  4 ++--
 builtin/commit.c        |  2 +-
 mailmap.c               | 20 +++++++++++++-------
 mailmap.h               | 14 +++++++++-----
 pretty.c                |  2 +-
 pretty.h                |  2 +-
 revision.c              |  2 +-
 revision.h              |  3 ++-
 shortlog.h              |  3 ++-
 10 files changed, 33 insertions(+), 21 deletions(-)

On Sun, Jan 03 2021, brian m. carlson wrote:

Re my comment on 1/5 wouldn't it make more sense to use the new API here:

    -       debug_mm("mailmap: clearing %d entries...\n", map->nr);
    +       debug_mm("mailmap: clearing %u entries...\n", mailmap_entries(mailmap));

I changed %d to %u here, maybe we'd also want to do that earlier while we're at it (not code you're changing):

    -       debug_mm("mailmap: removing entries for <%s>, with %d sub-entries\n",
    +       debug_mm("mailmap: removing entries for <%s>, with %u sub-entries\n",

Currently, Git always looks up entries in the mailmap in a case-insensitive way, both for names and addresses, which is, as explained below, suboptimal.

First, for email addresses, RFC 5321 is clear that only domains are case insensitive; local-parts (the portion before the at sign) are not. It states this:

  The local-part of a mailbox MUST BE treated as case sensitive.
  Therefore, SMTP implementations MUST take care to preserve the case of
  mailbox local-parts.

There exist systems today where local-parts remain case sensitive (and this author has one), and as such, it's incorrect for us to case fold them in any way. Let's add a failing test that indicates this is a problem, while still keeping the test for case-insensitive domains.

Note that it's also incorrect for us to case-fold names because we don't guarantee that we're using the locale of the author, and it's impossible to case-fold names in a locale-insensitive way. Turkish and Azeri contain both a dotted and dotless I, and the uppercase ASCII I folds not to the lowercase ASCII I, but to a dotless version, and vice versa with the lowercase I. There are many words in Turkish which differ only in the dottedness of the I, so it is likely that there are also personal names which differ in the same way.

That would be a problem even if our implementation were perfect, which it is not. We currently fold only ASCII characters, so this feature has never worked correctly for the vast majority of the users on the planet, regardless of the locale. For example, on Linux, even in a Spanish locale, we don't handle "Simón" properly. Even if we did handle that, we'd probably also want to implement Unicode normalization, which we don't.

In general, case-folding text is extremely language- and locale-specific and requires intimacy with the spelling and grammar of the language in question and careful attention to the Unicode details in order to produce a result that is meaningful to humans and conforms with linguistic and societal norms.

Because we do not have any of the required context with a plain personal name, we cannot hope to possibly case-fold personal names correctly. We should stop trying to do so and just treat them as a series of bytes, so let's add a test that we don't case-fold personal names as well.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t4203-mailmap.sh | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

RFC 5321 is clear that the local-part of an email address (the part before the at sign) is case sensitive, and this has been the case since the original RFC 821. It directs us that "the local-part MUST be interpreted and assigned semantics only by the host specified in the domain part of the address."

Since we are not that party, it's not correct for us to compare them case insensitively. However, we do still want to compare the domain parts case insensitively, so let's add a helper that downcases the domain and then compare byte by byte.

Similarly, it's not possible for us to correctly case-fold text in a locale-insensitive way, so our handling of personal names has also been subject to bugs. Additionally, we've never handled non-ASCII characters correctly, which means that our previous comparisons really only worked well for a fraction of the people on the planet. Since our code wasn't right and it's basically impossible to compare personal names without regard to case, let's also switch our matching of names to be byte by byte.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 mailmap.c          | 27 ++++++++++++++++++++++++---
 t/t4203-mailmap.sh |  4 ++--
 2 files changed, 26 insertions(+), 5 deletions(-)

On Sun, Jan 03 2021, brian m. carlson wrote:

[For the purposes of this reply I'm also replying to your commit message in 3/5, which I think makes things less confusing than two E-Mails]

It seems better to quote the part of this where it goes on to say:

    However, exploiting the case sensitivity of mailbox local-parts
    impedes interoperability and is discouraged.  Mailbox domains follow
    normal DNS rules and are hence not case sensitive.

And also that RFC 5321 says earlier

    "a host that expects to receive mail SHOULD avoid defining mailboxes
    where the Local-part requires (or uses) the Quoted-string form or
    where the Local-part is case- sensitive.  For any purposes that
    require generating or comparing Local-parts (e.g., to specific
    mailbox names), all quoted forms MUST be treated as equivalent,".

So if we're taking RFC 5321 MUST requirements as something we've got to do we're still not following it. But as I'll go on to note I think rationale by RFC 5321 is perhaps not the best thing to do here...

I don't really care much about this feature of mailmap, but I'm struggling a bit to understand this part.

We're not an SMTP server or SMTP server library, so we don't have to worry about mail mis-routing or whatever.

We just have to worry about cases where you're not all of these people in one project's commit metadata and/or .mailmap, and thus mailmap rules would match too generously:

    "brian m. carlson" <sandals@crustytoothpaste.net>
    "brian m. carlson" <SANDALS@crustytoothpaste.net>
    "BRIAN M. CARLSON" <sandals@crustytoothpaste.net>
    "BRIAN M. CARLSON" <SANDALS@crustytoothpaste.net>

Is that really plausible? In any case, neither of these two patches make reference to us already having changed this in the past in 1.6.2 and & there being reports on the ML about the bug & us changing it back. See https://lore.kernel.org/git/f182fb1700e8dea15459fd02ced2a6e5797bec99.1238458535u.git.johannes.schindelin@gmx.de/

Maybe we should still do this, but I think for a v3 it makes sense to summarize that discussion etc.

As one of those people, aren't you confusing two things here. I daresay that almost everyone with a non-ASCII name doesn't have a non-ASCII E-Mail address, sure some do, but treating those as one and the same doesn't seem to make sense.

Yes you can have non-ASCII on the LHS of @ in an E-Mail address, it just seems to me that's very rare.

Do we have known cases where we're making use of this case-insensitive matching of the *name* part? Showing some of those non-ASCII cases in the tests in 3/5 would be nice.

[... end <snip> of commit 3/5 ...]

I'm still undecided about whether we should be calling strcasecmp() to begin with, but I don't really find this convincing. Just because we only support brian@ and BRIAN@ being the same but not ævar@ and ÆVAR@ we should break the existing behavior for everyone?

It seems to me if we're not going to use strcasecmp() we can get rid of this whole namemap_cmp() function. See the comment just above it & de2f95ebed2 (mailmap: work around implementations with pure inline strcasecmp, 2013-09-12). I.e. we had a wrapper function here just because we were using strcasecmp().

Speaking of pedantic readings of RFC 5321, aren't we closing the door here to proper DQUOTE handling? I.e. the local-part containing a @ within quotes :)

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

After reading the old discussion again, I am not sure if this is worth doing. To many people, it is a promise we've made and kept that we treat addresses including the local part case-insensitively when summarising commits by ident strings.

I'd really wish that this series were structured to have 5/5 early and 3&4/5 squashed into a single final patch.

On Wed, Jan 06 2021, Junio C Hamano wrote:

Something that I only realized after I sent <87czykvg19.fsf@evledraar.gmail.com>: Any problems .mailmap has with Turkish "dotless I" we have already with "git log --author=<name> -i".

Not to say that there isn't some problem to solve here, just that if we do it's a more general issue than mailmap.

As can be inferred from my upthread reply I thought that was ASCII-only, but it turns out we do set LC_CTYPE based on the user's locale, and it also applies for English-speakers. E.g. in LANG=en_US.UTF-8 "--author=ævar -i" will work.

Of course that doesn't address the point of whether we should be DWYM-ing E-Mail addresses, just the sub-claim that one reason we shouldn't is because of impossible-to-solve Unicode edge cases.

Many people, through the course of their lives, will change either a name or an email address. For this reason, we have the mailmap, to map from a user's former name or email address to their current, canonical forms. Normally, this works well as it is.

However, sometimes people change a name or an email address and wish to wholly disassociate themselves from that former name or email address. For example, a person may transition from one gender to another, changing their name, or they may have changed their name to disassociate themselves from an abusive family or partner. In such a case, using the former name or address in any way may be undesirable and the person may wish to replace it as completely as possible.

For projects which wish to support this, introduce hashed forms into the mailmap. These forms, which start with "@sha256:" followed by a SHA-256 hash of the entry, can be used in place of the form used in the commit field. This form is intentionally designed to be unlikely to conflict with legitimate use cases. For example, this is not a valid email address according to RFC 5322. In the unlikely event that a user has put such a form into the actual commit as their name, we will accept it.

While the form of the data is designed to accept multiple hash algorithms, we intentionally do not support SHA-1. There is little reason to support such a weak algorithm in new use cases and no backwards compatibility to consider. Moreover, SHA-256 is faster than the SHA1DC implementation we use, so this not only improves performance, but simplifies the current implementation somewhat as well.

Note that it is, of course, possible to perform a lookup on all commit objects to determine the actual entry which matches the hashed form of the data. However, this is an improvement over the status quo.

The performance of this patch with no hashed entries is very similar to the performance without this patch. Considering a git log command to look up author and committer information on 981,680 commits in the Linux kernel history, either with an unhashed mailmap or a mailmap with all old values hashed:

                                   Shortest  Longest  Average  Change
  Git 2.30                         7.876     8.297    8.143
  This patch, unhashed             7.923     8.484    8.237    + 1.15%
  This patch, hashed               14.510    14.783   14.672   +80.17%
  This patch, hashed, unoptimized  15.425    16.318   15.901   +95.27%

Thus, the average performance after this patch is within normal variation of the pre-patch performance. It's unlikely that users will notice the difference in practice, even on much larger repositories, unless they're using the new feature.

To minimize the performance impact of the hashing process, we maintain a reference count of each mailmap entry and when we encounter an entry we must hash, we insert the same object under the unhashed key as well. We also keep a count of the number of hashed entries. This means we must hash an object at most once and once we've seen all the hashed objects, we won't hash any more objects. Times without this optimization are listed above in the unoptimized entry.

This has the potential to cause a performance problem as we insert items into a sorted list, but changing the implementation to use a khash map instead does not result in a significantly faster implementation, despite the improved insertion speed. Performance in the unhashed case is slightly worse, so this approach was not adopted since it provides few benefits.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/mailmap.txt | 28 +++++++++++
 mailmap.c                 | 99 ++++++++++++++++++++++++++++++++++-----
 mailmap.h                 |  2 +
 t/t4203-mailmap.sh        | 35 ++++++++++++++
 4 files changed, 152 insertions(+), 12 deletions(-)

On Sun, Jan 03 2021, brian m. carlson wrote:

I think it makes sense to split up 1-4/5 here from 5/5 in this series since they're really unrelated changes, although due to the changes in 1-4 they'll conflict.

The cover letter noted "As mentioned in the original thread, I think a hash rather than an encoding is the right choice here.". Reading the v1 I think you're referring to https://lore.kernel.org/git/X9wUGaR3IXcpV0nT@camp.crustytoothpaste.net/

In v1 I pointed out you needed to read some combination of the cover letter & the patch to see what this was intended for (see [1]). I think for v3 the commit itself should summarize the trade-offs & design choices.

We'll emit the commit author information as-is in that case under "git show", or run the mapping and map it via mailmap? Anyway, it seems there's a test for this. Probably better to just point to it.

I agree with most of this aside from the "weak algorithm" part. That seems like an irrelevant aside for this specific use of a hashing algorithm, no? We could even use MD5 here, so SHA256-only is just setting is up for not needing to deal with SHA1 forever in this one place in some SHA256 future repo.

Am I reading this right that if there's a single hashed entry in .mailmap anything using %aE or %aN is around 2x as slow?

Your v1 mentioned that a project might "insert entries for many contributors in order to make discovery of "interesting" entries significantly less convenient." which is gone in the v2 patch. As noted in [1] I don't see how it helps the obscurity much, but if that's still the intended use we'd expect to get more slowdowns in the wild if users intend to convert their whole mailmap to this form if they want a single entry to use the form.

Anyway, as you might have guessed I'm still not a fan of this direction.

But most of it is because I honestly don't get why this specific approach is required to achieve the stated aims, there's a few of them, so here's an attempt to break them down:

1. User changed their name and doesn't want themselves or others to see
  their old name

For the case where Joe Developer is now known as Jane Doe in most cases
you don't need to put the old name at all into the .mailmap. E.g. for
git.git this patch to our .mailmap produces the same output for `log
--all --pretty="%h %an%ae%aN%aE"`:
    
     brian m. carlson <sandals@crustytoothpaste.net>
    -brian m. carlson <sandals@crustytoothpaste.net> <sandals@crustytoothpaste.ath.cx>
    -brian m. carlson <sandals@crustytoothpaste.net> <bk2204@github.com>
    +<sandals@crustytoothpaste.net> <sandals@crustytoothpaste.ath.cx>
    +<sandals@crustytoothpaste.net> <bk2204@github.com>

So the new->name/email mapping (as opposed to new->email) is really only needed for some really obscure cases where two people shared an E-Mail or something.

So we're talking about hiding the old E-Mail, presumably because it was joe@ intsead of jane@, so in that case we could just support URI encoding:

    Jane Doe <jane@example.com>
    <jane@example.com> <%6A%6F%65@%64%65%76%65%6C%6F%70%65%72.%63%6F%6D>

Made via:

    $ perl -MURI::Escape=uri_escape -wE 'say uri_escape q[joe@developer.com], "^@."'
    %6A%6F%65@%64%65%76%65%6C%6F%70%65%72.%63%6F%6D

Which also has the nice attribute that people can make it obvious what part they want to hide, since this is really a feature to enable social politeness & consideration:

    Jane Doe <jane@example.com>
    # I don't want to be known by my old name, thanks
    <jane@example.com> <%6A%6F%65@developer.com>

2. Hiding from your enemies

For the other use-case of "abusive family or partner" I had the comment in v1 of "but not so much that you'd still take the risk of submitting a patch to .mailmap?".

Now that's obviously phrased in an off-the-cuff manner, but I'm serious.

I think it is important that the non-security of this feature obviously looks like some trivial encoding, because that's what it is. People get lulled into a false sense of security with these things all the time (e.g. thinking their "Authorization" HTTP header is safe to post on a public pastebin). So we should as much as possible make this look like the non-security it is.

3. Enabling people not to treat .mailmap as binary or a multi-encoding
file.

I mentioned this in my [1]. Your implementation doesn't do this, but e.g. it would be very nice for a project that switched from latin-1 to utf-8 to be able to do, in some cases:

    # 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)
    #

    Ævar Arnfjörð Bjarmason <avarab@gmail.com> %C6var %41rnfj%F6r%F0 %42jarmason <avarab@gmail.com>

Or some combination thereof, so e.g. previously Big5/latin1 who migrated to UTF-8 don't need to have non-valid UTF-8 in .mailmap

4. Spam

You mentioned this in your [2] (but not as a use-case in the v2 re-rolled commit message):

    And we know that spammers and recruiters (which, in this case, are
    also spammers) do indeed scrape repositories via the repository web
    interfaces.

Surely these people are most interested in the current E-Mail addresses, which if they're scraping the common web interfaces (e.g. Github, GitLab) are easily accessible there. It doesn't seem very plausible that someone would care enough to scrape .mailmap for old addresses but not just update their scraper to clone & run "git log" for the purposes of e.g. their recruitment E-Mails.

5. Interaction with other systems

Something I mentioned in the last 3 paragraphs of my [1]. I think you're only considering the cases where git itself does the mailmap translation, but we have 3rd party systems that make use of the format in good ways (also doing the Joe->Jane mapping). Making it a hassle for those systems makes it more likely that Jane doesn't get the mapping she wants.

1. https://lore.kernel.org/git/87eejswql6.fsf@evledraar.gmail.com/
2. https://lore.kernel.org/git/X9wUGaR3IXcpV0nT@camp.crustytoothpaste.net/

On 2021-01-05 at 14:21:40, Ævar Arnfjörð Bjarmason wrote:

Okay, I'll drop them.

I can do that. It's a very long commit message anyway, but if you think it would be better in the commit message, I can add it.

It will be handled correctly via the mailmap code, in which case we'll make a no-op transformation. If the user is not using the mailmap, then it will be handled trivially.

One should avoid the use of weak algorithms when possible even if they are not being used in a way that makes them weak because it incentivizes others to use them, often in a way that is insecure. I had a conversation with a junior candidate during an interview who said they used SHA-1 in a particular case "because Git uses it." That's why I mentioned it.

No, that's not the case. As soon as we see every hashed entry, we will stop hashing new entries. Linux is not necessarily the best case for this because it has a long history with many one-off contributors long ago in the history.

I'll explain that further in the commit message and add some more metrics.

Peff objected to that text, so I removed it.

As mentioned above, it depends on who you put in the mailmap. If they're the most recent 50 contributors, it'll probably be pretty cheap. If you put the oldest contributors in there and they've not sent any recent commits, it will be more expensive.

I've got that impression pretty strongly.

I do want to point out that generally I'm pretty willing to change approaches and do things differently. I've completely redone a decent number of patches in the past in response to feedback on the list.

I'm not changing the approach here because, as mentioned below, I don't think that just encoding meets the use cases I'm targeting here. So I have heard your suggestions and to be clear, I do value your input on this (and on other topics), it's just that I disagree that such a change is one I should make.

That's unlikely, but it does happen.  That's why we have it.

I don't think this feature is going to get used if we just encode names or email addresses. In the United States, when someone transitions, they get a court order to change their name. I don't think a lot of corporate environments are going to want to just encode an old name or email address in a trivially invertible way given that. This is typically a topic handled with some sensitivity in most companies.

I will tell you that I would not just use an encoded version if I were changing my name for any of the reasons I've mentioned. That wouldn't cut it for me, and I wouldn't use such a feature. The feature I'm implementing is a feature I've talked with trans folks about, and that's why I'm implementing this as it is. The response I got was essentially, "It's not everything I want, but it's an improvement."

If the decision is that we want to go with encoding instead of hashing, then I'll drop this patch. I'm not going to put my name or sign-off on that because I don't think it meets the need I'm addressing here.

The entire problem, of course, is that we bake a human's personal name and email address immutably into a Merkle tree. We know full well that people do change their names and email addresses all the time (e.g., marriage, job changes), and yet we have this design. In retrospect, we should have done something different, but hindsight is 20/20 and I'm just trying to do the best we can with what we've got.

No, my use case isn't "hiding from an abusive family or partner". It's "I'm finally free of that **** and I never want to hear their name again." (I've known people in this situation.) Also, the similar use case of, "my family member, with whom I share an uncommon name, murdered someone, which I obviously found abhorrent, and I would like to not be associated with them when my name is Googled." And yes, I knew an acquaintance many years ago whose family member murdered someone.

In other words, the person changed their name to disassociate themselves, not to hide from their abuser.

Unless the user is using the GitHub-provided noreply address or a similar address, which is common. This allows people to map all of their old addresses to such an address, which, judging from StackOverflow, is a thing people want to do.

I can tell you from dealing with abuse that raising the bar even the tiniest bit is very significant to stopping it. Most recruiters are not developers and they and spammers don't have Git installed. They're going to rely on Googling or other public search functionality, and this makes that harder.

Greylisting is exactly raising the bar the tiniest amount and it's extraordinarily effective.

This is an argument for never changing the format. Sometimes things change, and I don't want to avoid making a change because other implementations haven't implemented it yet. Under that approach, we'd never have the SHA-256 work.

On Wed, Jan 06 2021, brian m. carlson wrote:

Not replying to most of this E-Mail because I think there's nothing left to add / you clarified things for me in those cases / we respectfully disagree / any outstanding points we can pick up in your re-roll / whatever :)

Doesn't the difference in some sense boil down to either an implicit promise or an implicit assumption that the hashed version is forever going to be protected by some security-through-obscurity/inconvenience when it comes to git.git & its default tooling?

And would those users be as comfortable with the difference between encoded v.s. hashed if e.g. "git check-mailmap" learned to read the .mailmap and search-replace all the hashed versions with their materialized values, or if popular tools like Emacs learned to via a Git .mailmap in a "need translation" similar to *.gpg and *.gz. How about if popular web views of Git served up that materialized "check-mailmap" output by default?

None of which I think is implausible that we'll get as follow-up patches, I might even submit some at some point, not out of some spite. Just because I don't want to maintain out-of-tree code for an out-of-tree program that understands a Git .mailmap today, but where I'd need to search-replace the hashed versions.

Ditto it being very likely that popular editors or web viewers will gain support for this, just because it's tedious to manually hash & copy/paste & validate values.

In looking at some of the fsck code recently & having some yet-unsubmitted patches I thought of trying to compine it with mailmap. I.e. it seems like a natural feature for fsck to gain to warn you about unused mailmap entries, just like it can warn about unreachable/dangling objects. After all these are really just sort-of pointers into our Merkle tree. Spewing out all the mappings seems like an obvious addition to that, e.g. in spewing out an "optimized/non-redundant" (plain or hashed) mailmap to re-commit.

That's the main reason I'm uncomfortable with this approach, because it seems to me to implicitly rely on things that are tedious now, but which the march of history all but inevitably should make trivial if we were to integrate it. Unless we're *also* promising to forever intentionally (and artificially) keep it inconvenient.

E.g. the example of how long it takes to clone & extract this info from chromium.git in the v1 thread.

It seems like a fair assumption that we'll have some future version of git where you can ask a remote server about that sort of thing in milliseconds.

Not because of this hashed .mailmap thing in particular, just as an emergent effect that it's happy to serve up things it knows about the DAG from having walked & cached it in general. E.g. info from the commit-graph, what hash is contained in what ref, or how one value (such as a .mailmap entry) maps to another etc.

On 2021-01-10 at 19:24:34, Ævar Arnfjörð Bjarmason wrote:

Yes, I think we do rely on this being inconvenient. If you plan to submit such a patch, I'm going to let this series drop.

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

I am not sure if we want to even mention the "for example" here.

These are certainly all legitimate reasons to want this feature, but after reading the "for example", lack of a corresponding negative statement (e.g. sometimes people also change their name or address to hide their bad behaviour in the past that is associated with these names) needlessly stood out and made me wonder if we need to somehow defend the feature with "...but we do not mean to abet people in hiding their past bad behaviour with this mechanism". I'd prefer us not forced to defend the mechanism if we did not have to.

There were suggestions to use reversible encoding, IIRC, just for obscurity. I do not have a strong preference either way myself, but because such an approach would give the same improvement over the status quo, would be simpler, more performant and most importantly, it makes it clear that this is not serious security but casual obscurity, I'd want to be convinced why we want to use a hash here a bit more strongly.

Is this needed to be said? After all, we won't take @md5: or @blake2: or anything other than @sha256: in this version (and probably any forseeable versions). Unless we offer a way to plug-in algos of projects' choice, that is, and at that point, "SHA-1 is not accepted" is a statement too strong for us to make.

On 2021-01-05 at 20:05:22, Junio C Hamano wrote:

I added it because I imagine the use cases for this feature aren't immediately obvious to a lot of people and the general rule is that commit messages explain why we would implement such a feature. If you'd prefer I drop it and leave it up to the imagination (or to the list archives), I can do that.

I'll drop that line.

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Yeah, I understand that.

Thanks.