* [PATCH] Fetch missing submodule objects from default remote
@ 2026-01-12 21:36 Nasser Grainawi
2026-01-13 2:17 ` Jacob Keller
` (2 more replies)
0 siblings, 3 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-01-12 21:36 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jacob Keller, Nasser Grainawi
When be76c2128234d94b47f7087152ee55d08bb65d88 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>
---
The original commit was a bit over 7 years ago, so I guess it worked
fine for most users. I've now run into cases where it doesn't work while
using 'repo' tool manifests that use a non-"origin" remote name and
contain projects with submodules.
I kept this as a single commit because most of the code delta is in the new
test for the new submodule helper command, but it could easily be split into
two commits if that's preferred.
builtin/submodule--helper.c | 38 +++++
submodule.c | 17 ++-
t/meson.build | 1 +
t/t5526-fetch-submodules.sh | 52 +++++++
t/t5572-pull-submodule.sh | 21 ++-
t/t7425-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++
6 files changed, 312 insertions(+), 3 deletions(-)
create mode 100755 t/t7425-submodule-get-default-remote.sh
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..044fcad6c9 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..ef6cdab165 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',
+ 't7425-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..868dd6d130 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 submodule remote of different non-origin name from superproject' '
+ git -C child/a-submodule remote rename origin o2 &&
+
+ # Create commit that's unreachable from current master branch
+ git -C a-submodule checkout -b newmain2 master^ &&
+ 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/t7425-submodule-get-default-remote.sh b/t/t7425-submodule-get-default-remote.sh
new file mode 100755
index 0000000000..b842af9a2d
--- /dev/null
+++ b/t/t7425-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.51.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] Fetch missing submodule objects from default remote
2026-01-12 21:36 [PATCH] Fetch missing submodule objects from default remote Nasser Grainawi
@ 2026-01-13 2:17 ` Jacob Keller
2026-01-13 21:51 ` Ben Knoble
2026-01-14 19:48 ` [PATCH v2] submodule: fetch missing " Nasser Grainawi
2 siblings, 0 replies; 36+ messages in thread
From: Jacob Keller @ 2026-01-13 2:17 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, Patrick Steinhardt
On Mon, Jan 12, 2026 at 1:36 PM Nasser Grainawi
<nasser.grainawi@oss.qualcomm.com> wrote:
>
> When be76c2128234d94b47f7087152ee55d08bb65d88 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>
> ---
> The original commit was a bit over 7 years ago, so I guess it worked
> fine for most users. I've now run into cases where it doesn't work while
> using 'repo' tool manifests that use a non-"origin" remote name and
> contain projects with submodules.
>
> I kept this as a single commit because most of the code delta is in the new
> test for the new submodule helper command, but it could easily be split into
> two commits if that's preferred.
>
> builtin/submodule--helper.c | 38 +++++
> submodule.c | 17 ++-
> t/meson.build | 1 +
> t/t5526-fetch-submodules.sh | 52 +++++++
> t/t5572-pull-submodule.sh | 21 ++-
> t/t7425-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++
> 6 files changed, 312 insertions(+), 3 deletions(-)
> create mode 100755 t/t7425-submodule-get-default-remote.sh
>
I've had this exact same issue due to setting a default name of
upstream instead of origin due to how I like to name things for
fork-based workflows. The change looks good and the tests are
appreciated.
I recall trying to fix related issues with the following series, but I
think I lost track of its development and never saw it through to
merging:
https://lore.kernel.org/git/20250623-jk-submodule-helper-use-url-v4-0-133ef3d89569@gmail.com/
Reading through that series, it doesn't fix this particular hard
coding either, so this is an improvement regardless.
Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] Fetch missing submodule objects from default remote
2026-01-12 21:36 [PATCH] Fetch missing submodule objects from default remote Nasser Grainawi
2026-01-13 2:17 ` Jacob Keller
@ 2026-01-13 21:51 ` Ben Knoble
2026-01-13 22:41 ` Nasser Grainawi
2026-01-14 14:05 ` Junio C Hamano
2026-01-14 19:48 ` [PATCH v2] submodule: fetch missing " Nasser Grainawi
2 siblings, 2 replies; 36+ messages in thread
From: Ben Knoble @ 2026-01-13 21:51 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, Patrick Steinhardt, Jacob Keller
> Le 12 janv. 2026 à 16:36, Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> a écrit :
>
> When be76c2128234d94b47f7087152ee55d08bb65d88 added support for fetching
> a missing submodule object by id, it
Convention is to refer to published commits using the “reference” format supported by git log and git show :)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] Fetch missing submodule objects from default remote
2026-01-13 21:51 ` Ben Knoble
@ 2026-01-13 22:41 ` Nasser Grainawi
2026-01-14 2:19 ` D. Ben Knoble
2026-01-14 14:05 ` Junio C Hamano
1 sibling, 1 reply; 36+ messages in thread
From: Nasser Grainawi @ 2026-01-13 22:41 UTC (permalink / raw)
To: Ben Knoble; +Cc: git, Patrick Steinhardt, Jacob Keller
On Tue, Jan 13, 2026 at 2:51 PM Ben Knoble <ben.knoble@gmail.com> wrote:
>
>
>
> > Le 12 janv. 2026 à 16:36, Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> a écrit :
> >
> > When be76c2128234d94b47f7087152ee55d08bb65d88 added support for fetching
> > a missing submodule object by id, it
>
> Convention is to refer to published commits using the “reference” format supported by git log and git show :)
Oh, thanks for pointing that out! I missed it in the SubmittingPatches
doc. If I end up sending a v2 I'll include the update, but I assume
that alone isn't worth sending a new patch for?
If it helps to have it here, the corrected first paragraph should be:
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".
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] Fetch missing submodule objects from default remote
2026-01-13 22:41 ` Nasser Grainawi
@ 2026-01-14 2:19 ` D. Ben Knoble
2026-01-14 14:10 ` Junio C Hamano
0 siblings, 1 reply; 36+ messages in thread
From: D. Ben Knoble @ 2026-01-14 2:19 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, Patrick Steinhardt, Jacob Keller
On Tue, Jan 13, 2026 at 5:41 PM Nasser Grainawi
<nasser.grainawi@oss.qualcomm.com> wrote:
> On Tue, Jan 13, 2026 at 2:51 PM Ben Knoble <ben.knoble@gmail.com> wrote:
> > > Le 12 janv. 2026 à 16:36, Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> a écrit :
> > >
> > > When be76c2128234d94b47f7087152ee55d08bb65d88 added support for fetching
> > > a missing submodule object by id, it
> >
> > Convention is to refer to published commits using the “reference” format supported by git log and git show :)
>
> Oh, thanks for pointing that out! I missed it in the SubmittingPatches
> doc. If I end up sending a v2 I'll include the update, but I assume
> that alone isn't worth sending a new patch for?
>
> If it helps to have it here, the corrected first paragraph should be:
>
> 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".
Thanks. That should be sufficient for Junio to correct it when
applying, but I would amend the change locally in case we get further
iterations ;)
--
D. Ben Knoble
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] Fetch missing submodule objects from default remote
2026-01-13 21:51 ` Ben Knoble
2026-01-13 22:41 ` Nasser Grainawi
@ 2026-01-14 14:05 ` Junio C Hamano
2026-01-14 19:23 ` Nasser Grainawi
1 sibling, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-01-14 14:05 UTC (permalink / raw)
To: Ben Knoble; +Cc: Nasser Grainawi, git, Patrick Steinhardt, Jacob Keller
Ben Knoble <ben.knoble@gmail.com> writes:
>> Le 12 janv. 2026 à 16:36, Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> a écrit :
>>
>> When be76c2128234d94b47f7087152ee55d08bb65d88 added support for fetching
>> a missing submodule object by id, it
>
> Convention is to refer to published commits using the
> “reference” format supported by git log and git show :)
;-)
Other two conventions violated that you may want to point out are
(1) the commit title lacks the <area>: prefix and upcases the first
word, (2) we do not use // to introduce a comment line.
Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] Fetch missing submodule objects from default remote
2026-01-14 2:19 ` D. Ben Knoble
@ 2026-01-14 14:10 ` Junio C Hamano
2026-01-14 21:22 ` Ben Knoble
0 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-01-14 14:10 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Nasser Grainawi, git, Patrick Steinhardt, Jacob Keller
"D. Ben Knoble" <ben.knoble@gmail.com> writes:
> Thanks. That should be sufficient for Junio to correct it when
> applying, ...
I'd prefer not to see such recommendation to use the maintainer as a
janitor, though, as the number of contributors well outweigh the
number of maintainer(s).
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] Fetch missing submodule objects from default remote
2026-01-14 14:05 ` Junio C Hamano
@ 2026-01-14 19:23 ` Nasser Grainawi
0 siblings, 0 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-01-14 19:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Knoble, git, Patrick Steinhardt, Jacob Keller
On Wed, Jan 14, 2026 at 7:05 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Other two conventions violated that you may want to point out are
> (1) the commit title lacks the <area>: prefix and upcases the first
> word, (2) we do not use // to introduce a comment line.
Thanks. I'll fix those and send a v2 with the commit reference fixed too.
Also, I couldn't find mention of (2) in the CodingGuidelines doc. Did
I miss it or would you like a patch to add it?
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] submodule: fetch missing objects from default remote
2026-01-12 21:36 [PATCH] Fetch missing submodule objects from default remote Nasser Grainawi
2026-01-13 2:17 ` Jacob Keller
2026-01-13 21:51 ` Ben Knoble
@ 2026-01-14 19:48 ` Nasser Grainawi
2026-01-21 0:48 ` Nasser Grainawi
2026-01-22 15:27 ` [PATCH v3] " Nasser Grainawi
2 siblings, 2 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-01-14 19:48 UTC (permalink / raw)
To: git
Cc: Nasser Grainawi, D. Ben Knoble, Patrick Steinhardt, Jacob Keller,
Junio C Hamano
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>
---
Range-diff against v1:
1: 36ce158268 ! 1: 99c4792cff Fetch missing submodule objects from default remote
@@ Metadata
Author: Nasser Grainawi <nasser.grainawi@oss.qualcomm.com>
## Commit message ##
- Fetch missing submodule objects from default remote
+ submodule: fetch missing objects from default remote
- When be76c2128234d94b47f7087152ee55d08bb65d88 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".
+ 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
@@ submodule.c: static int get_next_submodule(struct child_process *cp, struct strb
+ strbuf_trim_trailing_newline(&remote_name);
+ strvec_push(&cp->args, remote_name.buf);
+ } else {
-+ // Fallback to "origin" if the helper fails
++ /* Fallback to "origin" if the helper fails */
+ strvec_push(&cp->args, "origin");
+ }
+ strbuf_release(&remote_name);
builtin/submodule--helper.c | 38 +++++
submodule.c | 17 ++-
t/meson.build | 1 +
t/t5526-fetch-submodules.sh | 52 +++++++
t/t5572-pull-submodule.sh | 21 ++-
t/t7425-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++
6 files changed, 312 insertions(+), 3 deletions(-)
create mode 100755 t/t7425-submodule-get-default-remote.sh
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..ef6cdab165 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',
+ 't7425-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..868dd6d130 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 submodule remote of different non-origin name from superproject' '
+ git -C child/a-submodule remote rename origin o2 &&
+
+ # Create commit that's unreachable from current master branch
+ git -C a-submodule checkout -b newmain2 master^ &&
+ 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/t7425-submodule-get-default-remote.sh b/t/t7425-submodule-get-default-remote.sh
new file mode 100755
index 0000000000..b842af9a2d
--- /dev/null
+++ b/t/t7425-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.51.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] Fetch missing submodule objects from default remote
2026-01-14 14:10 ` Junio C Hamano
@ 2026-01-14 21:22 ` Ben Knoble
0 siblings, 0 replies; 36+ messages in thread
From: Ben Knoble @ 2026-01-14 21:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nasser Grainawi, git, Patrick Steinhardt, Jacob Keller
>
> Le 14 janv. 2026 à 09:10, Junio C Hamano <gitster@pobox.com> a écrit :
>
> "D. Ben Knoble" <ben.knoble@gmail.com> writes:
>
>> Thanks. That should be sufficient for Junio to correct it when
>> applying, ...
>
> I'd prefer not to see such recommendation to use the maintainer as a
> janitor, though, as the number of contributors well outweigh the
> number of maintainer(s).
Certainly. Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] submodule: fetch missing objects from default remote
2026-01-14 19:48 ` [PATCH v2] submodule: fetch missing " Nasser Grainawi
@ 2026-01-21 0:48 ` Nasser Grainawi
2026-01-22 15:27 ` [PATCH v3] " Nasser Grainawi
1 sibling, 0 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-01-21 0:48 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Patrick Steinhardt, Jacob Keller, Junio C Hamano
On Wed, Jan 14, 2026 at 12:48 PM Nasser Grainawi
<nasser.grainawi@oss.qualcomm.com> wrote:
>
> Signed-off-by: Nasser Grainawi <nasser.grainawi@oss.qualcomm.com>
I realized I missed adding Jacob's Reviewed-by.
Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
> diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
> index 45f384dd32..868dd6d130 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 submodule remote of different non-origin name from superproject' '
> + git -C child/a-submodule remote rename origin o2 &&
> +
> + # Create commit that's unreachable from current master branch
The single quote in this comment is breaking the test. Sorry I didn't
re-run the tests before sending the patch. I'll wait for any other
comments and otherwise include this fix in a v3 tomorrow.
> diff --git a/t/t7425-submodule-get-default-remote.sh b/t/t7425-submodule-get-default-remote.sh
FYI, this test name conflicts with the new test added in topic
ar/submodule-gitdir-tweak (they both use t7425). Renaming this test to
't7426-...' and updating the name in t/meson.build is sufficient to
have all tests passing with 'seen'.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3] submodule: fetch missing objects from default remote
2026-01-14 19:48 ` [PATCH v2] submodule: fetch missing " Nasser Grainawi
2026-01-21 0:48 ` Nasser Grainawi
@ 2026-01-22 15:27 ` Nasser Grainawi
2026-01-22 18:49 ` Junio C Hamano
` (3 more replies)
1 sibling, 4 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-01-22 15:27 UTC (permalink / raw)
To: git
Cc: Nasser Grainawi, D. Ben Knoble, Patrick Steinhardt, Jacob Keller,
Junio C Hamano
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>
---
Range-diff against v2:
1: 99c4792cff ! 1: 1dd17e9f75 submodule: fetch missing objects from default remote
@@ Commit message
Change-Id: I0fec01b161aa13ed4c1c5e53477dad6912d1b5e6
Signed-off-by: Nasser Grainawi <nasser.grainawi@oss.qualcomm.com>
+ Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
## builtin/submodule--helper.c ##
@@ builtin/submodule--helper.c: static int get_default_remote_submodule(const char *module_path, char **default_
@@ t/t5572-pull-submodule.sh: test_expect_success 'fetch submodule remote of differ
+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's unreachable from current master branch
++ # Create commit that is unreachable from current master branch
+ git -C a-submodule checkout -b newmain2 master^ &&
+ test_commit -C a-submodule echo_o2 &&
+ test_commit -C a-submodule moreecho_o2 &&
builtin/submodule--helper.c | 38 +++++
submodule.c | 17 ++-
t/meson.build | 1 +
t/t5526-fetch-submodules.sh | 52 +++++++
t/t5572-pull-submodule.sh | 21 ++-
t/t7425-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++
6 files changed, 312 insertions(+), 3 deletions(-)
create mode 100755 t/t7425-submodule-get-default-remote.sh
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..ef6cdab165 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',
+ 't7425-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..faafe31409 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 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 master^ &&
+ 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/t7425-submodule-get-default-remote.sh b/t/t7425-submodule-get-default-remote.sh
new file mode 100755
index 0000000000..b842af9a2d
--- /dev/null
+++ b/t/t7425-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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-01-22 15:27 ` [PATCH v3] " Nasser Grainawi
@ 2026-01-22 18:49 ` Junio C Hamano
2026-01-22 20:16 ` Jacob Keller
2026-02-27 18:29 ` Nasser Grainawi
2026-01-22 21:21 ` Junio C Hamano
` (2 subsequent siblings)
3 siblings, 2 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-01-22 18:49 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
> 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>
> ---
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?
> builtin/submodule--helper.c | 38 +++++
> submodule.c | 17 ++-
> t/meson.build | 1 +
> t/t5526-fetch-submodules.sh | 52 +++++++
> t/t5572-pull-submodule.sh | 21 ++-
> t/t7425-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++
> 6 files changed, 312 insertions(+), 3 deletions(-)
> create mode 100755 t/t7425-submodule-get-default-remote.sh
>
> 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);
Hmph, I am not sure what is going on. What are we getting out of
parse_options() here? Would it be the same to see if we got
anything remaining on the command line by checking argc and call
usage_with_options() without calling parse_options(), or am I
missing something?
> + 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);
Do we know that the value of default_remote has no funny bytes in
it, like newline? In the end the name has to become part of
refs/remotes/<name>/HEAD that has to be a valid refname, so not
giving any facility to quote funny bytes and allowing the caller of
this helper to assume a LF terminated single line should be fine, so
I am guessing that the answer is yes, but I offhand do not know how
we know that we do not have to worry about such a situation in the
code path that begins with get_default_remote_submodule().
> 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);
OK, this part is quite straight-forward.
> 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
> + )
> +'
Hmph, these overly long lines are eyesore. I wonder if we can do
something about them?
I also wonder if we want to make sure we are getting from the remote
that is given the custom name in a more direct way (instead of "we
see that our fetch succeeds, and because there is no other remote,
it must have gotten what is needed from the renamed one"), or is it
too much paranoia?
> +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
> + )
> +'
Are we testing anything new in this test, compared to the previous
one? Both update the submodule sub1 by adding a new ref under
refs/changes/ hierachy and have "git fetch --recurse-submodules"
follow the changes.
> diff --git a/t/t7425-submodule-get-default-remote.sh b/t/t7425-submodule-get-default-remote.sh
> new file mode 100755
> index 0000000000..b842af9a2d
> --- /dev/null
> +++ b/t/t7425-submodule-get-default-remote.sh
> ...
> +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
> + )
> +'
OK, we have dedicated tests like these to ensure that when we say
"get default remote" we get what we expect, which is very good.
The "too much paranoia" question above is primarily about "are we
really saying get-default-remote properly at the place where it
matters?" IOW, we guarantee that the lower layer works as expected
with t7425, but the way we validate that we call the lower layer
correctly felt a bit indirect.
Other than these, looking very good.
Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-01-22 18:49 ` Junio C Hamano
@ 2026-01-22 20:16 ` Jacob Keller
2026-01-22 20:38 ` Junio C Hamano
2026-02-25 21:55 ` Junio C Hamano
2026-02-27 18:29 ` Nasser Grainawi
1 sibling, 2 replies; 36+ messages in thread
From: Jacob Keller @ 2026-01-22 20:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nasser Grainawi, git, D. Ben Knoble, Patrick Steinhardt
On Thu, Jan 22, 2026 at 10:49 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
>
> > 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>
> > ---
>
> 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>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-01-22 20:16 ` Jacob Keller
@ 2026-01-22 20:38 ` Junio C Hamano
2026-02-25 21:55 ` Junio C Hamano
1 sibling, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-01-22 20:38 UTC (permalink / raw)
To: Jacob Keller; +Cc: Nasser Grainawi, git, D. Ben Knoble, Patrick Steinhardt
Jacob Keller <jacob.keller@gmail.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>
Thanks. Queued.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-01-22 15:27 ` [PATCH v3] " Nasser Grainawi
2026-01-22 18:49 ` Junio C Hamano
@ 2026-01-22 21:21 ` Junio C Hamano
2026-01-23 23:26 ` Junio C Hamano
2026-03-01 2:53 ` [PATCH v4] " Nasser Grainawi
3 siblings, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-01-22 21:21 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
> t/t7425-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++
One thing I forgot to notice is that t7425 is already taken by
another topic in flight in 'seen'. Perhaps move it to t7426 or
something, perhaps?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-01-22 15:27 ` [PATCH v3] " Nasser Grainawi
2026-01-22 18:49 ` Junio C Hamano
2026-01-22 21:21 ` Junio C Hamano
@ 2026-01-23 23:26 ` Junio C Hamano
2026-01-24 2:18 ` Junio C Hamano
2026-03-01 2:53 ` [PATCH v4] " Nasser Grainawi
3 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-01-23 23:26 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
> diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
> index 45f384dd32..faafe31409 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 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 master^ &&
This test assumes that the first branch created by default is
'master', which will break in one of the CI jobs:
https://github.com/git/git/actions/runs/21304166518/job/61328461844#step:9:1942
If we are assuming that we are on the default branch when this
"Create commit" step runs, perhaps you can replace your "master^"
with "HEAD^" to achieve the same effect in a way that works
regardless of what the default branch is called?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-01-23 23:26 ` Junio C Hamano
@ 2026-01-24 2:18 ` Junio C Hamano
2026-02-20 23:12 ` Junio C Hamano
0 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-01-24 2:18 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
Junio C Hamano <gitster@pobox.com> writes:
> Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
>
>> diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
>> index 45f384dd32..faafe31409 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 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 master^ &&
>
> This test assumes that the first branch created by default is
> 'master', which will break in one of the CI jobs:
>
> https://github.com/git/git/actions/runs/21304166518/job/61328461844#step:9:1942
For now, I've queued two fix-up patches on top of the posted patch
to avoid CI breakages when the topic is merged to 'seen'. One is to
rename t7425-submodule-get-default-remote.sh to t7426-submodule-get-default-remote.sh
(both filename and the reference to it in t/meson.build), and the
other one is the following.
----- >8 -----
Subject: [PATCH] SQUASH??? fixup
The test as posted breaks when run with
$ make WITH_BREAKING_CHANGES=YesPlease test
as the added part assumes that the default branch name is "master".
This band-aid is sufficient for the purpose of the maintainer to get
the CI passing, but the real solution should probably be done better
in such a way that the latter step does not have to rely on the
creation of "anchorpoint" in the previous step. I'll leave it to
the contributor of the topic.
---
t/t5572-pull-submodule.sh | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
index faafe31409..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) &&
@@ -265,7 +266,7 @@ test_expect_success 'fetch submodule remote of different non-origin name from su
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 master^ &&
+ 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) &&
--
2.53.0-rc1-193-g609e9a7b29
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-01-24 2:18 ` Junio C Hamano
@ 2026-02-20 23:12 ` Junio C Hamano
2026-02-27 17:20 ` Nasser Grainawi
0 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-02-20 23:12 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
Junio C Hamano <gitster@pobox.com> writes:
>> ...
>> This test assumes that the first branch created by default is
>> 'master', which will break in one of the CI jobs:
>>
>> https://github.com/git/git/actions/runs/21304166518/job/61328461844#step:9:1942
>
> For now, I've queued two fix-up patches on top of the posted patch
> to avoid CI breakages when the topic is merged to 'seen'. One is to
> rename t7425-submodule-get-default-remote.sh to t7426-submodule-get-default-remote.sh
> (both filename and the reference to it in t/meson.build), and the
> other one is the following.
>
> ----- >8 -----
> Subject: [PATCH] SQUASH??? fixup
>
> The test as posted breaks when run with
>
> $ make WITH_BREAKING_CHANGES=YesPlease test
>
> as the added part assumes that the default branch name is "master".
>
> This band-aid is sufficient for the purpose of the maintainer to get
> the CI passing, but the real solution should probably be done better
> in such a way that the latter step does not have to rely on the
> creation of "anchorpoint" in the previous step. I'll leave it to
> the contributor of the topic.
> ---
This was from about a month ago, and we haven't heard from you.
Will we see a hopefully small and final update [PATCH v4] of this
topic sometime soon?
Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-01-22 20:16 ` Jacob Keller
2026-01-22 20:38 ` Junio C Hamano
@ 2026-02-25 21:55 ` Junio C Hamano
2026-03-02 22:06 ` Jacob Keller
1 sibling, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-02-25 21:55 UTC (permalink / raw)
To: Jacob Keller; +Cc: Nasser Grainawi, git, D. Ben Knoble, Patrick Steinhardt
Jacob Keller <jacob.keller@gmail.com> writes:
> 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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-02-20 23:12 ` Junio C Hamano
@ 2026-02-27 17:20 ` Nasser Grainawi
0 siblings, 0 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-02-27 17:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
On Fri, Feb 20, 2026 at 4:12 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> >> ...
> >> This test assumes that the first branch created by default is
> >> 'master', which will break in one of the CI jobs:
> >>
> >> https://github.com/git/git/actions/runs/21304166518/job/61328461844#step:9:1942
> >
> > For now, I've queued two fix-up patches on top of the posted patch
> > to avoid CI breakages when the topic is merged to 'seen'. One is to
> > rename t7425-submodule-get-default-remote.sh to t7426-submodule-get-default-remote.sh
> > (both filename and the reference to it in t/meson.build), and the
> > other one is the following.
> >
> > ----- >8 -----
> > Subject: [PATCH] SQUASH??? fixup
> >
> > The test as posted breaks when run with
> >
> > $ make WITH_BREAKING_CHANGES=YesPlease test
> >
> > as the added part assumes that the default branch name is "master".
> >
> > This band-aid is sufficient for the purpose of the maintainer to get
> > the CI passing, but the real solution should probably be done better
> > in such a way that the latter step does not have to rely on the
> > creation of "anchorpoint" in the previous step. I'll leave it to
> > the contributor of the topic.
> > ---
>
> This was from about a month ago, and we haven't heard from you.
> Will we see a hopefully small and final update [PATCH v4] of this
> topic sometime soon?
>
Yes, sorry about the delay. I was aiming for thorough and landed at
tardy. I'll get my replies to your inline comments sent shortly and
hopefully the v4 soon after that.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-01-22 18:49 ` Junio C Hamano
2026-01-22 20:16 ` Jacob Keller
@ 2026-02-27 18:29 ` Nasser Grainawi
1 sibling, 0 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-02-27 18:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
On Thu, Jan 22, 2026 at 11:49 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
> >
> > 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);
>
> Hmph, I am not sure what is going on. What are we getting out of
> parse_options() here? Would it be the same to see if we got
> anything remaining on the command line by checking argc and call
> usage_with_options() without calling parse_options(), or am I
> missing something?
I had found a few other places following this same pattern (for example:
gc.c maintenance_stop() and notes.c list()) and I thought it was because
parse_options() handles common options like '-h' and has standardized
messages for errors like unknown options.
> > + code = get_default_remote_submodule(path, &default_remote);
> > + if (code) {
> > + free(resolved_path);
> > + return code;
> > + }
> > +
> > + printf("%s\n", default_remote);
>
> Do we know that the value of default_remote has no funny bytes in
> it, like newline? In the end the name has to become part of
> refs/remotes/<name>/HEAD that has to be a valid refname, so not
> giving any facility to quote funny bytes and allowing the caller of
> this helper to assume a LF terminated single line should be fine, so
> I am guessing that the answer is yes, but I offhand do not know how
> we know that we do not have to worry about such a situation in the
> code path that begins with get_default_remote_submodule().
I don't think we know that, or at least I can't find proof of it. All I
can find is that remote.c issues a warning for remote names starting
with '/'. That seems to miss cases tested in t0602-reffiles-fsck.sh.
However, I'm not sure if this helper should be responsible for
validating the remote name as that seems like something remote.c should
be doing when parsing configs.
> Hmph, these overly long lines are eyesore. I wonder if we can do
> something about them?
I'll send a fixed version.
> I also wonder if we want to make sure we are getting from the remote
> that is given the custom name in a more direct way (instead of "we
> see that our fetch succeeds, and because there is no other remote,
> it must have gotten what is needed from the renamed one"), or is it
> too much paranoia?
I can capture the fetch command output and compare it to some expected
output where we have the remote paths, but that still doesn't show the
remote name. But it looks like I can inspect the GIT_TRACE output and
compare the `git submodule--helper get-default-remote` and subsequent
`git fetch` commands to expected output. I've added both methods to this
new test and the existing test it was modeled on so that it's obvious
there's a difference between them.
>
> > +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
> > + )
> > +'
>
> Are we testing anything new in this test, compared to the previous
> one? Both update the submodule sub1 by adding a new ref under
> refs/changes/ hierachy and have "git fetch --recurse-submodules"
> follow the changes.
I think the only difference is not creating a new superproject ref under
refs/heads/ when we fetch. I mirrored it after the test above 'fetch new
submodule commit on-demand in FETCH_HEAD', but for the intent of testing
this remote name feature, I don't think we need both. I don't mind
dropping it if you think it's unnecessary.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4] submodule: fetch missing objects from default remote
2026-01-22 15:27 ` [PATCH v3] " Nasser Grainawi
` (2 preceding siblings ...)
2026-01-23 23:26 ` Junio C Hamano
@ 2026-03-01 2:53 ` Nasser Grainawi
2026-03-02 22:09 ` Jacob Keller
` (2 more replies)
3 siblings, 3 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-03-01 2:53 UTC (permalink / raw)
To: git
Cc: Nasser Grainawi, D. Ben Knoble, Patrick Steinhardt, Jacob Keller,
Junio C Hamano
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>
---
I removed Jacob Keller's Reviewed-By since there are more significant
edits to the tests in v4 that they haven't reviewed.
v4 includes fixes for the issues Junio patched in seen and I confirmed
it merges cleanly to seen (with v3 reverted) as well as next and passes
tests.
Range-diff against v3:
1: 1dd17e9f75 ! 1: 9c5a5df9a2 submodule: fetch missing objects from default remote
@@ submodule.c: static int get_next_submodule(struct child_process *cp, struct strb
## t/meson.build ##
@@ t/meson.build: integration_tests = [
- 't7422-submodule-output.sh',
't7423-submodule-symlinks.sh',
't7424-submodule-mixed-ref-formats.sh',
-+ 't7425-submodule-get-default-remote.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',
## t/t5526-fetch-submodules.sh ##
+@@ t/t5526-fetch-submodules.sh: test_expect_success "fetch new submodule commits on-demand outside standard refs
+ git update-ref refs/changes/3 $E &&
+ (
+ 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
++ test_when_finished "rm -f $pwd/on-demand_submodule_fetch_trace" &&
++ GIT_TRACE="$pwd/on-demand_submodule_fetch_trace" \
++ git fetch --recurse-submodules origin refs/changes/3:refs/heads/my_branch 2>actual_fetch &&
++ test_cmp expect_fetch actual_fetch &&
+ 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" \
++ "$pwd/on-demand_submodule_fetch_trace" &&
++ test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ origin" \
++ "$pwd/on-demand_submodule_fetch_trace" &&
+ git checkout --recurse-submodules FETCH_HEAD
+ )
+ '
@@ t/t5526-fetch-submodules.sh: test_expect_success 'fetch new submodule commit intermittently referenced by sup
)
'
-+test_expect_success 'fetch --recurse-submodules works with custom remote names' '
++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}) &&
++ 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}) &&
++ 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" &&
++ 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 &&
++ 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
++ test_when_finished "rm -f $pwd/custom_on-demand_submodule_fetch_trace" &&
++ GIT_TRACE="$pwd/custom_on-demand_submodule_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 &&
++
+ 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" \
++ "$pwd/custom_on-demand_submodule_fetch_trace" &&
++ test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ custom_remote $D" \
++ "$pwd/custom_on-demand_submodule_fetch_trace" &&
+ git checkout --recurse-submodules FETCH_HEAD
+ )
+'
@@ t/t5572-pull-submodule.sh: test_expect_success 'fetch submodule remote of differ
+ test_path_is_file child/a-submodule/moreecho.t
+'
+
-+test_expect_success 'fetch submodule remote of different non-origin name from superproject' '
++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
-+ git -C a-submodule checkout -b newmain2 master^ &&
++ # 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) &&
@@ t/t5572-pull-submodule.sh: test_expect_success 'fetch submodule remote of differ
test_done
- ## t/t7425-submodule-get-default-remote.sh (new) ##
+ ## t/t7426-submodule-get-default-remote.sh (new) ##
@@
+#!/bin/sh
+
builtin/submodule--helper.c | 38 +++++
submodule.c | 17 ++-
t/meson.build | 1 +
t/t5526-fetch-submodules.sh | 111 +++++++++++++-
t/t5572-pull-submodule.sh | 21 ++-
t/t7426-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++
6 files changed, 370 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 e5174ee575..bf241a7a1e 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..19c0f05144 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -836,9 +836,34 @@ test_expect_success "fetch new submodule commits on-demand outside standard refs
git update-ref refs/changes/3 $E &&
(
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
+ test_when_finished "rm -f $pwd/on-demand_submodule_fetch_trace" &&
+ GIT_TRACE="$pwd/on-demand_submodule_fetch_trace" \
+ git fetch --recurse-submodules origin refs/changes/3:refs/heads/my_branch 2>actual_fetch &&
+ test_cmp expect_fetch actual_fetch &&
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" \
+ "$pwd/on-demand_submodule_fetch_trace" &&
+ test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ origin" \
+ "$pwd/on-demand_submodule_fetch_trace" &&
git checkout --recurse-submodules FETCH_HEAD
)
'
@@ -929,6 +954,90 @@ 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 &&
+ (
+ 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
+ test_when_finished "rm -f $pwd/custom_on-demand_submodule_fetch_trace" &&
+ GIT_TRACE="$pwd/custom_on-demand_submodule_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 &&
+
+ 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" \
+ "$pwd/custom_on-demand_submodule_fetch_trace" &&
+ test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ custom_remote $D" \
+ "$pwd/custom_on-demand_submodule_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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v3] submodule: fetch missing objects from default remote
2026-02-25 21:55 ` Junio C Hamano
@ 2026-03-02 22:06 ` Jacob Keller
0 siblings, 0 replies; 36+ messages in thread
From: Jacob Keller @ 2026-03-02 22:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nasser Grainawi, git, D. Ben Knoble, Patrick Steinhardt
On Wed, Feb 25, 2026 at 1:55 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.keller@gmail.com> writes:
>
> > 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?
>
I'll re-review the latest version.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] submodule: fetch missing objects from default remote
2026-03-01 2:53 ` [PATCH v4] " Nasser Grainawi
@ 2026-03-02 22:09 ` Jacob Keller
2026-03-02 22:11 ` Junio C Hamano
2026-03-03 2:11 ` Junio C Hamano
2026-03-03 20:09 ` [PATCH v5] " Nasser Grainawi
2 siblings, 1 reply; 36+ messages in thread
From: Jacob Keller @ 2026-03-02 22:09 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Junio C Hamano
On Sat, Feb 28, 2026 at 6:53 PM Nasser Grainawi
<nasser.grainawi@oss.qualcomm.com> wrote:
>
> 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>
> ---
> I removed Jacob Keller's Reviewed-By since there are more significant
> edits to the tests in v4 that they haven't reviewed.
>
> v4 includes fixes for the issues Junio patched in seen and I confirmed
> it merges cleanly to seen (with v3 reverted) as well as next and passes
> tests.
>
v4 looks good, thanks!
Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] submodule: fetch missing objects from default remote
2026-03-02 22:09 ` Jacob Keller
@ 2026-03-02 22:11 ` Junio C Hamano
0 siblings, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-03-02 22:11 UTC (permalink / raw)
To: Jacob Keller; +Cc: Nasser Grainawi, git, D. Ben Knoble, Patrick Steinhardt
Jacob Keller <jacob.keller@gmail.com> writes:
>> v4 includes fixes for the issues Junio patched in seen and I confirmed
>> it merges cleanly to seen (with v3 reverted) as well as next and passes
>> tests.
>>
>
> v4 looks good, thanks!
>
> Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
Thanks. Will wiggle your Reviewed-by in while queuing the patch.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] submodule: fetch missing objects from default remote
2026-03-01 2:53 ` [PATCH v4] " Nasser Grainawi
2026-03-02 22:09 ` Jacob Keller
@ 2026-03-03 2:11 ` Junio C Hamano
2026-03-03 19:00 ` Nasser Grainawi
2026-03-03 20:09 ` [PATCH v5] " Nasser Grainawi
2 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-03-03 2:11 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
> 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>
> ---
> I removed Jacob Keller's Reviewed-By since there are more significant
> edits to the tests in v4 that they haven't reviewed.
>
> v4 includes fixes for the issues Junio patched in seen and I confirmed
> it merges cleanly to seen (with v3 reverted) as well as next and passes
> tests.
The tests in this patch seems to be broken. I didn't notice it
before merging it to 'seen', so tonight's integration CI is expected
to fail at the tip of 'seen'.
*** prove (shell & unit tests) ***
[18:10:13] t5526-fetch-submodules.sh .. 39/?
error: bug in the test script: test_when_finished does nothing in a subshell
[18:10:13] t5526-fetch-submodules.sh .. 42/?
error: bug in the test script: test_when_finished does nothing in a subshell
[18:10:13] t5526-fetch-submodules.sh .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 2/56 subtests
[18:10:28]
Test Summary Report
-------------------
t5526-fetch-submodules.sh (Wstat: 256 (exited 1) Tests: 56 Failed: 2)
Failed tests: 40, 44
Non-zero exit status: 1
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] submodule: fetch missing objects from default remote
2026-03-03 2:11 ` Junio C Hamano
@ 2026-03-03 19:00 ` Nasser Grainawi
2026-03-03 19:26 ` Nasser Grainawi
0 siblings, 1 reply; 36+ messages in thread
From: Nasser Grainawi @ 2026-03-03 19:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
On Mon, Mar 2, 2026 at 7:11 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
>
> > 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>
> > ---
> > I removed Jacob Keller's Reviewed-By since there are more significant
> > edits to the tests in v4 that they haven't reviewed.
> >
> > v4 includes fixes for the issues Junio patched in seen and I confirmed
> > it merges cleanly to seen (with v3 reverted) as well as next and passes
> > tests.
>
> The tests in this patch seems to be broken. I didn't notice it
> before merging it to 'seen', so tonight's integration CI is expected
> to fail at the tip of 'seen'.
>
>
>
> *** prove (shell & unit tests) ***
> [18:10:13] t5526-fetch-submodules.sh .. 39/?
> error: bug in the test script: test_when_finished does nothing in a subshell
And this is how I learned about TEST_SHELL_PATH. My Ubuntu machine has
sh linked to dash, so these errors weren't showing up for me.
Also, I just noticed I have $pwd in a couple places and need that to be
$(pwd), so I will send both those fixes.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] submodule: fetch missing objects from default remote
2026-03-03 19:00 ` Nasser Grainawi
@ 2026-03-03 19:26 ` Nasser Grainawi
2026-03-03 20:02 ` Junio C Hamano
0 siblings, 1 reply; 36+ messages in thread
From: Nasser Grainawi @ 2026-03-03 19:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
On Tue, Mar 3, 2026 at 12:00 PM Nasser Grainawi
<nasser.grainawi@oss.qualcomm.com> wrote:
>
> Also, I just noticed I have $pwd in a couple places and need that to be
> $(pwd), so I will send both those fixes.
Sorry, ignore this. L11 in this script has `pwd=$(pwd)`, so the $pwd
usage was intentional and not problematic.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] submodule: fetch missing objects from default remote
2026-03-03 19:26 ` Nasser Grainawi
@ 2026-03-03 20:02 ` Junio C Hamano
0 siblings, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-03-03 20:02 UTC (permalink / raw)
To: Nasser Grainawi; +Cc: git, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
> On Tue, Mar 3, 2026 at 12:00 PM Nasser Grainawi
> <nasser.grainawi@oss.qualcomm.com> wrote:
>>
>> Also, I just noticed I have $pwd in a couple places and need that to be
>> $(pwd), so I will send both those fixes.
>
> Sorry, ignore this. L11 in this script has `pwd=$(pwd)`, so the $pwd
> usage was intentional and not problematic.
Oh, then it would make it easier to fix, actually, since $pwd is
some fixed directory established long before the control flow went
into the subshell so the fix for test_when_finished that are used to
remove things under $pwd/ (not $(pwd)/) would just be the matter of
moving them out of the subshell, setting the finished handlers up
before we go into the subshells.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v5] submodule: fetch missing objects from default remote
2026-03-01 2:53 ` [PATCH v4] " Nasser Grainawi
2026-03-02 22:09 ` Jacob Keller
2026-03-03 2:11 ` Junio C Hamano
@ 2026-03-03 20:09 ` Nasser Grainawi
2026-03-03 20:47 ` Ramsay Jones
2026-03-03 23:40 ` [PATCH v6] " Nasser Grainawi
2 siblings, 2 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-03-03 20:09 UTC (permalink / raw)
To: git
Cc: Nasser Grainawi, D. Ben Knoble, Patrick Steinhardt, Jacob Keller,
Junio C Hamano
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>
---
Fixes for test_when_finished usage within a subshell.
Range-diff against v4:
1: 9c5a5df9a2 ! 1: 673ad0372a submodule: fetch missing objects from default remote
@@ t/meson.build: integration_tests = [
## t/t5526-fetch-submodules.sh ##
@@ t/t5526-fetch-submodules.sh: 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 &&
@@ t/t5526-fetch-submodules.sh: test_expect_success "fetch new submodule commits on
+ * branch $C -> FETCH_HEAD
+ Fetching submodule submodule/subdir/deepsubmodule
+ EOF
-+ test_when_finished "rm -f $pwd/on-demand_submodule_fetch_trace" &&
-+ GIT_TRACE="$pwd/on-demand_submodule_fetch_trace" \
-+ git fetch --recurse-submodules origin refs/changes/3:refs/heads/my_branch 2>actual_fetch &&
++ 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 &&
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" \
-+ "$pwd/on-demand_submodule_fetch_trace" &&
++ "$FETCH_TRACE" &&
+ test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ origin" \
-+ "$pwd/on-demand_submodule_fetch_trace" &&
++ "$FETCH_TRACE" &&
git checkout --recurse-submodules FETCH_HEAD
)
'
@@ t/t5526-fetch-submodules.sh: test_expect_success 'fetch new submodule commit int
+ -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 &&
+ DEEP_START=$(git -C submodule/subdir/deepsubmodule rev-parse --short \
@@ t/t5526-fetch-submodules.sh: test_expect_success 'fetch new submodule commit int
+ * branch $C -> FETCH_HEAD
+ Fetching submodule submodule/subdir/deepsubmodule
+ EOF
-+ test_when_finished "rm -f $pwd/custom_on-demand_submodule_fetch_trace" &&
-+ GIT_TRACE="$pwd/custom_on-demand_submodule_fetch_trace" \
-+ git fetch --recurse-submodules origin \
++ 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
@@ t/t5526-fetch-submodules.sh: test_expect_success 'fetch new submodule commit int
+ 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" \
-+ "$pwd/custom_on-demand_submodule_fetch_trace" &&
++ "$FETCH_TRACE" &&
+ test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ custom_remote $D" \
-+ "$pwd/custom_on-demand_submodule_fetch_trace" &&
++ "$FETCH_TRACE" &&
+ git checkout --recurse-submodules FETCH_HEAD
+ )
+'
builtin/submodule--helper.c | 38 +++++
submodule.c | 17 ++-
t/meson.build | 1 +
t/t5526-fetch-submodules.sh | 112 +++++++++++++-
t/t5572-pull-submodule.sh | 21 ++-
t/t7426-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++
6 files changed, 371 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..cc8aec5b1c 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -834,11 +834,37 @@ 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 &&
+ 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 &&
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 +955,90 @@ 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 &&
+ 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 &&
+
+ 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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v5] submodule: fetch missing objects from default remote
2026-03-03 20:09 ` [PATCH v5] " Nasser Grainawi
@ 2026-03-03 20:47 ` Ramsay Jones
2026-03-03 21:26 ` Junio C Hamano
2026-03-03 23:40 ` [PATCH v6] " Nasser Grainawi
1 sibling, 1 reply; 36+ messages in thread
From: Ramsay Jones @ 2026-03-03 20:47 UTC (permalink / raw)
To: Nasser Grainawi, git
Cc: D. Ben Knoble, Patrick Steinhardt, Jacob Keller, Junio C Hamano
On 03/03/2026 8:09 pm, Nasser Grainawi wrote:
> 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>
> ---
> Fixes for test_when_finished usage within a subshell.
>
The 't5526-fetch-submodules.sh' test failed for me tonight. Having seen an
earlier email about the test_when_finished failures I wasn't too surprised
but, once I looked at the failure, it was obviously not the cause of this
failure. Indeed, when I ran the test by hand, it passed ... :)
It seems the 'seen' branch (@764d09c9ce) has this v5 version of the patch
and (in this test anyway) has some flakiness:
$ cd t
$ ./t5526-fetch-submodules.sh --stress >out 2>&1
$ cat out
FAIL 4.1
FAIL 3.1
FAIL 5.1
OK 0.1
OK 1.1
OK 6.1
OK 7.1
OK 2.1
Log(s) of failed test run(s):
Contents of '/home/ramsay/git/t/test-results/t5526-fetch-submodules.stress-3.out':
Initialized empty Git repository in /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-3/.git/
expecting success of 5526.1 'setup':
...
expecting success of 5526.44 'fetch new submodule commits on-demand outside standard refspec with custom remote name':
...
+ diff -u expect_fetch_custom actual_fetch_warnings_removed
--- expect_fetch_custom 2026-03-03 20:35:13.949600802 +0000
+++ actual_fetch_warnings_removed 2026-03-03 20:35:14.150601532 +0000
@@ -4,9 +4,9 @@
Fetching submodule sub1/subdir/deepsubmodule
Fetching submodule submodule
Fetching submodule submodule/subdir/deepsubmodule
-From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/./sub1
- * branch 43c17d99ab9d4fcabf7107e36660b27113b54663 -> FETCH_HEAD
-Fetching submodule sub1/subdir/deepsubmodule
From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/submodule
* branch e38933e027ee8a2000f603124aa899302a09a51f -> FETCH_HEAD
Fetching submodule submodule/subdir/deepsubmodule
+From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/./sub1
+ * branch 43c17d99ab9d4fcabf7107e36660b27113b54663 -> FETCH_HEAD
+Fetching submodule sub1/subdir/deepsubmodule
error: last command exited with $?=1
not ok 44 - fetch new submodule commits on-demand outside standard refspec with custom remote name
...
$
From which I guess that the order of the output is somewhat unpredictable.
Also, other test files in that patch didn't fail for me with 'make test', but it
could be possible that they are also flaky. I didn't look.
Thanks.
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v5] submodule: fetch missing objects from default remote
2026-03-03 20:47 ` Ramsay Jones
@ 2026-03-03 21:26 ` Junio C Hamano
2026-03-03 23:29 ` Nasser Grainawi
0 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-03-03 21:26 UTC (permalink / raw)
To: Ramsay Jones
Cc: Nasser Grainawi, git, D. Ben Knoble, Patrick Steinhardt,
Jacob Keller
Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> + diff -u expect_fetch_custom actual_fetch_warnings_removed
> --- expect_fetch_custom 2026-03-03 20:35:13.949600802 +0000
> +++ actual_fetch_warnings_removed 2026-03-03 20:35:14.150601532 +0000
> @@ -4,9 +4,9 @@
> Fetching submodule sub1/subdir/deepsubmodule
> Fetching submodule submodule
> Fetching submodule submodule/subdir/deepsubmodule
> -From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/./sub1
> - * branch 43c17d99ab9d4fcabf7107e36660b27113b54663 -> FETCH_HEAD
> -Fetching submodule sub1/subdir/deepsubmodule
> From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/submodule
> * branch e38933e027ee8a2000f603124aa899302a09a51f -> FETCH_HEAD
> Fetching submodule submodule/subdir/deepsubmodule
> +From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/./sub1
> + * branch 43c17d99ab9d4fcabf7107e36660b27113b54663 -> FETCH_HEAD
> +Fetching submodule sub1/subdir/deepsubmodule
> error: last command exited with $?=1
> not ok 44 - fetch new submodule commits on-demand outside standard refspec with custom remote name
>
> ...
>
> $
>
> From which I guess that the order of the output is somewhat unpredictable.
>
> Also, other test files in that patch didn't fail for me with 'make test', but it
> could be possible that they are also flaky. I didn't look.
Ah, looks like the command tries to fetch from multiple places in
parallel and it is up to the luck which one reports its result
first? We probably do not want such a "human readable progress
output should look exactly like this" test.
Thanks for reporting.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v5] submodule: fetch missing objects from default remote
2026-03-03 21:26 ` Junio C Hamano
@ 2026-03-03 23:29 ` Nasser Grainawi
0 siblings, 0 replies; 36+ messages in thread
From: Nasser Grainawi @ 2026-03-03 23:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ramsay Jones, git, D. Ben Knoble, Patrick Steinhardt,
Jacob Keller
On Tue, Mar 3, 2026 at 2:26 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>
> > + diff -u expect_fetch_custom actual_fetch_warnings_removed
> > --- expect_fetch_custom 2026-03-03 20:35:13.949600802 +0000
> > +++ actual_fetch_warnings_removed 2026-03-03 20:35:14.150601532 +0000
> > @@ -4,9 +4,9 @@
> > Fetching submodule sub1/subdir/deepsubmodule
> > Fetching submodule submodule
> > Fetching submodule submodule/subdir/deepsubmodule
> > -From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/./sub1
> > - * branch 43c17d99ab9d4fcabf7107e36660b27113b54663 -> FETCH_HEAD
> > -Fetching submodule sub1/subdir/deepsubmodule
> > From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/submodule
> > * branch e38933e027ee8a2000f603124aa899302a09a51f -> FETCH_HEAD
> > Fetching submodule submodule/subdir/deepsubmodule
> > +From /home/ramsay/git/t/trash directory.t5526-fetch-submodules.stress-5/./sub1
> > + * branch 43c17d99ab9d4fcabf7107e36660b27113b54663 -> FETCH_HEAD
> > +Fetching submodule sub1/subdir/deepsubmodule
> > error: last command exited with $?=1
> > not ok 44 - fetch new submodule commits on-demand outside standard refspec with custom remote name
> >
> > ...
> >
> > $
> >
> > From which I guess that the order of the output is somewhat unpredictable.
> >
> > Also, other test files in that patch didn't fail for me with 'make test', but it
> > could be possible that they are also flaky. I didn't look.
>
> Ah, looks like the command tries to fetch from multiple places in
> parallel and it is up to the luck which one reports its result
> first? We probably do not want such a "human readable progress
> output should look exactly like this" test.
>
> Thanks for reporting.
Yes, thank you. I'll drop that part of the test and just keep the
GIT_TRACE check.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v6] submodule: fetch missing objects from default remote
2026-03-03 20:09 ` [PATCH v5] " Nasser Grainawi
2026-03-03 20:47 ` Ramsay Jones
@ 2026-03-03 23:40 ` Nasser Grainawi
2026-03-09 23:40 ` Junio C Hamano
1 sibling, 1 reply; 36+ messages in thread
From: Nasser Grainawi @ 2026-03-03 23:40 UTC (permalink / raw)
To: git
Cc: Nasser Grainawi, D. Ben Knoble, Patrick Steinhardt, Jacob Keller,
Junio C Hamano
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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v6] submodule: fetch missing objects from default remote
2026-03-03 23:40 ` [PATCH v6] " Nasser Grainawi
@ 2026-03-09 23:40 ` Junio C Hamano
0 siblings, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-03-09 23:40 UTC (permalink / raw)
To: git; +Cc: Nasser Grainawi, D. Ben Knoble, Patrick Steinhardt, Jacob Keller
Nasser Grainawi <nasser.grainawi@oss.qualcomm.com> writes:
> 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.
This has gone quiet. Shall we declare victory and mark the topic
for 'next'?
Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2026-03-09 23:40 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-12 21:36 [PATCH] Fetch missing submodule objects from default remote Nasser Grainawi
2026-01-13 2:17 ` Jacob Keller
2026-01-13 21:51 ` Ben Knoble
2026-01-13 22:41 ` Nasser Grainawi
2026-01-14 2:19 ` D. Ben Knoble
2026-01-14 14:10 ` Junio C Hamano
2026-01-14 21:22 ` Ben Knoble
2026-01-14 14:05 ` Junio C Hamano
2026-01-14 19:23 ` Nasser Grainawi
2026-01-14 19:48 ` [PATCH v2] submodule: fetch missing " Nasser Grainawi
2026-01-21 0:48 ` Nasser Grainawi
2026-01-22 15:27 ` [PATCH v3] " Nasser Grainawi
2026-01-22 18:49 ` Junio C Hamano
2026-01-22 20:16 ` Jacob Keller
2026-01-22 20:38 ` Junio C Hamano
2026-02-25 21:55 ` Junio C Hamano
2026-03-02 22:06 ` Jacob Keller
2026-02-27 18:29 ` Nasser Grainawi
2026-01-22 21:21 ` Junio C Hamano
2026-01-23 23:26 ` Junio C Hamano
2026-01-24 2:18 ` Junio C Hamano
2026-02-20 23:12 ` Junio C Hamano
2026-02-27 17:20 ` Nasser Grainawi
2026-03-01 2:53 ` [PATCH v4] " Nasser Grainawi
2026-03-02 22:09 ` Jacob Keller
2026-03-02 22:11 ` Junio C Hamano
2026-03-03 2:11 ` Junio C Hamano
2026-03-03 19:00 ` Nasser Grainawi
2026-03-03 19:26 ` Nasser Grainawi
2026-03-03 20:02 ` Junio C Hamano
2026-03-03 20:09 ` [PATCH v5] " Nasser Grainawi
2026-03-03 20:47 ` Ramsay Jones
2026-03-03 21:26 ` Junio C Hamano
2026-03-03 23:29 ` Nasser Grainawi
2026-03-03 23:40 ` [PATCH v6] " Nasser Grainawi
2026-03-09 23:40 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox