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

[PATCH v5 09/10] fetch: fetch unpopulated, changed submodules

From
Glen Choo <chooglen@google.com>
Date
Mar 8, 2022, 00:14 UTC
Message-ID
<20220308001433.94995-10-chooglen@google.com>
In-Reply-To
<20220308001433.94995-1-chooglen@google.com>

"git fetch --recurse-submodules" only considers populated submodules (i.e. submodules that can be found by iterating the index), which makes "git fetch" behave differently based on which commit is checked out. As a result, even if the user has initialized all submodules correctly, they may not fetch the necessary submodule commits, and commands like "git checkout --recurse-submodules" might fail.

Teach "git fetch" to fetch cloned, changed submodules regardless of whether they are populated. This is in addition to the current behavior of fetching populated submodules (which is always attempted regardless of what was fetched in the superproject, or even if nothing was fetched in the superproject).

A submodule may be encountered multiple times (via the list of populated submodules or via the list of changed submodules). When this happens, "git fetch" only reads the 'populated copy' and ignores the 'changed copy'. Amend the verify_fetch_result() test helper so that we can assert on which 'copy' is being read.

Signed-off-by: Glen Choo <chooglen@google.com>
---
 Documentation/fetch-options.txt |  26 ++--
 Documentation/git-fetch.txt     |  10 +-
 builtin/fetch.c                 |  14 +-
 submodule.c                     | 178 +++++++++++++++++++--
 submodule.h                     |  12 +-
 t/t5526-fetch-submodules.sh     | 268 +++++++++++++++++++++++++++++++-
 6 files changed, 462 insertions(+), 46 deletions(-)
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index f903683189..6cdd9d43c5 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -186,15 +186,23 @@ endif::git-pull[]
 ifndef::git-pull[]
 --recurse-submodules[=yes|on-demand|no]::
 	This option controls if and under what conditions new commits of
-	populated submodules should be fetched too. It can be used as a
-	boolean option to completely disable recursion when set to 'no' or to
-	unconditionally recurse into all populated submodules when set to
-	'yes', which is the default when this option is used without any
-	value. Use 'on-demand' to only recurse into a populated submodule
-	when the superproject retrieves a commit that updates the submodule's
-	reference to a commit that isn't already in the local submodule
-	clone. By default, 'on-demand' is used, unless
-	`fetch.recurseSubmodules` is set (see linkgit:git-config[1]).
+	submodules should be fetched too. When recursing through submodules,
+	`git fetch` always attempts to fetch "changed" submodules, that is, a
+	submodule that has commits that are referenced by a newly fetched
+	superproject commit but are missing in the local submodule clone. A
+	changed submodule can be fetched as long as it is present locally e.g.
+	in `$GIT_DIR/modules/` (see linkgit:gitsubmodules[7]); if the upstream
+	adds a new submodule, that submodule cannot be fetched until it is
+	cloned e.g. by `git submodule update`.
++
+When set to 'on-demand', only changed submodules are fetched. When set
+to 'yes', all populated submodules are fetched and submodules that are
+both unpopulated and changed are fetched. When set to 'no', submodules
+are never fetched.
++
+When unspecified, this uses the value of `fetch.recurseSubmodules` if it
+is set (see linkgit:git-config[1]), defaulting to 'on-demand' if unset.
+When this option is used without any value, it defaults to 'yes'.
 endif::git-pull[]
 
 -j::
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 550c16ca61..e9d364669a 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -287,12 +287,10 @@ include::transfer-data-leaks.txt[]
 
 BUGS
 ----
-Using --recurse-submodules can only fetch new commits in already checked
-out submodules right now. When e.g. upstream added a new submodule in the
-just fetched commits of the superproject the submodule itself cannot be
-fetched, making it impossible to check out that submodule later without
-having to do a fetch again. This is expected to be fixed in a future Git
-version.
+Using --recurse-submodules can only fetch new commits in submodules that are
+present locally e.g. in `$GIT_DIR/modules/`. If the upstream adds a new
+submodule, that submodule cannot be fetched until it is cloned e.g. by `git
+submodule update`. This is expected to be fixed in a future Git version.
 
 SEE ALSO
 --------
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 95832ba1df..97a89763c8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2178,13 +2178,13 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 			max_children = fetch_parallel_config;
 
 		add_options_to_argv(&options);
-		result = fetch_populated_submodules(the_repository,
-						    &options,
-						    submodule_prefix,
-						    recurse_submodules,
-						    recurse_submodules_default,
-						    verbosity < 0,
-						    max_children);
+		result = fetch_submodules(the_repository,
+					  &options,
+					  submodule_prefix,
+					  recurse_submodules,
+					  recurse_submodules_default,
+					  verbosity < 0,
+					  max_children);
 		strvec_clear(&options);
 	}
 
