[v2,8/9] submodule: read shallows when finding changed submodules

Commit Message

Glen Choo Feb. 15, 2022, 5:23 p.m. UTC

In a repository with submodules, "git fetch --update-shallow" can fail.
This happens because "git fetch" does not read shallows when rev walking
the newly fetched commits to find changed submodules, thus the rev walk
may try to read the parent of a shallow and fail. This can occur when
--recurse-submodules is not passed, because the default behavior is
to fetch changed submodules i.e. --recurse-submodules=on-demand.

Fix this by reading shallows before the rev walk, and test for it.

Signed-off-by: Glen Choo <[e-mail]>
---
 submodule.c                 |  4 ++++
 t/t5526-fetch-submodules.sh | 10 ++++++++++
 2 files changed, 14 insertions(+)

Comments

Jonathan Tan Feb. 15, 2022, 10:03 p.m. UTC | #1

Glen Choo <[e-mail]> writes:
> @@ -901,6 +902,9 @@ static void collect_changed_submodules(struct repository *r,
>  
>  	save_warning = warn_on_object_refname_ambiguity;
>  	warn_on_object_refname_ambiguity = 0;
> +	/* make sure shallows are read */
> +	is_repository_shallow(the_repository);

This is presumably to initialize the data structures that some later
code reads without first calling is_repository_shallow(). Do we know
which part of the later code this is? If yes, it would be better to fix
it there.

Patch

diff --git a/submodule.c b/submodule.c
index 3558fddeb7..e62619bee0 100644
--- a/submodule.c
+++ b/submodule.c
@@ -22,6 +22,7 @@ 
 #include "parse-options.h"
 #include "object-store.h"
 #include "commit-reach.h"
+#include "shallow.h"
 
 static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
 static int initialized_fetch_ref_tips;
@@ -901,6 +902,9 @@  static void collect_changed_submodules(struct repository *r,
 
 	save_warning = warn_on_object_refname_ambiguity;
 	warn_on_object_refname_ambiguity = 0;
+	/* make sure shallows are read */
+	is_repository_shallow(the_repository);
+
 	repo_init_revisions(r, &rev, NULL);
 	setup_revisions(argv->nr, argv->v, &rev, &s_r_opt);
 	warn_on_object_refname_ambiguity = save_warning;
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index df44757468..ea70c3646f 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -1031,4 +1031,14 @@  test_expect_success 'recursive fetch after deinit a submodule' '
 	test_cmp expect actual
 '
 
+test_expect_success 'recursive fetch does not fail with --update-shallow' '
+	git clone --no-local --depth=2 --recurse-submodules . shallow &&
+	git init notshallow &&
+	(
+		cd notshallow &&
+		git submodule add ../submodule sub &&
+		git fetch --update-shallow ../shallow/.git refs/heads/*:refs/remotes/shallow/* --recurse-submodules
+	)
+'
+
 test_done