mailmap: Switch mailmap parsing to use the git_parse module

include/git2/mailmap.h

@@ -9,6 +9,7 @@
 
 #include "common.h"
 #include "types.h"
+#include "buffer.h"
 
 /**
  * @file git2/mailmap.h
@@ -35,22 +36,20 @@ typedef struct git_mailmap_entry {
 #define GIT_MAILMAP_ENTRY_INIT {GIT_MAILMAP_ENTRY_VERSION}
 
 /**
- * Create a mailmap object by parsing a mailmap file.
+ * Create a mailmap object by parsing a mailmap file. Malformed entries in the
+ * mailmap are ignored.
  *
  * The mailmap must be freed with 'git_mailmap_free'.
  *
  * @param out pointer to store the mailmap
- * @param data raw data buffer to parse
- * @param size size of the raw data buffer
- * @return 0 on success
+ * @param buffer buffer to parse the mailmap from
+ * @return 0 on success, otherwise an error code
  */
-GIT_EXTERN(int) git_mailmap_parse(
-	git_mailmap **out,
-	const char *data,
-	size_t size);
+GIT_EXTERN(int) git_mailmap_from_buffer(git_mailmap **out, git_buf *buffer);
 
 /**
- * Create a mailmap object from the given repository.
+ * Create a mailmap object from the given repository. Malformed entries in the
+ * mailmap are ignored.
  *
  * If the repository is not bare, the repository's working directory root will
  * be checked for the '.mailmap' file to be parsed.
@@ -62,20 +61,23 @@ GIT_EXTERN(int) git_mailmap_parse(
  *
  * @param out pointer to store the mailmap
  * @param repo repository to find the .mailmap in
- * @return 0 on success; GIT_ENOTFOUND if .mailmap could not be found.
+ * @return 0 on success; GIT_ENOTFOUND if .mailmap could not be found,
+ *         otherwise an error code.
  */
-GIT_EXTERN(int) git_mailmap_from_repo(
-	git_mailmap **out,
-	git_repository *repo);
+GIT_EXTERN(int) git_mailmap_from_repo(git_mailmap **out, git_repository *repo);
 
 /**
- * Free a mailmap created by 'git_mailmap_parse' or 'git_mailmap_from_repo'.
+ * Free a mailmap created by 'git_mailmap_from_buffer' or
+ * 'git_mailmap_from_repo'.
  */
 GIT_EXTERN(void) git_mailmap_free(git_mailmap *mailmap);
 
 /**
  * Resolve a name and email to the corresponding real name and email.
  *
+ * The lifetime of the string results is tied to the `mailmap`, `name`, and
+ * `email` parameters.
+ *
  * @param name_out either 'name', or the real name to use.
  *             You should NOT free this value.
  * @param email_out either 'email' or the real email to use,
@@ -85,11 +87,8 @@ GIT_EXTERN(void) git_mailmap_free(git_mailmap *mailmap);
  * @param email the email to resolve.
  */
 GIT_EXTERN(void) git_mailmap_resolve(
-	const char **name_out,
-	const char **email_out,
-	const git_mailmap *mailmap,
-	const char *name,
-	const char *email);
+	const char **name_out, const char **email_out,
+	const git_mailmap *mailmap, const char *name, const char *email);
 
 /**
  * Get the number of mailmap entries in this mailmap.
@@ -104,8 +103,7 @@ GIT_EXTERN(size_t) git_mailmap_entry_count(const git_mailmap *mailmap);
  * @return the mailmap entry at index, or NULL if it cannot be found.
  */
 GIT_EXTERN(const git_mailmap_entry *) git_mailmap_entry_byindex(
-	const git_mailmap *mailmap,
-	size_t idx);
+	const git_mailmap *mailmap, size_t idx);
 
 /**
  * Lookup a mailmap entry by name/email pair.
@@ -118,9 +116,7 @@ GIT_EXTERN(const git_mailmap_entry *) git_mailmap_entry_byindex(
  * @return the corresponding mailmap entry, or NULL if it cannot be found.
  */
 GIT_EXTERN(const git_mailmap_entry *) git_mailmap_entry_lookup(
-	const git_mailmap *mailmap,
-	const char *name,
-	const char *email);
+	const git_mailmap *mailmap, const char *name, const char *email);
 
 /** @} */
 GIT_END_DECL

