[PATCH] Add support for git's mailmap.

[john at keeping.me.uk (John Keeping)]

On Wed, Aug 24, 2016 at 02:28:16PM -0400, Jason A. Smith wrote:
> 
> If a mailmap file is present in the repo, it will be used to coalesce
> commits by the same person, just like git does. When no mailmap file is
> found then it functions as before.

Missing sign-off, see [1] for what this means.

[1] http://developercertificate.org/

> ---
>   cgit.h      |  3 +++
>   parsing.c   | 43 +++++++++++++++++++++++++++++++++++++++++++
>   ui-atom.c   |  9 +++++++--
>   ui-commit.c |  4 ++++
>   ui-log.c    |  4 ++++
>   ui-stats.c  |  8 ++++++--
>   6 files changed, 67 insertions(+), 4 deletions(-)
> 
> diff --git a/cgit.h b/cgit.h
> index 325432b..737ab92 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -24,6 +24,7 @@
>  #include <utf8.h>
>  #include <notes.h>
>  #include <graph.h>
> +#include <mailmap.h>
>  
>  /* Add isgraph(x) to Git's sane ctype support (see git-compat-util.h) */
>  #undef isgraph
> @@ -370,6 +371,8 @@ extern char *fmtalloc(const char *format,...);
>  extern struct commitinfo *cgit_parse_commit(struct commit *commit);
>  extern struct taginfo *cgit_parse_tag(struct tag *tag);
>  extern void cgit_parse_url(const char *url);
> +extern int cgit_read_mailmap(struct string_list *map);
> +extern int cgit_map_user(struct string_list *map, char **email, char **name);
>  
>  extern const char *cgit_repobasename(const char *reponame);
>  
> diff --git a/parsing.c b/parsing.c
> index 9dacb16..7e7edd1 100644
> --- a/parsing.c
> +++ b/parsing.c
> @@ -88,6 +88,49 @@ static void parse_user(const char *t, char **name, char **email, unsigned long *
>  	}
>  }
>  
> +/* Cgit surrounds the email address with angle brackets while git does not.
> +   Copy the email address, remove the angle brackets, perform the mailmap
> +   user lookup and finally replace the name and email address with the new
> +   values, and put the angle brackets back around the email. */

/*
 * None of the other multiline comments in this code base
 * look like the one above :-)
 */

> +int cgit_read_mailmap(struct string_list *map)
> +{
> +	int err = 0;
> +
> +	memset(map, 0, sizeof(*map));
> +	err = read_mailmap(map, NULL);
> +	return err;
> +}

Missing blank line here, but I'm not sure why the function above exists.
Other than the memset(), it doesn't do anything except call
read_mailmap() and the memset is definitely wrong, it leaks memory if
string_list is non-empty and if you're worried that map might be
uninitialized then this should be calling string_list_init(), but I'd
argue that it's up to the call site and since you always allocate on the
stack you can just assign STRING_LIST_INIT_NODUP on declaration.

> +int cgit_map_user(struct string_list *map, char **email, char **name)
> +{
> +	char *map_email, *map_name;
> +	size_t emaillen, namelen;
> +	int ret;
> +
> +	emaillen = strlen(*email);
> +	namelen = strlen(*name);
> +	map_email = xmalloc(emaillen + 1);
> +	map_name = xmalloc(namelen + 1);
> +	strncpy(map_email, *email, emaillen + 1);
> +	strncpy(map_name, *name, namelen + 1);
> +	memmove(map_email, map_email+1, emaillen-2);
> +	map_email[emaillen-2] = '\0';
> +	emaillen = emaillen - 2;
> +	ret = map_user(map, (const char**)&map_email, &emaillen,
> +			    (const char**)&map_name, &namelen);
> +	if (ret) {
> +		*email = xmalloc(strlen("<") + emaillen + strlen(">") + 1);
> +		sprintf(*email, "<%.*s>", (int)emaillen, map_email);
> +		*name = xmalloc(namelen + 1);
> +		strncpy(*name, map_name, namelen + 1);
> +	} else {
> +		/* When a mapping is found they are pointing to the
> +		   actual mailmap list entry and cannot be freed. */
> +
> +		free(map_email);
> +		free(map_name);
> +	}
> +
> +	return ret;
> +}

I think we can do a log better than this if we step back and think about
the call sites below.  There's no need to modify the author and
committer fields in struct commitinfo if we first refactor to introduce
local variables:

	const char *author = info->author;
	const char *author_email = info->author_email;

Then we can almost use map_user() directly if not for the pesky angle
brackets, but looking at the use sites, ui-atom.c is already prepared
for the email address not to have them and there are a few HTML sites
that are trivial to change.  The only difficult case is the email filter
and we could create a new cgit_print_ident() function to wrap all of
that up nicely.

It's a bigger change and will turn this single patch into a series, but
the end result will be much cleaner.

