Commit cf1a114ba029f57c3c42bd4e8c30565cac48ba90

Patrick Steinhardt 2019-06-13T19:10:22

config_file: do not include trailing '/' for "gitdir" conditionals When evaluating "gitdir:" and "gitdir/i:" conditionals, we currently compare the given pattern with the value of `git_repository_path`. Thing is though that `git_repository_path` returns the gitdir path with trailing '/', while we actually need to match against the gitdir without it. Fix this issue by stripping the trailing '/' previous to matching. Add various tests to ensure we get this right.

diff --git a/src/config_file.c b/src/config_file.c
index ae7a32b..4fb8327 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -701,7 +701,7 @@ static int do_match_gitdir(
 	const char *condition,
 	bool case_insensitive)
 {
-	git_buf pattern = GIT_BUF_INIT;
+	git_buf pattern = GIT_BUF_INIT, gitdir = GIT_BUF_INIT;
 	int error, fnmatch_flags;
 
 	if (condition[0] == '.' && git_path_is_dirsep(condition[1])) {
@@ -722,17 +722,24 @@ static int do_match_gitdir(
 		goto out;
 	}
 
+	if ((error = git_repository_item_path(&gitdir, repo, GIT_REPOSITORY_ITEM_GITDIR)) < 0)
+		goto out;
+
+	if (git_path_is_dirsep(gitdir.ptr[gitdir.size - 1]))
+		git_buf_truncate(&gitdir, gitdir.size - 1);
+
 	fnmatch_flags = FNM_PATHNAME|FNM_LEADING_DIR;
 	if (case_insensitive)
 		fnmatch_flags |= FNM_IGNORECASE;
 
-	if ((error = p_fnmatch(pattern.ptr, git_repository_path(repo), fnmatch_flags)) < 0)
+	if ((error = p_fnmatch(pattern.ptr, gitdir.ptr, fnmatch_flags)) < 0)
 		goto out;
 
 	*matches = (error == 0);
 
 out:
 	git_buf_dispose(&pattern);
+	git_buf_dispose(&gitdir);
 	return error;
 }
 
diff --git a/tests/config/conditionals.c b/tests/config/conditionals.c
index cb1f916..4c25728 100644
--- a/tests/config/conditionals.c
+++ b/tests/config/conditionals.c
@@ -47,53 +47,55 @@ static void assert_condition_includes(const char *keyword, const char *path, boo
 	git_config_free(cfg);
 }
 
+static char *sandbox_path(git_buf *buf, const char *suffix)
+{
+	char *path = p_realpath(clar_sandbox_path(), NULL);
+	cl_assert(path);
+	cl_git_pass(git_buf_attach(buf, path, 0));
+	cl_git_pass(git_buf_joinpath(buf, buf->ptr, suffix));
+	return buf->ptr;
+}
+
 void test_config_conditionals__gitdir(void)
 {
 	git_buf path = GIT_BUF_INIT;
-	char *sandbox_path;
 
 	assert_condition_includes("gitdir", ROOT_PREFIX "/", true);
-	assert_condition_includes("gitdir", "empty_standard_repo", true);
+	assert_condition_includes("gitdir", "empty_stand", false);
+	assert_condition_includes("gitdir", "empty_stand/", false);
+	assert_condition_includes("gitdir", "empty_stand/.git", false);
+	assert_condition_includes("gitdir", "empty_stand/.git/", false);
+	assert_condition_includes("gitdir", "empty_stand*/", true);
+	assert_condition_includes("gitdir", "empty_stand*/.git", true);
+	assert_condition_includes("gitdir", "empty_stand*/.git/", false);
 	assert_condition_includes("gitdir", "empty_standard_repo/", true);
-	assert_condition_includes("gitdir", "./", true);
+	assert_condition_includes("gitdir", "empty_standard_repo/.git", true);
+	assert_condition_includes("gitdir", "empty_standard_repo/.git/", false);
+
+	assert_condition_includes("gitdir", "./", false);
 
 	assert_condition_includes("gitdir", ROOT_PREFIX "/nonexistent", false);
 	assert_condition_includes("gitdir", ROOT_PREFIX "/empty_standard_repo", false);
-	assert_condition_includes("gitdir", "empty_stand", false);
 	assert_condition_includes("gitdir", "~/empty_standard_repo", false);
 
-	sandbox_path = p_realpath(clar_sandbox_path(), NULL);
+	assert_condition_includes("gitdir", sandbox_path(&path, "/"), true);
+	assert_condition_includes("gitdir", sandbox_path(&path, "/**"), true);
 
-	git_buf_joinpath(&path, sandbox_path, "/");
-	assert_condition_includes("gitdir", path.ptr, true);
+	assert_condition_includes("gitdir", sandbox_path(&path, "empty_standard_repo/"), true);
+	assert_condition_includes("gitdir", sandbox_path(&path, "empty_standard_repo/"), true);
+	assert_condition_includes("gitdir", sandbox_path(&path, "Empty_Standard_Repo"), false);
+	assert_condition_includes("gitdir", sandbox_path(&path, "Empty_Standard_Repo/"), false);
 
-	git_buf_joinpath(&path, sandbox_path, "/*");
-	assert_condition_includes("gitdir", path.ptr, true);
-
-	git_buf_joinpath(&path, sandbox_path, "empty_standard_repo");
-	assert_condition_includes("gitdir", path.ptr, true);
-
-	git_buf_joinpath(&path, sandbox_path, "Empty_Standard_Repo");
-	assert_condition_includes("gitdir", path.ptr, false);
-
-	git__free(sandbox_path);
 	git_buf_dispose(&path);
 }
 
 void test_config_conditionals__gitdir_i(void)
 {
 	git_buf path = GIT_BUF_INIT;
-	char *sandbox_path;
-
-	sandbox_path = p_realpath(clar_sandbox_path(), NULL);
-
-	git_buf_joinpath(&path, sandbox_path, "empty_standard_repo");
-	assert_condition_includes("gitdir/i", path.ptr, true);
 
-	git_buf_joinpath(&path, sandbox_path, "EMPTY_STANDARD_REPO");
-	assert_condition_includes("gitdir/i", path.ptr, true);
+	assert_condition_includes("gitdir/i", sandbox_path(&path, "empty_standard_repo/"), true);
+	assert_condition_includes("gitdir/i", sandbox_path(&path, "EMPTY_STANDARD_REPO/"), true);
 
-	git__free(sandbox_path);
 	git_buf_dispose(&path);
 }