Merge pull request #3500 from ethomson/submodules_with_path

Handle submodules with paths in `git_submodule_update`

src/submodule.c

@@ -1423,7 +1423,6 @@ static int submodule_update_head(git_submodule *submodule)
 	return 0;
 }
 
-
 int git_submodule_reload(git_submodule *sm, int force)
 {
 	int error = 0;
@@ -1433,35 +1432,30 @@ int git_submodule_reload(git_submodule *sm, int force)
 
 	assert(sm);
 
-	/* refresh index data */
-	if ((error = submodule_update_index(sm)) < 0)
-		return error;
-
-	/* refresh HEAD tree data */
-	if ((error = submodule_update_head(sm)) < 0)
-		return error;
+	if (!git_repository_is_bare(sm->repo)) {
+		/* refresh config data */
+		mods = gitmodules_snapshot(sm->repo);
+		if (mods != NULL) {
+			error = submodule_read_config(sm, mods);
+			git_config_free(mods);
 
-	/* done if bare */
-	if (git_repository_is_bare(sm->repo))
-		return error;
+			if (error < 0)
+				return error;
+		}
 
-	/* refresh config data */
-	mods = gitmodules_snapshot(sm->repo);
-	if (mods != NULL) {
-		error = submodule_read_config(sm, mods);
-		git_config_free(mods);
+		/* refresh wd data */
+		sm->flags &=
+			~(GIT_SUBMODULE_STATUS_IN_WD |
+			  GIT_SUBMODULE_STATUS__WD_OID_VALID |
+			  GIT_SUBMODULE_STATUS__WD_FLAGS);
 
-		if (error < 0) {
-			return error;
-		}
+		error = submodule_load_from_wd_lite(sm);
 	}
 
-	/* refresh wd data */
-	sm->flags &=
-		~(GIT_SUBMODULE_STATUS_IN_WD | GIT_SUBMODULE_STATUS__WD_OID_VALID |
-		  GIT_SUBMODULE_STATUS__WD_FLAGS);
+	if (error == 0 && (error = submodule_update_index(sm)) == 0)
+		error = submodule_update_head(sm);
 
-	return submodule_load_from_wd_lite(sm);
+	return error;
 }
 
 static void submodule_copy_oid_maybe(

tests/resources/submodule_with_path/.gitmodules

+[submodule "testrepo"]
+	path = lib/testrepo
+	url = ../testrepo.git

tests/resources/submodule_with_path/.gitted/HEAD

+ref: refs/heads/master

tests/resources/submodule_with_path/.gitted/config

+[core]
+	repositoryformatversion = 0
+	filemode = false
+	bare = false
+	logallrefupdates = true
+	symlinks = false
+	ignorecase = true
+	hideDotFiles = dotGitOnly

tests/resources/submodule_with_path/.gitted/refs/heads/master

+89ca686bb21bfb75dda99a02313831a0c418f921

tests/submodule/submodule_helpers.c

@@ -156,6 +156,21 @@ git_repository *setup_fixture_submodule_simple(void)
 	return repo;
 }
 
+git_repository *setup_fixture_submodule_with_path(void)
+{
+	git_repository *repo = cl_git_sandbox_init("submodule_with_path");
+
+	cl_fixture_sandbox("testrepo.git");
+	p_mkdir("submodule_with_path/lib", 0777);
+	p_mkdir("submodule_with_path/lib/testrepo", 0777);
+
+	cl_set_cleanup(cleanup_fixture_submodules, "testrepo.git");
+
+	cl_git_pass(git_repository_reinit_filesystem(repo, 1));
+
+	return repo;
+}
+
 void assert__submodule_exists(
 	git_repository *repo, const char *name,
 	const char *msg, const char *file, int line)

tests/submodule/submodule_helpers.h

@@ -5,6 +5,7 @@ extern git_repository *setup_fixture_submodules(void);
 extern git_repository *setup_fixture_submod2(void);
 extern git_repository *setup_fixture_submodule_simple(void);
 extern git_repository *setup_fixture_super(void);
+extern git_repository *setup_fixture_submodule_with_path(void);
 
 extern unsigned int get_submodule_status(git_repository *, const char *);
 

tests/submodule/update.c

@@ -131,6 +131,53 @@ void test_submodule_update__update_submodule(void)
 	git_submodule_free(sm);
 }
 
+void test_submodule_update__update_submodule_with_path(void)
+{
+	git_submodule *sm;
+	git_submodule_update_options update_options = GIT_SUBMODULE_UPDATE_OPTIONS_INIT;
+	unsigned int submodule_status = 0;
+	struct update_submodule_cb_payload update_payload = { 0 };
+
+	g_repo = setup_fixture_submodule_with_path();
+
+	update_options.checkout_opts.progress_cb = checkout_progress_cb;
+	update_options.checkout_opts.progress_payload = &update_payload;
+
+	update_options.fetch_opts.callbacks.update_tips = update_tips;
+	update_options.fetch_opts.callbacks.payload = &update_payload;
+
+	/* get the submodule */
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "testrepo"));
+
+	/* verify the initial state of the submodule */
+	cl_git_pass(git_submodule_status(&submodule_status, g_repo, "testrepo", GIT_SUBMODULE_IGNORE_UNSPECIFIED));
+	cl_assert_equal_i(submodule_status, GIT_SUBMODULE_STATUS_IN_HEAD |
+		GIT_SUBMODULE_STATUS_IN_INDEX |
+		GIT_SUBMODULE_STATUS_IN_CONFIG |
+		GIT_SUBMODULE_STATUS_WD_UNINITIALIZED);
+
+	/* initialize and update the submodule */
+	cl_git_pass(git_submodule_init(sm, 0));
+	cl_git_pass(git_submodule_update(sm, 0, &update_options));
+
+	/* verify state */
+	cl_git_pass(git_submodule_status(&submodule_status, g_repo, "testrepo", GIT_SUBMODULE_IGNORE_UNSPECIFIED));
+	cl_assert_equal_i(submodule_status, GIT_SUBMODULE_STATUS_IN_HEAD |
+		GIT_SUBMODULE_STATUS_IN_INDEX |
+		GIT_SUBMODULE_STATUS_IN_CONFIG |
+		GIT_SUBMODULE_STATUS_IN_WD);
+
+	cl_assert(git_oid_streq(git_submodule_head_id(sm), "a65fedf39aefe402d3bb6e24df4d4f5fe4547750") == 0);
+	cl_assert(git_oid_streq(git_submodule_wd_id(sm), "a65fedf39aefe402d3bb6e24df4d4f5fe4547750") == 0);
+	cl_assert(git_oid_streq(git_submodule_index_id(sm), "a65fedf39aefe402d3bb6e24df4d4f5fe4547750") == 0);
+
+	/* verify that the expected callbacks have been called. */
+	cl_assert_equal_i(1, update_payload.checkout_progress_called);
+	cl_assert_equal_i(1, update_payload.update_tips_called);
+
+	git_submodule_free(sm);
+}
+
 void test_submodule_update__update_and_init_submodule(void)
 {
 	git_submodule *sm;
@@ -390,3 +437,4 @@ void test_submodule_update__can_force_update(void)
 	git_object_free(branch_commit);
 	git_reference_free(branch_reference);
 }
+