threads / rfc / 34398

RFC patch, 4 partsbuiltin: add git-check-mailmap command

Subject: [RFC/PATCH 1/4] builtin: add git-check-mailmap command

## tl;dr

10 messages between Jul 10, 2013 and Jul 11, 2013. Diffs are folded; open one to read it.

replies: 9people: 3as markdown or json

Eric Sunshine· Jul 10, 2013, 19:03 UTC · lore

[RFC/PATCH 0/4] add git-check-mailmap command

This patch series adds builtin command check-mailmap, similar to check-attr and check-ignore, which allows direct testing of .mailmap configuration. More importantly, as plumbing accessible to scripts and other porcelain, check-mailmap publishes the stable, well-tested .mailmap functionality employed by built-in Git commands.

It is RFC because it lacks documentation; because its utility as a check-foo command is minimal compared to check-attr and check-ignore (although its utility as plumbing for scripts and porcelain is more significant); and because I want to make sure that the project is willing to accept yet another check-foo command.

The idea and motivation for git-check-mailmap arose from the effort to implement .mailmap support for git-contacts [1] (presently at es/contacts in 'pu').

Felipe's Ruby implementation of .mailmap support for his git-related script is introduced in patch 9/15 [2] of v5, and is augmented in patch 10/15 [3] to support invocation from within a subdirectory. His version supports configuration variable mailmap.file, but not mailmap.blob.

Rather than rewriting the functionality yet again, this time in Perl, along with all the details and nuances of the C version employed by Git builtins, I decided that it made more sense to expose the well-tested and polished C implementation as plumbing, thus allowing any script or porcelain to take advantage of it.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/229533/ [2]: http://article.gmane.org/gmane.comp.version-control.git/224782/ [3]: http://article.gmane.org/gmane.comp.version-control.git/224783/

Eric Sunshine (4):
  builtin: add git-check-mailmap command
  t4203: test check-mailmap command invocation
  t4203: test mailmap functionality directly rather than indirectly
  t4203: consolidate test-repository setup
 .gitignore                             |   1 +
 Makefile                               |   1 +
 builtin.h                              |   1 +
 builtin/check-mailmap.c                |  73 ++++++++++
 command-list.txt                       |   1 +
 contrib/completion/git-completion.bash |   1 +
 git.c                                  |   1 +
 t/t4203-mailmap.sh                     | 246 +++++++++++++++++----------------
 8 files changed, 209 insertions(+), 116 deletions(-)
 create mode 100644 builtin/check-mailmap.c
-- 
1.8.3.2
Eric Sunshine· Jul 10, 2013, 19:03 UTC · re: Eric Sunshine · lore

Introduce command check-mailmap, similar to check-attr and check-ignore, which allows direct testing of .mailmap configuration.

As plumbing accessible to scripts and other porcelain, check-mailmap publishes the stable, well-tested .mailmap functionality employed by built-in Git commands. Consequently, script authors need not re-implement .mailmap functionality manually, thus avoiding potential quirks and behavioral differences.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 .gitignore                             |  1 +
 Makefile                               |  1 +
 builtin.h                              |  1 +
 builtin/check-mailmap.c                | 73 ++++++++++++++++++++++++++++++++++
 command-list.txt                       |  1 +
 contrib/completion/git-completion.bash |  1 +
 git.c                                  |  1 +
 7 files changed, 79 insertions(+)
 create mode 100644 builtin/check-mailmap.c
Show changes to 7 files +79 −0

.gitignore, Makefile, builtin.h, builtin/check-mailmap.c, command-list.txt, contrib/completion/git-completion.bash, git.c

diff --git a/.gitignore b/.gitignore
index efa8db0..6b1fd1b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -23,6 +23,7 @@
 /git-cat-file
 /git-check-attr
 /git-check-ignore
+/git-check-mailmap
 /git-check-ref-format
 /git-checkout
 /git-checkout-index
diff --git a/Makefile b/Makefile
index 0600eb4..ef442eb 100644
--- a/Makefile
+++ b/Makefile
@@ -912,6 +912,7 @@ BUILTIN_OBJS += builtin/bundle.o
 BUILTIN_OBJS += builtin/cat-file.o
 BUILTIN_OBJS += builtin/check-attr.o
 BUILTIN_OBJS += builtin/check-ignore.o
