Merge pull request #4522 from csware/submodules-should-report-parse-errors

Submodules-API should report .gitmodules parse errors instead of ignoring them

src/submodule.c

@@ -91,7 +91,7 @@ __KHASH_IMPL(
 
 static int submodule_alloc(git_submodule **out, git_repository *repo, const char *name);
 static git_config_backend *open_gitmodules(git_repository *repo, int gitmod);
-static git_config *gitmodules_snapshot(git_repository *repo);
+static int gitmodules_snapshot(git_config **snap, git_repository *repo);
 static int get_url_base(git_buf *url, git_repository *repo);
 static int lookup_head_remote_key(git_buf *remote_key, git_repository *repo);
 static int lookup_default_remote(git_remote **remote, git_repository *repo);
@@ -554,8 +554,11 @@ int git_submodule__map(git_repository *repo, git_strmap *map)
 		data.map = map;
 		data.repo = repo;
 
-		if ((mods = gitmodules_snapshot(repo)) == NULL)
+		if ((error = gitmodules_snapshot(&mods, repo)) < 0) {
+			if (error == GIT_ENOTFOUND)
+				error = 0;
 			goto cleanup;
+		}
 
 		data.mods = mods;
 		if ((error = git_config_foreach(
@@ -1565,7 +1568,8 @@ int git_submodule_reload(git_submodule *sm, int force)
 
 	if (!git_repository_is_bare(sm->repo)) {
 		/* refresh config data */
-		mods = gitmodules_snapshot(sm->repo);
+		if ((error = gitmodules_snapshot(&mods, sm->repo)) < 0 && error != GIT_ENOTFOUND)
+			return error;
 		if (mods != NULL) {
 			error = submodule_read_config(sm, mods);
 			git_config_free(mods);
@@ -1969,32 +1973,37 @@ static int submodule_load_from_wd_lite(git_submodule *sm)
 }
 
 /**
- * Returns a snapshot of $WORK_TREE/.gitmodules.
+ * Requests a snapshot of $WORK_TREE/.gitmodules.
  *
- * We ignore any errors and just pretend the file isn't there.
+ * Returns GIT_ENOTFOUND in case no .gitmodules file exist
  */
-static git_config *gitmodules_snapshot(git_repository *repo)
+static int gitmodules_snapshot(git_config **snap, git_repository *repo)
 {
 	const char *workdir = git_repository_workdir(repo);
-	git_config *mods = NULL, *snap = NULL;
+	git_config *mods = NULL;
 	git_buf path = GIT_BUF_INIT;
+	int error;
 
-	if (workdir != NULL) {
-		if (git_buf_joinpath(&path, workdir, GIT_MODULES_FILE) != 0)
-			return NULL;
+	if (!workdir)
+		return GIT_ENOTFOUND;
 
-		if (git_config_open_ondisk(&mods, path.ptr) < 0)
-			mods = NULL;
-	}
+	if ((error = git_buf_joinpath(&path, workdir, GIT_MODULES_FILE)) < 0)
+		return error;
 
-	git_buf_free(&path);
+	if ((error = git_config_open_ondisk(&mods, path.ptr)) < 0)
+		goto cleanup;
+
+	if ((error = git_config_snapshot(snap, mods)) < 0)
+		goto cleanup;
+
+	error = 0;
 
-	if (mods) {
-		git_config_snapshot(&snap, mods);
+cleanup:
+	if (mods)
 		git_config_free(mods);
-	}
+	git_buf_free(&path);
 
-	return snap;
+	return error;
 }
 
 static git_config_backend *open_gitmodules(

tests/submodule/lookup.c

@@ -445,3 +445,19 @@ void test_submodule_lookup__foreach_in_bare_repository_fails(void)
 
 	cl_git_fail(git_submodule_foreach(g_repo, foreach_cb, NULL));
 }
+
+void test_submodule_lookup__fail_invalid_gitmodules(void)
+{
+	git_submodule *sm;
+	sm_lookup_data data;
+	memset(&data, 0, sizeof(data));
+
+	cl_git_rewritefile("submod2/.gitmodules",
+			   "[submodule \"Test_App\"\n"
+			   "    path = Test_App\n"
+			   "    url = ../Test_App\n");
+
+	cl_git_fail(git_submodule_lookup(&sm, g_repo, "Test_App"));
+
+	cl_git_fail(git_submodule_foreach(g_repo, sm_lookup_cb, &data));
+}