[PATCH] shortlog: pass the mailmap into the revision walker
From: CB Bailey
shortlog always respects the mailmap in its output. Pass the mailmap
into the revision walker to allow the mailmap to be used with revision
limiting options such as '--author'.
This removes some apparently inconsistent behaviors when using
'--author', such as not finding some or all commits for a given author
which do appear under that author in an unrestricted invocation of
shortlog or commits being summarized under a different author than the
specified author.
---
builtin/shortlog.c | 2 ++
t/t4203-mailmap.sh | 28
2 files changed, 30 insertions(+)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 88f88e97b2..a6fb00ade8 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -188,6 +188,8 @@ static void get_from_rev(struct rev_info *rev, struct
shortlog *log)
{
struct commit *commit;
+ rev->mailmap = &log->mailmap;
+
if (prepare_revision_walk(rev))
die(_("revision walk setup failed"));
while ((commit = get_revision(rev)) != NULL)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 43b1522ea2..9bee35b06c 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -383,6 +383,34 @@ test_expect_success 'Shortlog output (complex mapping)' '
'
+test_expect_success 'Shortlog output (complex mapping, filtered)' '
+
+ printf " 1\tA U Thor \n" >expect &&
+
+ git shortlog -es --author="A U Thor" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 1\tCTO \n" >expect &&
+
+ git shortlog -es --author=CTO HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 2\tOther Author \n" >expect &&
+
+ git shortlog -es --author="Other Author" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 2\tSanta Claus \n" >expect &&
+
+ git shortlog -es --author="Santa Claus" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 1\tSome Dude \n" >expect &&
+
+ git shortlog -es --author="Some Dude" HEAD >actual &&
+ test_cmp expect actual
+'
+
# git log with --pretty format which uses the name and email mailmap
placemarkers
cat >expect <<\EOF
Author CTO maps to CTO
--
2.17.0.rc0
[PATCH] shortlog: pass the mailmap into the revision walker
From: CB Bailey
shortlog always respects the mailmap in its output. Pass the mailmap
into the revision walker to allow the mailmap to be used with revision
limiting options such as '--author'.
This removes some apparently inconsistent behaviors when using
'--author', such as not finding some or all commits for a given author
which do appear under that author in an unrestricted invocation of
shortlog or commits being summarized under a different author than the
specified author.
Signed-off-by: CB Bailey
---
Resending with omitted s-o-b.
builtin/shortlog.c | 2 ++
t/t4203-mailmap.sh | 28
2 files changed, 30 insertions(+)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 88f88e97b2..a6fb00ade8 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -188,6 +188,8 @@ static void get_from_rev(struct rev_info *rev, struct
shortlog *log)
{
struct commit *commit;
+ rev->mailmap = &log->mailmap;
+
if (prepare_revision_walk(rev))
die(_("revision walk setup failed"));
while ((commit = get_revision(rev)) != NULL)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 43b1522ea2..9bee35b06c 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -383,6 +383,34 @@ test_expect_success 'Shortlog output (complex mapping)' '
'
+test_expect_success 'Shortlog output (complex mapping, filtered)' '
+
+ printf " 1\tA U Thor \n" >expect &&
+
+ git shortlog -es --author="A U Thor" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 1\tCTO \n" >expect &&
+
+ git shortlog -es --author=CTO HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 2\tOther Author \n" >expect &&
+
+ git shortlog -es --author="Other Author" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 2\tSanta Claus \n" >expect &&
+
+ git shortlog -es --author="Santa Claus" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 1\tSome Dude \n" >expect &&
+
+ git shortlog -es --author="Some Dude" HEAD >actual &&
+ test_cmp expect actual
+'
+
# git log with --pretty format which uses the name and email mailmap
placemarkers
cat >expect <<\EOF
Author CTO maps to CTO
--
2.17.0.rc0
[RFC/PATCH] Use mailmap by default in log, show and whatchanged
From: CB Bailey People who have changed their name or email address will usually know that they need to set 'log.mailmap' in order to have their new details reflected for old commits with 'git log', but others who interact with them may not know or care enough to enable this option. Change the default for 'git log' and friends to always use mailmap so that everyone gets to see canonical names and email addresses. Signed-off-by: CB Bailey --- Related to my patch to make shortlog pass the mailmap into the revision walker which may end up being configuratble behavior, I thought I'd propose this for discussion. For people who change their names during their involvement in a project, it can be important that the people with whom they work only see their correct name, even when browsing old history. I had a dig around in the mailing list archives and couldn't find any specific reason not to use a mailmap in log where one is in use. I did find this message which suggests that it makes sense to make it the default for human-consumed formats. This patch doesn't affect "--pretty=raw" formatting. Documentation/config/log.txt | 4 ++-- Documentation/git-log.txt| 12 +--- builtin/log.c| 2 +- t/t4203-mailmap.sh | 18 ++ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt index 78d9e4453a..8a01eed46b 100644 --- a/Documentation/config/log.txt +++ b/Documentation/config/log.txt @@ -39,5 +39,5 @@ log.showSignature:: linkgit:git-whatchanged[1] assume `--show-signature`. log.mailmap:: - If true, makes linkgit:git-log[1], linkgit:git-show[1], and - linkgit:git-whatchanged[1] assume `--use-mailmap`. + If false, makes linkgit:git-log[1], linkgit:git-show[1], and + linkgit:git-whatchanged[1] assume `--no-use-mailmap`. diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index 90761f1694..7815c9a6cb 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -50,9 +50,11 @@ OPTIONS commit was reached. --use-mailmap:: - Use mailmap file to map author and committer names and email - addresses to canonical real names and email addresses. See - linkgit:git-shortlog[1]. +--no-use-mailmap:: + Use (or don't use) mailmap file to map author and committer names and + email addresses to canonical real names and email addresses. The default + is to use the mailmap file, but this can be overriden with the + `log.mailmap` configuration option. See linkgit:git-shortlog[1]. --full-diff:: Without this flag, `git log -p ...` shows commits that @@ -205,6 +207,10 @@ log.showRoot:: `git log -p` output would be shown without a diff attached. The default is `true`. +log.mailmap:: + If `false`, makes `git log` and related commands assume + `--no-use-mailmap`. + log.showSignature:: If `true`, `git log` and related commands will act as if the `--show-signature` option was passed to them. diff --git a/builtin/log.c b/builtin/log.c index e8e51068bd..41a5eefb20 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -46,7 +46,7 @@ static int default_follow; static int default_show_signature; static int decoration_style; static int decoration_given; -static int use_mailmap_config; +static int use_mailmap_config = 1; static const char *fmt_patch_subject_prefix = "PATCH"; static const char *fmt_pretty; diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index 43b1522ea2..efb145c4cd 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -424,6 +424,24 @@ EOF test_expect_success 'Log output with --use-mailmap' ' git log --use-mailmap | grep Author >actual && + test_cmp expect actual && +# --use-mailmap is the default + git log | grep Author >actual && + test_cmp expect actual +' + +cat >expect <<\EOF +Author: CTO +Author: claus +Author: santa +Author: nick2 +Author: nick2 +Author: nick1 +Author: A U Thor +EOF + +test_expect_success 'Log output with --no-use-mailmap' ' + git log --no-use-mailmap | grep Author >actual && test_cmp expect actual ' -- 2.20.0
Re: [RFC/PATCH] Use mailmap by default in log, show and whatchanged
On Thu, Dec 13, 2018 at 12:09:40PM +, CB Bailey wrote: > I had a dig around in the mailing list archives and couldn't find any > specific reason not to use a mailmap in log where one is in use. I did > find this message which suggests that it makes sense to make it the > default for human-consumed formats. This patch doesn't affect > "--pretty=raw" formatting. This week I'm having an issue with sending messages a few seconds before I've completed them. This message: https://public-inbox.org/git/20121112231815.ge10...@sigill.intra.peff.net/ CB
Re: Deadname rewriting
On Fri, Jun 21, 2019 at 11:34:06PM +0200, Ævar Arnfjörð Bjarmason wrote: > >> This topic was discussed at the last git contributor summit (brought up > >> by CB Bailey) resulting in this patch, which I see didn't make it in & > >> needs to be resurrected again: > >> https://public-inbox.org/git/20181212171052.13415-1...@hashpling.org/ > > > > Thanks for the link. > > > > I didn't know about config options for mailmap.file and log.mailmap > > before. These do make this option much more useful, especially when we > > can insert default settings for them into /etc/gitconfig across the > > company. > > Right, and to the extent that we don't --use-mailmap by default I think > that's mainly because nobody's cared enough to advocate for it. I think > it would be a sensible default. That was this patch: https://public-inbox.org/git/20181213120940.26477-1...@hashpling.org/ There were no objections so I was going to re-propose it but I haven't got around to this for a number of reasons, many of which are not Git related. Ideally, I wanted to fix all of the known issues with mailmap such as some behaviors of shortlog fixed with the shortlog patch above. I also noticed some more artifacts that I would like to be fixed. In particular the RFC 822 style "trailers" should be rewritten by default. Having something like this pop up is not likely to be acceptable in a project which uses trailers: commit abcd... Author: Bob important commit message Signed-off-by: Alice
[PATCH] t2024: mark test using "checkout -p" with PERL prerequisite
checkout with the -p switch uses the "add interactive" framework which is written in Perl. Add a PERL prerequisite to skip this test when built with NO_PERL. Signed-off-by: CB Bailey --- t/t2024-checkout-dwim.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh index f79dfbbdd6..0e512c3066 100755 --- a/t/t2024-checkout-dwim.sh +++ b/t/t2024-checkout-dwim.sh @@ -75,7 +75,7 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' ' test_branch master ' -test_expect_success 'checkout of branch from multiple remotes fails with advice' ' +test_expect_success PERL 'checkout of branch from multiple remotes fails with advice' ' git checkout -B master && test_might_fail git branch -D foo && test_must_fail git checkout foo 2>stderr && -- 2.14.3 (Apple Git-98)
[PATCH] t4038: Remove non-portable '-a' option passed to test_cmp
From: CB Bailey Signed-off-by: CB Bailey --- t/t4038-diff-combined.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh index d4afe12554..b9d876efa2 100755 --- a/t/t4038-diff-combined.sh +++ b/t/t4038-diff-combined.sh @@ -509,7 +509,7 @@ test_expect_success FUNNYNAMES '--combined-all-paths and --raw and funny names' test_expect_success FUNNYNAMES '--combined-all-paths and --raw -and -z and funny names' ' printf "aaf8087c3cbd4db8e185a2d074cf27c53cfb75d7\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\0file\twith\ttabs\0i\tam\ttabbed\0fickle\tnaming\0" >expect && git diff-tree -c -M --raw --combined-all-paths -z HEAD >actual && - test_cmp -a expect actual + test_cmp expect actual ' test_expect_success FUNNYNAMES '--combined-all-paths and --cc and funny names' ' -- 2.23.0.1.g68c7121409
Re: [PATCH v4 0/3] use mailmap by default in git log
On Thu, Jul 11, 2019 at 01:37:24PM -0500, Ariadne Conill wrote: > It is not uncommon for people to change their name or e-mail address. > To facilitate this, Git provides support for the `.mailmap` file, > which contains a list of identities and previously used e-mail > addresses that are associated with that identity. > > Unfortunately, while Git's support for the `.mailmap` file is generally > excellent, I recently discovered that `git log` does not treat the > mail map file the same as the other tools, instead requiring an > explicit flag to use the mailmap file. > > I believe this is an unfortunate flaw, as the mailmap file should > ideally contain the most current known contact information for a > contributor, allowing anyone to contact the contributor about their > patches in the future. > > This should be the finished version of the patch set, thanks to > everyone who has helped review it! Thank you very much for following up on this. I've been meaning to revisit my RFC from last year on this topic for some time but unfortunately Git work has not been able to be a priority for me for some time. I think that this patch everything that covered 'log' specifically that I'm aware of, 'shortlog' still has some issues that I'd like to address. CB
Re: [PATCH] CODE_OF_CONDUCT: mention individual project-leader emails
On 26/09/2019 08:20, Jeff King wrote: > On Tue, Sep 24, 2019 at 04:52:56PM -0700, Emily Shaffer wrote: >> I helped my other FOSS project to adopt a Code of Conduct earlier in >> the year (https://github.com/openbmc/docs/blob/master/code-of-conduct.md) >> and we got around this by asking for volunteers from the technical >> steering committee to agree to have their contact info listed on the >> escalation path; at the end of the escalation path we also listed >> someone external to the project (which we were able to do because we >> had been adopted by the Linux Foundation, and they have someone for >> that). > > Yeah, I think this is sort of the same thing except that I > pre-volunteered the whole project committee. ;) > > We could have a separate list of contacts for the code of conduct, but > it seems simplest to just use the existing group that we already have, > unless there's a compelling reason not to. I, too, wondered if it might be more appropriate to have the list of names and email addresses separated from the repository and just linked from the CoC. Perhaps someone would need to expunge themselves from the list permanently, or perhaps we'd want to protect against a hypothetical person in a position of control changing the list to their trusted cronies. I cannot think of a realistic scenario or practical setup which would actually guarantee any such benefits and this solution is simple and practical. Overall for this proposed CoC patch: ACK CB
Re: [PATCH v3] diffcore-break: use a goto instead of a redundant if statement
On Sat, Sep 28, 2019 at 06:56:46PM -0600, Alex Henrie wrote:
> The condition "if (q->nr <= j)" checks whether the loop exited normally
> or via a break statement. This check can be avoided by replacing the
> jump to the end of the loop with a jump to the end of the function.
>
> With the break replaced by a goto, the two diff_q calls then can be
> replaced with a single diff_q call outside of the outer if statement.
>
> Reviewed-by: Derrick Stolee
> Signed-off-by: Alex Henrie
> ---
> diffcore-break.c | 15 +++
> 1 file changed, 7 insertions(+), 8 deletions(-)
For easier discussion, I've snipped the original patch and replaced with
one with enough context to show the entire function.
I was reviewing this patch and it appeared to introduce a change in
behaviour.
> diff --git a/diffcore-break.c b/diffcore-break.c
> index 875aefd3fe..f6ab74141b 100644
> --- a/diffcore-break.c
> +++ b/diffcore-break.c
> @@ -262,44 +262,43 @@ static void merge_broken(struct diff_filepair *p,
>
> void diffcore_merge_broken(void)
> {
> struct diff_queue_struct *q = &diff_queued_diff;
> struct diff_queue_struct outq;
> int i, j;
>
> DIFF_QUEUE_CLEAR(&outq);
>
> for (i = 0; i < q->nr; i++) {
> struct diff_filepair *p = q->queue[i];
> if (!p)
> /* we already merged this with its peer */
> continue;
> else if (p->broken_pair &&
>!strcmp(p->one->path, p->two->path)) {
> /* If the peer also survived rename/copy, then
>* we merge them back together.
>*/
> for (j = i + 1; j < q->nr; j++) {
> struct diff_filepair *pp = q->queue[j];
> if (pp->broken_pair &&
> !strcmp(pp->one->path, pp->two->path) &&
> !strcmp(p->one->path, pp->two->path)) {
> /* Peer survived. Merge them */
> merge_broken(p, pp, &outq);
> q->queue[j] = NULL;
> - break;
> + goto done;
Previously, if the condition matched in the inner loop, the function
would null out the entry in the queue that that inner loop had reached
(q->queue[j] = NULL) and then break out of the inner loop. This meant
that the outer loop would skip over this entry (if (!p)).
The change introduced seems to break out of both loops as soon as we
reach one match, whereas before other subsequent matches would be
considered and merged. Not only this, but the outer 'else' case for all
subsequent entries is skipped so the rest of the entries the original
queue are missing from 'outq'.
> }
> }
> - if (q->nr <= j)
> - /* The peer did not survive, so we keep
> - * it in the output.
> - */
> - diff_q(&outq, p);
> + /* The peer did not survive, so we keep
> + * it in the output.
> + */
> }
> - else
> - diff_q(&outq, p);
> + diff_q(&outq, p);
> }
> +
> +done:
> free(q->queue);
> *q = outq;
>
> return;
> }
I spent a bit of time trying to see if this change was user visible
which turned out to be unneeded as t4008-diff-break-rewrite.sh already
fails with this change for me in my environment, initially with this
test but also 3 other tests in this file.
> expecting success of 4008.6 'run diff with -B (#3)':
> git diff-index -B reference >current &&
> cat >expect <<-EOF &&
> :100644 100644 $blob0_id $blob1_id M100 file0
> :100644 100644 $blob1_id $blob0_id M100 file1
> EOF
> compare_diff_raw expect current
>
> --- .tmp-12019-09-29 09:21:07.089070076 +
> +++ .tmp-22019-09-29 09:21:07.093070086 +
> @@ -1,2 +1 @@
> :100644 100644 548142c327a6790ff8821d67c2ee1eff7a656b52
> 6ff87c4664981e4397625791c8ea3bbb5f2279a3 M# file0
> -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3
> 548142c327a6790ff8821d67c2ee1eff7a656b52 M# file1
> not ok 6 - run diff with -B (#3)
Re: [PATCH] filter-branch: use printf instead of echo -e
On Mon, Mar 19, 2018 at 03:49:05PM +0100, Michele Locati wrote: > In order to echo a tab character, it's better to use printf instead of > "echo -e", because it's more portable (for instance, "echo -e" doesn't work > as expected on a Mac). > > This solves the "fatal: Not a valid object name" error in git-filter-branch > when using the --state-branch option. > > Signed-off-by: Michele Locati > --- > git-filter-branch.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-filter-branch.sh b/git-filter-branch.sh > index 1b7e4b2cd..21d84eff3 100755 > --- a/git-filter-branch.sh > +++ b/git-filter-branch.sh > @@ -627,7 +627,7 @@ then > print H "$_:$f\n" or die; > } > close(H) or die;' || die "Unable to save state") > - state_tree=$(/bin/echo -e "100644 blob $state_blob\tfilter.map" | git > mktree) > + state_tree=$(printf '100644 blob %s\tfilter.map\n' "$state_blob" | git > mktree) > if test -n "$state_commit" > then > state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" > -p "$state_commit") I think the change from 'echo -e' to printf is good because of the better portability reason that you cite. Looking at the change, I am now curious as to why '/bin/echo' is used. Testing on a Mac, bash's built in 'echo' recognizes '-e' whereas '/bin/echo' does not. This is just an observation, I still prefer the move to 'printf' that you suggest. There are two further uses of '/bin/echo' in git-filter-branch.sh which are portable (no "-e", just printing a word that cannot be confused for an option). One is visible in your diff context and the other is just below it. For consistency with other echos in git-filter-branch.sh, I think that these should probably use simple 'echo' rather than '/bin/echo' to use a builtin where available. CB
Re: Raise your hand to Ack jk/code-of-conduct if your Ack fell thru cracks
On Wed, Oct 09, 2019 at 09:14:59AM +0900, Junio C Hamano wrote: > Johannes Schindelin writes: > > > Junio, would you mind picking it up, please? > > I trust you enough that I won't go back to the cited messages to > double check that these acks are real, but I'd still wait for a few > days for people who expressed their Acks but your scan missed, or > those who wanted to give their Acks but forgot to do so, to raise > their hands on this thread. Acked-by: CB Bailey I raise my hand.