+BUILTIN_OBJS += builtin/check-mailmap.o
 BUILTIN_OBJS += builtin/check-ref-format.o
 BUILTIN_OBJS += builtin/checkout-index.o
 BUILTIN_OBJS += builtin/checkout.o
diff --git a/builtin.h b/builtin.h
index 1ed8edb..8afa2de 100644
--- a/builtin.h
+++ b/builtin.h
@@ -40,6 +40,7 @@ extern int cmd_checkout(int argc, const char **argv, const char *prefix);
 extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
 extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ignore(int argc, const char **argv, const char *prefix);
+extern int cmd_check_mailmap(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c
new file mode 100644
index 0000000..cbcc6db
--- /dev/null
+++ b/builtin/check-mailmap.c
@@ -0,0 +1,73 @@
+#include "builtin.h"
+#include "mailmap.h"
+#include "parse-options.h"
+#include "string-list.h"
+
+static int use_stdin;
+static int null_out;
+static const char * const check_mailmap_usage[] = {
+N_("git check-mailmap [options] <contact>..."),
+NULL
+};
+
+static const struct option check_mailmap_options[] = {
+	OPT_BOOLEAN(0, "stdin", &use_stdin,
+		    N_("also read contacts from stdin")),
+	OPT_BOOLEAN('z', NULL, &null_out,
+		    N_("null-terminate output lines")),
+	OPT_END()
+};
+
+static void check_mailmap(struct string_list *mailmap, const char *contact)
+{
+	const char *name, *mail;
+	size_t namelen, maillen;
+	struct ident_split ident;
+	char term = null_out ? '\0' : '\n';
+
+	if (split_ident_line(&ident, contact, strlen(contact)))
+		die(_("unable to parse contact: %s"), contact);
+
+	name = ident.name_begin;
+	namelen = ident.name_end - ident.name_begin;
+	mail = ident.mail_begin;
+	maillen = ident.mail_end - ident.mail_begin;
+
+	map_user(mailmap, &mail, &maillen, &name, &namelen);
+
+	if (namelen)
+		printf("%.*s <%.*s>%c",
+			(int)namelen, name, (int)maillen, mail, term);
+	else
+		printf("<%.*s>%c", (int)maillen, mail, term);
+}
+
+int cmd_check_mailmap(int argc, const char **argv, const char *prefix)
+{
+	int i;
+	struct string_list mailmap = STRING_LIST_INIT_NODUP;
+
+	git_config(git_default_config, NULL);
+	argc = parse_options(argc, argv, prefix, check_mailmap_options,
+		check_mailmap_usage, 0);
+	if (argc == 0 && !use_stdin)
+		die(_("no contacts specified"));
+
+	read_mailmap(&mailmap, NULL);
+
+	for (i = 0; i < argc; ++i)
+		check_mailmap(&mailmap, argv[i]);
+	maybe_flush_or_die(stdout, "contact to stdout");
+
+	if (use_stdin) {
+		struct strbuf buf = STRBUF_INIT;
+		while (strbuf_getline(&buf, stdin, '\n') != EOF) {
+			check_mailmap(&mailmap, buf.buf);
+			maybe_flush_or_die(stdout, "contact to stdout");
+		}
+		strbuf_release(&buf);
+	}
+
+	clear_mailmap(&mailmap);
+	return 0;
+}
diff --git a/command-list.txt b/command-list.txt
index bf83303..08b04e2 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -13,6 +13,7 @@ git-bundle                              mainporcelain
 git-cat-file                            plumbinginterrogators
 git-check-attr                          purehelpers
 git-check-ignore                        purehelpers
+git-check-mailmap                       purehelpers
 git-checkout                            mainporcelain common
 git-checkout-index                      plumbingmanipulators
 git-check-ref-format                    purehelpers
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ebc40d4..c118550 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -648,6 +648,7 @@ __git_list_porcelain_commands ()
 		cat-file)         : plumbing;;
 		check-attr)       : plumbing;;
 		check-ignore)     : plumbing;;
+		check-mailmap)    : plumbing;;
 		check-ref-format) : plumbing;;
 		checkout-index)   : plumbing;;
 		commit-tree)      : plumbing;;