src/blame.h

@@ -67,6 +67,7 @@ typedef struct git_blame__entry {
 struct git_blame {
 	char *path;
 	git_repository *repository;
+	git_mailmap *mailmap;
 	git_blame_options options;
 
 	git_vector hunks;
@@ -84,8 +85,6 @@ struct git_blame {
 	int num_lines;
 	const char *final_buf;
 	git_off_t final_buf_size;
-
-	git_mailmap *mailmap;
 };
 
 git_blame *git_blame__alloc(

src/signature.c

@@ -99,27 +99,8 @@ int git_signature_new(git_signature **sig_out, const char *name, const char *ema
 
 int git_signature_dup(git_signature **dest, const git_signature *source)
 {
-	git_signature *signature;
-
-	if (source == NULL)
-		return 0;
-
-	signature = git__calloc(1, sizeof(git_signature));
-	GITERR_CHECK_ALLOC(signature);
-
-	signature->name = git__strdup(source->name);
-	GITERR_CHECK_ALLOC(signature->name);
-
-	signature->email = git__strdup(source->email);
-	GITERR_CHECK_ALLOC(signature->email);
-
-	signature->when.time = source->when.time;
-	signature->when.offset = source->when.offset;
-	signature->when.sign = source->when.sign;
-
-	*dest = signature;
-
-	return 0;
+	/* If mailmap is NULL, this method just copies the signature */
+	return git_signature_with_mailmap(dest, source, NULL);
 }
 
 int git_signature_with_mailmap(
@@ -132,32 +113,26 @@ int git_signature_with_mailmap(
 	const char *email = NULL;
 
 	if (source == NULL)
-		goto on_error;
+		return 0;
 
 	git_mailmap_resolve(&name, &email, mailmap, source->name, source->email);
 
 	signature = git__calloc(1, sizeof(git_signature));
-	if (!signature)
-		goto on_error;
+	GITERR_CHECK_ALLOC(signature);
 
 	signature->name = git__strdup(name);
-	if (!signature->name)
-		goto on_error;
+	GITERR_CHECK_ALLOC(signature->name);
 
 	signature->email = git__strdup(email);
-	if (!signature->email)
-		goto on_error;
+	GITERR_CHECK_ALLOC(signature->email);
 
 	signature->when.time = source->when.time;
 	signature->when.offset = source->when.offset;
 	signature->when.sign = source->when.sign;
 
 	*dest = signature;
-	return 0;
 
-on_error:
-	git_signature_free(signature);
-	return -1;
+	return 0;
 }
 
 int git_signature__pdup(git_signature **dest, const git_signature *source, git_pool *pool)

tests/clar_libgit2.c

@@ -227,8 +227,7 @@ git_repository *cl_git_sandbox_reopen(void)
 		_cl_repo = NULL;
 
 		cl_git_pass(git_repository_open(
-			&_cl_repo,
-			cl_fixture_basename(_cl_sandbox)));
+			&_cl_repo, cl_fixture_basename(_cl_sandbox)));
 	}
 
 	return _cl_repo;

tests/mailmap/basic.c

@@ -15,7 +15,10 @@ const char TEST_MAILMAP[] =
 
 void test_mailmap_basic__initialize(void)
 {
-	cl_git_pass(git_mailmap_parse(&mailmap, TEST_MAILMAP, sizeof(TEST_MAILMAP) - 1));
+	git_buf buf = GIT_BUF_INIT;
+	git_buf_attach_notowned(&buf, TEST_MAILMAP, sizeof(TEST_MAILMAP) - 1);
+
+	cl_git_pass(git_mailmap_from_buffer(&mailmap, &buf));
 }
 
 void test_mailmap_basic__cleanup(void)
@@ -53,9 +56,7 @@ void test_mailmap_basic__lookup_not_found(void)
 void test_mailmap_basic__lookup(void)
 {
 	const git_mailmap_entry *entry = git_mailmap_entry_lookup(
-		mailmap,
-		"Typoed the name once",
-		"foo@baz.com");
+		mailmap, "Typoed the name once", "foo@baz.com");
 	cl_assert(entry);
 	cl_assert(!git__strcmp(entry->real_name, "Foo bar"));
 }
@@ -65,11 +66,7 @@ void test_mailmap_basic__empty_email_query(void)
 	const char *name;
 	const char *email;
 	git_mailmap_resolve(
-		&name,
-		&email,
-		mailmap,
-		"Author name",
-		"otheremail@foo.com");
+		&name, &email, mailmap, "Author name", "otheremail@foo.com");
 	cl_assert(!git__strcmp(name, "Author name"));
 	cl_assert(!git__strcmp(email, "email@foo.com"));
 }
