submodule: update index check to check path before directory and fix tests

2 files changed, 35 insertions, 27 deletions

diff --git a/src/submodule.c b/src/submodule.c
index 87b8801c0..d1a0c822b 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -149,39 +149,44 @@ static int find_by_path(const git_config_entry *entry, void *payload)
 	return 0;
 }
 
-static int can_add_submodule(git_repository *repo, const char *path) {
-	int error;
+/*
+ * Checks to see if the submodule shares its name with a file or directory that
+ * already exists on the index. If so, the submodule cannot be added.
+ */
+static int can_add_submodule(git_repository *repo, const char *path)
+{
+	int error = 0;
 	git_index *index;
 	git_buf dir = GIT_BUF_INIT;
 
-	if ((error = git_buf_sets(&dir, path)) < 0)
-		return error;
-
-	if ((error = git_path_to_dir(&dir)) < 0)
-		return error;
-
-	/* get the index for the repo */
-
 	if ((error = git_repository_index__weakptr(&index, repo)) < 0)
-		return error;
-
-	/* see if the submodule name exists as a file on the index */
+		goto out;
 
 	if ((error = git_index_find(NULL, index, path)) == 0) {
 		giterr_set(GITERR_SUBMODULE,
-			"'%s' already exists in the index", path);
-		return GIT_EEXISTS;
+			"File '%s' already exists in the index", path);
+		error = GIT_EEXISTS;
+		goto out;
 	}
+	error = 0;
 
-	/* see if the submodule name exists as a directory on the index */
+	if ((error = git_buf_sets(&dir, path)) < 0)
+		goto out;
+
+	if ((error = git_path_to_dir(&dir)) < 0)
+		goto out;
 
 	if ((error = git_index_find_prefix(NULL, index, dir.ptr)) == 0) {
 		giterr_set(GITERR_SUBMODULE,
-			"'%s' already exists in the index", path);
-		return GIT_EEXISTS;
+			"Directory '%s' already exists in the index", path);
+		error = GIT_EEXISTS;
+		goto out;
 	}
+	error = 0;
 
-	return 0;
+out:
+	git_buf_free(&dir);
+	return error;
 }
 
 /**
diff --git a/tests/submodule/add.c b/tests/submodule/add.c
index 125ac9a55..ef7faa8c9 100644
--- a/tests/submodule/add.c
+++ b/tests/submodule/add.c
@@ -136,19 +136,21 @@ void test_submodule_add__path_exists_in_index(void)
 	git_buf dirname = GIT_BUF_INIT;
 	git_buf filename = GIT_BUF_INIT;
 
-	/* In this repo, HEAD (master) has no remote tracking branc h*/
 	g_repo = cl_git_sandbox_init("testrepo");
 
-	cl_git_pass(git_buf_joinpath(&dirname, git_repository_workdir(g_repo), "TestGitRepository"));
+	cl_git_pass(git_buf_joinpath(&dirname, git_repository_workdir(g_repo), "subdirectory"));
 	cl_git_pass(git_buf_joinpath(&filename, dirname.ptr, "test.txt"));
 
 	cl_git_pass(p_mkdir(dirname.ptr, 0700));
 	cl_git_mkfile(filename.ptr, "This is some content");
 
 	cl_git_pass(git_repository_index__weakptr(&index, g_repo));
-	cl_git_pass(git_index_add_bypath(index, "TestGitRepository/test.txt"));
+	cl_git_pass(git_index_add_bypath(index, "subdirectory/test.txt"));
+
+	cl_git_pass(p_unlink(filename.ptr));
+	cl_git_pass(p_rmdir(dirname.ptr));
 
-	cl_git_fail_with(git_submodule_add_setup(&sm, g_repo, "./", "TestGitRepository", 1), GIT_EEXISTS);
+	cl_git_fail_with(git_submodule_add_setup(&sm, g_repo, "./", "subdirectory", 1), GIT_EEXISTS);
 
 	git_submodule_free(sm);
 	git_buf_free(&dirname);
@@ -161,17 +163,18 @@ void test_submodule_add__file_exists_in_index(void)
 	git_submodule *sm;
 	git_buf name = GIT_BUF_INIT;
 
-	/* In this repo, HEAD (master) has no remote tracking branc h*/
 	g_repo = cl_git_sandbox_init("testrepo");
 
-	cl_git_pass(git_buf_joinpath(&name, git_repository_workdir(g_repo), "TestGitRepository"));
+	cl_git_pass(git_buf_joinpath(&name, git_repository_workdir(g_repo), "subdirectory"));
 
 	cl_git_mkfile(name.ptr, "Test content");
 
 	cl_git_pass(git_repository_index__weakptr(&index, g_repo));
-	cl_git_pass(git_index_add_bypath(index, "TestGitRepository"));
+	cl_git_pass(git_index_add_bypath(index, "subdirectory"));
+
+	cl_git_pass(p_unlink(name.ptr));
 
-	cl_git_fail_with(git_submodule_add_setup(&sm, g_repo, "./", "TestGitRepository", 1), GIT_EEXISTS);
+	cl_git_fail_with(git_submodule_add_setup(&sm, g_repo, "./", "subdirectory", 1), GIT_EEXISTS);
 
 	git_submodule_free(sm);
 	git_buf_free(&name);