Subject: [RFC PATCH 2/2] Submodules: Don't parse .gitmodules when it contains merge conflicts

From: Jens Lehmann <[email-masked]>
Date: Thu, 12 May 2011 23:03:01 +0200

Until now commands like "git status", "git diff" and "git fetch" would
fail when the .gitmodules file contained merge conflicts because the
config parser would call die() when hitting the conflict markers:

    "fatal: bad config file line <n> in <path>/.gitmodules"

While this was behavior was on the safe side, it is really unhelpful to
the user to have commands like status and diff fail, as these are needed
to find out what's going on. And the error message is only mildly helpful,
as it points to the right file but doesn't mention that it is unmerged.

Improve the situation by checking if the index records .gitmodules as
unmerged. When that is the case we can't make any assumptions about the
configuration to be found there after the merge conflict is resolved by
the user, so assume that all recursion is disabled unless .git/config or
the global config say otherwise.

As soon as the merge conflict is resolved and the .gitmodules file has
been staged subsequent commands again honor any configuration done there.

Signed-off-by: Jens Lehmann <[email-masked]>
---
 submodule.c                 |   23 +++++++++++++++++++++--
 t/t7506-status-submodule.sh |    6 +++---
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/submodule.c b/submodule.c
index 5294cef..cdf844c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -14,6 +14,13 @@ static struct string_list config_fetch_recurse_submodules_for_name;
 static struct string_list config_ignore_for_name;
 static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
 static struct string_list changed_submodule_paths;
+/* The following flag is set if the .gitmodules file is unmerged. We then
+ * disable recursion for all submodules where .git/config doesn't have a
+ * matching config entry because we can't guess what might be configured in
+ * .gitmodules unless the user resolves the conflict. When a command line
+ * option is given (which always overrides configuration) this flag will be
+ * ignored. */
+static int gitmodules_is_unmerged;

 static int add_submodule_odb(const char *path)
 {
@@ -63,6 +70,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 		ignore_option = unsorted_string_list_lookup(&config_ignore_for_name, path_option->util);
 		if (ignore_option)
 			handle_ignore_submodules_arg(diffopt, ignore_option->util);
+		else if (gitmodules_is_unmerged)
+			DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
 	}
 }

@@ -82,9 +91,18 @@ void gitmodules_config(void)
 	const char *work_tree = get_git_work_tree();
 	if (work_tree) {
 		struct strbuf gitmodules_path = STRBUF_INIT;
+		int pos;
 		strbuf_addstr(&gitmodules_path, work_tree);
 		strbuf_addstr(&gitmodules_path, "/.gitmodules");
-		git_config_from_file(submodule_config, gitmodules_path.buf, NULL);
+		if (read_cache() < 0)
+			die("index file corrupt");
+		pos = cache_name_pos(".gitmodules", 11);
+		if (pos >= -1) {
+			/* We have a clean, untracked or missing .gitmodules, try to parse it */
+			git_config_from_file(submodule_config, gitmodules_path.buf, NULL);
+		} else {
+			gitmodules_is_unmerged = 1;
+		}
 		strbuf_release(&gitmodules_path);
 	}
 }
@@ -434,7 +452,8 @@ int fetch_populated_submodules(int num_options, const char **options,
 					default_argv = "on-demand";
 				}
 			} else {
-				if (config_fetch_recurse_submodules == RECURSE_SUBMODULES_OFF)
+				if ((config_fetch_recurse_submodules == RECURSE_SUBMODULES_OFF) ||
+				    gitmodules_is_unmerged)
 					continue;
 				if (config_fetch_recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) {
 					if (!unsorted_string_list_lookup(&changed_submodule_paths, ce->name))
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index 3b1806c..b31b64e 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -204,7 +204,7 @@ cat > status_expect << EOF
 #
 EOF

-test_expect_failure 'status with merge conflict in .gitmodules' '
+test_expect_success 'status with merge conflict in .gitmodules' '
 	git clone . super &&
 	test_create_repo_with_commit sub1 &&
 	test_tick &&
@@ -263,7 +263,7 @@ index badaa4c,44f999a..0000000
 ++>>>>>>> add_sub1
 EOF

-test_expect_failure 'diff with merge conflict in .gitmodules' '
+test_expect_success 'diff with merge conflict in .gitmodules' '
 	(
 		cd super &&
 		git diff > ../diff_actual 2>&1
@@ -271,7 +271,7 @@ test_expect_failure 'diff with merge conflict in .gitmodules' '
 	test_cmp diff_actual diff_expect
 '

-test_expect_failure 'diff --submodule with merge conflict in .gitmodules' '
+test_expect_success 'diff --submodule with merge conflict in .gitmodules' '
 	(
 		cd super &&
 		git diff --submodule > ../diff_submodule_actual 2>&1
-- 
1.7.5.1.251.ga75dd