@@ -79,20 +76,13 @@ void test_mailmap_basic__name_matching(void)
 	const char *name;
 	const char *email;
 	git_mailmap_resolve(
-		&name,
-		&email,
-		mailmap,
-		"Other Name",
-		"yetanotheremail@foo.com");
+		&name, &email, mailmap, "Other Name", "yetanotheremail@foo.com");
 	cl_assert(!git__strcmp(name, "Other Name"));
 	cl_assert(!git__strcmp(email, "email@foo.com"));
 
 	git_mailmap_resolve(
-		&name,
-		&email,
-		mailmap,
-		"Other Name That Doesn't Match",
-		"yetanotheremail@foo.com");
+		&name, &email, mailmap,
+		"Other Name That Doesn't Match", "yetanotheremail@foo.com");
 	cl_assert(!git__strcmp(name, "Other Name That Doesn't Match"));
 	cl_assert(!git__strcmp(email, "yetanotheremail@foo.com"));
 }

tests/mailmap/blame.c

@@ -15,11 +15,8 @@ void test_mailmap_blame__initialize(void)
 
 void test_mailmap_blame__cleanup(void)
 {
-	cl_git_sandbox_cleanup();
-	g_repo = NULL;
-
 	git_blame_free(g_blame);
-	g_blame = NULL;
+	cl_git_sandbox_cleanup();
 }
 
 void test_mailmap_blame__hunks(void)
@@ -32,7 +29,7 @@ void test_mailmap_blame__hunks(void)
 
 	opts.flags |= GIT_BLAME_USE_MAILMAP;
 
-	cl_check_pass(git_blame_file(&g_blame, g_repo, "file.txt", &opts));
+	cl_git_pass(git_blame_file(&g_blame, g_repo, "file.txt", &opts));
 	if (!g_blame)
 		return;
 
@@ -54,7 +51,7 @@ void test_mailmap_blame__hunks_no_mailmap(void)
 
 	g_repo = cl_git_sandbox_init("mailmap");
 
-	cl_check_pass(git_blame_file(&g_blame, g_repo, "file.txt", &opts));
+	cl_git_pass(git_blame_file(&g_blame, g_repo, "file.txt", &opts));
 	if (!g_blame)
 		return;
 

tests/mailmap/parsing.c