diff --git a/git.c b/git.c
index 4359086..02c3035 100644
--- a/git.c
+++ b/git.c
@@ -324,6 +324,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "cat-file", cmd_cat_file, RUN_SETUP },
 		{ "check-attr", cmd_check_attr, RUN_SETUP },
 		{ "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
+		{ "check-mailmap", cmd_check_mailmap, RUN_SETUP },
 		{ "check-ref-format", cmd_check_ref_format },
 		{ "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
 		{ "checkout-index", cmd_checkout_index,
-- 
1.8.3.2
Duy Nguyen· Jul 11, 2013, 02:31 UTC · re: Eric Sunshine · lore

Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command

On Thu, Jul 11, 2013 at 2:03 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 5 quoted lines
> +static const struct option check_mailmap_options[] = {
> +       OPT_BOOLEAN(0, "stdin", &use_stdin,
> +                   N_("also read contacts from stdin")),
> +       OPT_BOOLEAN('z', NULL, &null_out,
> +                   N_("null-terminate output lines")),

I think OPT_BOOLEAN is deprecated in favor of OPT_BOOL (or OPT_COUNTUP if you really want -z -z -z to mean differently than -z)

> +       maybe_flush_or_die(stdout, "contact to stdout");
On error this function will print
write failure on 'contact to stdout'

maybe maybe_flush_or_die(stdout, "write contact to stdout") or something? From i18n point of view, maybe_flush_or_die should not compose a sentence this way. Let the second argument be a complete sentence so that translators have more freedom. But that's a different issue. -- Duy

Eric Sunshine· Jul 11, 2013, 05:50 UTC · re: Duy Nguyen · lore

Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command

On Wed, Jul 10, 2013 at 10:31 PM, Duy Nguyen <pclouds@gmail.com> wrote:
Show 9 quoted lines
> On Thu, Jul 11, 2013 at 2:03 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> +static const struct option check_mailmap_options[] = {
>> +       OPT_BOOLEAN(0, "stdin", &use_stdin,
>> +                   N_("also read contacts from stdin")),
>> +       OPT_BOOLEAN('z', NULL, &null_out,
>> +                   N_("null-terminate output lines")),
>
> I think OPT_BOOLEAN is deprecated in favor of OPT_BOOL (or OPT_COUNTUP
> if you really want -z -z -z to mean differently than -z)

Thanks, I knew this and intended to change it to OPT_BOOL but forgot. (The OPT_BOOLEAN was inherited from check-attr.c and check-ignore.c which I used as templates for check-mailmap.c.)

Show 11 quoted lines
>> +       maybe_flush_or_die(stdout, "contact to stdout");
>
> On error this function will print
>
> write failure on 'contact to stdout'
>
> maybe maybe_flush_or_die(stdout, "write contact to stdout") or
> something? From i18n point of view, maybe_flush_or_die should not
> compose a sentence this way. Let the second argument be a complete
> sentence so that translators have more freedom. But that's a different
> issue.

Indeed, it's not ideal. I chose "contact to stdout" for consistency with other callers, not because of a fondness for it.

  % git grep maybe_flush_or_die
  builtin/blame.c: maybe_flush_or_die(stdout, "stdout");
  builtin/check-attr.c: maybe_flush_or_die(stdout, "attribute to stdout");
  builtin/check-attr.c: maybe_flush_or_die(stdout, "attribute to stdout");
  builtin/check-ignore.c: maybe_flush_or_die(stdout, "check-ignore to stdout");
  builtin/check-ignore.c: maybe_flush_or_die(stdout, "ignore to stdout");
  builtin/hash-object.c: maybe_flush_or_die(stdout, "hash to stdout");
  builtin/rev-list.c: maybe_flush_or_die(stdout, "stdout");
  log-tree.c: maybe_flush_or_die(stdout, "stdout");

They seem pretty evenly split between just "stdout" and "foo to stdout". (I actually prefer plain "stdout" and will happily change it to that.)

-- ES
Duy Nguyen· Jul 11, 2013, 06:47 UTC · re: Eric Sunshine · lore

Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command

On Thu, Jul 11, 2013 at 12:50 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 28 quoted lines
>>> +       maybe_flush_or_die(stdout, "contact to stdout");
>>
>> On error this function will print
>>
>> write failure on 'contact to stdout'
>>
>> maybe maybe_flush_or_die(stdout, "write contact to stdout") or
>> something? From i18n point of view, maybe_flush_or_die should not
>> compose a sentence this way. Let the second argument be a complete
>> sentence so that translators have more freedom. But that's a different
>> issue.
>
> Indeed, it's not ideal. I chose "contact to stdout" for consistency
> with other callers, not because of a fondness for it.
>
>   % git grep maybe_flush_or_die
>   builtin/blame.c: maybe_flush_or_die(stdout, "stdout");
>   builtin/check-attr.c: maybe_flush_or_die(stdout, "attribute to stdout");
>   builtin/check-attr.c: maybe_flush_or_die(stdout, "attribute to stdout");
>   builtin/check-ignore.c: maybe_flush_or_die(stdout, "check-ignore to stdout");
>   builtin/check-ignore.c: maybe_flush_or_die(stdout, "ignore to stdout");
>   builtin/hash-object.c: maybe_flush_or_die(stdout, "hash to stdout");
>   builtin/rev-list.c: maybe_flush_or_die(stdout, "stdout");
>   log-tree.c: maybe_flush_or_die(stdout, "stdout");
>
> They seem pretty evenly split between just "stdout" and "foo to
> stdout". (I actually prefer plain "stdout" and will happily change it
> to that.)

Then probably stick to the convention. If this i18n thing is fixed, it has to be fixed everywhere anyway.

-- 
Duy
Antoine Pelisse· Jul 11, 2013, 06:45 UTC · re: Eric Sunshine · lore

Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command

On Wed, Jul 10, 2013 at 9:03 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 16 quoted lines
> +static void check_mailmap(struct string_list *mailmap, const char *contact)
> +{
> +       const char *name, *mail;
> +       size_t namelen, maillen;
> +       struct ident_split ident;
> +       char term = null_out ? '\0' : '\n';
> +
> +       if (split_ident_line(&ident, contact, strlen(contact)))
> +               die(_("unable to parse contact: %s"), contact);
> +
> +       name = ident.name_begin;
> +       namelen = ident.name_end - ident.name_begin;
> +       mail = ident.mail_begin;
> +       maillen = ident.mail_end - ident.mail_begin;
> +
> +       map_user(mailmap, &mail, &maillen, &name, &namelen);

Would it be useful to check the return value of this function, to display a message when the name can't mapped ?

Show 6 quoted lines
> +       if (namelen)
> +               printf("%.*s <%.*s>%c",
> +                       (int)namelen, name, (int)maillen, mail, term);
> +       else
> +               printf("<%.*s>%c", (int)maillen, mail, term);
> +}
Eric Sunshine· Jul 11, 2013, 10:28 UTC · re: Antoine Pelisse · lore

Re: [RFC/PATCH 1/4] builtin: add git-check-mailmap command

On Thu, Jul 11, 2013 at 2:45 AM, Antoine Pelisse <apelisse@gmail.com> wrote:
Show 20 quoted lines
> On Wed, Jul 10, 2013 at 9:03 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> +static void check_mailmap(struct string_list *mailmap, const char *contact)
>> +{
>> +       const char *name, *mail;
>> +       size_t namelen, maillen;
>> +       struct ident_split ident;
>> +       char term = null_out ? '\0' : '\n';
>> +
>> +       if (split_ident_line(&ident, contact, strlen(contact)))
>> +               die(_("unable to parse contact: %s"), contact);
>> +
>> +       name = ident.name_begin;
>> +       namelen = ident.name_end - ident.name_begin;
>> +       mail = ident.mail_begin;
>> +       maillen = ident.mail_end - ident.mail_begin;
>> +
>> +       map_user(mailmap, &mail, &maillen, &name, &namelen);
>
> Would it be useful to check the return value of this function, to
> display a message when the name can't mapped ?

I thought about it but decided against the added complexity (at least initially) for a few reasons.

There are only two callers in the existing code which even look at the return value:

  % git grep 'map_user('
  builtin/blame.c: map_user(&mailmap, &mailbuf, &maillen,
  builtin/shortlog.c: map_user(&log->mailmap, &mailbuf, &maillen,
&namebuf, &namelen);
  pretty.c: map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
  pretty.c: return mail_map->nr && map_user(mail_map, email,
email_len, name, name_len);
  revision.c: if (map_user(mailmap, &mail, &maillen, &name, &namelen)) {

Of those, mailmap_name() in pretty.c merely passes the return value along to its callers, but its callers don't care about it:

  % git grep 'mailmap_name('
  pretty.c: mailmap_name(&mail, &maillen, &name, &namelen);

commit_rewrite_person() in revision.c uses the return value apparently only as an optimization to decide if it should go through the effort of replacing the "old" person with the re-mapped person, and then passes the return value along to its callers, none of which care:

  % git grep 'commit_rewrite_person('
  revision.c: commit_rewrite_person(&buf, "\nauthor ", opt->mailmap);
  revision.c: commit_rewrite_person(&buf, "\ncommitter ", opt->mailmap);

The sort of optimization taken by commit_rewrite_person() is effectively lost to script and porcelain clients of check-mailmap since the cost of executing the command probably swamps any savings the client might otherwise achieve by knowing whether the person was remapped. Also, modulo possible whitespace changes, a client can compare what it passed in to what is received back to determine if remapping occurred.

As plumbing, the expectation is that most clients will pass a value in and use the output without further interpretation. If check-mailmap unconditionally adds some "mapped" or "not mapped" indicator to each returned value, then that places extra burden on all clients by forcing them to parse the result. Alternately, if the indicator is controlled by a command-line option, then (at the least) that increases documentation costs.

For people using check-mailmap as a .mailmap debugging aid, such an indicator might indeed be useful, however, it seems prudent to keep things simple initially by omitting the bells and whistles until practical experience shows that more features (and complexity) are warranted.

Show 6 quoted lines
>> +       if (namelen)
>> +               printf("%.*s <%.*s>%c",
>> +                       (int)namelen, name, (int)maillen, mail, term);
>> +       else
>> +               printf("<%.*s>%c", (int)maillen, mail, term);
>> +}
Eric Sunshine· Jul 10, 2013, 19:03 UTC · re: Eric Sunshine · lore

[RFC/PATCH 2/4] t4203: test check-mailmap command invocation

Test the command-line interface of check-mailmap. (Actual .mailmap functionality is already covered by existing tests.)

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t4203-mailmap.sh | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)
Show changes to t/t4203-mailmap.sh +50 −0
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 842b754..8645492 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -13,6 +13,11 @@ fuzz_blame () {
 }
 
 test_expect_success setup '
+	cat >contacts <<-\EOF &&
+	A U Thor <author@example.com>
+	nick1 <bugs@company.xx>
+	EOF
+
 	echo one >one &&
 	git add one &&
 	test_tick &&
@@ -23,6 +28,51 @@ test_expect_success setup '
 	git commit --author "nick1 <bugs@company.xx>" -m second
 '
 
+test_expect_success 'check-mailmap no arguments' '
+	test_must_fail git check-mailmap
+'
+
+test_expect_success 'check-mailmap arguments' '
+	cat >expect <<-\EOF &&
+	A U Thor <author@example.com>
+	nick1 <bugs@company.xx>
+	EOF
+	git check-mailmap \
+		"A U Thor <author@example.com>" \
+		"nick1 <bugs@company.xx>" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'check-mailmap --stdin' '
+	cat >expect <<-\EOF &&
+	A U Thor <author@example.com>
+	nick1 <bugs@company.xx>
+	EOF
+	git check-mailmap --stdin <contacts >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'check-mailmap --stdin arguments' '
+	cat >expect <<-\EOF &&
+	Internal Guy <bugs@company.xy>
+	EOF
+	cat <contacts >>expect &&
+	git check-mailmap --stdin "Internal Guy <bugs@company.xy>" \
+		<contacts >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'check-mailmap -z' '
+	printf "A U Thor <author@example.com>\0" >expect &&
+	printf "nick1 <bugs@company.xx>\0" >>expect &&
+	git check-mailmap -z --stdin <contacts >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'check-mailmap bogus contact' '
+	test_must_fail git check-mailmap bogus
+'
+
 cat >expect <<\EOF
 A U Thor (1):
       initial
-- 
1.8.3.2
Eric Sunshine· Jul 10, 2013, 19:04 UTC · re: Eric Sunshine · lore

[RFC/PATCH 3/4] t4203: test mailmap functionality directly rather than indirectly

With the introduction of check-mailmap, it is now possible to check .mailmap functionality directly rather than indirectly as a side-effect of other commands (such as git-shortlog), therefore, do so.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t4203-mailmap.sh | 133 ++++++++++++++++++-----------------------------------
 1 file changed, 45 insertions(+), 88 deletions(-)
Show changes to t/t4203-mailmap.sh +45 −88
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 8645492..48a000b 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -74,128 +74,96 @@ test_expect_success 'check-mailmap bogus contact' '
 '
 
 cat >expect <<\EOF
-A U Thor (1):
-      initial
-
-nick1 (1):
-      second
-
+A U Thor <author@example.com>
+nick1 <bugs@company.xx>
 EOF
 
 test_expect_success 'No mailmap' '
-	git shortlog HEAD >actual &&
+	git check-mailmap --stdin <contacts >actual &&
 	test_cmp expect actual
 '
 
 cat >expect <<\EOF
-Repo Guy (1):
-      initial
-
-nick1 (1):
-      second
-
+Repo Guy <author@example.com>
+nick1 <bugs@company.xx>
 EOF
 
 test_expect_success 'default .mailmap' '
 	echo "Repo Guy <author@example.com>" > .mailmap &&
-	git shortlog HEAD >actual &&
+	git check-mailmap --stdin <contacts >actual &&
 	test_cmp expect actual
 '
 
 # Using a mailmap file in a subdirectory of the repo here, but
 # could just as well have been a file outside of the repository
 cat >expect <<\EOF
-Internal Guy (1):
-      second
-
-Repo Guy (1):
-      initial
-
+Repo Guy <author@example.com>
+Internal Guy <bugs@company.xx>
 EOF
 test_expect_success 'mailmap.file set' '
 	mkdir -p internal_mailmap &&
 	echo "Internal Guy <bugs@company.xx>" > internal_mailmap/.mailmap &&
 	git config mailmap.file internal_mailmap/.mailmap &&
-	git shortlog HEAD >actual &&
+	git check-mailmap --stdin <contacts >actual &&
 	test_cmp expect actual
 '
 
 cat >expect <<\EOF
-External Guy (1):
-      initial
-
-Internal Guy (1):
-      second
-
+External Guy <author@example.com>
+Internal Guy <bugs@company.xx>
 EOF
 test_expect_success 'mailmap.file override' '
 	echo "External Guy <author@example.com>" >> internal_mailmap/.mailmap &&
 	git config mailmap.file internal_mailmap/.mailmap &&
-	git shortlog HEAD >actual &&
+	git check-mailmap --stdin <contacts >actual &&
 	test_cmp expect actual
 '
 
 cat >expect <<\EOF
-Repo Guy (1):
-      initial
-
-nick1 (1):
-      second
-
+Repo Guy <author@example.com>
+nick1 <bugs@company.xx>
 EOF
 
 test_expect_success 'mailmap.file non-existent' '
 	rm internal_mailmap/.mailmap &&
 	rmdir internal_mailmap &&
-	git shortlog HEAD >actual &&
+	git check-mailmap --stdin <contacts >actual &&
 	test_cmp expect actual
 '
 
 cat >expect <<\EOF
-Internal Guy (1):
-      second
-
-Repo Guy (1):
-      initial
-
+Repo Guy <author@example.com>
+Internal Guy <bugs@company.xy>
 EOF
 
 test_expect_success 'name entry after email entry' '
 	mkdir -p internal_mailmap &&
 	echo "<bugs@company.xy> <bugs@company.xx>" >internal_mailmap/.mailmap &&
 	echo "Internal Guy <bugs@company.xx>" >>internal_mailmap/.mailmap &&
-	git shortlog HEAD >actual &&
+	git check-mailmap --stdin <contacts >actual &&
 	test_cmp expect actual
 '
 
 cat >expect <<\EOF
-Internal Guy (1):
-      second
-
-Repo Guy (1):
-      initial
-
+Repo Guy <author@example.com>
+Internal Guy <bugs@company.xy>
 EOF
 
 test_expect_success 'name entry after email entry, case-insensitive' '
 	mkdir -p internal_mailmap &&
 	echo "<bugs@company.xy> <bugs@company.xx>" >internal_mailmap/.mailmap &&
 	echo "Internal Guy <BUGS@Company.xx>" >>internal_mailmap/.mailmap &&
-	git shortlog HEAD >actual &&
+	git check-mailmap --stdin <contacts >actual &&
 	test_cmp expect actual
 '
 
 cat >expect <<\EOF
-A U Thor (1):
-      initial
-
-nick1 (1):
-      second
-
+A U Thor <author@example.com>
+nick1 <bugs@company.xx>
 EOF
 test_expect_success 'No mailmap files, but configured' '
 	rm -f .mailmap internal_mailmap/.mailmap &&
-	git shortlog HEAD >actual &&
+	git check-mailmap --stdin <contacts >actual &&
 	test_cmp expect actual
 '
 
@@ -217,54 +185,43 @@ test_expect_success 'setup mailmap blob tests' '
 
 test_expect_success 'mailmap.blob set' '
 	cat >expect <<-\EOF &&
-	Blob Guy (1):
-	      second
-
-	Repo Guy (1):
-	      initial
-
+	Repo Guy <author@example.com>
+	Blob Guy <bugs@company.xx>
 	EOF
-	git -c mailmap.blob=map:just-bugs shortlog HEAD >actual &&
+	git -c mailmap.blob=map:just-bugs check-mailmap --stdin \
+		<contacts >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'mailmap.blob overrides .mailmap' '
 	cat >expect <<-\EOF &&
-	Blob Guy (2):
-	      initial
-	      second
-
+	Blob Guy <author@example.com>
+	Blob Guy <bugs@company.xx>
 	EOF
-	git -c mailmap.blob=map:both shortlog HEAD >actual &&
+	git -c mailmap.blob=map:both check-mailmap --stdin \
+		<contacts  >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'mailmap.file overrides mailmap.blob' '
 	cat >expect <<-\EOF &&
-	Blob Guy (1):
-	      second
-
-	Internal Guy (1):
-	      initial
-
+	Internal Guy <author@example.com>
+	Blob Guy <bugs@company.xx>
 	EOF
 	git \
 	  -c mailmap.blob=map:both \
 	  -c mailmap.file=internal.map \
-	  shortlog HEAD >actual &&
+	  check-mailmap --stdin <contacts >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'mailmap.blob can be missing' '
 	cat >expect <<-\EOF &&
-	Repo Guy (1):
-	      initial
-
-	nick1 (1):
-	      second
-
+	Repo Guy <author@example.com>
+	nick1 <bugs@company.xx>
 	EOF
-	git -c mailmap.blob=map:nonexistent shortlog HEAD >actual &&
+	git -c mailmap.blob=map:nonexistent check-mailmap --stdin \
+		<contacts >actual &&
 	test_cmp expect actual
 '
 
@@ -273,12 +230,12 @@ test_expect_success 'mailmap.blob defaults to off in non-bare repo' '
 	(
 		cd non-bare &&
 		test_commit one .mailmap "Fake Name <author@example.com>" &&
-		echo "     1	Fake Name" >expect &&
-		git shortlog -ns HEAD >actual &&
+		echo "Fake Name <author@example.com>" >expect &&
+		git check-mailmap "A U Thor <author@example.com>" >actual &&
 		test_cmp expect actual &&
 		rm .mailmap &&
-		echo "     1	A U Thor" >expect &&
-		git shortlog -ns HEAD >actual &&
+		echo "A U Thor <author@example.com>" >expect &&
+		git check-mailmap "A U Thor <author@example.com>" >actual &&
 		test_cmp expect actual
 	)
 '
@@ -287,8 +244,8 @@ test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '
 	git clone --bare non-bare bare &&
 	(
 		cd bare &&
-		echo "     1	Fake Name" >expect &&
-		git shortlog -ns HEAD >actual &&
+		echo "Fake Name <author@example.com>" >expect &&
+		git check-mailmap "A U Thor <author@example.com>" >actual &&
 		test_cmp expect actual
 	)
 '
-- 
1.8.3.2
Eric Sunshine· Jul 10, 2013, 19:04 UTC · re: Eric Sunshine · lore

[RFC/PATCH 4/4] t4203: consolidate test-repository setup

The simple two-commit test-repository created by 'setup' is no longer needed by any of the tests retrofitted to use check-mailmap. Subsequent, more complex tests of git-shortlog, git-log, and git-blame functionality expand the repository by adding five commits. Consolidate the creation of this seven-commit repository into a single repository-setup function.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

The code near the top of the file which creates two commits, and the code later which creates five commits have been consolidated into a single repository-setup function. The patch is primarily code movement, but the noisy diff makes it look more busy than it actually is.

 t/t4203-mailmap.sh | 69 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 31 deletions(-)
Show changes to t/t4203-mailmap.sh +38 −31
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 48a000b..7e8b3cc 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -13,19 +13,10 @@ fuzz_blame () {
 }
 
 test_expect_success setup '
-	cat >contacts <<-\EOF &&
+	cat >contacts <<-\EOF
 	A U Thor <author@example.com>
 	nick1 <bugs@company.xx>
 	EOF
-
-	echo one >one &&
-	git add one &&
-	test_tick &&
-	git commit -m initial &&
-	echo two >>one &&
-	git add one &&
-	test_tick &&
-	git commit --author "nick1 <bugs@company.xx>" -m second
 '
 
 test_expect_success 'check-mailmap no arguments' '
@@ -168,8 +159,10 @@ test_expect_success 'No mailmap files, but configured' '
 '
 
 test_expect_success 'setup mailmap blob tests' '
+	git checkout --orphan empty &&
+	git commit --allow-empty --allow-empty-message &&
 	git checkout -b map &&
-	test_when_finished "git checkout master" &&
+	test_when_finished "git checkout empty" &&
 	cat >just-bugs <<-\EOF &&
 	Blob Guy <bugs@company.xx>
 	EOF
@@ -254,28 +247,19 @@ test_expect_success 'cleanup after mailmap.blob tests' '
 	rm -f .mailmap
 '
 
-# Extended mailmap configurations should give us the following output for shortlog
-cat >expect <<\EOF
-A U Thor <author@example.com> (1):
-      initial
-
-CTO <cto@company.xx> (1):
-      seventh
+test_expect_success 'setup mailmap test repo' '
+	git checkout --orphan master &&
 
-Other Author <other@author.xx> (2):
-      third
-      fourth
-
-Santa Claus <santa.claus@northpole.xx> (2):
-      fifth
-      sixth
-
-Some Dude <some@dude.xx> (1):
-      second
+	echo one >one &&
+	git add one &&
+	test_tick &&
+	git commit -m initial &&
 
-EOF
+	echo two >>one &&
+	git add one &&
+	test_tick &&
+	git commit --author "nick1 <bugs@company.xx>" -m second &&
 
-test_expect_success 'Shortlog output (complex mapping)' '
 	echo three >>one &&
 	git add one &&
 	test_tick &&
@@ -299,8 +283,31 @@ test_expect_success 'Shortlog output (complex mapping)' '
 	echo seven >>one &&
 	git add one &&
 	test_tick &&
-	git commit --author "CTO <cto@coompany.xx>" -m seventh &&
+	git commit --author "CTO <cto@coompany.xx>" -m seventh
+'
+
+# Extended mailmap configurations should give us the following output for shortlog
+cat >expect <<\EOF
+A U Thor <author@example.com> (1):
+      initial
+
+CTO <cto@company.xx> (1):
+      seventh
 
+Other Author <other@author.xx> (2):
+      third
+      fourth
+
+Santa Claus <santa.claus@northpole.xx> (2):
+      fifth
+      sixth
+
+Some Dude <some@dude.xx> (1):
+      second
+
+EOF
+
+test_expect_success 'Shortlog output (complex mapping)' '
 	mkdir -p internal_mailmap &&
 	echo "Committed <committer@example.com>" > internal_mailmap/.mailmap &&
 	echo "<cto@company.xx>                       <cto@coompany.xx>" >> internal_mailmap/.mailmap &&
-- 
1.8.3.2

← back to recent threads