{"thread":{"id":"66047","subject":"[PATCH] submodule: resolve insteadof-aliases when matching remote","startedAt":"2026-07-21T21:31:08Z","lastAt":"2026-08-11T02:57:52Z","messageCount":5,"participants":["Éric NICOLAS","Junio C Hamano","Jacob Keller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"548744","messageId":"20260721213042.3357346-1-ccjmne@gmail.com","threadId":"66047","inReplyTo":null,"subject":"[PATCH] submodule: resolve insteadof-aliases when matching remote","fromName":"Éric NICOLAS","fromEmail":"ccjmne@gmail.com","sentAt":"2026-07-21T21:30:42Z","receivedAt":"2026-07-21T21:31:08Z","isPatch":true,"body":"When ca62f524c1 introduced a mechanism to identify which remote is to be\nused by a submodule, we had it compare the URL stored in the .gitmodules\ninventory to those of each available remote.\n\nHowever, when using URL aliasing via url.<base>.insteadOf, we store\nin .gitmodules the URL pre-resolution of the alias, whereas the\ncorresponding remote set up in the submodule reports using the\n*resolved* URL.  This mechanism therefore fails to find a match then,\nand resorts to the fallback logic, which does use either the only\nconfigured remote if there is only one, or attempts using \"origin\"\notherwise.\n\nResolve the alias in the URL inventoried in .gitmodules before comparing\nit against those of the corresponding submodule's configured remotes.\n\nSigned-off-by: Éric NICOLAS <ccjmne@gmail.com>\n---\n remote.c                    | 15 ++++++++++++---\n t/t7406-submodule-update.sh | 21 +++++++++++++++++++++\n 2 files changed, 33 insertions(+), 3 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex b17648d6ef..ae187fb3d6 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1821,17 +1821,26 @@ const char *repo_default_remote(struct repository *repo)\n \n const char *repo_remote_from_url(struct repository *repo, const char *url)\n {\n+\tchar *rewritten_url;\n+\tconst char *url_to_match;\n+\tconst char *remote_name = NULL;\n+\n \tread_config(repo, 0);\n+\trewritten_url = alias_url(url, &repo->remote_state->rewrites);\n+\turl_to_match = rewritten_url ? rewritten_url : url;\n \n \tfor (int i = 0; i < repo->remote_state->remotes_nr; i++) {\n \t\tstruct remote *remote = repo->remote_state->remotes[i];\n \t\tif (!remote)\n \t\t\tcontinue;\n \n-\t\tif (remote_has_url(remote, url))\n-\t\t\treturn remote->name;\n+\t\tif (remote_has_url(remote, url_to_match)) {\n+\t\t\tremote_name = remote->name;\n+\t\t\tbreak;\n+\t\t}\n \t}\n-\treturn NULL;\n+\tfree(rewritten_url);\n+\treturn remote_name;\n }\n \n int branch_has_merge_config(struct branch *branch)\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 9554720152..84e2cbbef9 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -256,6 +256,27 @@ test_expect_success 'submodule update --remote should fetch upstream changes' '\n \t)\n '\n \n+test_expect_success 'submodule update --remote resolves URL rewrites' '\n+\ttest_config_global \"url.$(pwd)/.insteadOf\" local: &&\n+\tmkdir aliased-super aliased-submodule &&\n+\t(\n+\t\tcd aliased-submodule &&\n+\t\tgit init &&\n+\t\techo line >file &&\n+\t\tgit add file &&\n+\t\tgit commit -m \"Initial commit\"\n+\t) &&\n+\t(\n+\t\tcd aliased-super &&\n+\t\tgit init &&\n+\t\tgit submodule add local:aliased-submodule submodule &&\n+\t\tgit submodule update --force submodule &&\n+\t\tgit -C submodule remote rename origin upstream &&\n+\t\tgit -C submodule remote add fork user@host &&\n+\t\tgit submodule update --remote submodule\n+\t)\n+'\n+\n test_expect_success 'submodule update --remote should fetch upstream changes with .' '\n \t(\n \t\tcd super &&\n-- \n2.55.0\n\n"},{"id":"548795","messageId":"xmqqbjbyole1.fsf@gitster.g","threadId":"66047","inReplyTo":"20260721213042.3357346-1-ccjmne@gmail.com","subject":"Re: [PATCH] submodule: resolve insteadof-aliases when matching remote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-22T19:49:10Z","receivedAt":"2026-07-22T19:49:13Z","isPatch":true,"body":"Éric NICOLAS <ccjmne@gmail.com> writes:\n\n> When ca62f524c1 introduced a mechanism to identify which remote is to be\n> used by a submodule, we had it compare the URL stored in the .gitmodules\n> inventory to those of each available remote.\n\nPlease refer to an existing commit using this format:\n\n    When ca62f524c1 (submodule: look up remotes by URL first,\n    2025-06-23) introduced ...\n\n> However, when using URL aliasing via url.<base>.insteadOf, we store\n> in .gitmodules the URL pre-resolution of the alias, whereas the\n> corresponding remote set up in the submodule reports using the\n> *resolved* URL.  This mechanism therefore fails to find a match then,\n\nSince anything involving the .gitmodules file is often security-\nsensitive, it is always a good idea to go beyond just saying 'X fails\nto do Y.'  We should also explain why that failure is a bad thing (or\nperhaps a good thing) and for what reason.\n\nIf this aliasing were controlled by a remote entity (for example, if\nan upstream project modified the .gitmodules file to redirect us\nsomewhere unexpected), failing to find a match could actually be a\nsafety feature, shielding us from bad actors trying to hijack the\nlocal repository.  Since that is not the case here, adding 'fails to\nfind a match, which is unfortunate because...' would make the commit\nmessage much stronger.\n\n> and resorts to the fallback logic, which does use either the only\n> configured remote if there is only one, or attempts using \"origin\"\n> otherwise.\n>\n> Resolve the alias in the URL inventoried in .gitmodules before comparing\n> it against those of the corresponding submodule's configured remotes.\n>\n> Signed-off-by: Éric NICOLAS <ccjmne@gmail.com>\n> ---\n>  remote.c                    | 15 ++++++++++++---\n>  t/t7406-submodule-update.sh | 21 +++++++++++++++++++++\n>  2 files changed, 33 insertions(+), 3 deletions(-)\n>\n> diff --git a/remote.c b/remote.c\n> index b17648d6ef..ae187fb3d6 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -1821,17 +1821,26 @@ const char *repo_default_remote(struct repository *repo)\n>  \n>  const char *repo_remote_from_url(struct repository *repo, const char *url)\n>  {\n> +\tchar *rewritten_url;\n> +\tconst char *url_to_match;\n> +\tconst char *remote_name = NULL;\n> +\n>  \tread_config(repo, 0);\n> +\trewritten_url = alias_url(url, &repo->remote_state->rewrites);\n> +\turl_to_match = rewritten_url ? rewritten_url : url;\n\nBeing a bit lazy, I probably would have just reused 'url' directly:\n\n\tif ((rewritten_url = alias_url(url, &repo->remote_state->rewrites)))\n\t\turl = rewritten_url;\n\nThis lets us avoid introducing a brand-new 'url_to_match' variable,\nwhose lifetime is essentially just taking over for 'url' anyway.\n\n>  \tfor (int i = 0; i < repo->remote_state->remotes_nr; i++) {\n>  \t\tstruct remote *remote = repo->remote_state->remotes[i];\n>  \t\tif (!remote)\n>  \t\t\tcontinue;\n>  \n> -\t\tif (remote_has_url(remote, url))\n> -\t\t\treturn remote->name;\n> +\t\tif (remote_has_url(remote, url_to_match)) {\n> +\t\t\tremote_name = remote->name;\n> +\t\t\tbreak;\n> +\t\t}\n\nWhile the new code preserves the original 'first one wins' behavior,\nit does make me wonder why we do not issue a warning or raise an\nerror when multiple URLs match.  Leaving such an ambiguous\nconfiguration unflagged feels like a silent bug waiting to happen.\n\nBut it is of course outside the scope of this topic.\n\n>  \t}\n> -\treturn NULL;\n> +\tfree(rewritten_url);\n> +\treturn remote_name;\n>  }\n\nThanks.\n"},{"id":"548799","messageId":"20260723002132.3989727-1-ccjmne@gmail.com","threadId":"66047","inReplyTo":"20260721213042.3357346-1-ccjmne@gmail.com","subject":"[PATCH v2] submodule: resolve insteadOf aliases when matching remote","fromName":"Éric NICOLAS","fromEmail":"ccjmne@gmail.com","sentAt":"2026-07-23T00:21:32Z","receivedAt":"2026-07-23T00:22:03Z","isPatch":true,"body":"When ca62f524c1 (submodule: look up remotes by URL first, 2025-06-23)\nintroduced a mechanism to identify which remote is to be used by a\nsubmodule, it compared the URL stored in the .gitmodules inventory to\nthat of each available remote.\n\nThe URLs of remotes are rewritten according to url.<base>.insteadOf,\nwhereas those stored in the .gitmodules aren't.  When such aliasing\napplies, no match can be made between the two corresponding sides, and\nthe procedure degrades to its fallback logic electing either the only\nconfigured remote if there is only one, or \"origin\" otherwise.\n\nThat behaviour is unfortunate when no remote is called \"origin\",\nbecause its last resort will have a submodule update command look for a\nnon-existent remote-tracking reference and fail to proceed, instead of\nusing the remote whose rewritten URL matches.\n\nResolve the alias in the URL inventoried in .gitmodules before comparing\nit against those of the corresponding submodule's configured remotes.\n\nSigned-off-by: Éric NICOLAS <ccjmne@gmail.com>\n---\nThank you for your guidance.\n\nChanges in v2:\n\n- Reword the commit message more purposefully\n- Adjust the implementation as suggested, avoiding a superfluous\n  variable\n- Tidy up the integration test\n\n remote.c                    | 14 +++++++++++---\n t/t7406-submodule-update.sh | 19 +++++++++++++++++++\n 2 files changed, 30 insertions(+), 3 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex b17648d6ef..b1fed58e79 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1821,17 +1821,25 @@ const char *repo_default_remote(struct repository *repo)\n \n const char *repo_remote_from_url(struct repository *repo, const char *url)\n {\n+\tchar *rewritten_url;\n+\tconst char *remote_name = NULL;\n+\n \tread_config(repo, 0);\n+\tif ((rewritten_url = alias_url(url, &repo->remote_state->rewrites)))\n+\t\turl = rewritten_url;\n \n \tfor (int i = 0; i < repo->remote_state->remotes_nr; i++) {\n \t\tstruct remote *remote = repo->remote_state->remotes[i];\n \t\tif (!remote)\n \t\t\tcontinue;\n \n-\t\tif (remote_has_url(remote, url))\n-\t\t\treturn remote->name;\n+\t\tif (remote_has_url(remote, url)) {\n+\t\t\tremote_name = remote->name;\n+\t\t\tbreak;\n+\t\t}\n \t}\n-\treturn NULL;\n+\tfree(rewritten_url);\n+\treturn remote_name;\n }\n \n int branch_has_merge_config(struct branch *branch)\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 9554720152..10adeabf0f 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -256,6 +256,25 @@ test_expect_success 'submodule update --remote should fetch upstream changes' '\n \t)\n '\n \n+test_expect_success 'submodule update --remote resolves URL rewrites' '\n+\ttest_config_global \"url.$(pwd)/.insteadOf\" local: &&\n+\tmkdir alias-super alias-submodule &&\n+\t(\n+\t\tcd alias-submodule &&\n+\t\tgit init &&\n+\t\tgit commit --allow-empty --message \"Initial commit\"\n+\t) &&\n+\t(\n+\t\tcd alias-super &&\n+\t\tgit init &&\n+\t\tgit submodule add local:alias-submodule submodule &&\n+\t\tgit submodule update --force &&\n+\t\tgit -C submodule remote rename origin upstream &&\n+\t\tgit -C submodule remote add fork user@host &&\n+\t\tgit submodule update --remote\n+\t)\n+'\n+\n test_expect_success 'submodule update --remote should fetch upstream changes with .' '\n \t(\n \t\tcd super &&\n\nRange-diff against v1:\n1:  ed507998b3 ! 1:  4363eb3cb1 submodule: resolve insteadof-aliases when matching remote\n    @@ Metadata\n     Author: Éric NICOLAS <ccjmne@gmail.com>\n     \n      ## Commit message ##\n    -    submodule: resolve insteadof-aliases when matching remote\n    +    submodule: resolve insteadOf aliases when matching remote\n     \n    -    When ca62f524c1 introduced a mechanism to identify which remote is to be\n    -    used by a submodule, we had it compare the URL stored in the .gitmodules\n    -    inventory to those of each available remote.\n    +    When ca62f524c1 (submodule: look up remotes by URL first, 2025-06-23)\n    +    introduced a mechanism to identify which remote is to be used by a\n    +    submodule, it compared the URL stored in the .gitmodules inventory to\n    +    that of each available remote.\n     \n    -    However, when using URL aliasing via url.<base>.insteadOf, we store\n    -    in .gitmodules the URL pre-resolution of the alias, whereas the\n    -    corresponding remote set up in the submodule reports using the\n    -    *resolved* URL.  This mechanism therefore fails to find a match then,\n    -    and resorts to the fallback logic, which does use either the only\n    -    configured remote if there is only one, or attempts using \"origin\"\n    -    otherwise.\n    +    The URLs of remotes are rewritten according to url.<base>.insteadOf,\n    +    whereas those stored in the .gitmodules aren't.  When such aliasing\n    +    applies, no match can be made between the two corresponding sides, and\n    +    the procedure degrades to its fallback logic electing either the only\n    +    configured remote if there is only one, or \"origin\" otherwise.\n    +\n    +    That behaviour is unfortunate when no remote is called \"origin\",\n    +    because its last resort will have a submodule update command look for a\n    +    non-existent remote-tracking reference and fail to proceed, instead of\n    +    using the remote whose rewritten URL matches.\n     \n         Resolve the alias in the URL inventoried in .gitmodules before comparing\n         it against those of the corresponding submodule's configured remotes.\n    @@ remote.c: const char *repo_default_remote(struct repository *repo)\n      const char *repo_remote_from_url(struct repository *repo, const char *url)\n      {\n     +\tchar *rewritten_url;\n    -+\tconst char *url_to_match;\n     +\tconst char *remote_name = NULL;\n     +\n      \tread_config(repo, 0);\n    -+\trewritten_url = alias_url(url, &repo->remote_state->rewrites);\n    -+\turl_to_match = rewritten_url ? rewritten_url : url;\n    ++\tif ((rewritten_url = alias_url(url, &repo->remote_state->rewrites)))\n    ++\t\turl = rewritten_url;\n      \n      \tfor (int i = 0; i < repo->remote_state->remotes_nr; i++) {\n      \t\tstruct remote *remote = repo->remote_state->remotes[i];\n    @@ remote.c: const char *repo_default_remote(struct repository *repo)\n      \n     -\t\tif (remote_has_url(remote, url))\n     -\t\t\treturn remote->name;\n    -+\t\tif (remote_has_url(remote, url_to_match)) {\n    ++\t\tif (remote_has_url(remote, url)) {\n     +\t\t\tremote_name = remote->name;\n     +\t\t\tbreak;\n     +\t\t}\n    @@ t/t7406-submodule-update.sh: test_expect_success 'submodule update --remote shou\n      \n     +test_expect_success 'submodule update --remote resolves URL rewrites' '\n     +\ttest_config_global \"url.$(pwd)/.insteadOf\" local: &&\n    -+\tmkdir aliased-super aliased-submodule &&\n    ++\tmkdir alias-super alias-submodule &&\n     +\t(\n    -+\t\tcd aliased-submodule &&\n    ++\t\tcd alias-submodule &&\n     +\t\tgit init &&\n    -+\t\techo line >file &&\n    -+\t\tgit add file &&\n    -+\t\tgit commit -m \"Initial commit\"\n    ++\t\tgit commit --allow-empty --message \"Initial commit\"\n     +\t) &&\n     +\t(\n    -+\t\tcd aliased-super &&\n    ++\t\tcd alias-super &&\n     +\t\tgit init &&\n    -+\t\tgit submodule add local:aliased-submodule submodule &&\n    -+\t\tgit submodule update --force submodule &&\n    ++\t\tgit submodule add local:alias-submodule submodule &&\n    ++\t\tgit submodule update --force &&\n     +\t\tgit -C submodule remote rename origin upstream &&\n     +\t\tgit -C submodule remote add fork user@host &&\n    -+\t\tgit submodule update --remote submodule\n    ++\t\tgit submodule update --remote\n     +\t)\n     +'\n     +\n-- \n2.55.0\n\n"},{"id":"548916","messageId":"xmqqldb05dlo.fsf@gitster.g","threadId":"66047","inReplyTo":"20260723002132.3989727-1-ccjmne@gmail.com","subject":"Re: [PATCH v2] submodule: resolve insteadOf aliases when matching remote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-24T20:36:51Z","receivedAt":"2026-07-24T20:36:54Z","isPatch":true,"body":"Éric NICOLAS <ccjmne@gmail.com> writes:\n\n> - Reword the commit message more purposefully\n> - Adjust the implementation as suggested, avoiding a superfluous\n>   variable\n> - Tidy up the integration test\n\nQueued.\n\nIs everybody happy with this version?\n\nThanks.\n\n> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\n> index 9554720152..10adeabf0f 100755\n> --- a/t/t7406-submodule-update.sh\n> +++ b/t/t7406-submodule-update.sh\n> @@ -256,6 +256,25 @@ test_expect_success 'submodule update --remote should fetch upstream changes' '\n>  \t)\n>  '\n>  \n> +test_expect_success 'submodule update --remote resolves URL rewrites' '\n> +\ttest_config_global \"url.$(pwd)/.insteadOf\" local: &&\n> +\tmkdir alias-super alias-submodule &&\n> +\t(\n> +\t\tcd alias-submodule &&\n> +\t\tgit init &&\n> +\t\tgit commit --allow-empty --message \"Initial commit\"\n> +\t) &&\n> +\t(\n> +\t\tcd alias-super &&\n> +\t\tgit init &&\n> +\t\tgit submodule add local:alias-submodule submodule &&\n> +\t\tgit submodule update --force &&\n> +\t\tgit -C submodule remote rename origin upstream &&\n> +\t\tgit -C submodule remote add fork user@host &&\n> +\t\tgit submodule update --remote\n> +\t)\n> +'\n\n\n"},{"id":"550231","messageId":"CA+P7+xpADY-cfzfjmaXboJMdQfcjRLFNoxhWf4weU00-Q0g2rA@mail.gmail.com","threadId":"66047","inReplyTo":"xmqqldb05dlo.fsf@gitster.g","subject":"Re: [PATCH v2] submodule: resolve insteadOf aliases when matching remote","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2026-08-11T02:57:40Z","receivedAt":"2026-08-11T02:57:52Z","isPatch":true,"body":"On Fri, Jul 24, 2026 at 1:36 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Éric NICOLAS <ccjmne@gmail.com> writes:\n>\n> > - Reword the commit message more purposefully\n> > - Adjust the implementation as suggested, avoiding a superfluous\n> >   variable\n> > - Tidy up the integration test\n>\n> Queued.\n>\n> Is everybody happy with this version?\n>\n> Thanks.\n\nYes, consider it:\n\nReviewed-by: Jacob Keller <jacob.keller@gmail.com>\n\nAppreciate the fix, I think I had ran into this at some point and it\ngot put on a pile of \"to finish debugging later\" and never fixed.\nThanks!\n\n>\n> > diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\n> > index 9554720152..10adeabf0f 100755\n> > --- a/t/t7406-submodule-update.sh\n> > +++ b/t/t7406-submodule-update.sh\n> > @@ -256,6 +256,25 @@ test_expect_success 'submodule update --remote should fetch upstream changes' '\n> >       )\n> >  '\n> >\n> > +test_expect_success 'submodule update --remote resolves URL rewrites' '\n> > +     test_config_global \"url.$(pwd)/.insteadOf\" local: &&\n> > +     mkdir alias-super alias-submodule &&\n> > +     (\n> > +             cd alias-submodule &&\n> > +             git init &&\n> > +             git commit --allow-empty --message \"Initial commit\"\n> > +     ) &&\n> > +     (\n> > +             cd alias-super &&\n> > +             git init &&\n> > +             git submodule add local:alias-submodule submodule &&\n> > +             git submodule update --force &&\n> > +             git -C submodule remote rename origin upstream &&\n> > +             git -C submodule remote add fork user@host &&\n> > +             git submodule update --remote\n> > +     )\n> > +'\n>\n>\n"}]}