git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/7] submodule: use separate submodule repositories

From
Andrew Oakley <andrew@adoakley.name>
Date
Sep 29, 2020, 15:53 UTC
Message-ID
<20200929155350.49066-3-andrew@adoakley.name>
In-Reply-To
<20200929155350.49066-1-andrew@adoakley.name>

This commit allows `git fetch --recuse-submodules` to work correctly with partial clones, including the case where it is only the parent repository that is a partial clone.

Using a separate repository with its own object store should allow any objects that need to be fetched from a promisor or an alternate object store to work correctly.

Replacing get_submodule_ref_store with get_main_ref_store for the correct repo makes the refs store lookup objects in the correct repo (for at least the cases relevant for a fetch). We still can't fetch objects from promisor remotes, but do_oid_object_info_extended detects this and fails gracefully.

Signed-off-by: Andrew Oakley <andrew@adoakley.name>
---
 submodule.c | 105 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 60 insertions(+), 45 deletions(-)
diff --git a/submodule.c b/submodule.c
index 543b1123ae..3889dc7d9a 100644
--- a/submodule.c
+++ b/submodule.c
@@ -89,11 +89,11 @@ int is_staging_gitmodules_ok(struct index_state *istate)
 	return 1;
 }
 
-static int for_each_remote_ref_submodule(const char *submodule,
+static int for_each_remote_ref_submodule(struct repository *subrepo,
 					 each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_remote_ref(get_submodule_ref_store(submodule),
-					fn, cb_data);
+       return refs_for_each_remote_ref(get_main_ref_store(subrepo), fn,
+				       cb_data);
 }
 
 /*
@@ -879,6 +879,27 @@ static void free_submodules_oids(struct string_list *submodules)
 	string_list_clear(submodules, 1);
 }
 
+static struct repository* get_changed_submodule_repo(struct repository *r,
+						     const char *name_or_path)
+{
+	const struct submodule *submodule;
+	struct repository *subrepo;
+
+	submodule = submodule_from_name(r, &null_oid, name_or_path);
+	if (!submodule) {
+		/* Not a named submodule, try just using the path */
+		return open_submodule(name_or_path);
+	}
+
+	subrepo = xmalloc(sizeof(*subrepo));
+	if (repo_submodule_init(subrepo, r, submodule)) {
+		free(subrepo);
+		return NULL;
+	}
+
+	return subrepo;
+}
+
 static int has_remote(const char *refname, const struct object_id *oid,
 		      int flags, void *cb_data)
 {
@@ -895,7 +916,6 @@ static int append_oid_to_argv(const struct object_id *oid, void *data)
 struct has_commit_data {
 	struct repository *repo;
 	int result;
-	const char *path;
 };
 
 static int check_has_commit(const struct object_id *oid, void *data)
@@ -916,28 +936,22 @@ static int check_has_commit(const struct object_id *oid, void *data)
 		return 0;
 	default:
 		die(_("submodule entry '%s' (%s) is a %s, not a commit"),
-		    cb->path, oid_to_hex(oid), type_name(type));
+		    cb->repo->submodule_prefix, oid_to_hex(oid),
+		    type_name(type));
 	}
 }
 