diff --git a/submodule.c b/submodule.c
index b36ef26752..1f5f39ce18 100644
--- a/submodule.c
+++ b/submodule.c
@@ -808,9 +808,25 @@ static const char *default_name_or_path(const char *path_or_name)
 
 /*
  * Holds relevant information for a changed submodule. Used as the .util
- * member of the changed submodule string_list_item.
+ * member of the changed submodule name string_list_item.
+ *
+ * (super_oid, path) allows the submodule config to be read from _some_
+ * .gitmodules file. We store this information the first time we find a
+ * superproject commit that points to the submodule, but this is
+ * arbitrary - we can choose any (super_oid, path) that matches the
+ * submodule's name.
  */
 struct changed_submodule_data {
+	/*
+	 * The first superproject commit in the rev walk that points to
+	 * the submodule.
+	 */
+	const struct object_id *super_oid;
+	/*
+	 * Path to the submodule in the superproject commit referenced
+	 * by 'super_oid'.
+	 */
+	char *path;
 	/* The submodule commits that have changed in the rev walk. */
 	struct oid_array new_commits;
 };
@@ -818,6 +834,7 @@ struct changed_submodule_data {
 static void changed_submodule_data_clear(struct changed_submodule_data *cs_data)
 {
 	oid_array_clear(&cs_data->new_commits);
+	free(cs_data->path);
 }
 
 static void collect_changed_submodules_cb(struct diff_queue_struct *q,
@@ -862,9 +879,14 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,
 			continue;
 
 		item = string_list_insert(changed, name);
-		if (!item->util)
+		if (item->util)
+			cs_data = item->util;
+		else {
 			item->util = xcalloc(1, sizeof(struct changed_submodule_data));
-		cs_data = item->util;
+			cs_data = item->util;
+			cs_data->super_oid = commit_oid;
+			cs_data->path = xstrdup(p->two->path);
+		}
 		oid_array_append(&cs_data->new_commits, &p->two->oid);
 	}
 }
@@ -1253,14 +1275,36 @@ void check_for_new_submodule_commits(struct object_id *oid)
 	oid_array_append(&ref_tips_after_fetch, oid);
 }
 
