[PATCH v6] submodule: fetch missing objects from default remote
- From
Nasser Grainawi <nasser.grainawi@oss.qualcomm.com>
- Date
- Mar 3, 2026, 23:40 UTC
- Message-ID
- <20260303234044.105157-1-nasser.grainawi@oss.qualcomm.com>
- In-Reply-To
- <20260303200906.4118348-1-nasser.grainawi@oss.qualcomm.com>
When be76c21282 (fetch: ensure submodule objects fetched, 2018-12-06) added support for fetching a missing submodule object by id, it hardcoded the remote name as "origin" and deferred anything more complicated for a later patch. Implement the NEEDSWORK item to remove the hardcoded assumption by adding and using a submodule helper subcmd 'get-default-remote'. Fixing this lets 'git fetch --recurse-submodules' succeed when the fetched commit(s) in the superproject trigger a submodule fetch, and that submodule's default remote name is not "origin".
Add non-"origin" remote tests to t5526-fetch-submodules.sh and t5572-pull-submodule.sh demonstrating this works as expected and add dedicated tests for get-default-remote.
Signed-off-by: Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> Reviewed-by: Jacob Keller <jacob.keller@gmail.com> --- Removed the fetch progress output comparison from the tests as the ordering is unpredictable.
Range-diff against v5:
1: 673ad0372a ! 1: ff3034f05f submodule: fetch missing objects from default remote
@@ t/t5526-fetch-submodules.sh: test_expect_success "fetch new submodule commits on
(
cd downstream &&
- git fetch --recurse-submodules origin refs/changes/3:refs/heads/my_branch &&
-+ DEEP_START=$(git -C submodule/subdir/deepsubmodule rev-parse --short origin/deep) &&
-+ DEEP_END=$(git -C "$pwd/deepsubmodule" rev-parse --short deep) &&
-+ cat >"expect_fetch" <<-EOF &&
-+ From $pwd/.
-+ * [new ref] refs/changes/3 -> my_branch
-+ Fetching submodule sub1
-+ Fetching submodule sub1/subdir/deepsubmodule
-+ Fetching submodule submodule
-+ Fetching submodule submodule/subdir/deepsubmodule
-+ From $pwd/deepsubmodule
-+ $DEEP_START..$DEEP_END deep -> origin/deep
-+ From $pwd/./sub1
-+ * branch $D -> FETCH_HEAD
-+ Fetching submodule sub1/subdir/deepsubmodule
-+ From $pwd/submodule
-+ * branch $C -> FETCH_HEAD
-+ Fetching submodule submodule/subdir/deepsubmodule
-+ EOF
+ GIT_TRACE="$FETCH_TRACE" git fetch --recurse-submodules origin \
-+ refs/changes/3:refs/heads/my_branch 2>actual_fetch &&
-+ test_cmp expect_fetch actual_fetch &&
++ refs/changes/3:refs/heads/my_branch &&
git -C submodule cat-file -t $C &&
git -C sub1 cat-file -t $D &&
+ test_grep "trace: built-in: git submodule--helper get-default-remote sub1" \
@@ t/t5526-fetch-submodules.sh: test_expect_success 'fetch new submodule commit int
+ test_when_finished "rm -f \"$FETCH_TRACE\"" &&
+ (
+ cd downstream &&
-+ DEEP_START=$(git -C submodule/subdir/deepsubmodule rev-parse --short \
-+ origin/deep) &&
-+ DEEP_END=$(git -C "$pwd/deepsubmodule" rev-parse --short deep) &&
-+ cat >"expect_fetch_custom" <<-EOF &&
-+ From $pwd/.
-+ * [new ref] refs/changes/custom3 -> my_other_branch
-+ Fetching submodule sub1
-+ Fetching submodule sub1/subdir/deepsubmodule
-+ Fetching submodule submodule
-+ Fetching submodule submodule/subdir/deepsubmodule
-+ From $pwd/./sub1
-+ * branch $D -> FETCH_HEAD
-+ Fetching submodule sub1/subdir/deepsubmodule
-+ From $pwd/submodule
-+ * branch $C -> FETCH_HEAD
-+ Fetching submodule submodule/subdir/deepsubmodule
-+ EOF
+ GIT_TRACE="$FETCH_TRACE" git fetch --recurse-submodules origin \
-+ refs/changes/custom3:refs/heads/my_other_branch \
-+ 2>actual_fetch_custom &&
-+ # the without .gitmodules test above causes warnings
-+ grep -v "^warning: " actual_fetch_custom >actual_fetch_warnings_removed &&
-+ test_cmp expect_fetch_custom actual_fetch_warnings_removed &&
-+
++ refs/changes/custom3:refs/heads/my_other_branch &&
+ git -C submodule cat-file -t $C &&
+ git -C sub1 cat-file -t $D &&
+ test_grep "trace: built-in: git submodule--helper get-default-remote sub1" \builtin/submodule--helper.c | 38 +++++ submodule.c | 17 ++- t/meson.build | 1 + t/t5526-fetch-submodules.sh | 71 ++++++++- t/t5572-pull-submodule.sh | 21 ++- t/t7426-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++ 6 files changed, 330 insertions(+), 4 deletions(-) create mode 100755 t/t7426-submodule-get-default-remote.sh
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b621d14275..0a4676f3ba 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -113,6 +113,43 @@ static int get_default_remote_submodule(const char *module_path, char **default_ return 0; } +static int module_get_default_remote(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) +{ + const char *path; + char *resolved_path = NULL; + char *default_remote = NULL; + int code; + struct option options[] = { + OPT_END() + }; + const char *const usage[] = { + N_("git submodule--helper get-default-remote <path>"), + NULL + }; + + argc = parse_options(argc, argv, prefix, options, usage, 0); + if (argc != 1) + usage_with_options(usage, options); + + path = argv[0]; + if (prefix && *prefix && !is_absolute_path(path)) { + resolved_path = xstrfmt("%s%s", prefix, path); + path = resolved_path; + } + + code = get_default_remote_submodule(path, &default_remote); + if (code) { + free(resolved_path); + return code; + } + + printf("%s\n", default_remote); + free(default_remote); + free(resolved_path); + return 0; +} + /* the result should be freed by the caller. */ static char *get_submodule_displaypath(const char *path, const char *prefix, const char *super_prefix) @@ -3788,6 +3825,7 @@ int cmd_submodule__helper(int argc, OPT_SUBCOMMAND("set-url", &fn, module_set_url), OPT_SUBCOMMAND("set-branch", &fn, module_set_branch), OPT_SUBCOMMAND("create-branch", &fn, module_create_branch), + OPT_SUBCOMMAND("get-default-remote", &fn, module_get_default_remote), OPT_END() }; argc = parse_options(argc, argv, prefix, options, usage, 0); diff --git a/submodule.c b/submodule.c index 508938e4da..906febfa0e 100644 --- a/submodule.c +++ b/submodule.c @@ -1708,6 +1708,8 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err, if (spf->oid_fetch_tasks_nr) { struct fetch_task *task = spf->oid_fetch_tasks[spf->oid_fetch_tasks_nr - 1]; + struct child_process cp_remote = CHILD_PROCESS_INIT; + struct strbuf remote_name = STRBUF_INIT; spf->oid_fetch_tasks_nr--; child_process_init(cp); @@ -1721,8 +1723,19 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err, strvec_pushf(&cp->args, "--submodule-prefix=%s%s/", spf->prefix, task->sub->path); - /* NEEDSWORK: have get_default_remote from submodule--helper */ - strvec_push(&cp->args, "origin"); + cp_remote.git_cmd = 1; + strvec_pushl(&cp_remote.args, "submodule--helper", + "get-default-remote", task->sub->path, NULL); + + if (!capture_command(&cp_remote, &remote_name, 0)) { + strbuf_trim_trailing_newline(&remote_name); + strvec_push(&cp->args, remote_name.buf); + } else { + /* Fallback to "origin" if the helper fails */ + strvec_push(&cp->args, "origin"); + } + strbuf_release(&remote_name); + oid_array_for_each_unique(task->commits, append_oid_to_argv, &cp->args); diff --git a/t/meson.build b/t/meson.build index 6d91470ebc..cfa3de5962 100644 --- a/t/meson.build +++ b/t/meson.build @@ -894,6 +894,7 @@ integration_tests = [ 't7423-submodule-symlinks.sh', 't7424-submodule-mixed-ref-formats.sh', 't7425-submodule-gitdir-path-extension.sh', + 't7426-submodule-get-default-remote.sh', 't7450-bad-git-dotfiles.sh', 't7500-commit-template-squash-signoff.sh', 't7501-commit-basic-functionality.sh', diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 5e566205ba..1242ee9185 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -834,11 +834,18 @@ test_expect_success "fetch new submodule commits on-demand outside standard refs git commit -m "updated submodules outside of refs/heads" && E=$(git rev-parse HEAD) && git update-ref refs/changes/3 $E && + FETCH_TRACE="$(pwd)/trace.out" && + test_when_finished "rm -f \"$FETCH_TRACE\"" && ( cd downstream && - git fetch --recurse-submodules origin refs/changes/3:refs/heads/my_branch && + GIT_TRACE="$FETCH_TRACE" git fetch --recurse-submodules origin \ + refs/changes/3:refs/heads/my_branch && git -C submodule cat-file -t $C && git -C sub1 cat-file -t $D && + test_grep "trace: built-in: git submodule--helper get-default-remote sub1" \ + "$FETCH_TRACE" && + test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ origin" \ + "$FETCH_TRACE" && git checkout --recurse-submodules FETCH_HEAD ) ' @@ -929,6 +936,68 @@ test_expect_success 'fetch new submodule commit intermittently referenced by sup ) ' +test_expect_success 'fetch new submodule commits on-demand outside standard refspec with custom remote name' ' + # depends on the previous test for setup + + # Rename the remote in sub1 from "origin" to "custom_remote" + git -C downstream/sub1 remote rename origin custom_remote && + + # Create new commits in the original submodules + C=$(git -C submodule commit-tree \ + -m "change outside refs/heads for custom remote" HEAD^{tree}) && + git -C submodule update-ref refs/changes/custom1 $C && + git update-index --cacheinfo 160000 $C submodule && + test_tick && + + D=$(git -C sub1 commit-tree \ + -m "change outside refs/heads for custom remote" HEAD^{tree}) && + git -C sub1 update-ref refs/changes/custom2 $D && + git update-index --cacheinfo 160000 $D sub1 && + + git commit \ + -m "updated submodules outside of refs/heads for custom remote" && + E=$(git rev-parse HEAD) && + git update-ref refs/changes/custom3 $E && + FETCH_TRACE="$(pwd)/trace.out" && + test_when_finished "rm -f \"$FETCH_TRACE\"" && + ( + cd downstream && + GIT_TRACE="$FETCH_TRACE" git fetch --recurse-submodules origin \ + refs/changes/custom3:refs/heads/my_other_branch && + git -C submodule cat-file -t $C && + git -C sub1 cat-file -t $D && + test_grep "trace: built-in: git submodule--helper get-default-remote sub1" \ + "$FETCH_TRACE" && + test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ custom_remote $D" \ + "$FETCH_TRACE" && + git checkout --recurse-submodules FETCH_HEAD + ) +' + +test_expect_success 'fetch new submodule commit on-demand in FETCH_HEAD from custom remote' ' + # depends on the previous test for setup + + C=$(git -C submodule commit-tree -m "another change outside refs/heads for custom remote" HEAD^{tree}) && + git -C submodule update-ref refs/changes/custom4 $C && + git update-index --cacheinfo 160000 $C submodule && + test_tick && + + D=$(git -C sub1 commit-tree -m "another change outside refs/heads for custom remote" HEAD^{tree}) && + git -C sub1 update-ref refs/changes/custom5 $D && + git update-index --cacheinfo 160000 $D sub1 && + + git commit -m "updated submodules outside of refs/heads" && + E=$(git rev-parse HEAD) && + git update-ref refs/changes/custom6 $E && + ( + cd downstream && + git fetch --recurse-submodules origin refs/changes/custom6 && + git -C submodule cat-file -t $C && + git -C sub1 cat-file -t $D && + git checkout --recurse-submodules FETCH_HEAD + ) +' + add_commit_push () { dir="$1" && msg="$2" && diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh index 45f384dd32..42d14328b6 100755 --- a/t/t5572-pull-submodule.sh +++ b/t/t5572-pull-submodule.sh @@ -257,7 +257,26 @@ test_expect_success 'fetch submodule remote of different name from superproject' git -C a-submodule reset --hard HEAD^^ && git -C child pull --no-recurse-submodules && - git -C child submodule update + git -C child submodule update && + test_path_is_file child/a-submodule/moreecho.t +' + +test_expect_success 'fetch non-origin submodule remote named different from superproject' ' + git -C child/a-submodule remote rename origin o2 && + + # Create commit that is unreachable from current master branch + # newmain is already reset in the previous test + test_commit -C a-submodule echo_o2 && + test_commit -C a-submodule moreecho_o2 && + subc=$(git -C a-submodule rev-parse --short HEAD) && + + git -C parent/a-submodule fetch && + git -C parent/a-submodule checkout "$subc" && + git -C parent commit -m "update submodule o2" a-submodule && + git -C a-submodule reset --hard HEAD^^ && + + git -C child pull --recurse-submodules && + test_path_is_file child/a-submodule/moreecho_o2.t ' test_done diff --git a/t/t7426-submodule-get-default-remote.sh b/t/t7426-submodule-get-default-remote.sh new file mode 100755 index 0000000000..b842af9a2d --- /dev/null +++ b/t/t7426-submodule-get-default-remote.sh @@ -0,0 +1,186 @@ +#!/bin/sh + +test_description='git submodule--helper get-default-remote' + +TEST_NO_CREATE_REPO=1 +. ./test-lib.sh + +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + +test_expect_success 'setup repositories' ' + # Create a repository to be used as submodule + git init sub && + test_commit --no-tag -C sub "initial commit in sub" file.txt "sub content" && + + # Create main repository + git init super && + ( + cd super && + mkdir subdir && + test_commit --no-tag -C subdir "initial commit in super" main.txt "super content" && + git submodule add ../sub subpath && + git commit -m "add submodule 'sub' at subpath" + ) +' + +test_expect_success 'get-default-remote returns origin for initialized submodule' ' + ( + cd super && + git submodule update --init && + echo "origin" >expect && + git submodule--helper get-default-remote subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote works from subdirectory' ' + ( + cd super/subdir && + echo "origin" >expect && + git submodule--helper get-default-remote ../subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote fails with non-existent path' ' + ( + cd super && + test_must_fail git submodule--helper get-default-remote nonexistent 2>err && + test_grep "could not get a repository handle" err + ) +' + +test_expect_success 'get-default-remote fails with non-submodule path' ' + ( + cd super && + test_must_fail git submodule--helper get-default-remote subdir 2>err && + test_grep "could not get a repository handle" err + ) +' + +test_expect_success 'get-default-remote fails without path argument' ' + ( + cd super && + test_must_fail git submodule--helper get-default-remote 2>err && + test_grep "usage:" err + ) +' + +test_expect_success 'get-default-remote fails with too many arguments' ' + ( + cd super && + test_must_fail git submodule--helper get-default-remote subpath subdir 2>err && + test_grep "usage:" err + ) +' + +test_expect_success 'setup submodule with non-origin default remote name' ' + # Create another submodule path with a different remote name + ( + cd super && + git submodule add ../sub upstream-subpath && + git commit -m "add second submodule in upstream-subpath" && + git submodule update --init upstream-subpath && + + # Change the remote name in the submodule + cd upstream-subpath && + git remote rename origin upstream + ) +' + +test_expect_success 'get-default-remote returns non-origin remote name' ' + ( + cd super && + echo "upstream" >expect && + git submodule--helper get-default-remote upstream-subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote handles submodule with multiple remotes' ' + ( + cd super/subpath && + git remote add other-upstream ../../sub && + git remote add myfork ../../sub + ) && + + ( + cd super && + echo "origin" >expect && + git submodule--helper get-default-remote subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote handles submodule with multiple remotes and none are origin' ' + ( + cd super/upstream-subpath && + git remote add yet-another-upstream ../../sub && + git remote add yourfork ../../sub + ) && + + ( + cd super && + echo "upstream" >expect && + git submodule--helper get-default-remote upstream-subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'setup nested submodule with non-origin remote' ' + git init innersub && + test_commit --no-tag -C innersub "initial commit in innersub" inner.txt "innersub content" && + + ( + cd sub && + git submodule add ../innersub innersubpath && + git commit -m "add nested submodule at innersubpath" + ) && + + ( + cd super/upstream-subpath && + git pull upstream && + git submodule update --init --recursive . && + ( + cd innersubpath && + git remote rename origin another_upstream + ) + ) +' + +test_expect_success 'get-default-remote works with nested submodule' ' + ( + cd super && + echo "another_upstream" >expect && + git submodule--helper get-default-remote upstream-subpath/innersubpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote works with submodule that has no remotes' ' + # Create a submodule directory manually without remotes + ( + cd super && + git init no-remote-sub && + test_commit --no-tag -C no-remote-sub "local commit" local.txt "local content" + ) && + + # Add it as a submodule + ( + cd super && + git submodule add ./no-remote-sub && + git commit -m "add local submodule 'no-remote-sub'" + ) && + + ( + cd super && + # Should fall back to "origin" remote name when no remotes exist + echo "origin" >expect && + git submodule--helper get-default-remote no-remote-sub >actual && + test_cmp expect actual + ) +' + +test_done
-- 2.52.0