-static int submodule_has_commits(struct repository *r,
-				 const char *path,
+static int submodule_has_commits(struct repository *subrepo,
 				 struct oid_array *commits)
 {
-	struct has_commit_data has_commit = { r, 1, path };
+	struct has_commit_data has_commit = { subrepo, 1 };
 
 	/*
-	 * Perform a cheap, but incorrect check for the existence of 'commits'.
-	 * This is done by adding the submodule's object store to the in-core
-	 * object store, and then querying for each commit's existence.  If we
-	 * do not have the commit object anywhere, there is no chance we have
-	 * it in the object store of the correct submodule and have it
-	 * reachable from a ref, so we can fail early without spawning rev-list
-	 * which is expensive.
+	 * Perform a check for the existence of 'commits' in the submodule's
+	 * object store.  If we do not have the commit object, there is no
+	 * chance we have it reachable from a ref, so we can fail early without
+	 * spawning rev-list which is expensive.
 	 */
-	if (add_submodule_odb(path))
-		return 0;
-
 	oid_array_for_each_unique(commits, check_has_commit, &has_commit);
 
 	if (has_commit.result) {
@@ -956,7 +970,7 @@ static int submodule_has_commits(struct repository *r,
 		prepare_submodule_repo_env(&cp.env_array);
 		cp.git_cmd = 1;
 		cp.no_stdin = 1;
-		cp.dir = path;
+		cp.dir = subrepo->submodule_prefix;
 
 		if (capture_command(&cp, &out, GIT_MAX_HEXSZ + 1) || out.len)
 			has_commit.result = 0;
@@ -967,11 +981,12 @@ static int submodule_has_commits(struct repository *r,
 	return has_commit.result;
 }
 
-static int submodule_needs_pushing(struct repository *r,
-				   const char *path,
+static int submodule_needs_pushing(struct repository *subrepo,
 				   struct oid_array *commits)
 {
-	if (!submodule_has_commits(r, path, commits))
+	const char *path = subrepo->submodule_prefix;
+
+	if (!submodule_has_commits(subrepo, commits))
 		/*
 		 * NOTE: We do consider it safe to return "no" here. The
 		 * correct answer would be "We do not know" instead of
@@ -985,7 +1000,7 @@ static int submodule_needs_pushing(struct repository *r,
 		 */
 		return 0;
 
-	if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
+	if (for_each_remote_ref_submodule(subrepo, has_remote, NULL) > 0) {
 		struct child_process cp = CHILD_PROCESS_INIT;
 		struct strbuf buf = STRBUF_INIT;
 		int needs_pushing = 0;
@@ -1032,20 +1047,18 @@ int find_unpushed_submodules(struct repository *r,
 
 	for_each_string_list_item(name, &submodules) {
 		struct oid_array *commits = name->util;
-		const struct submodule *submodule;
-		const char *path = NULL;
-
-		submodule = submodule_from_name(r, &null_oid, name->string);
-		if (submodule)
-			path = submodule->path;
-		else
-			path = default_name_or_path(name->string);
+		struct repository *subrepo;
 
-		if (!path)
+		subrepo = get_changed_submodule_repo(r, name->string);
+		if (!subrepo)
 			continue;
 
-		if (submodule_needs_pushing(r, path, commits))
-			string_list_insert(needs_pushing, path);
+		if (submodule_needs_pushing(subrepo, commits))
+			string_list_insert(needs_pushing,
+					   subrepo->submodule_prefix);
+
+		repo_clear(subrepo);
+		free(subrepo);
 	}
 
 	free_submodules_oids(&submodules);
@@ -1060,7 +1073,12 @@ static int push_submodule(const char *path,
 			  const struct string_list *push_options,
 			  int dry_run)
 {
-	if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
+	struct repository *subrepo = open_submodule(path);
+	int have_remote = for_each_remote_ref_submodule(subrepo, has_remote, NULL);
+	repo_clear(subrepo);
+	free(subrepo);
+
+	if (have_remote > 0) {
 		struct child_process cp = CHILD_PROCESS_INIT;
 		strvec_push(&cp.args, "push");
 		if (dry_run)
@@ -1219,22 +1237,19 @@ static void calculate_changed_submodule_paths(struct repository *r,
 
 	for_each_string_list_item(name, changed_submodule_names) {
 		struct oid_array *commits = name->util;
-		const struct submodule *submodule;
-		const char *path = NULL;
-
-		submodule = submodule_from_name(r, &null_oid, name->string);
-		if (submodule)
-			path = submodule->path;
-		else
-			path = default_name_or_path(name->string);
+		struct repository *subrepo;
 
-		if (!path)
+		subrepo = get_changed_submodule_repo(r, name->string);
+		if (!subrepo)
 			continue;
 
-		if (submodule_has_commits(r, path, commits)) {
+		if (submodule_has_commits(subrepo, commits)) {
 			oid_array_clear(commits);
 			*name->string = '\0';
 		}
+
+		repo_clear(subrepo);
+		free(subrepo);
 	}
 
 	string_list_remove_empty_items(changed_submodule_names, 1);

-- 
2.26.2
Previous: Andrew OakleyNext: Andrew Oakley
Message 3 of 11 in “Submodules and partial clones”
  1. 0/7 Submodules and partial clonesAndrew Oakley, Sep 29, 2020
  2. 6/7 submodule: remove add_submodule_odbAndrew Oakley, Sep 29, 2020
  3. 2/7 submodule: use separate submodule repositoriesAndrew Oakley, Sep 29, 2020
  4. 3/7 Add failing test for partial clones with submodulesAndrew Oakley, Sep 29, 2020
  5. 4/7 refs: use correct repo in refs_peel_refAndrew Oakley, Sep 29, 2020
  6. 1/7 refs: store owning repository for object lookupAndrew Oakley, Sep 29, 2020
  7. 7/7 submodule: use partial clone filterAndrew Oakley, Sep 29, 2020
  8. 5/7 merge-recursive: use separate submodule repositoryAndrew Oakley, Sep 29, 2020
  9. Jonathan TanSep 29, 2020
  10. Andrew OakleySep 30, 2020
  11. Jonathan TanSep 30, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.