[PATCHv2 3/4] submodule.c: submodule_move_head works with broken submodules

From

Stefan Beller <sbeller@google.com>

Date

Apr 18, 2017, 21:37 UTC

Message-ID

<20170418213725.7901-4-sbeller@google.com>

In-Reply-To

<20170418213725.7901-1-sbeller@google.com>

If the submodule is initialized but doesn't look like it is populated,
this is a red flag and can indicate multiple sorts of failures:
(1) The submodule may be recorded at an object name, that is missing.
(2) The submodule '.git' file link may be broken and it is not pointing
    at a repository.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule.c               | 28 ++++++++++++++++++++++++----
 t/lib-submodule-update.sh | 23 ++++++++++++++++++++---
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/submodule.c b/submodule.c
index ccf8932731..20ed5b5681 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1332,10 +1332,24 @@ int submodule_move_head(const char *path,
 	int ret = 0;
 	struct child_process cp = CHILD_PROCESS_INIT;
 	const struct submodule *sub;
+	int *error_code_ptr, error_code;
 
 	if (!is_submodule_initialized(path))
 		return 0;
 
+	if (flags & SUBMODULE_MOVE_HEAD_FORCE)
+		/*
+		 * Pass non NULL pointer to is_submodule_populated_gently
+		 * to prevent die()-ing. We'll use connect_work_tree_and_git_dir
+		 * to fixup the submodule in the force case later.
+		 */
+		error_code_ptr = &error_code;
+	else
+		error_code_ptr = NULL;
+
+	if (old && !is_submodule_populated_gently(path, error_code_ptr))
+		return 0;
+
 	sub = submodule_from_path(null_sha1, path);
 
 	if (!sub)
@@ -1353,15 +1367,21 @@ int submodule_move_head(const char *path,
 				absorb_git_dir_into_superproject("", path,
 					ABSORB_GITDIR_RECURSE_SUBMODULES);
 		} else {
-			struct strbuf sb = STRBUF_INIT;
-			strbuf_addf(&sb, "%s/modules/%s",
+			char *gitdir = xstrfmt("%s/modules/%s",
 				    get_git_common_dir(), sub->name);
-			connect_work_tree_and_git_dir(path, sb.buf);
-			strbuf_release(&sb);
+			connect_work_tree_and_git_dir(path, gitdir);
+			free(gitdir);
 
 			/* make sure the index is clean as well */
 			submodule_reset_index(path);
 		}
+
+		if (old && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {
+			char *gitdir = xstrfmt("%s/modules/%s",
+				    get_git_common_dir(), sub->name);
+			connect_work_tree_and_git_dir(path, gitdir);
+			free(gitdir);
+		}
 	}
 
 	prepare_submodule_repo_env_no_git_dir(&cp.env_array);
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 22dd9e060c..f0b1b18206 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -1213,14 +1213,31 @@ test_submodule_forced_switch_recursing () {
 		)
 	'
 	# Updating a submodule from an invalid sha1 updates
-	test_expect_success "$command: modified submodule does not update submodule work tree from invalid commit" '
+	test_expect_success "$command: modified submodule does update submodule work tree from invalid commit" '
 		prolog &&
 		reset_work_tree_to_interested invalid_sub1 &&
 		(
 			cd submodule_update &&
 			git branch -t valid_sub1 origin/valid_sub1 &&
-			test_must_fail $command valid_sub1 &&
-			test_superproject_content origin/invalid_sub1
+			$command valid_sub1 &&
+			test_superproject_content origin/valid_sub1 &&
+			test_submodule_content sub1 origin/valid_sub1
+		)
+	'
+
+	# Old versions of Git were buggy writing the .git link file
+	# (e.g. before f8eaa0ba98b and then moving the superproject repo
+	# whose submodules contained absolute paths)
+	test_expect_success "$command: updating submodules fixes .git links" '
+		prolog &&
+		reset_work_tree_to_interested add_sub1 &&
+		(
+			cd submodule_update &&
+			git branch -t modify_sub1 origin/modify_sub1 &&
+			echo "gitdir: bogus/path" >sub1/.git &&
+			$command modify_sub1 &&
+			test_superproject_content origin/modify_sub1 &&
+			test_submodule_content sub1 origin/modify_sub1
 		)
 	'
 }

-- 
2.12.2.642.g1b8cc69eee.dirty