>  #ifdef NO_ICONV
>  #define reencode(a, b, c)
>  #else
> diff --git a/ui-atom.c b/ui-atom.c
> index 41838d3..e0d1144 100644
> --- a/ui-atom.c
> +++ b/ui-atom.c
> @@ -11,7 +11,8 @@
>  #include "html.h"
>  #include "ui-shared.h"
>  
> -static void add_entry(struct commit *commit, const char *host)
> +static void add_entry(struct commit *commit, const char *host,
> +		      struct string_list *mailmap)
>  {
>  	char delim = '&';
>  	char *hex;
> @@ -19,6 +20,7 @@ static void add_entry(struct commit *commit, const char *host)
>  	struct commitinfo *info;
>  
>  	info = cgit_parse_commit(commit);
> +	cgit_map_user(mailmap, &(info->author_email), &(info->author));
>  	hex = oid_to_hex(&commit->object.oid);
>  	html("<entry>\n");
>  	html("<title>");
> @@ -89,6 +91,7 @@ void cgit_print_atom(char *tip, char *path, int max_count)
>  	const char *argv[] = {NULL, tip, NULL, NULL, NULL};
>  	struct commit *commit;
>  	struct rev_info rev;
> +	struct string_list mailmap;
>  	int argc = 2;
>  
>  	if (ctx.qry.show_all)
> @@ -108,6 +111,8 @@ void cgit_print_atom(char *tip, char *path, int max_count)
>  	rev.show_root_diff = 0;
>  	rev.max_count = max_count;
>  	setup_revisions(argc, argv, &rev, NULL);
> +	cgit_read_mailmap(&mailmap);
> +	rev.mailmap = &mailmap;
>  	prepare_revision_walk(&rev);
>  
>  	host = cgit_hosturl();
> @@ -139,7 +144,7 @@ void cgit_print_atom(char *tip, char *path, int max_count)
>  		free(repourl);
>  	}
>  	while ((commit = get_revision(&rev)) != NULL) {
> -		add_entry(commit, host);
> +		add_entry(commit, host, &mailmap);
>  		free_commit_buffer(commit);
>  		free_commit_list(commit->parents);
>  		commit->parents = NULL;
> diff --git a/ui-commit.c b/ui-commit.c
> index 099d294..27d5931 100644
> --- a/ui-commit.c
> +++ b/ui-commit.c
> @@ -18,6 +18,7 @@ void cgit_print_commit(char *hex, const char *prefix)
>  	struct commit *commit, *parent;
>  	struct commitinfo *info, *parent_info;
>  	struct commit_list *p;
> +	struct string_list mailmap;
>  	struct strbuf notes = STRBUF_INIT;
>  	unsigned char sha1[20];
>  	char *tmp, *tmp2;
> @@ -38,6 +39,9 @@ void cgit_print_commit(char *hex, const char *prefix)
>  		return;
>  	}
>  	info = cgit_parse_commit(commit);
> +	cgit_read_mailmap(&mailmap);
> +	cgit_map_user(&mailmap, &(info->author_email), &(info->author));
> +	cgit_map_user(&mailmap, &(info->committer_email), &(info->committer));
>  
>  	format_display_notes(sha1, &notes, PAGE_ENCODING, 0);
>  
> diff --git a/ui-log.c b/ui-log.c
> index c97b8e0..90fe5f8 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -238,6 +238,7 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
>  			 oid_to_hex(&commit->object.oid), ctx.qry.vpath);
>  	show_commit_decorations(commit);
>  	html("</td><td>");
> +	cgit_map_user(revs->mailmap, &(info->author_email), &(info->author));
>  	cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
>  	html_txt(info->author);
>  	cgit_close_filter(ctx.repo->email_filter);
> @@ -360,6 +361,7 @@ static char *next_token(char **src)
>  void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern,
>  		    char *path, int pager, int commit_graph, int commit_sort)
>  {
> +	struct string_list mailmap;
>  	struct rev_info rev;
>  	struct commit *commit;
>  	struct argv_array rev_argv = ARGV_ARRAY_INIT;
> @@ -443,6 +445,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
>  		DIFF_XDL_SET(&rev.diffopt, IGNORE_WHITESPACE);
>  
>  	compile_grep_patterns(&rev.grep_filter);
> +	cgit_read_mailmap(&mailmap);
> +	rev.mailmap = &mailmap;
>  	prepare_revision_walk(&rev);
>  
>  	if (pager) {
> diff --git a/ui-stats.c b/ui-stats.c
> index 7acd358..9ca55f6 100644
> --- a/ui-stats.c
> +++ b/ui-stats.c
> @@ -159,7 +159,7 @@ const char *cgit_find_stats_periodname(int idx)
>  }
>  
>  static void add_commit(struct string_list *authors, struct commit *commit,
> -	const struct cgit_period *period)
> +	const struct cgit_period *period, struct string_list *mailmap)
>  {
>  	struct commitinfo *info;
>  	struct string_list_item *author, *item;
> @@ -171,6 +171,7 @@ static void add_commit(struct string_list *authors, struct commit *commit,
>  	uintptr_t *counter;
>  
>  	info = cgit_parse_commit(commit);
> +	cgit_map_user(mailmap, &(info->author_email), &(info->author));
>  	tmp = xstrdup(info->author);
>  	author = string_list_insert(authors, tmp);
>  	if (!author->util)
> @@ -209,6 +210,7 @@ static int cmp_total_commits(const void *a1, const void *a2)
>  static struct string_list collect_stats(const struct cgit_period *period)
>  {
>  	struct string_list authors;
> +	struct string_list mailmap;
>  	struct rev_info rev;
>  	struct commit *commit;
>  	const char *argv[] = {NULL, ctx.qry.head, NULL, NULL, NULL, NULL};
> @@ -237,10 +239,12 @@ static struct string_list collect_stats(const struct cgit_period *period)
>  	rev.verbose_header = 1;
>  	rev.show_root_diff = 0;
>  	setup_revisions(argc, argv, &rev, NULL);
> +	cgit_read_mailmap(&mailmap);
> +	rev.mailmap = &mailmap;
>  	prepare_revision_walk(&rev);
>  	memset(&authors, 0, sizeof(authors));
>  	while ((commit = get_revision(&rev)) != NULL) {
> -		add_commit(&authors, commit, period);
> +		add_commit(&authors, commit, period, &mailmap);
>  		free_commit_buffer(commit);
>  		free_commit_list(commit->parents);
>  		commit->parents = NULL;
> 

> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit