Add diff drivers tests (and fix bugs)

This adds real tests for user-configured diff drivers and in the
process found a bunch of bugs.

src/diff_driver.c

@@ -38,6 +38,7 @@ struct git_diff_driver {
 	uint32_t other_flags;
 	git_array_t(regex_t) fn_patterns;
 	regex_t  word_pattern;
+	char name[GIT_FLEX_ARRAY];
 };
 
 struct git_diff_driver_registry {
@@ -132,15 +133,18 @@ static int git_diff_driver_load(
 	int error = 0, bval;
 	git_diff_driver_registry *reg;
 	git_diff_driver *drv;
+	size_t namelen = strlen(driver_name);
+	khiter_t pos;
 	git_config *cfg;
 	git_buf name = GIT_BUF_INIT;
 	const char *val;
+	bool found_driver = false;
 
 	reg = git_repository_driver_registry(repo);
 	if (!reg)
 		return -1;
 	else {
-		khiter_t pos = git_strmap_lookup_index(reg->drivers, driver_name);
+		pos = git_strmap_lookup_index(reg->drivers, driver_name);
 		if (git_strmap_valid_index(reg->drivers, pos)) {
 			*out = git_strmap_value_at(reg->drivers, pos);
 			return 0;
@@ -153,9 +157,10 @@ static int git_diff_driver_load(
 		return GIT_ENOTFOUND;
 	}
 
-	drv = git__calloc(1, sizeof(git_diff_driver));
+	drv = git__calloc(1, sizeof(git_diff_driver) + namelen + 1);
 	GITERR_CHECK_ALLOC(drv);
 	drv->type = DIFF_DRIVER_AUTO;
+	memcpy(drv->name, driver_name, namelen);
 
 	if ((error = git_buf_printf(&name, "diff.%s.binary", driver_name)) < 0)
 		goto fail;
@@ -176,51 +181,62 @@ static int git_diff_driver_load(
 		/* if diff.<driver>.binary is false, force binary checks off */
 		/* but still may have custom function context patterns, etc. */
 		drv->binary_flags = GIT_DIFF_FORCE_TEXT;
+		found_driver = true;
 	}
 
 	/* TODO: warn if diff.<name>.command or diff.<name>.textconv are set */
 
-	if ((error = git_buf_printf(&name, "diff.%s.xfuncname", driver_name)) < 0)
-		goto fail;
+	git_buf_truncate(&name, namelen + strlen("diff.."));
+	git_buf_put(&name, "xfuncname", strlen("xfuncname"));
 	if ((error = git_config_get_multivar(
 			cfg, name.ptr, NULL, diff_driver_xfuncname, drv)) < 0) {
 		if (error != GIT_ENOTFOUND)
 			goto fail;
-		/* no diff.<driver>.xfuncname values, so just continue */
-		giterr_clear();
+		giterr_clear(); /* no diff.<driver>.xfuncname, so just continue */
 	}
 
-	if ((error = git_buf_printf(&name, "diff.%s.funcname", driver_name)) < 0)
-		goto fail;
+	git_buf_truncate(&name, namelen + strlen("diff.."));
+	git_buf_put(&name, "funcname", strlen("funcname"));
 	if ((error = git_config_get_multivar(
 			cfg, name.ptr, NULL, diff_driver_funcname, drv)) < 0) {
 		if (error != GIT_ENOTFOUND)
 			goto fail;
-		/* no diff.<driver>.funcname values, so just continue */
-		giterr_clear();
+		giterr_clear(); /* no diff.<driver>.funcname, so just continue */
 	}
 
 	/* if we found any patterns, set driver type to use correct callback */
-	if (git_array_size(drv->fn_patterns) > 0)
+	if (git_array_size(drv->fn_patterns) > 0) {
 		drv->type = DIFF_DRIVER_PATTERNLIST;
+		found_driver = true;
+	}
 
-	if ((error = git_buf_printf(&name, "diff.%s.wordregex", driver_name)) < 0)
-		goto fail;
+	git_buf_truncate(&name, namelen + strlen("diff.."));
+	git_buf_put(&name, "wordregex", strlen("wordregex"));
 	if ((error = git_config_get_string(&val, cfg, name.ptr)) < 0) {
 		if (error != GIT_ENOTFOUND)
 			goto fail;
-		/* no diff.<driver>.wordregex, so just continue */
-		giterr_clear();
+		giterr_clear(); /* no diff.<driver>.wordregex, so just continue */
 	} else if ((error = regcomp(&drv->word_pattern, val, REG_EXTENDED)) != 0) {
 		/* TODO: warning about bad regex instead of failure */
 		error = giterr_set_regex(&drv->word_pattern, error);
 		goto fail;
+	} else {
+		found_driver = true;
 	}
 
 	/* TODO: look up diff.<driver>.algorithm to turn on minimal / patience
 	 * diff in drv->other_flags
 	 */
 
+	/* if no driver config found, fall back on AUTO driver */
+	if (!found_driver)
+		goto fail;
+
+	/* store driver in registry */
+	git_strmap_insert(reg->drivers, drv->name, drv, error);
+	if (error < 0)
+		goto fail;
+
 	*out = drv;
 	return 0;
 
@@ -244,18 +260,15 @@ int git_diff_driver_lookup(
 	if ((error = git_attr_get(&value, repo, 0, path, "diff")) < 0)
 		return error;
 
-	if (GIT_ATTR_FALSE(value)) {
+	if (GIT_ATTR_UNSPECIFIED(value))
+		/* just use the auto value */;
+	else if (GIT_ATTR_FALSE(value))
 		*out = &global_drivers[DIFF_DRIVER_BINARY];
-		return 0;
-	}
-
-	else if (GIT_ATTR_TRUE(value)) {
+	else if (GIT_ATTR_TRUE(value))
 		*out = &global_drivers[DIFF_DRIVER_TEXT];
-		return 0;
-	}
 
 	/* otherwise look for driver information in config and build driver */
-	if ((error = git_diff_driver_load(out, repo, value)) < 0) {
+	else if ((error = git_diff_driver_load(out, repo, value)) < 0) {
 		if (error != GIT_ENOTFOUND)
 			return error;
 		else
@@ -263,7 +276,9 @@ int git_diff_driver_lookup(
 	}
 
 use_auto:
-	*out = &global_drivers[DIFF_DRIVER_AUTO];
+	if (!*out)
+		*out = &global_drivers[DIFF_DRIVER_AUTO];
+
 	return 0;
 }
 
@@ -326,7 +341,7 @@ static int diff_context_line__pattern_match(
 
 	GIT_UNUSED(line_len);
 
-	for (i = 0; i > git_array_size(driver->fn_patterns); ++i) {
+	for (i = 0; i < git_array_size(driver->fn_patterns); ++i) {
 		if (!regexec(git_array_get(driver->fn_patterns, i), line, 0, NULL, 0))
 			return true;
 	}

tests-clar/diff/drivers.c

+#include "clar_libgit2.h"
+#include "diff_helpers.h"
+#include "repository.h"
+#include "diff_driver.h"
+
+static git_repository *g_repo = NULL;
+static git_config *g_cfg = NULL;
+
+void test_diff_drivers__initialize(void)
+{
+}
+
+void test_diff_drivers__cleanup(void)
+{
+	git_config_free(g_cfg);
+	g_cfg = NULL;
+
+	cl_git_sandbox_cleanup();
+	g_repo = NULL;
+}
+
+void test_diff_drivers__patterns(void)
+{
+	const char *one_sha = "19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13";
+	git_tree *one;
+	git_diff_list *diff;
+	git_diff_patch *patch;
+	char *text;
+	const char *expected0 = "diff --git a/untimely.txt b/untimely.txt\nindex 9a69d96..57fd0cf 100644\n--- a/untimely.txt\n+++ b/untimely.txt\n@@ -22,3 +22,5 @@ Comes through the blood of the vanguards who\n   dreamed--too soon--it had sounded.\r\n \r\n                 -- Rudyard Kipling\r\n+\r\n+Some new stuff\r\n";
+	const char *expected1 = "diff --git a/untimely.txt b/untimely.txt\nindex 9a69d96..57fd0cf 100644\nBinary files a/untimely.txt and b/untimely.txt differ\n";
+	const char *expected2 = "diff --git a/untimely.txt b/untimely.txt\nindex 9a69d96..57fd0cf 100644\n--- a/untimely.txt\n+++ b/untimely.txt\n@@ -22,3 +22,5 @@ Heaven delivers on earth the Hour that cannot be\n   dreamed--too soon--it had sounded.\r\n \r\n                 -- Rudyard Kipling\r\n+\r\n+Some new stuff\r\n";
+
+	g_repo = cl_git_sandbox_init("renames");
+
+	one = resolve_commit_oid_to_tree(g_repo, one_sha);
+
+	/* no diff */
+
+	cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, one, NULL));
+	cl_assert_equal_i(0, (int)git_diff_num_deltas(diff));
+	git_diff_list_free(diff);
+
+	/* default diff */
+
+	cl_git_append2file("renames/untimely.txt", "\r\nSome new stuff\r\n");
+
+	cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, one, NULL));
+	cl_assert_equal_i(1, (int)git_diff_num_deltas(diff));
+
+	cl_git_pass(git_diff_get_patch(&patch, NULL, diff, 0));
+	cl_git_pass(git_diff_patch_to_str(&text, patch));
+	cl_assert_equal_s(expected0, text);
+
+	git__free(text);
+	git_diff_patch_free(patch);
+	git_diff_list_free(diff);
+
+	/* attribute diff set to false */
+
+	cl_git_rewritefile("renames/.gitattributes", "untimely.txt -diff\n");
+
+	cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, one, NULL));
+	cl_assert_equal_i(1, (int)git_diff_num_deltas(diff));
+
+	cl_git_pass(git_diff_get_patch(&patch, NULL, diff, 0));
+	cl_git_pass(git_diff_patch_to_str(&text, patch));
+	cl_assert_equal_s(expected1, text);
+
+	git__free(text);
+	git_diff_patch_free(patch);
+	git_diff_list_free(diff);
+
+	/* attribute diff set to unconfigured value (should use default) */
+
+	cl_git_rewritefile("renames/.gitattributes", "untimely.txt diff=kipling0\n");
+
+	cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, one, NULL));
+	cl_assert_equal_i(1, (int)git_diff_num_deltas(diff));
+
+	cl_git_pass(git_diff_get_patch(&patch, NULL, diff, 0));
+	cl_git_pass(git_diff_patch_to_str(&text, patch));
+	cl_assert_equal_s(expected0, text);
+
+	git__free(text);
+	git_diff_patch_free(patch);
+	git_diff_list_free(diff);
+
+	/* let's define that driver */
+
+	cl_git_pass(git_repository_config(&g_cfg, g_repo));
+	cl_git_pass(git_config_set_bool(g_cfg, "diff.kipling0.binary", 1));
+
+	cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, one, NULL));
+	cl_assert_equal_i(1, (int)git_diff_num_deltas(diff));
+
+	cl_git_pass(git_diff_get_patch(&patch, NULL, diff, 0));
+	cl_git_pass(git_diff_patch_to_str(&text, patch));
+	cl_assert_equal_s(expected1, text);
+
+	git__free(text);
+	git_diff_patch_free(patch);
+	git_diff_list_free(diff);
+
+	/* let's use a real driver with some regular expressions */
+
+	git_diff_driver_registry_free(g_repo->diff_drivers);
+	g_repo->diff_drivers = NULL;
+
+	cl_git_pass(git_repository_config(&g_cfg, g_repo));
+	cl_git_pass(git_config_set_bool(g_cfg, "diff.kipling0.binary", 0));
+	cl_git_pass(git_config_set_string(g_cfg, "diff.kipling0.xfuncname", "^H"));
+
+	cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, one, NULL));
+	cl_assert_equal_i(1, (int)git_diff_num_deltas(diff));
+
+	cl_git_pass(git_diff_get_patch(&patch, NULL, diff, 0));
+	cl_git_pass(git_diff_patch_to_str(&text, patch));
+	cl_assert_equal_s(expected2, text);
+
+	git__free(text);
+	git_diff_patch_free(patch);
+	git_diff_list_free(diff);
+
+	git_tree_free(one);
+}
+