Re: [PATCH v3] submodule: fetch missing objects from default remote
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 25, 2026, 21:55 UTC
- Message-ID
- <xmqq7bs08p4r.fsf@gitster.g>
- In-Reply-To
- <CA+P7+xoYA6zhwVeCV6d5yW4ZxOKNLam3r-ayEDHpYpFqWyLduw@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
Show 15 quoted lines
> On Thu, Jan 22, 2026 at 10:49 AM Junio C Hamano <gitster@pobox.com> wrote: >> >> Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes: >> ... >> Thanks. Jacob, this v3 is not exactly the same as v1 that you >> reviewed (and range-diff relative to v2 does not capture what got >> changed between the version you saw and this version), but I just >> checked that they are "essentially identical" except for the >> proposed log message. Are you happy with having your Reviewed-by on >> this version? >> > > I re-reviewed the patch and everything looks fine to me: > > Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
We actually needed a pair of small fixups on top, but I presume that even with them your reviewed-by still stands?
The following is with these fixes squashed in.
builtin/submodule--helper.c | 38 +++++++ submodule.c | 17 ++- t/meson.build | 1 + t/t5526-fetch-submodules.sh | 52 +++++++++ t/t5572-pull-submodule.sh | 24 ++++- t/t7426-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++++++++++ 6 files changed, 314 insertions(+), 4 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index d537ab087a..b180a24091 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -112,6 +112,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) @@ -3608,6 +3645,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 40a5c6fb9d..6599657f34 100644 --- a/submodule.c +++ b/submodule.c @@ -1706,6 +1706,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); @@ -1719,8 +1721,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 459c52a489..6ff54cf276 100644 --- a/t/meson.build +++ b/t/meson.build @@ -887,6 +887,7 @@ integration_tests = [ 't7422-submodule-output.sh', 't7423-submodule-symlinks.sh', 't7424-submodule-mixed-ref-formats.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..a5a273b392 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -929,6 +929,58 @@ test_expect_success 'fetch new submodule commit intermittently referenced by sup ) ' +test_expect_success 'fetch --recurse-submodules works with custom remote names' ' + # 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 && + ( + cd downstream && + 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 && + 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..dfc07d050b 100755 --- a/t/t5572-pull-submodule.sh +++ b/t/t5572-pull-submodule.sh @@ -246,7 +246,8 @@ test_expect_success 'fetch submodule remote of different name from superproject' git -C child submodule update --init && # Needs to create unreachable commit from current master branch. - git -C a-submodule checkout -b newmain HEAD^ && + git -C a-submodule tag anchorpoint HEAD && + git -C a-submodule checkout -b newmain anchorpoint^ && test_commit -C a-submodule echo && test_commit -C a-submodule moreecho && subc=$(git -C a-submodule rev-parse --short HEAD) && @@ -257,7 +258,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 submodule remote of different non-origin name from superproject' ' + git -C child/a-submodule remote rename origin o2 && + + # Create commit that is unreachable from current master branch + git -C a-submodule checkout -b newmain2 anchorpoint^ && + 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