+/*
+ * Returns 1 if there is at least one submodule gitdir in
+ * $GIT_DIR/modules and 0 otherwise. This follows
+ * submodule_name_to_gitdir(), which looks for submodules in
+ * $GIT_DIR/modules, not $GIT_COMMON_DIR.
+ *
+ * A submodule can be moved to $GIT_DIR/modules manually by running "git
+ * submodule absorbgitdirs", or it may be initialized there by "git
+ * submodule update".
+ */
+static int repo_has_absorbed_submodules(struct repository *r)
+{
+	int ret;
+	struct strbuf buf = STRBUF_INIT;
+
+	strbuf_repo_git_path(&buf, r, "modules/");
+	ret = file_exists(buf.buf) && !is_empty_dir(buf.buf);
+	strbuf_release(&buf);
+	return ret;
+}
+
 static void calculate_changed_submodule_paths(struct repository *r,
 		struct string_list *changed_submodule_names)
 {
 	struct strvec argv = STRVEC_INIT;
 	struct string_list_item *name;
 
-	/* No need to check if there are no submodules configured */
-	if (!submodule_from_path(r, NULL, NULL))
+	/* No need to check if no submodules would be fetched */
+	if (!submodule_from_path(r, NULL, NULL) &&
+	    !repo_has_absorbed_submodules(r))
 		return;
 
 	strvec_push(&argv, "--"); /* argv[0] program name */
@@ -1333,7 +1377,16 @@ int submodule_touches_in_range(struct repository *r,
 }
 
 struct submodule_parallel_fetch {
-	int count;
+	/*
+	 * The index of the last index entry processed by
+	 * get_fetch_task_from_index().
+	 */
+	int index_count;
+	/*
+	 * The index of the last string_list entry processed by
+	 * get_fetch_task_from_changed().
+	 */
+	int changed_count;
 	struct strvec args;
 	struct repository *r;
 	const char *prefix;
@@ -1342,7 +1395,16 @@ struct submodule_parallel_fetch {
 	int quiet;
 	int result;
 
+	/*
+	 * Names of submodules that have new commits. Generated by
+	 * walking the newly fetched superproject commits.
+	 */
 	struct string_list changed_submodule_names;
+	/*
+	 * Names of submodules that have already been processed. Lets us
+	 * avoid fetching the same submodule more than once.
+	 */
+	struct string_list seen_submodule_names;
 
 	/* Pending fetches by OIDs */
 	struct fetch_task **oid_fetch_tasks;
@@ -1353,6 +1415,7 @@ struct submodule_parallel_fetch {
 #define SPF_INIT { \
 	.args = STRVEC_INIT, \
 	.changed_submodule_names = STRING_LIST_INIT_DUP, \
+	.seen_submodule_names = STRING_LIST_INIT_DUP, \
 	.submodules_with_errors = STRBUF_INIT, \
 }
 
@@ -1390,6 +1453,7 @@ struct fetch_task {
 	const struct submodule *sub;
 	unsigned free_sub : 1; /* Do we need to free the submodule? */
 	const char *default_argv; /* The default fetch mode. */
+	struct strvec git_args; /* Args for the child git process. */
 
 	struct oid_array *commits; /* Ensure these commits are fetched */
 };
@@ -1425,6 +1489,8 @@ static void fetch_task_release(struct fetch_task *p)
 	if (p->repo)
 		repo_clear(p->repo);
 	FREE_AND_NULL(p->repo);
+
+	strvec_clear(&p->git_args);
 }
 
 static struct repository *get_submodule_repo_for(struct repository *r,
@@ -1463,6 +1529,9 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
 		task->free_sub = 1;
 	}
 
+	if (string_list_lookup(&spf->seen_submodule_names, task->sub->name))
+		goto cleanup;
+
 	switch (get_fetch_recurse_config(task->sub, spf))
 	{
 	default:
@@ -1493,10 +1562,12 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
 }
 
 static struct fetch_task *
-get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err)
+get_fetch_task_from_index(struct submodule_parallel_fetch *spf,
+			  struct strbuf *err)
 {
-	for (; spf->count < spf->r->index->cache_nr; spf->count++) {
-		const struct cache_entry *ce = spf->r->index->cache[spf->count];
+	for (; spf->index_count < spf->r->index->cache_nr; spf->index_count++) {
+		const struct cache_entry *ce =
+			spf->r->index->cache[spf->index_count];
 		struct fetch_task *task;
 
 		if (!S_ISGITLINK(ce->ce_mode))
@@ -1511,7 +1582,7 @@ get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err)
 				strbuf_addf(err, _("Fetching submodule %s%s\n"),
 					    spf->prefix, ce->name);
 
-			spf->count++;
+			spf->index_count++;
 			return task;
 		} else {
 			struct strbuf empty_submodule_path = STRBUF_INIT;
@@ -1539,11 +1610,83 @@ get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err)
 	return NULL;
 }
 
+static struct fetch_task *
+get_fetch_task_from_changed(struct submodule_parallel_fetch *spf,
+			    struct strbuf *err)
+{
+	for (; spf->changed_count < spf->changed_submodule_names.nr;
+	     spf->changed_count++) {
+		struct string_list_item item =
+			spf->changed_submodule_names.items[spf->changed_count];
+		struct changed_submodule_data *cs_data = item.util;
+		struct fetch_task *task;
+
+		if (!is_tree_submodule_active(spf->r, cs_data->super_oid,cs_data->path))
+			continue;
+
+		task = fetch_task_create(spf, cs_data->path,
+					 cs_data->super_oid);
+		if (!task)
+			continue;
+
+		if (!task->repo) {
+			strbuf_addf(err, _("Could not access submodule '%s' at commit %s\n"),
+				    cs_data->path,
+				    find_unique_abbrev(cs_data->super_oid, DEFAULT_ABBREV));
+
+			fetch_task_release(task);
+			free(task);
+			continue;
+		}
+
+		if (!spf->quiet)
+			strbuf_addf(err,
+				    _("Fetching submodule %s%s at commit %s\n"),
+				    spf->prefix, task->sub->path,
+				    find_unique_abbrev(cs_data->super_oid,
+						       DEFAULT_ABBREV));
+
+		spf->changed_count++;
+		/*
+		 * NEEDSWORK: Submodules set/unset a value for
+		 * core.worktree when they are populated/unpopulated by
+		 * "git checkout" (and similar commands, see
+		 * submodule_move_head() and
+		 * connect_work_tree_and_git_dir()), but if the
+		 * submodule is unpopulated in another way (e.g. "git
+		 * rm", "rm -r"), core.worktree will still be set even
+		 * though the directory doesn't exist, and the child
+		 * process will crash while trying to chdir into the
+		 * nonexistent directory.
+		 *
+		 * In this case, we know that the submodule has no
+		 * working tree, so we can work around this by
+		 * setting "--work-tree=." (--bare does not work because
+		 * worktree settings take precedence over bare-ness).
+		 * However, this is not necessarily true in other cases,
+		 * so a generalized solution is still necessary.
+		 *
+		 * Possible solutions:
+		 * - teach "git [add|rm]" to unset core.worktree and
+		 *   discourage users from removing submodules without
+		 *   using a Git command.
+		 * - teach submodule child processes to ignore stale
+		 *   core.worktree values.
+		 */
+		strvec_push(&task->git_args, "--work-tree=.");
+		return task;
+	}
+	return NULL;
+}
+
 static int get_next_submodule(struct child_process *cp, struct strbuf *err,
 			      void *data, void **task_cb)
 {
 	struct submodule_parallel_fetch *spf = data;
-	struct fetch_task *task = get_fetch_task(spf, err);
+	struct fetch_task *task =
+		get_fetch_task_from_index(spf, err);
+	if (!task)
+		task = get_fetch_task_from_changed(spf, err);
 
 	if (task) {
 		struct strbuf submodule_prefix = STRBUF_INIT;
@@ -1553,6 +1696,8 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err,
 		prepare_submodule_repo_env_in_gitdir(&cp->env_array);
 		cp->git_cmd = 1;
 		strvec_init(&cp->args);
+		if (task->git_args.nr)
+			strvec_pushv(&cp->args, task->git_args.v);
 		strvec_pushv(&cp->args, spf->args.v);
 		strvec_push(&cp->args, task->default_argv);
 		strvec_push(&cp->args, "--submodule-prefix");
@@ -1564,6 +1709,7 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err,
 		*task_cb = task;
 
 		strbuf_release(&submodule_prefix);
+		string_list_insert(&spf->seen_submodule_names, task->sub->name);
 		return 1;
 	}
 
@@ -1678,11 +1824,11 @@ static int fetch_finish(int retvalue, struct strbuf *err,
 	return 0;
 }
 
-int fetch_populated_submodules(struct repository *r,
-			       const struct strvec *options,
-			       const char *prefix, int command_line_option,
-			       int default_option,
-			       int quiet, int max_parallel_jobs)
+int fetch_submodules(struct repository *r,
+		     const struct strvec *options,
+		     const char *prefix, int command_line_option,
+		     int default_option,
+		     int quiet, int max_parallel_jobs)
 {
 	int i;
 	struct submodule_parallel_fetch spf = SPF_INIT;
diff --git a/submodule.h b/submodule.h
index 784ceffc0e..61bebde319 100644
--- a/submodule.h
+++ b/submodule.h
@@ -88,12 +88,12 @@ int should_update_submodules(void);
  */
 const struct submodule *submodule_from_ce(const struct cache_entry *ce);
 void check_for_new_submodule_commits(struct object_id *oid);
-int fetch_populated_submodules(struct repository *r,
-			       const struct strvec *options,
-			       const char *prefix,
-			       int command_line_option,
-			       int default_option,
-			       int quiet, int max_parallel_jobs);
+int fetch_submodules(struct repository *r,
+		     const struct strvec *options,
+		     const char *prefix,
+		     int command_line_option,
+		     int default_option,
+		     int quiet, int max_parallel_jobs);
 unsigned is_submodule_modified(const char *path, int ignore_untracked);
 int submodule_uses_gitfile(const char *path);
 
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index aa6bb9867c..43dada8544 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -12,17 +12,29 @@ pwd=$(pwd)
 
 write_expected_sub () {
 	NEW_HEAD=$1 &&
+	SUPER_HEAD=$2 &&
 	cat >"$pwd/expect.err.sub" <<-EOF
-	Fetching submodule submodule
+	Fetching submodule submodule${SUPER_HEAD:+ at commit $SUPER_HEAD}
 	From $pwd/submodule
 	   OLD_HEAD..$NEW_HEAD  sub        -> origin/sub
 	EOF
 }
 
+write_expected_sub2 () {
+	NEW_HEAD=$1 &&
+	SUPER_HEAD=$2 &&
+	cat >"$pwd/expect.err.sub2" <<-EOF
+	Fetching submodule submodule2${SUPER_HEAD:+ at commit $SUPER_HEAD}
+	From $pwd/submodule2
+	   OLD_HEAD..$NEW_HEAD  sub2       -> origin/sub2
+	EOF
+}
+
 write_expected_deep () {
 	NEW_HEAD=$1 &&
+	SUB_HEAD=$2 &&
 	cat >"$pwd/expect.err.deep" <<-EOF
-	Fetching submodule submodule/subdir/deepsubmodule
+	Fetching submodule submodule/subdir/deepsubmodule${SUB_HEAD:+ at commit $SUB_HEAD}
 	From $pwd/deepsubmodule
 	   OLD_HEAD..$NEW_HEAD  deep       -> origin/deep
 	EOF
@@ -106,6 +118,10 @@ verify_fetch_result () {
 	then
 		cat expect.err.deep >>expect.err.combined
 	fi &&
+	if test -f expect.err.sub2
+	then
+		cat expect.err.sub2 >>expect.err.combined
+	fi &&
 	sed -e 's/[0-9a-f][0-9a-f]*\.\./OLD_HEAD\.\./' "$ACTUAL_ERR" >actual.err.cmp &&
 	test_cmp expect.err.combined actual.err.cmp
 }
@@ -419,6 +435,147 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess
 	verify_fetch_result actual.err
 '
 
+# These tests verify that we can fetch submodules that aren't in the
+# index.
+#
+# First, test the simple case where the index is empty and we only fetch
+# submodules that are not in the index.
+test_expect_success 'setup downstream branch without submodules' '
+	(
+		cd downstream &&
+		git checkout --recurse-submodules -b no-submodules &&
+		git rm .gitmodules &&
+		git rm submodule &&
+		git commit -m "no submodules" &&
+		git checkout --recurse-submodules super
+	)
+'
+
+test_expect_success "'--recurse-submodules=on-demand' should fetch submodule commits if the submodule is changed but the index has no submodules" '
+	add_submodule_commits &&
+	add_superproject_commits &&
+	# Fetch the new superproject commit
+	(
+		cd downstream &&
+		git switch --recurse-submodules no-submodules &&
+		git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err
+	) &&
+	super_head=$(git rev-parse --short HEAD) &&
+	sub_head=$(git -C submodule rev-parse --short HEAD) &&
+	deep_head=$(git -C submodule/subdir/deepsubmodule rev-parse --short HEAD) &&
+
+	# assert that these are fetched from commits, not the index
+	write_expected_sub $sub_head $super_head &&
+	write_expected_deep $deep_head $sub_head &&
+
+	test_must_be_empty actual.out &&
+	verify_fetch_result actual.err
+'
+
+test_expect_success "'--recurse-submodules' should fetch submodule commits if the submodule is changed but the index has no submodules" '
+	add_submodule_commits &&
+	add_superproject_commits &&
+	# Fetch the new superproject commit
+	(
+		cd downstream &&
+		git switch --recurse-submodules no-submodules &&
+		git fetch --recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	super_head=$(git rev-parse --short HEAD) &&
+	sub_head=$(git -C submodule rev-parse --short HEAD) &&
+	deep_head=$(git -C submodule/subdir/deepsubmodule rev-parse --short HEAD) &&
+
+	# assert that these are fetched from commits, not the index
+	write_expected_sub $sub_head $super_head &&
+	write_expected_deep $deep_head $sub_head &&
+
+	test_must_be_empty actual.out &&
+	verify_fetch_result actual.err
+'
+
+test_expect_success "'--recurse-submodules' should ignore changed, inactive submodules" '
+	add_submodule_commits &&
+	add_superproject_commits &&
+
+	# Fetch the new superproject commit
+	(
+		cd downstream &&
+		git switch --recurse-submodules no-submodules &&
+		git -c submodule.submodule.active=false fetch --recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	test_must_be_empty actual.out &&
+	super_head=$(git rev-parse --short HEAD) &&
+	write_expected_super $super_head &&
+	# Neither should be fetched because the submodule is inactive
+	rm expect.err.sub &&
+	rm expect.err.deep &&
+	verify_fetch_result actual.err
+'
+
+# Now that we know we can fetch submodules that are not in the index,
+# test that we can fetch index and non-index submodules in the same
+# operation.
+test_expect_success 'setup downstream branch with other submodule' '
+	mkdir submodule2 &&
+	(
+		cd submodule2 &&
+		git init &&
+		echo sub2content >sub2file &&
+		git add sub2file &&
+		git commit -a -m new &&
+		git branch -M sub2
+	) &&
+	git checkout -b super-sub2-only &&
+	git submodule add "$pwd/submodule2" submodule2 &&
+	git commit -m "add sub2" &&
+	git checkout super &&
+	(
+		cd downstream &&
+		git fetch --recurse-submodules origin &&
+		git checkout super-sub2-only &&
+		# Explicitly run "git submodule update" because sub2 is new
+		# and has not been cloned.
+		git submodule update --init &&
+		git checkout --recurse-submodules super
+	)
+'
+
+test_expect_success "'--recurse-submodules' should fetch submodule commits in changed submodules and the index" '
+	test_when_finished "rm expect.err.sub2" &&
+	# Create new commit in origin/super
+	add_submodule_commits &&
+	add_superproject_commits &&
+
+	# Create new commit in origin/super-sub2-only
+	git checkout super-sub2-only &&
+	(
+		cd submodule2 &&
+		test_commit --no-tag foo
+	) &&
+	git add submodule2 &&
+	git commit -m "new submodule2" &&
+
+	git checkout super &&
+	(
+		cd downstream &&
+		git fetch --recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	test_must_be_empty actual.out &&
+	sub2_head=$(git -C submodule2 rev-parse --short HEAD) &&
+	super_head=$(git rev-parse --short super) &&
+	super_sub2_only_head=$(git rev-parse --short super-sub2-only) &&
+	write_expected_sub2 $sub2_head $super_sub2_only_head &&
+
+	# write_expected_super cannot handle >1 branch. Since this is a
+	# one-off, construct expect.err.super manually.
+	cat >"$pwd/expect.err.super" <<-EOF &&
+	From $pwd/.
+	   OLD_HEAD..$super_head  super           -> origin/super
+	   OLD_HEAD..$super_sub2_only_head  super-sub2-only -> origin/super-sub2-only
+	EOF
+	verify_fetch_result actual.err
+'
+
 test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" '
 	add_submodule_commits &&
 	echo a >> file &&
@@ -861,4 +1018,111 @@ test_expect_success 'recursive fetch after deinit a submodule' '
 	test_cmp expect actual
 '
 
+test_expect_success 'setup repo with upstreams that share a submodule name' '
+	mkdir same-name-1 &&
+	(
+		cd same-name-1 &&
+		git init -b main &&
+		test_commit --no-tag a
+	) &&
+	git clone same-name-1 same-name-2 &&
+	# same-name-1 and same-name-2 both add a submodule with the
+	# name "submodule"
+	(
+		cd same-name-1 &&
+		mkdir submodule &&
+		git -C submodule init -b main &&
+		test_commit -C submodule --no-tag a1 &&
+		git submodule add "$pwd/same-name-1/submodule" &&
+		git add submodule &&
+		git commit -m "super-a1"
+	) &&
+	(
+		cd same-name-2 &&
+		mkdir submodule &&
+		git -C submodule init -b main &&
+		test_commit -C submodule --no-tag a2 &&
+		git submodule add "$pwd/same-name-2/submodule" &&
+		git add submodule &&
+		git commit -m "super-a2"
+	) &&
+	git clone same-name-1 -o same-name-1 same-name-downstream &&
+	(
+		cd same-name-downstream &&
+		git remote add same-name-2 ../same-name-2 &&
+		git fetch --all &&
+		# init downstream with same-name-1
+		git submodule update --init
+	)
+'
+
+test_expect_success 'fetch --recurse-submodules updates name-conflicted, populated submodule' '
+	test_when_finished "git -C same-name-downstream checkout main" &&
+	(
+		cd same-name-1 &&
+		test_commit -C submodule --no-tag b1 &&
+		git add submodule &&
+		git commit -m "super-b1"
+	) &&
+	(
+		cd same-name-2 &&
+		test_commit -C submodule --no-tag b2 &&
+		git add submodule &&
+		git commit -m "super-b2"
+	) &&
+	(
+		cd same-name-downstream &&
+		# even though the .gitmodules is correct, we cannot
+		# fetch from same-name-2
+		git checkout same-name-2/main &&
+		git fetch --recurse-submodules same-name-1 &&
+		test_must_fail git fetch --recurse-submodules same-name-2
+	) &&
+	super_head1=$(git -C same-name-1 rev-parse HEAD) &&
+	git -C same-name-downstream cat-file -e $super_head1 &&
+
+	super_head2=$(git -C same-name-2 rev-parse HEAD) &&
+	git -C same-name-downstream cat-file -e $super_head2 &&
+
+	sub_head1=$(git -C same-name-1/submodule rev-parse HEAD) &&
+	git -C same-name-downstream/submodule cat-file -e $sub_head1 &&
+
+	sub_head2=$(git -C same-name-2/submodule rev-parse HEAD) &&
+	test_must_fail git -C same-name-downstream/submodule cat-file -e $sub_head2
+'
+
+test_expect_success 'fetch --recurse-submodules updates name-conflicted, unpopulated submodule' '
+	(
+		cd same-name-1 &&
+		test_commit -C submodule --no-tag c1 &&
+		git add submodule &&
+		git commit -m "super-c1"
+	) &&
+	(
+		cd same-name-2 &&
+		test_commit -C submodule --no-tag c2 &&
+		git add submodule &&
+		git commit -m "super-c2"
+	) &&
+	(
+		cd same-name-downstream &&
+		git checkout main &&
+		git rm .gitmodules &&
+		git rm submodule &&
+		git commit -m "no submodules" &&
+		git fetch --recurse-submodules same-name-1
+	) &&
+	head1=$(git -C same-name-1/submodule rev-parse HEAD) &&
+	head2=$(git -C same-name-2/submodule rev-parse HEAD) &&
+	(
+		cd same-name-downstream/.git/modules/submodule &&
+		# The submodule has core.worktree pointing to the "git
+		# rm"-ed directory, overwrite the invalid value. See
+		# comment in get_fetch_task_from_changed() for more
+		# information.
+		git --work-tree=. cat-file -e $head1 &&
+		test_must_fail git --work-tree=. cat-file -e $head2
+	)
+'
+
 test_done
-- 
2.33.GIT
Previous: Glen ChooNext: Glen Choo
Message 134 of 149 in “fetch --recurse-submodules: fetch unpopulated submodules”
  1. 0/8 fetch --recurse-submodules: fetch unpopulated submodulesGlen Choo, Feb 10, 2022
  2. 1/8 submodule: inline submodule_commits() into callerGlen Choo, Feb 10, 2022
  3. 2/8 submodule: store new submodule commits oid_array in a structGlen Choo, Feb 10, 2022
  4. Jonathan TanFeb 10, 2022
  5. Junio C HamanoFeb 10, 2022
  6. 4/8 t5526: introduce test helper to assert on fetchesGlen Choo, Feb 10, 2022
  7. 3/8 submodule: make static functions read submodules from commitsGlen Choo, Feb 10, 2022
  8. Jonathan TanFeb 10, 2022
  9. Glen ChooFeb 11, 2022
  10. Glen ChooFeb 11, 2022
  11. 5/8 t5526: use grep to assert on fetchesGlen Choo, Feb 10, 2022
  12. Jonathan TanFeb 10, 2022
  13. 6/8 submodule: extract get_fetch_task()Glen Choo, Feb 10, 2022
  14. Jonathan TanFeb 10, 2022
  15. 7/8 fetch: fetch unpopulated, changed submodulesGlen Choo, Feb 10, 2022
  16. Junio C HamanoFeb 10, 2022
  17. Glen ChooFeb 11, 2022
  18. Junio C HamanoFeb 11, 2022
  19. Jonathan TanFeb 10, 2022
  20. Glen ChooFeb 14, 2022
  21. Glen ChooFeb 14, 2022
  22. Glen ChooFeb 14, 2022
  23. 8/8 submodule: fix bug and remove add_submodule_odb()Glen Choo, Feb 10, 2022
  24. Junio C HamanoFeb 10, 2022
  25. Glen ChooFeb 11, 2022
  26. Jonathan TanFeb 10, 2022
  27. Glen ChooFeb 11, 2022
  28. Junio C HamanoFeb 11, 2022
  29. Glen ChooFeb 14, 2022
  30. Junio C HamanoFeb 10, 2022
  31. Glen ChooFeb 10, 2022
  32. Junio C HamanoFeb 10, 2022
  33. Glen ChooFeb 11, 2022
  34. 0/9 fetch --recurse-submodules: fetch unpopulated submodulesGlen Choo, Feb 15, 2022
  35. 1/9 t5526: introduce test helper to assert on fetchesGlen Choo, Feb 15, 2022
  36. Ævar Arnfjörð BjarmasonFeb 15, 2022
  37. 2/9 t5526: use grep to assert on fetchesGlen Choo, Feb 15, 2022
  38. Ævar Arnfjörð BjarmasonFeb 15, 2022
  39. Glen ChooFeb 16, 2022
  40. Ævar Arnfjörð BjarmasonFeb 16, 2022
  41. Glen ChooFeb 17, 2022
  42. Ævar Arnfjörð BjarmasonFeb 17, 2022
  43. Glen ChooFeb 17, 2022
  44. 3/9 submodule: make static functions read submodules from commitsGlen Choo, Feb 15, 2022
  45. Jonathan TanFeb 15, 2022
  46. Glen ChooFeb 16, 2022
  47. Ævar Arnfjörð BjarmasonFeb 15, 2022
  48. Glen ChooFeb 16, 2022
  49. 4/9 submodule: inline submodule_commits() into callerGlen Choo, Feb 15, 2022
  50. Ævar Arnfjörð BjarmasonFeb 15, 2022
  51. 5/9 submodule: store new submodule commits oid_array in a structGlen Choo, Feb 15, 2022
  52. Ævar Arnfjörð BjarmasonFeb 15, 2022
  53. 6/9 submodule: extract get_fetch_task()Glen Choo, Feb 15, 2022
  54. 7/9 fetch: fetch unpopulated, changed submodulesGlen Choo, Feb 15, 2022
  55. Jonathan TanFeb 15, 2022
  56. Glen ChooFeb 16, 2022
  57. Glen ChooFeb 16, 2022
  58. Ævar Arnfjörð BjarmasonFeb 16, 2022
  59. Glen ChooFeb 16, 2022
  60. Ævar Arnfjörð BjarmasonFeb 15, 2022
  61. 8/9 submodule: read shallows when finding changed submodulesGlen Choo, Feb 15, 2022
  62. Jonathan TanFeb 15, 2022
  63. 9/9 submodule: fix latent check_has_commit() bugGlen Choo, Feb 15, 2022
  64. Jonathan TanFeb 15, 2022
  65. 00/10 fetch --recurse-submodules: fetch unpopulated submodulesGlen Choo, Feb 24, 2022
  66. 01/10 t5526: introduce test helper to assert on fetchesGlen Choo, Feb 24, 2022
  67. Junio C HamanoFeb 25, 2022
  68. 02/10 t5526: stop asserting on stderr literallyGlen Choo, Feb 24, 2022
  69. Ævar Arnfjörð BjarmasonFeb 24, 2022
  70. Glen ChooFeb 24, 2022
  71. Eric SunshineFeb 24, 2022
  72. Jonathan TanFeb 24, 2022
  73. Glen ChooFeb 25, 2022
  74. 04/10 submodule: make static functions read submodules from commitsGlen Choo, Feb 24, 2022
  75. 03/10 t5526: create superproject commits with test helperGlen Choo, Feb 24, 2022
  76. Jonathan TanFeb 24, 2022
  77. Glen ChooFeb 25, 2022
  78. Ævar Arnfjörð BjarmasonFeb 25, 2022
  79. Glen ChooFeb 28, 2022
  80. 05/10 submodule: inline submodule_commits() into callerGlen Choo, Feb 24, 2022
  81. 06/10 submodule: store new submodule commits oid_array in a structGlen Choo, Feb 24, 2022
  82. 07/10 submodule: extract get_fetch_task()Glen Choo, Feb 24, 2022
  83. Jonathan TanFeb 24, 2022
  84. 08/10 submodule: move logic into fetch_task_create()Glen Choo, Feb 24, 2022
  85. Jonathan TanFeb 24, 2022
  86. 10/10 submodule: fix latent check_has_commit() bugGlen Choo, Feb 24, 2022
  87. 09/10 fetch: fetch unpopulated, changed submodulesGlen Choo, Feb 24, 2022
  88. Junio C HamanoFeb 24, 2022
  89. Glen ChooFeb 25, 2022
  90. Junio C HamanoFeb 25, 2022
  91. Glen ChooFeb 25, 2022
  92. Jonathan TanFeb 25, 2022
  93. Glen ChooFeb 25, 2022
  94. Jonathan TanMar 4, 2022
  95. Glen ChooMar 5, 2022
  96. Jonathan TanMar 4, 2022
  97. Junio C HamanoFeb 26, 2022
  98. Johannes SchindelinMar 1, 2022
  99. Junio C HamanoMar 1, 2022
  100. Glen ChooMar 2, 2022
  101. Junio C HamanoMar 1, 2022
  102. 00/10 fetch --recurse-submodules: fetch unpopulated submodulesGlen Choo, Mar 4, 2022
  103. 01/10 t5526: introduce test helper to assert on fetchesGlen Choo, Mar 4, 2022
  104. Junio C HamanoMar 4, 2022
  105. Glen ChooMar 4, 2022
  106. 02/10 t5526: stop asserting on stderr literallyGlen Choo, Mar 4, 2022
  107. Junio C HamanoMar 4, 2022
  108. Jonathan TanMar 4, 2022
  109. Junio C HamanoMar 4, 2022
  110. Glen ChooMar 5, 2022
  111. 03/10 t5526: create superproject commits with test helperGlen Choo, Mar 4, 2022
  112. Jonathan TanMar 4, 2022
  113. 04/10 submodule: make static functions read submodules from commitsGlen Choo, Mar 4, 2022
  114. 05/10 submodule: inline submodule_commits() into callerGlen Choo, Mar 4, 2022
  115. 06/10 submodule: store new submodule commits oid_array in a structGlen Choo, Mar 4, 2022
  116. 07/10 submodule: extract get_fetch_task()Glen Choo, Mar 4, 2022
  117. 09/10 fetch: fetch unpopulated, changed submodulesGlen Choo, Mar 4, 2022
  118. Junio C HamanoMar 4, 2022
  119. Glen ChooMar 4, 2022
  120. Junio C HamanoMar 5, 2022
  121. Glen ChooMar 5, 2022
  122. Junio C HamanoMar 8, 2022
  123. Jonathan TanMar 4, 2022
  124. 10/10 submodule: fix latent check_has_commit() bugGlen Choo, Mar 4, 2022
  125. Junio C HamanoMar 4, 2022
  126. 08/10 submodule: move logic into fetch_task_create()Glen Choo, Mar 4, 2022
  127. Junio C HamanoMar 4, 2022
  128. 00/10 fetch --recurse-submodules: fetch unpopulated submodulesGlen Choo, Mar 8, 2022
  129. 01/10 t5526: introduce test helper to assert on fetchesGlen Choo, Mar 8, 2022
  130. 03/10 t5526: create superproject commits with test helperGlen Choo, Mar 8, 2022
  131. 02/10 t5526: stop asserting on stderr literallyGlen Choo, Mar 8, 2022
  132. 04/10 submodule: make static functions read submodules from commitsGlen Choo, Mar 8, 2022
  133. 05/10 submodule: inline submodule_commits() into callerGlen Choo, Mar 8, 2022
  134. 09/10 fetch: fetch unpopulated, changed submodulesGlen Choo, Mar 8, 2022
  135. 10/10 submodule: fix latent check_has_commit() bugGlen Choo, Mar 8, 2022
  136. 08/10 submodule: move logic into fetch_task_create()Glen Choo, Mar 8, 2022
  137. 06/10 submodule: store new submodule commits oid_array in a structGlen Choo, Mar 8, 2022
  138. 07/10 submodule: extract get_fetch_task()Glen Choo, Mar 8, 2022
  139. Junio C HamanoMar 8, 2022
  140. Glen ChooMar 8, 2022
  141. Junio C HamanoMar 9, 2022
  142. Glen ChooMar 9, 2022
  143. Junio C HamanoMar 9, 2022
  144. Glen ChooMar 9, 2022
  145. Glen ChooMar 16, 2022
  146. Junio C HamanoMar 16, 2022
  147. Glen ChooMar 16, 2022
  148. Junio C HamanoMar 16, 2022
  149. Jonathan TanMar 8, 2022

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.