threads / bug / 59896

bug: submodule update fails to fetch

Subject: bug: submodule update fails to fetch

## tl;dr

12 messages between Jun 22, 2023 and Oct 9, 2024.

replies: 11people: 5as markdown or json

Sergei Golubchik· Jun 22, 2023, 11:09 UTC · lore
Hi,

Sometimes (my local repository has lots of branches) after switching branches

  git submodule update --init --recursive
fails with something like
  fatal: transport 'file' not allowed
  fatal: Fetched in submodule path 'wsrep-lib', but it did not contain e238c0d240c2557229b0523a4a032f3cf8b41639. Direct fetching of that commit failed.

the submodule transport is not 'file' (it's https) and the direct fetching of the commit actually works:

  cd wsrep-lib
  git fetch origin e238c0d240c2557229b0523a4a032f3cf8b41639
  git checkout e238c0d240c2557229b0523a4a032f3cf8b41639
  cd ..
after that
  git submodule update --init --recursive

succeeds. This happens deterministically, but depends on the old and new commits in the last checkout. As a workaround we've had to change our CI to do

  git submodule foreach --recursive 'git fetch origin $sha1;git checkout --force FETCH_HEAD'
This is the bit from `git bugreport`:

[System Info] git version: git version 2.39.3 cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Linux 5.15.88-gentoo #1 SMP Wed Feb 15 16:42:45 CET 2023 x86_64 compiler info: gnuc: 11.3 libc info: glibc: 2.36 $SHELL (typically, interactive shell): /bin/bash

[Enabled Hooks]

Regards, Sergei

Taylor Blau· Jun 22, 2023, 13:07 UTC · re: Sergei Golubchik · lore

Re: bug: submodule update fails to fetch

On Thu, Jun 22, 2023 at 01:09:07PM +0200, Sergei Golubchik wrote:
Show 26 quoted lines
> Hi,
>
> Sometimes (my local repository has lots of branches) after switching
> branches
>
>   git submodule update --init --recursive
>
> fails with something like
>
>   fatal: transport 'file' not allowed
>   fatal: Fetched in submodule path 'wsrep-lib', but it did not contain e238c0d240c2557229b0523a4a032f3cf8b41639. Direct fetching of that commit failed.
>
> the submodule transport is not 'file' (it's https) and the direct
> fetching of the commit actually works:
>
>   cd wsrep-lib
>   git fetch origin e238c0d240c2557229b0523a4a032f3cf8b41639
>   git checkout e238c0d240c2557229b0523a4a032f3cf8b41639
>   cd ..
>
> after that
>
>   git submodule update --init --recursive
>
> succeeds. This happens deterministically, but depends on the old and new
> commits in the last checkout. As a workaround we've had to change our CI to do

It makes sense that after manually fetching the desired tip that the submodule update goes through OK, because there is nothing to do (the checked-out state matches what's in .gitmodules), so we don't have to use any transport mechanism.

I recently changed the submodule update rules to disallow file-based submodules when not directly executed by the user. See a1d4f67c12 (transport: make `protocol.file.allow` be "user" by default, 2022-07-29) for more of the details there.

So in the short term, you should able to work around what you're seeing by setting `protocol.file.allow` to "always" with something like

    $ git config --global protocol.file.allow always

But in the short-term, I am curious why we are complaining about needing to use the file transport when you claim that the submodule actually needs the HTTPS transport.

Are you able to share a copy of your repository, and/or its .gitmodules file, and your repository-local .gitconfig, as well? Do you have some `url.<base>.insteadOf` value configured elsewhere that would be rewriting those paths for you?

Thanks, Taylor

Sergei Golubchik· Jun 22, 2023, 16:39 UTC · re: Taylor Blau · lore

Re: bug: submodule update fails to fetch

Hi, Taylor,
On Jun 22, Taylor Blau wrote:
Show 31 quoted lines
> On Thu, Jun 22, 2023 at 01:09:07PM +0200, Sergei Golubchik wrote:
> > Hi,
> >
> > Sometimes (my local repository has lots of branches) after switching
> > branches
> >
> >   git submodule update --init --recursive
> >
> > fails with something like
> >
> >   fatal: transport 'file' not allowed
> >   fatal: Fetched in submodule path 'wsrep-lib', but it did not contain e238c0d240c2557229b0523a4a032f3cf8b41639. Direct fetching of that commit failed.
> >
> > the submodule transport is not 'file' (it's https) and the direct
> > fetching of the commit actually works:
> >
> >   cd wsrep-lib
> >   git fetch origin e238c0d240c2557229b0523a4a032f3cf8b41639
> >   git checkout e238c0d240c2557229b0523a4a032f3cf8b41639
> >   cd ..
> >
> > after that
> >
> >   git submodule update --init --recursive
> >
> > succeeds.
> 
> It makes sense that after manually fetching the desired tip that the
> submodule update goes through OK, because there is nothing to do (the
> checked-out state matches what's in .gitmodules), so we don't have to
> use any transport mechanism.

Right. I just used it to show that git thinks the submodule was updated correctly and doesn't try to do anything after that.

> I recently changed the submodule update rules to disallow file-based
> submodules when not directly executed by the user. See a1d4f67c12
> (transport: make `protocol.file.allow` be "user" by default, 2022-07-29)
> for more of the details there.
Yes, I've seen it. That submodule shouldn't be affected:
  $ git remote -v
  origin  https://github.com/codership/wsrep-lib.git (fetch)
  origin  https://github.com/codership/wsrep-lib.git (push)

so I wouldn't want to circumvent your fix and allow the file transport that we aren't using.

Show 8 quoted lines
> But in the short-term, I am curious why we are complaining about needing
> to use the file transport when you claim that the submodule actually
> needs the HTTPS transport.
> 
> Are you able to share a copy of your repository, and/or its .gitmodules
> file, and your repository-local .gitconfig, as well? Do you have some
> `url.<base>.insteadOf` value configured elsewhere that would be
> rewriting those paths for you?

No insteadOf. Let me try to... Okay, here's the bug. In submodule--helper.c, fetch_in_submodule() function, there're lines: --------------------------- 2211 if (oid) { 2212 char *hex = oid_to_hex(oid); 2213 char *remote = get_default_remote(); 2214 2215 strvec_pushl(&cp.args, remote, hex, NULL); 2216 free(remote); 2217 } ---------------------------

this get_default_remote() appears to be getting the default remote name for the main repository and then uses it to fetch from the submodule.

It happens that my default remote isn't "origin" (long story), it's "github", but in the submodule it's of course "origin", there's no "github" remote there. As a result, `git submodule update` runs the command

  git fetch github ${commit_hash}
in the submodule, and that's interpreted as 'file' transport.

To repeat this you need a repository where the default remote isn't "origin" and a submodule where the commit cannot be fetched by simply `git fetch` and needs a direct fetch.

Hope this helps.

Regards, Sergei

Jacob Keller· Jun 23, 2023, 07:26 UTC · re: Sergei Golubchik · lore

Re: bug: submodule update fails to fetch

On Thu, Jun 22, 2023 at 9:57 AM Sergei Golubchik <vuvova@gmail.com> wrote:
Show 95 quoted lines
>
> Hi, Taylor,
>
> On Jun 22, Taylor Blau wrote:
> > On Thu, Jun 22, 2023 at 01:09:07PM +0200, Sergei Golubchik wrote:
> > > Hi,
> > >
> > > Sometimes (my local repository has lots of branches) after switching
> > > branches
> > >
> > >   git submodule update --init --recursive
> > >
> > > fails with something like
> > >
> > >   fatal: transport 'file' not allowed
> > >   fatal: Fetched in submodule path 'wsrep-lib', but it did not contain e238c0d240c2557229b0523a4a032f3cf8b41639. Direct fetching of that commit failed.
> > >
> > > the submodule transport is not 'file' (it's https) and the direct
> > > fetching of the commit actually works:
> > >
> > >   cd wsrep-lib
> > >   git fetch origin e238c0d240c2557229b0523a4a032f3cf8b41639
> > >   git checkout e238c0d240c2557229b0523a4a032f3cf8b41639
> > >   cd ..
> > >
> > > after that
> > >
> > >   git submodule update --init --recursive
> > >
> > > succeeds.
> >
> > It makes sense that after manually fetching the desired tip that the
> > submodule update goes through OK, because there is nothing to do (the
> > checked-out state matches what's in .gitmodules), so we don't have to
> > use any transport mechanism.
>
> Right. I just used it to show that git thinks the submodule was updated
> correctly and doesn't try to do anything after that.
>
> > I recently changed the submodule update rules to disallow file-based
> > submodules when not directly executed by the user. See a1d4f67c12
> > (transport: make `protocol.file.allow` be "user" by default, 2022-07-29)
> > for more of the details there.
>
> Yes, I've seen it. That submodule shouldn't be affected:
>
>   $ git remote -v
>   origin  https://github.com/codership/wsrep-lib.git (fetch)
>   origin  https://github.com/codership/wsrep-lib.git (push)
>
> so I wouldn't want to circumvent your fix and allow the file transport
> that we aren't using.
>
> > But in the short-term, I am curious why we are complaining about needing
> > to use the file transport when you claim that the submodule actually
> > needs the HTTPS transport.
> >
> > Are you able to share a copy of your repository, and/or its .gitmodules
> > file, and your repository-local .gitconfig, as well? Do you have some
> > `url.<base>.insteadOf` value configured elsewhere that would be
> > rewriting those paths for you?
>
> No insteadOf. Let me try to...
> Okay, here's the bug. In submodule--helper.c, fetch_in_submodule()
> function, there're lines:
> ---------------------------
> 2211         if (oid) {
> 2212                 char *hex = oid_to_hex(oid);
> 2213                 char *remote = get_default_remote();
> 2214
> 2215                 strvec_pushl(&cp.args, remote, hex, NULL);
> 2216                 free(remote);
> 2217         }
> ---------------------------
>
> this get_default_remote() appears to be getting the default remote name
> for the main repository and then uses it to fetch from the submodule.
>
> It happens that my default remote isn't "origin" (long story), it's
> "github", but in the submodule it's of course "origin", there's no
> "github" remote there. As a result, `git submodule update` runs the
> command
>
>   git fetch github ${commit_hash}
>
> in the submodule, and that's interpreted as 'file' transport.
>
> To repeat this you need a repository where the default remote isn't
> "origin" and a submodule where the commit cannot be fetched by simply
> `git fetch` and needs a direct fetch.
>
> Hope this helps.
>
> Regards,
> Sergei

I recently experienced something similar when changing the default remote name for some of my repositories. I had wondered what causes problems because submodule update would sometimes fail but other times succeed.

Daniel Black· Oct 1, 2024, 07:24 UTC · re: Sergei Golubchik · lore

[PATCH] submodule: correct remote name with fetch

The fetching of submodules erroniously used the main repository remote name instead of the submodule remote name[1].

Correct this by using the correct function to reteive the remote name.

1. https://www.spinics.net/lists/git/msg462320.html
Signed-off-by: Daniel Black <daniel@mariadb.org>
---
 builtin/submodule--helper.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a1ada86952..210ae2570a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2322,7 +2322,12 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,
 		strvec_pushf(&cp.args, "--depth=%d", depth);
 	if (oid) {
 		char *hex = oid_to_hex(oid);
-		char *remote = get_default_remote();
+		char *remote;
+		int code;
+
+		code = get_default_remote_submodule(module_path, &remote);
+		if (code)
+			return code;
 
 		strvec_pushl(&cp.args, remote, hex, NULL);
 		free(remote);
-- 
2.46.2
Junio C Hamano· Oct 1, 2024, 17:27 UTC · re: Daniel Black · lore

Re: [PATCH] submodule: correct remote name with fetch

Daniel Black <daniel@mariadb.org> writes:
> The fetching of submodules erroniously used
> the main repository remote name instead of the
> submodule remote name[1].

Please write the problem description in the present tense, i.e. "the code does this, which is incorrect in this way".

> Correct this by using the correct function
> to reteive the remote name.
That's gives the same as "fix it" ;-).

Can we phrase it in end-user observable terms, for example, "instead of grabbing the default remote of the superproject repository, ask the default remote of the submodule we are going to run 'git fetch' in". It is a mere implementation detail of doing so, to call get_default_remote_submodule() instead of get_default_remote().

> 1. https://www.spinics.net/lists/git/msg462320.html

Please use the URL that shows the Message-Id when referring to a message in the list archive. E.g.,

    https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/
    https://public-inbox.org/git/ZJR5SPDj4Wt_gmRO@pweza/
Show 20 quoted lines
> Signed-off-by: Daniel Black <daniel@mariadb.org>
> ---
>  builtin/submodule--helper.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index a1ada86952..210ae2570a 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2322,7 +2322,12 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,
>  		strvec_pushf(&cp.args, "--depth=%d", depth);
>  	if (oid) {
>  		char *hex = oid_to_hex(oid);
> -		char *remote = get_default_remote();
> +		char *remote;
> +		int code;
> +
> +		code = get_default_remote_submodule(module_path, &remote);
> +		if (code)
> +			return code;

We never failed in this function to return without calling run_command(). Now we do.

Shouldn't we clean up the child_process structure we allocated and prepared before returning like this? It would just be the matter of calling child_process_clear(&cp), perhaps?

>  		strvec_pushl(&cp.args, remote, hex, NULL);
>  		free(remote);
Thanks.
Junio C Hamano· Oct 1, 2024, 17:34 UTC · re: Junio C Hamano · lore

Re: [PATCH] submodule: correct remote name with fetch

Junio C Hamano <gitster@pobox.com> writes:
Sorry, forgot to say one more thing before pressing [SEND].
>>  builtin/submodule--helper.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)

Please add a test or two, that fail without this change (to demonstrate the existing bug) and succeed with this change (to make sure if somebody breaks the code you fixed again in the future, such a breakage is caught).

Thanks.
Daniel Black· Oct 8, 2024, 01:49 UTC · re: Junio C Hamano · lore

I have this so far, but I need a hand with the test case.
With a manual hack of the code:
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2450,7 +2450,7 @@ static int run_update_procedure(const struct update_data *ud)
                 * Now we tried the usual fetch, but `oid` may
                 * not be reachable from any of the refs.
                 */
-               if (!is_tip_reachable(ud->sm_path, &ud->oid) &&
+               if (is_tip_reachable(ud->sm_path, &ud->oid) &&
                    fetch_in_submodule(ud->sm_path, ud->depth, ud->quiet, &ud->oid))
                        return die_message(_("Fetched in submodule path '%s', but it did not "
                                             "contain %s. Direct fetching of that commit failed."),

I'm able to using the GIT_TRACE= of git submodule update arrive at the
correct fetching from origin in the test case where before the code fix
it was fetching from remote o1.

trace: run_command: cd sub; unset GIT_PREFIX; GIT_DIR=.git git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all
trace: start_command: /home/dan/repos/git/git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all
trace: built-in: git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all
trace: run_command: cd sub; unset GIT_PREFIX; GIT_DIR=.git git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all
trace: start_command: /home/dan/repos/git/git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all
trace: built-in: git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all
trace: run_command: cd sub; unset GIT_PREFIX; GIT_DIR=.git git fetch origin 27fe1b65df6f55a58636afcba364dfcb64916cd6
trace: start_command: /home/dan/repos/git/git fetch origin 27fe1b65df6f55a58636afcba364dfcb64916cd6

As Sergei said in the original referenced message "a submodule where the commit cannot be fetched by simply
`git fetch` and needs a direct fetch".

So this is the test case that I'm having trouble generating.
Daniel Black· Oct 8, 2024, 01:49 UTC · re: Daniel Black · lore

[RFC PATCH v2] submodule: correct remote name with fetch

The code fetches the submodules remote based on the superproject remote name instead of the submodule remote name[1].

Instead of grabbing the default remote of the superproject repository, ask the default remote of the submodule we are going to run 'git fetch' in.

1. https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/
Signed-off-by: Daniel Black <daniel@mariadb.org>
---
 builtin/submodule--helper.c |  9 ++++-
 t/t5568-fetch-submodule.sh  | 65 +++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100755 t/t5568-fetch-submodule.sh
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a1ada86952..567d21d330 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2322,7 +2322,14 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,
 		strvec_pushf(&cp.args, "--depth=%d", depth);
 	if (oid) {
 		char *hex = oid_to_hex(oid);
-		char *remote = get_default_remote();
+		char *remote;
+		int code;
+
+		code = get_default_remote_submodule(module_path, &remote);
+		if (code) {
+			child_process_clear(&cp);
+			return code;
+		}
 
 		strvec_pushl(&cp.args, remote, hex, NULL);
 		free(remote);
diff --git a/t/t5568-fetch-submodule.sh b/t/t5568-fetch-submodule.sh
new file mode 100755
index 0000000000..56978bcfd7
--- /dev/null
+++ b/t/t5568-fetch-submodule.sh
@@ -0,0 +1,65 @@
+#!/bin/sh
+
+test_description='fetch can handle submodules origin names'
+
+GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
+export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+SUPER="$HTTPD_DOCUMENT_ROOT_PATH/super"
+SUB="$HTTPD_DOCUMENT_ROOT_PATH/sub"
+SUPER_URI="$HTTPD_URL/smart/super"
+SUB_URI="$HTTPD_URL/smart/sub"
+
+setup() {
+	SERVER="$1"
+	git init "$SERVER" &&
+	test_when_finished 'rm -rf "$SERVER"' &&
+	test_config -C "$SERVER" http.receivepack true
+}
+
+test_expect_success 'fetch submodule remote of different name from superproject' '
+	setup "$SUPER" &&
+	test_create_repo super &&
+	test_commit -C super bar &&
+	git -C super remote add upstream "$SUPER_URI" &&
+	test_config -C super push.default upstream &&
+	git -C super push --set-upstream upstream master:main &&
+
+	setup "$SUB" &&
+	test_create_repo sub &&
+	test_commit -C sub foo &&
+	git -C sub branch newmain &&
+	test_commit -C sub morefoo &&
+	test_commit -C sub moremorefoo &&
+	git -C sub remote add upstream "$SUB_URI" &&
+	test_config -C sub push.default upstream &&
+	git -C sub push --set-upstream upstream master:main &&
+
+	git -C super submodule add --branch main -- "$SUB_URI" sub &&
+	git -C super commit -am "add submodule" &&
+	git -C super push &&
+
+	# Needs to create unreachable commit from current master branch.
+	git -C sub checkout newmain &&
+	test_commit -C sub echo &&
+	test_commit -C sub moreecho &&
+	git -C sub push  --set-upstream upstream newmain:newmain &&
+
+	git clone --origin o1 --branch main -- "$SUPER_URI" superproject &&
+	git -C superproject submodule update --init &&
+
+	git -C super/sub fetch &&
+	git -C super/sub checkout origin/newmain &&
+	git -C super commit -m "update submodule" sub &&
+	git -C super push && 
+
+	git -C superproject pull --no-recurse-submodules &&
+	git -C superproject submodule update
+'
+
+test_done
-- 
2.46.2
Junio C Hamano· Oct 8, 2024, 19:23 UTC · re: Daniel Black · lore

Re: [RFC PATCH v2] submodule: correct remote name with fetch

Daniel Black <daniel@mariadb.org> writes:
Show 11 quoted lines
> The code fetches the submodules remote based on the superproject remote name
> instead of the submodule remote name[1].
>
> Instead of grabbing the default remote of the superproject repository, ask
> the default remote of the submodule we are going to run 'git fetch' in.
>
> 1. https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/
>
> Signed-off-by: Daniel Black <daniel@mariadb.org>
> ---
>  builtin/submodule--helper.c |  9 ++++-
The proposed log message is very well written.
>  t/t5568-fetch-submodule.sh  | 65 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+), 1 deletion(-)
>  create mode 100755 t/t5568-fetch-submodule.sh
Hmph,
	$ git grep "submodule update" t/

gives quite a many hits in existing tests. Didn't any of them have sufficient preparation steps that testing of this bugfix can reuse?

A test on "submodule update" behaviour tends to need quite a lot of preparation. Preparing the superproject, addition of a submodule to it, cloning of these two projects, and then half-preparing a clone of these super-sub arrangement. All of that needs to happen before we can say "submodule update" and observe the outcome to see if the bug still exists. If we can piggy-back on a test script that already has such preparation, it would be far more preferrable than having to do another set of preparation.

Another thing. If this is not about a bug that only manifests when the HTTP transport is in use, it is strongly preferred to avoid turning it into an httpd test. Some developers and/or environments skip them.

Show 17 quoted lines
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index a1ada86952..567d21d330 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2322,7 +2322,14 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,
>  		strvec_pushf(&cp.args, "--depth=%d", depth);
>  	if (oid) {
>  		char *hex = oid_to_hex(oid);
> -		char *remote = get_default_remote();
> +		char *remote;
> +		int code;
> +
> +		code = get_default_remote_submodule(module_path, &remote);
> +		if (code) {
> +			child_process_clear(&cp);
> +			return code;
> +		}

The get_default_remote_submodule() helper eventually calls into repo_get_default_remote() that returns an allocated string in remote, but it only does so when it succeeds, so this early return does not have to worry about leaking "remote" here. Good.

The code change looks quite straight-forward and looking good.
>  		strvec_pushl(&cp.args, remote, hex, NULL);
>  		free(remote);
Thanks.
Daniel Black· Oct 9, 2024, 03:32 UTC · re: Junio C Hamano · lore

[PATCH v3] submodule: correct remote name with fetch

The code fetches the submodules remote based on the superproject remote name instead of the submodule remote name[1].

Instead of grabbing the default remote of the superproject repository, ask the default remote of the submodule we are going to run 'git fetch' in.

1. https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/
Signed-off-by: Daniel Black <daniel@mariadb.org>
---
 builtin/submodule--helper.c |  9 ++++++++-
 t/t5572-pull-submodule.sh   | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index dc89488a7d..b6b5f1ebde 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2333,7 +2333,14 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,
 		strvec_pushf(&cp.args, "--depth=%d", depth);
 	if (oid) {
 		char *hex = oid_to_hex(oid);
-		char *remote = get_default_remote();
+		char *remote;
+		int code;
+
+		code = get_default_remote_submodule(module_path, &remote);
+		if (code) {
+			child_process_clear(&cp);
+			return code;
+		}
 
 		strvec_pushl(&cp.args, remote, hex, NULL);
 		free(remote);
diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
index 916e58c166..9b6cf8d88b 100755
--- a/t/t5572-pull-submodule.sh
+++ b/t/t5572-pull-submodule.sh
@@ -230,6 +230,7 @@ test_expect_success 'branch has no merge base with remote-tracking counterpart'
 
 	test_create_repo a-submodule &&
 	test_commit -C a-submodule foo &&
+	test_commit -C a-submodule bar &&
 
 	test_create_repo parent &&
 	git -C parent submodule add "$(pwd)/a-submodule" &&
@@ -246,4 +247,23 @@ test_expect_success 'branch has no merge base with remote-tracking counterpart'
 	git -C child pull --recurse-submodules --rebase
 '
 
+test_expect_success 'fetch submodule remote of different name from superproject' '
+	git -C child remote rename origin o1 &&
+	git -C child submodule update --init &&
+
+	# Needs to create unreachable commit from current master branch.
+	git -C a-submodule checkout -b newmain HEAD^ &&
+	test_commit -C a-submodule echo &&
+	test_commit -C a-submodule moreecho &&
+	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" a-submodule &&
+	git -C a-submodule reset --hard HEAD^^ &&
+
+	git -C child pull --no-recurse-submodules &&
+	git -C child submodule update
+'
+
 test_done
-- 
2.46.2
Junio C Hamano· Oct 9, 2024, 17:51 UTC · re: Daniel Black · lore

Re: [PATCH v3] submodule: correct remote name with fetch

Daniel Black <daniel@mariadb.org> writes:
Show 13 quoted lines
> The code fetches the submodules remote based on the superproject remote name
> instead of the submodule remote name[1].
>
> Instead of grabbing the default remote of the superproject repository, ask
> the default remote of the submodule we are going to run 'git fetch' in.
>
> 1. https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/
>
> Signed-off-by: Daniel Black <daniel@mariadb.org>
> ---
>  builtin/submodule--helper.c |  9 ++++++++-
>  t/t5572-pull-submodule.sh   | 20 ++++++++++++++++++++
>  2 files changed, 28 insertions(+), 1 deletion(-)
Excellent.  Will queue.
Thanks.
Show 36 quoted lines
> diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh
> index 916e58c166..9b6cf8d88b 100755
> --- a/t/t5572-pull-submodule.sh
> +++ b/t/t5572-pull-submodule.sh
> @@ -230,6 +230,7 @@ test_expect_success 'branch has no merge base with remote-tracking counterpart'
>  
>  	test_create_repo a-submodule &&
>  	test_commit -C a-submodule foo &&
> +	test_commit -C a-submodule bar &&
>  
>  	test_create_repo parent &&
>  	git -C parent submodule add "$(pwd)/a-submodule" &&
> @@ -246,4 +247,23 @@ test_expect_success 'branch has no merge base with remote-tracking counterpart'
>  	git -C child pull --recurse-submodules --rebase
>  '
>  
> +test_expect_success 'fetch submodule remote of different name from superproject' '
> +	git -C child remote rename origin o1 &&
> +	git -C child submodule update --init &&
> +
> +	# Needs to create unreachable commit from current master branch.
> +	git -C a-submodule checkout -b newmain HEAD^ &&
> +	test_commit -C a-submodule echo &&
> +	test_commit -C a-submodule moreecho &&
> +	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" a-submodule &&
> +	git -C a-submodule reset --hard HEAD^^ &&
> +
> +	git -C child pull --no-recurse-submodules &&
> +	git -C child submodule update
> +'
> +
>  test_done

← back to recent threads