[RFC] mailmap.blob overrides default .mailmap

[RFC] mailmap.blob overrides default .mailmap

[Cornelius Weig]

Hi,

 I was reading into the mailmap handling today and I'm a bit puzzled by the overriding behavior.

This is what the documentation says about precedence (emphasis mine):
-------------
mailmap.file
    The location of an augmenting mailmap file. The default mailmap, located
    in the root of the repository, is loaded first, then the mailmap file
    pointed to by this variable. The location of the mailmap file may be in a
    repository subdirectory, or somewhere outside of the repository itself.
    See git-shortlog(1) and git-blame(1).

mailmap.blob
    Like mailmap.file, but consider the value as a reference to a blob in the
    repository. If both mailmap.file and mailmap.blob are given, both are
!!! parsed, with _entries from mailmap.file taking precedence_. In a bare
    repository, this defaults to HEAD:.mailmap. In a non-bare repository, it
    defaults to empty.
------------

So from the doc I would have expected that files always get precedence over the blob. IOW entries from .mailmap override entries from mailmap.blob. However, this is not the case.

The code shows why (mailmap.c):
	err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
	if (startup_info->have_repository)
		err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);
	err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);


Apparently this is not an oversight, because there is an explicit test for this overriding behavior (t4203 'mailmap.blob overrides .mailmap').

So I wonder: what is the rationale behind this? I find this mixed overriding behavior hard to explain and difficult to understand.

Re: [RFC] mailmap.blob overrides default .mailmap

[Stefan Beller]

On Tue, Feb 7, 2017 at 3:56 AM, Cornelius Weig
<cornelius.weig@tngtech.com> wrote:
> Hi,
>
>  I was reading into the mailmap handling today and I'm a bit puzzled by the overriding behavior.
>
> This is what the documentation says about precedence (emphasis mine):
> -------------
> mailmap.file
>     The location of an augmenting mailmap file. The default mailmap, located
>     in the root of the repository, is loaded first, then the mailmap file
>     pointed to by this variable. The location of the mailmap file may be in a
>     repository subdirectory, or somewhere outside of the repository itself.
>     See git-shortlog(1) and git-blame(1).
>
> mailmap.blob
>     Like mailmap.file, but consider the value as a reference to a blob in the
>     repository. If both mailmap.file and mailmap.blob are given, both are
> !!! parsed, with _entries from mailmap.file taking precedence_. In a bare
>     repository, this defaults to HEAD:.mailmap. In a non-bare repository, it
>     defaults to empty.
> ------------
>
> So from the doc I would have expected that files always get precedence over the blob. IOW entries from .mailmap override entries from mailmap.blob. However, this is not the case.
>
> The code shows why (mailmap.c):
>         err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
>         if (startup_info->have_repository)
>                 err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);
>         err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);
>
>
> Apparently this is not an oversight, because there is an explicit test for this overriding behavior (t4203 'mailmap.blob overrides .mailmap').

which is blamed to 08610900 (mailmap: support reading mailmap from
blobs, 2012-12-12),
cc'ing Jeff who may remember what he was doing back then, as the
commit message doesn't discuss the implications on ordering.

>
> So I wonder: what is the rationale behind this? I find this mixed overriding behavior hard to explain and difficult to understand.
>

Re: [RFC] mailmap.blob overrides default .mailmap

[Jeff King]

On Tue, Feb 07, 2017 at 09:27:19AM -0800, Stefan Beller wrote:

> > The code shows why (mailmap.c):
> >         err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
> >         if (startup_info->have_repository)
> >                 err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);
> >         err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);
> >
> >
> > Apparently this is not an oversight, because there is an explicit
> > test for this overriding behavior (t4203 'mailmap.blob overrides
> > .mailmap').
> 
> which is blamed to 08610900 (mailmap: support reading mailmap from
> blobs, 2012-12-12),
> cc'ing Jeff who may remember what he was doing back then, as the
> commit message doesn't discuss the implications on ordering.

I think it was mostly that I had to define _some_ order. This made sense
to me as similar to things like attributes or excludes, where we prefer
clone-specific data over in-history data (so .git/info/attributes takes
precedence over .gitattributes).

So any mailmap.* would take precedence over the in-tree .mailmap file.
And then between mailmap.file and mailmap.blob, the "blob" form is
more "in-tree" than the "file" form (especially because we turn it on by
default in bare repos, so it really is identical to the in-tree form
there).

I think the easiest way to think of it is the same as we do config. We
read the files in a particular order, least-important to most-important,
and apply "last one wins" (so more-important entries overwrite
less-important ones).

-Peff

Re: [RFC] mailmap.blob overrides default .mailmap

[Cornelius Weig]

On 02/07/2017 08:28 PM, Jeff King wrote:
> 
> I think it was mostly that I had to define _some_ order. This made sense
> to me as similar to things like attributes or excludes, where we prefer
> clone-specific data over in-history data (so .git/info/attributes takes
> precedence over .gitattributes).
> 
> So any mailmap.* would take precedence over the in-tree .mailmap file.
> And then between mailmap.file and mailmap.blob, the "blob" form is
> more "in-tree" than the "file" form (especially because we turn it on by
> default in bare repos, so it really is identical to the in-tree form
> there).

So the clone-specific data wins over in-history makes perfect sense to me. Therefore, mailmap.file should win over mailmap.blob, agreed.

On the other hand, a checked-in .mailmap file and a mailmap-blob are both as in-history as the other to me. Now consider the following settings:

$ git config --unset mailmap.file
$ git config mailmap.blob HEAD:.mailmap
$ sed -i 's:peff@peff.com:no-valid-address:' .mailmap
$ git log -1 --author 'Jeff King'

So with the .mailmap being dirty, which address would one expect to be printed? I would expect 'no-valid-address', but it's 'peff@peff.com'.

Even though the .mailmap file is more visible and also closer to the user, the mailmap.blob wins over it. I find that somewhat counter-intuitive. Of course, setting `git config mailmap.file .mailmap`, would do the trick.

Re: [RFC] mailmap.blob overrides default .mailmap

[Jeff King]

On Tue, Feb 07, 2017 at 11:45:31PM +0100, Cornelius Weig wrote:

> On the other hand, a checked-in .mailmap file and a mailmap-blob are
> both as in-history as the other to me. Now consider the following
> settings:

I think it depends how you use them. You could point mailmap.blob to
some other ref entirely (even one that you fetched from another
repository).

I'd expect normal use to point it to HEAD:.mailmap, though (and that was
certainly the use case I wrote it for). On the other hand, the point of
pointing it to that particular blob is that it works even when you
_don't_ have a checkout (and this kicks in automatically in a bare
repo).

> $ git config --unset mailmap.file
> $ git config mailmap.blob HEAD:.mailmap
> $ sed -i 's:peff@peff.com:no-valid-address:' .mailmap
> $ git log -1 --author 'Jeff King'

In case anybody wants to experiment, there are a bunch of things that
make this a non-working example (at least on git.git):

  - my address is actually peff.net :)

  - There mailmap which mentions peff.net maps peff@github.com to
    peff.net, so this change would require --author=peff@github.com.

  - We don't apply mailmaps for the default output of "git log". You can
    format with "%aN %aE", or just use "git shortlog -ns --author=peff"
    which does map.

But that aside, yeah, you can make an argument to expect one way or the
other, depending on the situation you set up. I don't have a strong
feeling about it, but my gut feeling is that no ordering is
significantly better than the other, and that puts me in favor of
leaving it as-is purely out of inertia and backwards-compatibility.

-Peff