@@ -14,18 +14,15 @@ void test_mailmap_parsing__initialize(void)
 
 void test_mailmap_parsing__cleanup(void)
 {
-	cl_git_sandbox_cleanup();
-	g_repo = NULL;
-
 	git_mailmap_free(g_mailmap);
-	g_mailmap = NULL;
+	cl_git_sandbox_cleanup();
 }
 
 static void check_mailmap_entries(
 	const git_mailmap *mailmap, const mailmap_entry *entries, size_t entries_size)
 {
 	const git_mailmap_entry *parsed = NULL;
-	size_t idx = 0;
+	size_t idx;
 
 	/* Check that the parsed entries match */
 	cl_assert_equal_sz(entries_size, git_mailmap_entry_count(mailmap));
@@ -43,7 +40,7 @@ static void check_mailmap_resolve(
 {
 	const char *resolved_name = NULL;
 	const char *resolved_email = NULL;
-	size_t idx = 0;
+	size_t idx;
 
 	/* Check that the resolver behaves correctly */
 	for (idx = 0; idx < resolved_size; ++idx) {
@@ -60,23 +57,17 @@ static void check_mailmap_resolve(
 
 void test_mailmap_parsing__string(void)
 {
-	cl_check_pass(git_mailmap_parse(
-		&g_mailmap,
-		string_mailmap,
-		strlen(string_mailmap)));
+	git_buf buf = GIT_BUF_INIT;
+	git_buf_attach_notowned(&buf, string_mailmap, strlen(string_mailmap));
+	cl_git_pass(git_mailmap_from_buffer(&g_mailmap, &buf));
 
 	/* We should have parsed all of the entries */
-	check_mailmap_entries(
-		g_mailmap,
-		entries, ARRAY_SIZE(entries));
+	check_mailmap_entries(g_mailmap, entries, ARRAY_SIZE(entries));
 
 	/* Check that resolving the entries works */
+	check_mailmap_resolve(g_mailmap, resolved, ARRAY_SIZE(resolved));
 	check_mailmap_resolve(
-		g_mailmap,
-		resolved, ARRAY_SIZE(resolved));
-	check_mailmap_resolve(
-		g_mailmap,
-		resolved_untracked, ARRAY_SIZE(resolved_untracked));
+		g_mailmap, resolved_untracked, ARRAY_SIZE(resolved_untracked));
 }
 
 void test_mailmap_parsing__fromrepo(void)
@@ -84,40 +75,30 @@ void test_mailmap_parsing__fromrepo(void)
 	g_repo = cl_git_sandbox_init("mailmap");
 	cl_check(!git_repository_is_bare(g_repo));
 
-	cl_check_pass(git_mailmap_from_repo(&g_mailmap, g_repo));
+	cl_git_pass(git_mailmap_from_repo(&g_mailmap, g_repo));
 
 	/* We should have parsed all of the entries */
-	check_mailmap_entries(
-		g_mailmap,
-		entries, ARRAY_SIZE(entries));
+	check_mailmap_entries(g_mailmap, entries, ARRAY_SIZE(entries));
 
 	/* Check that resolving the entries works */
+	check_mailmap_resolve(g_mailmap, resolved, ARRAY_SIZE(resolved));
 	check_mailmap_resolve(
-		g_mailmap,
-		resolved, ARRAY_SIZE(resolved));
-	check_mailmap_resolve(
-		g_mailmap,
-		resolved_untracked, ARRAY_SIZE(resolved_untracked));
+		g_mailmap, resolved_untracked, ARRAY_SIZE(resolved_untracked));
 }
 
 void test_mailmap_parsing__frombare(void)
 {
 	g_repo = cl_git_sandbox_init("mailmap/.gitted");
-	cl_check_pass(git_repository_set_bare(g_repo));
+	cl_git_pass(git_repository_set_bare(g_repo));
 	cl_check(git_repository_is_bare(g_repo));
 
-	cl_check_pass(git_mailmap_from_repo(&g_mailmap, g_repo));
+	cl_git_pass(git_mailmap_from_repo(&g_mailmap, g_repo));
 
 	/* We should have parsed all of the entries, except for the untracked one */
-	check_mailmap_entries(
-		g_mailmap,
-		entries, ARRAY_SIZE(entries) - 1);
+	check_mailmap_entries(g_mailmap, entries, ARRAY_SIZE(entries) - 1);
 
 	/* Check that resolving the entries works */
+	check_mailmap_resolve(g_mailmap, resolved, ARRAY_SIZE(resolved));
 	check_mailmap_resolve(
-		g_mailmap,
-		resolved, ARRAY_SIZE(resolved));
-	check_mailmap_resolve(
-		g_mailmap,
-		resolved_bare, ARRAY_SIZE(resolved_bare));
+		g_mailmap, resolved_bare, ARRAY_SIZE(resolved_bare));
 }