{"thread":{"id":"59896","subject":"bug: submodule update fails to fetch","startedAt":"2023-06-22T11:09:23Z","lastAt":"2024-10-09T17:51:42Z","messageCount":12,"participants":["Sergei Golubchik","Taylor Blau","Jacob Keller","Daniel Black","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"478681","messageId":"ZJQr0_aC-NlLXDgj@pweza","threadId":"59896","inReplyTo":null,"subject":"bug: submodule update fails to fetch","fromName":"Sergei Golubchik","fromEmail":"vuvova@gmail.com","sentAt":"2023-06-22T11:09:07Z","receivedAt":"2023-06-22T11:09:23Z","isPatch":false,"sender":{"key":"vuvova@gmail.com","avatar":null},"body":"Hi,\n\nSometimes (my local repository has lots of branches) after switching\nbranches\n\n git submodule update --init --recursive\n\nfails with something like\n\n fatal: transport 'file' not allowed\n fatal: Fetched in submodule path 'wsrep-lib', but it did not contain e238c0d240c2557229b0523a4a032f3cf8b41639. Direct fetching of that commit failed.\n\nthe submodule transport is not 'file' (it's https) and the direct\nfetching of the commit actually works:\n\n cd wsrep-lib\n git fetch origin e238c0d240c2557229b0523a4a032f3cf8b41639\n git checkout e238c0d240c2557229b0523a4a032f3cf8b41639\n cd ..\n\nafter that\n\n git submodule update --init --recursive\n\nsucceeds. This happens deterministically, but depends on the old and new\ncommits in the last checkout. As a workaround we've had to change our CI to do\n\n git submodule foreach --recursive 'git fetch origin $sha1;git checkout --force FETCH_HEAD'\n\nThis is the bit from `git bugreport`:\n\n[System Info]\ngit version:\ngit version 2.39.3\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Linux 5.15.88-gentoo #1 SMP Wed Feb 15 16:42:45 CET 2023 x86_64\ncompiler info: gnuc: 11.3\nlibc info: glibc: 2.36\n$SHELL (typically, interactive shell): /bin/bash\n\n[Enabled Hooks]\n\nRegards,\nSergei\n"},{"id":"478689","messageId":"ZJRHlJvE4BMue1/Z@nand.local","threadId":"59896","inReplyTo":"ZJQr0_aC-NlLXDgj@pweza","subject":"Re: bug: submodule update fails to fetch","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-06-22T13:07:32Z","receivedAt":"2023-06-22T13:07:47Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Jun 22, 2023 at 01:09:07PM +0200, Sergei Golubchik wrote:\n> Hi,\n>\n> Sometimes (my local repository has lots of branches) after switching\n> branches\n>\n> git submodule update --init --recursive\n>\n> fails with something like\n>\n> fatal: transport 'file' not allowed\n> fatal: Fetched in submodule path 'wsrep-lib', but it did not contain e238c0d240c2557229b0523a4a032f3cf8b41639. Direct fetching of that commit failed.\n>\n> the submodule transport is not 'file' (it's https) and the direct\n> fetching of the commit actually works:\n>\n> cd wsrep-lib\n> git fetch origin e238c0d240c2557229b0523a4a032f3cf8b41639\n> git checkout e238c0d240c2557229b0523a4a032f3cf8b41639\n> cd ..\n>\n> after that\n>\n> git submodule update --init --recursive\n>\n> succeeds. This happens deterministically, but depends on the old and new\n> commits in the last checkout. As a workaround we've had to change our CI to do\n\nIt makes sense that after manually fetching the desired tip that the\nsubmodule update goes through OK, because there is nothing to do (the\nchecked-out state matches what's in .gitmodules), so we don't have to\nuse any transport mechanism.\n\nI recently changed the submodule update rules to disallow file-based\nsubmodules when not directly executed by the user. See a1d4f67c12\n(transport: make `protocol.file.allow` be \"user\" by default, 2022-07-29)\nfor more of the details there.\n\nSo in the short term, you should able to work around what you're seeing\nby setting `protocol.file.allow` to \"always\" with something like\n\n $ git config --global protocol.file.allow always\n\nBut in the short-term, I am curious why we are complaining about needing\nto use the file transport when you claim that the submodule actually\nneeds the HTTPS transport.\n\nAre you able to share a copy of your repository, and/or its .gitmodules\nfile, and your repository-local .gitconfig, as well? Do you have some\n`url..insteadOf` value configured elsewhere that would be\nrewriting those paths for you?\n\nThanks,\nTaylor\n"},{"id":"478691","messageId":"ZJR5SPDj4Wt_gmRO@pweza","threadId":"59896","inReplyTo":"ZJRHlJvE4BMue1/Z@nand.local","subject":"Re: bug: submodule update fails to fetch","fromName":"Sergei Golubchik","fromEmail":"vuvova@gmail.com","sentAt":"2023-06-22T16:39:36Z","receivedAt":"2023-06-22T16:40:00Z","isPatch":false,"sender":{"key":"vuvova@gmail.com","avatar":null},"body":"Hi, Taylor,\n\nOn Jun 22, Taylor Blau wrote:\n> On Thu, Jun 22, 2023 at 01:09:07PM +0200, Sergei Golubchik wrote:\n> > Hi,\n> >\n> > Sometimes (my local repository has lots of branches) after switching\n> > branches\n> >\n> > git submodule update --init --recursive\n> >\n> > fails with something like\n> >\n> > fatal: transport 'file' not allowed\n> > fatal: Fetched in submodule path 'wsrep-lib', but it did not contain e238c0d240c2557229b0523a4a032f3cf8b41639. Direct fetching of that commit failed.\n> >\n> > the submodule transport is not 'file' (it's https) and the direct\n> > fetching of the commit actually works:\n> >\n> > cd wsrep-lib\n> > git fetch origin e238c0d240c2557229b0523a4a032f3cf8b41639\n> > git checkout e238c0d240c2557229b0523a4a032f3cf8b41639\n> > cd ..\n> >\n> > after that\n> >\n> > git submodule update --init --recursive\n> >\n> > succeeds.\n> \n> It makes sense that after manually fetching the desired tip that the\n> submodule update goes through OK, because there is nothing to do (the\n> checked-out state matches what's in .gitmodules), so we don't have to\n> use any transport mechanism.\n\nRight. I just used it to show that git thinks the submodule was updated\ncorrectly and doesn't try to do anything after that.\n\n> I recently changed the submodule update rules to disallow file-based\n> submodules when not directly executed by the user. See a1d4f67c12\n> (transport: make `protocol.file.allow` be \"user\" by default, 2022-07-29)\n> for more of the details there.\n\nYes, I've seen it. That submodule shouldn't be affected:\n\n $ git remote -v\n origin https://github.com/codership/wsrep-lib.git (fetch)\n origin https://github.com/codership/wsrep-lib.git (push)\n\nso I wouldn't want to circumvent your fix and allow the file transport\nthat we aren't using.\n\n> But in the short-term, I am curious why we are complaining about needing\n> to use the file transport when you claim that the submodule actually\n> needs the HTTPS transport.\n> \n> Are you able to share a copy of your repository, and/or its .gitmodules\n> file, and your repository-local .gitconfig, as well? Do you have some\n> `url..insteadOf` value configured elsewhere that would be\n> rewriting those paths for you?\n\nNo insteadOf. Let me try to...\nOkay, here's the bug. In submodule--helper.c, fetch_in_submodule()\nfunction, there're lines:\n---------------------------\n2211 if (oid) {\n2212 char *hex = oid_to_hex(oid);\n2213 char *remote = get_default_remote();\n2214\n2215 strvec_pushl(&cp.args, remote, hex, NULL);\n2216 free(remote);\n2217 }\n---------------------------\n\nthis get_default_remote() appears to be getting the default remote name\nfor the main repository and then uses it to fetch from the submodule.\n\nIt happens that my default remote isn't \"origin\" (long story), it's\n\"github\", but in the submodule it's of course \"origin\", there's no\n\"github\" remote there. As a result, `git submodule update` runs the\ncommand\n\n git fetch github ${commit_hash}\n\nin the submodule, and that's interpreted as 'file' transport.\n\nTo repeat this you need a repository where the default remote isn't\n\"origin\" and a submodule where the commit cannot be fetched by simply\n`git fetch` and needs a direct fetch.\n\nHope this helps.\n\nRegards,\nSergei\n"},{"id":"478722","messageId":"CA+P7+xoj=4tdMtsyB3=8DADkygDV9Tsiv03puSyh4mVR-Nz2ag@mail.gmail.com","threadId":"59896","inReplyTo":"ZJR5SPDj4Wt_gmRO@pweza","subject":"Re: bug: submodule update fails to fetch","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2023-06-23T07:26:46Z","receivedAt":"2023-06-23T07:27:02Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Jun 22, 2023 at 9:57 AM Sergei Golubchik wrote:\n>\n> Hi, Taylor,\n>\n> On Jun 22, Taylor Blau wrote:\n> > On Thu, Jun 22, 2023 at 01:09:07PM +0200, Sergei Golubchik wrote:\n> > > Hi,\n> > >\n> > > Sometimes (my local repository has lots of branches) after switching\n> > > branches\n> > >\n> > > git submodule update --init --recursive\n> > >\n> > > fails with something like\n> > >\n> > > fatal: transport 'file' not allowed\n> > > fatal: Fetched in submodule path 'wsrep-lib', but it did not contain e238c0d240c2557229b0523a4a032f3cf8b41639. Direct fetching of that commit failed.\n> > >\n> > > the submodule transport is not 'file' (it's https) and the direct\n> > > fetching of the commit actually works:\n> > >\n> > > cd wsrep-lib\n> > > git fetch origin e238c0d240c2557229b0523a4a032f3cf8b41639\n> > > git checkout e238c0d240c2557229b0523a4a032f3cf8b41639\n> > > cd ..\n> > >\n> > > after that\n> > >\n> > > git submodule update --init --recursive\n> > >\n> > > succeeds.\n> >\n> > It makes sense that after manually fetching the desired tip that the\n> > submodule update goes through OK, because there is nothing to do (the\n> > checked-out state matches what's in .gitmodules), so we don't have to\n> > use any transport mechanism.\n>\n> Right. I just used it to show that git thinks the submodule was updated\n> correctly and doesn't try to do anything after that.\n>\n> > I recently changed the submodule update rules to disallow file-based\n> > submodules when not directly executed by the user. See a1d4f67c12\n> > (transport: make `protocol.file.allow` be \"user\" by default, 2022-07-29)\n> > for more of the details there.\n>\n> Yes, I've seen it. That submodule shouldn't be affected:\n>\n> $ git remote -v\n> origin https://github.com/codership/wsrep-lib.git (fetch)\n> origin https://github.com/codership/wsrep-lib.git (push)\n>\n> so I wouldn't want to circumvent your fix and allow the file transport\n> that we aren't using.\n>\n> > But in the short-term, I am curious why we are complaining about needing\n> > to use the file transport when you claim that the submodule actually\n> > needs the HTTPS transport.\n> >\n> > Are you able to share a copy of your repository, and/or its .gitmodules\n> > file, and your repository-local .gitconfig, as well? Do you have some\n> > `url..insteadOf` value configured elsewhere that would be\n> > rewriting those paths for you?\n>\n> No insteadOf. Let me try to...\n> Okay, here's the bug. In submodule--helper.c, fetch_in_submodule()\n> function, there're lines:\n> ---------------------------\n> 2211 if (oid) {\n> 2212 char *hex = oid_to_hex(oid);\n> 2213 char *remote = get_default_remote();\n> 2214\n> 2215 strvec_pushl(&cp.args, remote, hex, NULL);\n> 2216 free(remote);\n> 2217 }\n> ---------------------------\n>\n> this get_default_remote() appears to be getting the default remote name\n> for the main repository and then uses it to fetch from the submodule.\n>\n> It happens that my default remote isn't \"origin\" (long story), it's\n> \"github\", but in the submodule it's of course \"origin\", there's no\n> \"github\" remote there. As a result, `git submodule update` runs the\n> command\n>\n> git fetch github ${commit_hash}\n>\n> in the submodule, and that's interpreted as 'file' transport.\n>\n> To repeat this you need a repository where the default remote isn't\n> \"origin\" and a submodule where the commit cannot be fetched by simply\n> `git fetch` and needs a direct fetch.\n>\n> Hope this helps.\n>\n> Regards,\n> Sergei\n\nI recently experienced something similar when changing the default\nremote name for some of my repositories. I had wondered what causes\nproblems because submodule update would sometimes fail but other times\nsucceed.\n"},{"id":"503801","messageId":"20241001072423.1165932-1-daniel@mariadb.org","threadId":"59896","inReplyTo":"ZJQr0_aC-NlLXDgj@pweza","subject":"[PATCH] submodule: correct remote name with fetch","fromName":"Daniel Black","fromEmail":"daniel@mariadb.org","sentAt":"2024-10-01T07:24:23Z","receivedAt":"2024-10-01T07:25:43Z","isPatch":true,"sender":{"key":"daniel@mariadb.org","avatar":"https://avatars.githubusercontent.com/u/462287?v=4"},"body":"The fetching of submodules erroniously used\nthe main repository remote name instead of the\nsubmodule remote name[1].\n\nCorrect this by using the correct function\nto reteive the remote name.\n\n1. https://www.spinics.net/lists/git/msg462320.html\n\nSigned-off-by: Daniel Black \n---\n builtin/submodule--helper.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a1ada86952..210ae2570a 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2322,7 +2322,12 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,\n \t\tstrvec_pushf(&cp.args, \"--depth=%d\", depth);\n \tif (oid) {\n \t\tchar *hex = oid_to_hex(oid);\n-\t\tchar *remote = get_default_remote();\n+\t\tchar *remote;\n+\t\tint code;\n+\n+\t\tcode = get_default_remote_submodule(module_path, &remote);\n+\t\tif (code)\n+\t\t\treturn code;\n \n \t\tstrvec_pushl(&cp.args, remote, hex, NULL);\n \t\tfree(remote);\n-- \n2.46.2\n\n"},{"id":"503848","messageId":"xmqqed4zzqbg.fsf@gitster.g","threadId":"59896","inReplyTo":"20241001072423.1165932-1-daniel@mariadb.org","subject":"Re: [PATCH] submodule: correct remote name with fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-01T17:27:31Z","receivedAt":"2024-10-01T17:27:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Black writes:\n\n> The fetching of submodules erroniously used\n> the main repository remote name instead of the\n> submodule remote name[1].\n\nPlease write the problem description in the present tense, i.e. \"the\ncode does this, which is incorrect in this way\".\n\n> Correct this by using the correct function\n> to reteive the remote name.\n\nThat's gives the same as \"fix it\" ;-).\n\nCan we phrase it in end-user observable terms, for example, \"instead\nof grabbing the default remote of the superproject repository, ask\nthe default remote of the submodule we are going to run 'git fetch'\nin\". It is a mere implementation detail of doing so, to call\nget_default_remote_submodule() instead of get_default_remote().\n\n> 1. https://www.spinics.net/lists/git/msg462320.html\n\nPlease use the URL that shows the Message-Id when referring to a\nmessage in the list archive. E.g.,\n\n https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/\n https://public-inbox.org/git/ZJR5SPDj4Wt_gmRO@pweza/\n\n> Signed-off-by: Daniel Black \n> ---\n> builtin/submodule--helper.c | 7 ++++++-\n> 1 file changed, 6 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index a1ada86952..210ae2570a 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -2322,7 +2322,12 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,\n> \t\tstrvec_pushf(&cp.args, \"--depth=%d\", depth);\n> \tif (oid) {\n> \t\tchar *hex = oid_to_hex(oid);\n> -\t\tchar *remote = get_default_remote();\n> +\t\tchar *remote;\n> +\t\tint code;\n> +\n> +\t\tcode = get_default_remote_submodule(module_path, &remote);\n> +\t\tif (code)\n> +\t\t\treturn code;\n\nWe never failed in this function to return without calling\nrun_command(). Now we do.\n\nShouldn't we clean up the child_process structure we allocated and\nprepared before returning like this? It would just be the matter of\ncalling child_process_clear(&cp), perhaps?\n\n> \t\tstrvec_pushl(&cp.args, remote, hex, NULL);\n> \t\tfree(remote);\n\nThanks.\n"},{"id":"503849","messageId":"xmqq7carzpzi.fsf@gitster.g","threadId":"59896","inReplyTo":"xmqqed4zzqbg.fsf@gitster.g","subject":"Re: [PATCH] submodule: correct remote name with fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-01T17:34:41Z","receivedAt":"2024-10-01T17:34:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\nSorry, forgot to say one more thing before pressing [SEND].\n\n>> builtin/submodule--helper.c | 7 ++++++-\n>> 1 file changed, 6 insertions(+), 1 deletion(-)\n\nPlease add a test or two, that fail without this change (to\ndemonstrate the existing bug) and succeed with this change (to make\nsure if somebody breaks the code you fixed again in the future, such\na breakage is caught).\n\nThanks.\n"},{"id":"504372","messageId":"20241008015835.41678-1-daniel@mariadb.org","threadId":"59896","inReplyTo":"xmqq7carzpzi.fsf@gitster.g","subject":"","fromName":"Daniel Black","fromEmail":"daniel@mariadb.org","sentAt":"2024-10-08T01:49:53Z","receivedAt":"2024-10-08T01:59:17Z","isPatch":false,"sender":{"key":"daniel@mariadb.org","avatar":"https://avatars.githubusercontent.com/u/462287?v=4"},"body":"I have this so far, but I need a hand with the test case.\n\nWith a manual hack of the code:\n\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2450,7 +2450,7 @@ static int run_update_procedure(const struct update_data *ud)\n * Now we tried the usual fetch, but `oid` may\n * not be reachable from any of the refs.\n */\n- if (!is_tip_reachable(ud->sm_path, &ud->oid) &&\n+ if (is_tip_reachable(ud->sm_path, &ud->oid) &&\n fetch_in_submodule(ud->sm_path, ud->depth, ud->quiet, &ud->oid))\n return die_message(_(\"Fetched in submodule path '%s', but it did not \"\n \"contain %s. Direct fetching of that commit failed.\"),\n\nI'm able to using the GIT_TRACE= of git submodule update arrive at the\ncorrect fetching from origin in the test case where before the code fix\nit was fetching from remote o1.\n\ntrace: run_command: cd sub; unset GIT_PREFIX; GIT_DIR=.git git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all\ntrace: start_command: /home/dan/repos/git/git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all\ntrace: built-in: git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all\ntrace: run_command: cd sub; unset GIT_PREFIX; GIT_DIR=.git git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all\ntrace: start_command: /home/dan/repos/git/git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all\ntrace: built-in: git rev-list -n 1 27fe1b65df6f55a58636afcba364dfcb64916cd6 --not --all\ntrace: run_command: cd sub; unset GIT_PREFIX; GIT_DIR=.git git fetch origin 27fe1b65df6f55a58636afcba364dfcb64916cd6\ntrace: start_command: /home/dan/repos/git/git fetch origin 27fe1b65df6f55a58636afcba364dfcb64916cd6\n\nAs Sergei said in the original referenced message \"a submodule where the commit cannot be fetched by simply\n`git fetch` and needs a direct fetch\".\n\nSo this is the test case that I'm having trouble generating.\n\n"},{"id":"504373","messageId":"20241008015835.41678-2-daniel@mariadb.org","threadId":"59896","inReplyTo":"20241008015835.41678-1-daniel@mariadb.org","subject":"[RFC PATCH v2] submodule: correct remote name with fetch","fromName":"Daniel Black","fromEmail":"daniel@mariadb.org","sentAt":"2024-10-08T01:49:54Z","receivedAt":"2024-10-08T01:59:31Z","isPatch":true,"sender":{"key":"daniel@mariadb.org","avatar":"https://avatars.githubusercontent.com/u/462287?v=4"},"body":"The code fetches the submodules remote based on the superproject remote name\ninstead of the submodule remote name[1].\n\nInstead of grabbing the default remote of the superproject repository, ask\nthe default remote of the submodule we are going to run 'git fetch' in.\n\n1. https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/\n\nSigned-off-by: Daniel Black \n---\n builtin/submodule--helper.c | 9 ++++-\n t/t5568-fetch-submodule.sh | 65 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 73 insertions(+), 1 deletion(-)\n create mode 100755 t/t5568-fetch-submodule.sh\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a1ada86952..567d21d330 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2322,7 +2322,14 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,\n \t\tstrvec_pushf(&cp.args, \"--depth=%d\", depth);\n \tif (oid) {\n \t\tchar *hex = oid_to_hex(oid);\n-\t\tchar *remote = get_default_remote();\n+\t\tchar *remote;\n+\t\tint code;\n+\n+\t\tcode = get_default_remote_submodule(module_path, &remote);\n+\t\tif (code) {\n+\t\t\tchild_process_clear(&cp);\n+\t\t\treturn code;\n+\t\t}\n \n \t\tstrvec_pushl(&cp.args, remote, hex, NULL);\n \t\tfree(remote);\ndiff --git a/t/t5568-fetch-submodule.sh b/t/t5568-fetch-submodule.sh\nnew file mode 100755\nindex 0000000000..56978bcfd7\n--- /dev/null\n+++ b/t/t5568-fetch-submodule.sh\n@@ -0,0 +1,65 @@\n+#!/bin/sh\n+\n+test_description='fetch can handle submodules origin names'\n+\n+GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1\n+export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+SUPER=\"$HTTPD_DOCUMENT_ROOT_PATH/super\"\n+SUB=\"$HTTPD_DOCUMENT_ROOT_PATH/sub\"\n+SUPER_URI=\"$HTTPD_URL/smart/super\"\n+SUB_URI=\"$HTTPD_URL/smart/sub\"\n+\n+setup() {\n+\tSERVER=\"$1\"\n+\tgit init \"$SERVER\" &&\n+\ttest_when_finished 'rm -rf \"$SERVER\"' &&\n+\ttest_config -C \"$SERVER\" http.receivepack true\n+}\n+\n+test_expect_success 'fetch submodule remote of different name from superproject' '\n+\tsetup \"$SUPER\" &&\n+\ttest_create_repo super &&\n+\ttest_commit -C super bar &&\n+\tgit -C super remote add upstream \"$SUPER_URI\" &&\n+\ttest_config -C super push.default upstream &&\n+\tgit -C super push --set-upstream upstream master:main &&\n+\n+\tsetup \"$SUB\" &&\n+\ttest_create_repo sub &&\n+\ttest_commit -C sub foo &&\n+\tgit -C sub branch newmain &&\n+\ttest_commit -C sub morefoo &&\n+\ttest_commit -C sub moremorefoo &&\n+\tgit -C sub remote add upstream \"$SUB_URI\" &&\n+\ttest_config -C sub push.default upstream &&\n+\tgit -C sub push --set-upstream upstream master:main &&\n+\n+\tgit -C super submodule add --branch main -- \"$SUB_URI\" sub &&\n+\tgit -C super commit -am \"add submodule\" &&\n+\tgit -C super push &&\n+\n+\t# Needs to create unreachable commit from current master branch.\n+\tgit -C sub checkout newmain &&\n+\ttest_commit -C sub echo &&\n+\ttest_commit -C sub moreecho &&\n+\tgit -C sub push --set-upstream upstream newmain:newmain &&\n+\n+\tgit clone --origin o1 --branch main -- \"$SUPER_URI\" superproject &&\n+\tgit -C superproject submodule update --init &&\n+\n+\tgit -C super/sub fetch &&\n+\tgit -C super/sub checkout origin/newmain &&\n+\tgit -C super commit -m \"update submodule\" sub &&\n+\tgit -C super push && \n+\n+\tgit -C superproject pull --no-recurse-submodules &&\n+\tgit -C superproject submodule update\n+'\n+\n+test_done\n-- \n2.46.2\n\n"},{"id":"504480","messageId":"xmqqzfnez9dv.fsf@gitster.g","threadId":"59896","inReplyTo":"20241008015835.41678-2-daniel@mariadb.org","subject":"Re: [RFC PATCH v2] submodule: correct remote name with fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-08T19:23:40Z","receivedAt":"2024-10-08T19:23:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Black writes:\n\n> The code fetches the submodules remote based on the superproject remote name\n> instead of the submodule remote name[1].\n>\n> Instead of grabbing the default remote of the superproject repository, ask\n> the default remote of the submodule we are going to run 'git fetch' in.\n>\n> 1. https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/\n>\n> Signed-off-by: Daniel Black \n> ---\n> builtin/submodule--helper.c | 9 ++++-\n\nThe proposed log message is very well written.\n\n> t/t5568-fetch-submodule.sh | 65 +++++++++++++++++++++++++++++++++++++\n> 2 files changed, 73 insertions(+), 1 deletion(-)\n> create mode 100755 t/t5568-fetch-submodule.sh\n\nHmph,\n\n\t$ git grep \"submodule update\" t/\n\ngives quite a many hits in existing tests. Didn't any of them have\nsufficient preparation steps that testing of this bugfix can reuse?\n\nA test on \"submodule update\" behaviour tends to need quite a lot of\npreparation. Preparing the superproject, addition of a submodule to\nit, cloning of these two projects, and then half-preparing a clone\nof these super-sub arrangement. All of that needs to happen before\nwe can say \"submodule update\" and observe the outcome to see if the\nbug still exists. If we can piggy-back on a test script that already\nhas such preparation, it would be far more preferrable than having to\ndo another set of preparation.\n\nAnother thing. If this is not about a bug that only manifests when\nthe HTTP transport is in use, it is strongly preferred to avoid\nturning it into an httpd test. Some developers and/or environments\nskip them.\n\n\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index a1ada86952..567d21d330 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -2322,7 +2322,14 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,\n> \t\tstrvec_pushf(&cp.args, \"--depth=%d\", depth);\n> \tif (oid) {\n> \t\tchar *hex = oid_to_hex(oid);\n> -\t\tchar *remote = get_default_remote();\n> +\t\tchar *remote;\n> +\t\tint code;\n> +\n> +\t\tcode = get_default_remote_submodule(module_path, &remote);\n> +\t\tif (code) {\n> +\t\t\tchild_process_clear(&cp);\n> +\t\t\treturn code;\n> +\t\t}\n\nThe get_default_remote_submodule() helper eventually calls into\nrepo_get_default_remote() that returns an allocated string in\nremote, but it only does so when it succeeds, so this early return\ndoes not have to worry about leaking \"remote\" here. Good.\n\nThe code change looks quite straight-forward and looking good.\n\n> \t\tstrvec_pushl(&cp.args, remote, hex, NULL);\n> \t\tfree(remote);\n\nThanks.\n"},{"id":"504510","messageId":"20241009033257.1316690-1-daniel@mariadb.org","threadId":"59896","inReplyTo":"xmqqzfnez9dv.fsf@gitster.g","subject":"[PATCH v3] submodule: correct remote name with fetch","fromName":"Daniel Black","fromEmail":"daniel@mariadb.org","sentAt":"2024-10-09T03:32:54Z","receivedAt":"2024-10-09T03:33:08Z","isPatch":true,"sender":{"key":"daniel@mariadb.org","avatar":"https://avatars.githubusercontent.com/u/462287?v=4"},"body":"The code fetches the submodules remote based on the superproject remote name\ninstead of the submodule remote name[1].\n\nInstead of grabbing the default remote of the superproject repository, ask\nthe default remote of the submodule we are going to run 'git fetch' in.\n\n1. https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/\n\nSigned-off-by: Daniel Black \n---\n builtin/submodule--helper.c | 9 ++++++++-\n t/t5572-pull-submodule.sh | 20 ++++++++++++++++++++\n 2 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex dc89488a7d..b6b5f1ebde 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2333,7 +2333,14 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,\n \t\tstrvec_pushf(&cp.args, \"--depth=%d\", depth);\n \tif (oid) {\n \t\tchar *hex = oid_to_hex(oid);\n-\t\tchar *remote = get_default_remote();\n+\t\tchar *remote;\n+\t\tint code;\n+\n+\t\tcode = get_default_remote_submodule(module_path, &remote);\n+\t\tif (code) {\n+\t\t\tchild_process_clear(&cp);\n+\t\t\treturn code;\n+\t\t}\n \n \t\tstrvec_pushl(&cp.args, remote, hex, NULL);\n \t\tfree(remote);\ndiff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh\nindex 916e58c166..9b6cf8d88b 100755\n--- a/t/t5572-pull-submodule.sh\n+++ b/t/t5572-pull-submodule.sh\n@@ -230,6 +230,7 @@ test_expect_success 'branch has no merge base with remote-tracking counterpart'\n \n \ttest_create_repo a-submodule &&\n \ttest_commit -C a-submodule foo &&\n+\ttest_commit -C a-submodule bar &&\n \n \ttest_create_repo parent &&\n \tgit -C parent submodule add \"$(pwd)/a-submodule\" &&\n@@ -246,4 +247,23 @@ test_expect_success 'branch has no merge base with remote-tracking counterpart'\n \tgit -C child pull --recurse-submodules --rebase\n '\n \n+test_expect_success 'fetch submodule remote of different name from superproject' '\n+\tgit -C child remote rename origin o1 &&\n+\tgit -C child submodule update --init &&\n+\n+\t# Needs to create unreachable commit from current master branch.\n+\tgit -C a-submodule checkout -b newmain HEAD^ &&\n+\ttest_commit -C a-submodule echo &&\n+\ttest_commit -C a-submodule moreecho &&\n+\tsubc=$(git -C a-submodule rev-parse --short HEAD) &&\n+\n+\tgit -C parent/a-submodule fetch &&\n+\tgit -C parent/a-submodule checkout \"$subc\" &&\n+\tgit -C parent commit -m \"update submodule\" a-submodule &&\n+\tgit -C a-submodule reset --hard HEAD^^ &&\n+\n+\tgit -C child pull --no-recurse-submodules &&\n+\tgit -C child submodule update\n+'\n+\n test_done\n-- \n2.46.2\n\n"},{"id":"504617","messageId":"xmqqv7y1tb9v.fsf@gitster.g","threadId":"59896","inReplyTo":"20241009033257.1316690-1-daniel@mariadb.org","subject":"Re: [PATCH v3] submodule: correct remote name with fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-09T17:51:40Z","receivedAt":"2024-10-09T17:51:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Black writes:\n\n> The code fetches the submodules remote based on the superproject remote name\n> instead of the submodule remote name[1].\n>\n> Instead of grabbing the default remote of the superproject repository, ask\n> the default remote of the submodule we are going to run 'git fetch' in.\n>\n> 1. https://lore.kernel.org/git/ZJR5SPDj4Wt_gmRO@pweza/\n>\n> Signed-off-by: Daniel Black \n> ---\n> builtin/submodule--helper.c | 9 ++++++++-\n> t/t5572-pull-submodule.sh | 20 ++++++++++++++++++++\n> 2 files changed, 28 insertions(+), 1 deletion(-)\n\nExcellent. Will queue.\n\nThanks.\n\n> diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh\n> index 916e58c166..9b6cf8d88b 100755\n> --- a/t/t5572-pull-submodule.sh\n> +++ b/t/t5572-pull-submodule.sh\n> @@ -230,6 +230,7 @@ test_expect_success 'branch has no merge base with remote-tracking counterpart'\n> \n> \ttest_create_repo a-submodule &&\n> \ttest_commit -C a-submodule foo &&\n> +\ttest_commit -C a-submodule bar &&\n> \n> \ttest_create_repo parent &&\n> \tgit -C parent submodule add \"$(pwd)/a-submodule\" &&\n> @@ -246,4 +247,23 @@ test_expect_success 'branch has no merge base with remote-tracking counterpart'\n> \tgit -C child pull --recurse-submodules --rebase\n> '\n> \n> +test_expect_success 'fetch submodule remote of different name from superproject' '\n> +\tgit -C child remote rename origin o1 &&\n> +\tgit -C child submodule update --init &&\n> +\n> +\t# Needs to create unreachable commit from current master branch.\n> +\tgit -C a-submodule checkout -b newmain HEAD^ &&\n> +\ttest_commit -C a-submodule echo &&\n> +\ttest_commit -C a-submodule moreecho &&\n> +\tsubc=$(git -C a-submodule rev-parse --short HEAD) &&\n> +\n> +\tgit -C parent/a-submodule fetch &&\n> +\tgit -C parent/a-submodule checkout \"$subc\" &&\n> +\tgit -C parent commit -m \"update submodule\" a-submodule &&\n> +\tgit -C a-submodule reset --hard HEAD^^ &&\n> +\n> +\tgit -C child pull --no-recurse-submodules &&\n> +\tgit -C child submodule update\n> +'\n> +\n> test_done\n"}]}