{"thread":{"id":"63684","subject":"[PATCH v4 0/7] submodule: improve remote lookup logic","startedAt":"2025-06-23T23:17:02Z","lastAt":"2025-06-23T23:17:07Z","messageCount":8,"participants":["Jacob Keller"],"isPatch":true,"patchVersion":4,"patchTotal":7},"messages":[{"id":"520605","messageId":"20250623-jk-submodule-helper-use-url-v4-0-133ef3d89569@gmail.com","threadId":"63684","inReplyTo":null,"subject":"[PATCH v4 0/7] submodule: improve remote lookup logic","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-06-23T23:11:28Z","receivedAt":"2025-06-23T23:17:02Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"This series improves the git submodule remote lookup logic implemented in\nsubmodule--helper.\n\nA few cleanups are done first:\n\n* Remove the branch->merge_name array and replace it by directly using\n  branch->merge[i]->src immediately. This is simpler and easier to reason\n  about. While cleaning this up, also fix the issues with branch_release()\n  not tearing down everything properly.\n\n* remote_clear() failed to release the remote->push and remote->fetch\n  refspec data. Fix this.\n\n* The starts_with_dot(_dot)_slash helper functions are moved to dir.h for\n  re-use, as these are used both within submodule--helper.c and\n  submodule-config.c\n\n* Several remote.c helper functions are refactored to take repository\n  pointers, enabling use with a submodule repository pointer.\n\nNext, the submodule--helper.c logic replaces the repo_get_default_remote()\nfunction with a repo_default_remote() function in remote.c, which is based\non the more robust configuration reading logic. This helper uses similar\nlogic but also allows returning the only valid remote in the case where a\nrepository has exactly one remote. This way we do not fall back to \"origin\"\nif a user has renamed the remote without adding another.\n\nThis improved logic is a good first step, but won't handle cases where\nthere are multiple remotes, with none of them being named \"origin\".\n\nFor the final improvement, notice that the parent project already stores\nthe URL for the submodule in its .git/config or .gitmodules file. This URL\nis what we use to set the remote in the first place when cloning.\n\nAdd a repo_remote_from_url() helper which will iterate through the remotes\nand find the first remote with that URL. Use this in\nget_default_remote_submodule() to first try and find a remote by its URL.\nIf unsuccessful, we still keep the original fallback logic, in the off\nchance that the user has changed the URL from within the submodule.\n\nThis method is more robust as it is less likely that the user has manually\nchanged the submodule URL within the submodule but not also within the\n.git/config.\n\nWith this change, all commands which need the submodule remote will first\nlook up by URL before trying to use the fallback logic, and should now be\nable to find a suitable remote regardless of now they are renamed.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\nChanges in v4:\n- Fix branch_has_merge_config to use branch->set_merge\n- FREE_AND_NULL branch->merge in merge_clear()\n- Link to v3: https://lore.kernel.org/r/20250618-jk-submodule-helper-use-url-v3-0-7c60f2679271@gmail.com\n\nChanges in v3:\n- Completely remove branch->merge_name, making the resulting logic much\n  easier to understand.\n- Link to v2: https://lore.kernel.org/r/20250617-jk-submodule-helper-use-url-v2-0-04cbb003177d@gmail.com\n\nChanges in v2:\n- Remove repo_get_default_remote() entirely. The extra checks it does are\n  really only necessary if you're doing manual configuration lookup. This\n  avoids the confusion of similarly named functions and is less code.\n- Fix leaks in branch_release() and remote_clear().\n- Add a forward declaration of struct repository.\n- Verified tests pass with leak sanitizer now.\n- Link to v1: https://lore.kernel.org/r/20250610-jk-submodule-helper-use-url-v1-0-6d14c1504e91@gmail.com\n\n---\nJacob Keller (7):\n      remote: remove branch->merge_name and fix branch_release()\n      remote: fix tear down of struct remote\n      dir: move starts_with_dot(_dot)_slash to dir.h\n      remote: remove the_repository from some functions\n      submodule--helper: improve logic for fallback remote name\n      submodule: move get_default_remote_submodule()\n      submodule: look up remotes by URL first\n\n dir.h                       |  23 +++++++\n remote.h                    |   8 ++-\n branch.c                    |   4 +-\n builtin/pull.c              |   2 +-\n builtin/submodule--helper.c | 106 ++++++++++++-------------------\n remote.c                    | 149 ++++++++++++++++++++++++++++----------------\n submodule-config.c          |  12 ----\n t/t7406-submodule-update.sh |  61 ++++++++++++++++++\n 8 files changed, 230 insertions(+), 135 deletions(-)\n---\nbase-commit: 16bd9f20a403117f2e0d9bcda6c6e621d3763e77\nchange-id: 20250610-jk-submodule-helper-use-url-e55d3c379faf\n\nBest regards,\n-- \nJacob Keller <jacob.keller@gmail.com>\n\n"},{"id":"520606","messageId":"20250623-jk-submodule-helper-use-url-v4-1-133ef3d89569@gmail.com","threadId":"63684","inReplyTo":"20250623-jk-submodule-helper-use-url-v4-0-133ef3d89569@gmail.com","subject":"[PATCH v4 1/7] remote: remove branch->merge_name and fix branch_release()","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-06-23T23:11:29Z","receivedAt":"2025-06-23T23:17:03Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThe branch structure has both branch->merge_name and branch->merge for\ntracking the merge information. The former is allocated by add_merge()\nand stores the names read from the configuration file. The latter is\nallocated by set_merge() which is called by branch_get() when an\nexternal caller requests a branch.\n\nThis leads to the confusing situation where branch->merge_nr tracks both\nthe size of branch->merge (once its allocated) and branch->merge_name.\nThe branch_release() function incorrectly assumes that branch->merge is\nalways set when branch->merge_nr is non-zero, and can potentially crash\nif read_config() is called without branch_get() being called on every\nbranch.\n\nIn addition, branch_release() fails to free some of the memory\nassociated with the structure including:\n\n * Failure to free the refspec_item containers in branch->merge[i]\n * Failure to free the strings in branch->merge_name[i]\n * Failure to free the branch->merge_name parent array.\n\nThe set_merge() function sets branch->merge_nr to 0 when there is no\nvalid remote_name, to avoid external callers seeing a non-zero merge_nr\nbut a NULL merge array. This results in failure to release most of the\nmerge data as well.\n\nThese issues could be fixed directly, and indeed I initially proposed\nsuch a change at [1] in the past. While this works, there was some\nconfusion during review because of the inconsistencies.\n\nInstead, its time to clean up the situation properly. Remove\nbranch->merge_name entirely. Instead, allocate branch->merge earlier\nwithin add_merge() instead of within set_merge(). Instead of having\nset_merge() copy from merge_name[i] to merge[i]->src, just have\nadd_merge() directly initialize merge[i]->src.\n\nModify the add_merge() to call xstrdup() itself, instead of having\nthe caller of add_merge() do so. This makes it more obvious which code\nowns the memory.\n\nUpdate all callers which use branch->merge_name[i] to use\nbranch->merge[i]->src instead.\n\nAdd a merge_clear() function which properly releases all of the\nmerge-related memory, and which sets branch->merge_nr to zero. Use this\nboth in branch_release() and in set_merge(), fixing the leak when\nset_merge() finds no valid remote_name.\n\nAdd a set_merge variable to the branch structure, which indicates\nwhether set_merge() has been called. This replaces the previous use of a\nNULL check against the branch->merge array.\n\nWith these changes, the merge array is always allocated when merge_nr is\nnon-zero.\n\nThis use of refspec_item to store the names should be safe. External\ncallers should be using branch_get() to obtain a pointer to the branch,\nwhich will call set_merge(), and the callers internal to remote.c\nalready handle the partially initialized refpsec_item structure safely.\n\nThis end result is cleaner, and avoids duplicating the merge names\ntwice.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\nLink: [1] https://lore.kernel.org/git/20250617-jk-submodule-helper-use-url-v2-1-04cbb003177d@gmail.com/\n---\n remote.h       |  4 ++--\n branch.c       |  4 ++--\n builtin/pull.c |  2 +-\n remote.c       | 44 ++++++++++++++++++++++++++++----------------\n 4 files changed, 33 insertions(+), 21 deletions(-)\n\ndiff --git a/remote.h b/remote.h\nindex 7e4943ae3a70ecefa3332d211084762ca30b59b6..76d93bf88d1fb8c0e2cbc2bc99558f23a256155c 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -315,8 +315,8 @@ struct branch {\n \n \tchar *pushremote_name;\n \n-\t/* An array of the \"merge\" lines in the configuration. */\n-\tconst char **merge_name;\n+\t/* True if set_merge() has been called to finalize the merge array */\n+\tint set_merge;\n \n \t/**\n \t * An array of the struct refspecs used for the merge lines. That is,\ndiff --git a/branch.c b/branch.c\nindex 6d01d7d6bdb2e4d969429433b1b6bc88446a96c5..93f5b4e8dd9fe53ae4412827c458bade7c341278 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -230,7 +230,7 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref)\n \t\treturn -1;\n \t}\n \n-\tif (branch->merge_nr < 1 || !branch->merge_name || !branch->merge_name[0]) {\n+\tif (branch->merge_nr < 1 || !branch->merge || !branch->merge[0] || !branch->merge[0]->src) {\n \t\twarning(_(\"asked to inherit tracking from '%s', but no merge configuration is set\"),\n \t\t\tbare_ref);\n \t\treturn -1;\n@@ -238,7 +238,7 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref)\n \n \ttracking->remote = branch->remote_name;\n \tfor (i = 0; i < branch->merge_nr; i++)\n-\t\tstring_list_append(tracking->srcs, branch->merge_name[i]);\n+\t\tstring_list_append(tracking->srcs, branch->merge[i]->src);\n \treturn 0;\n }\n \ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex a1ebc6ad3328e074b105246f6bf5c41b063c17c9..f4556ae155ce22ea91f9878d772eb9228fe4e604 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -487,7 +487,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs\n \t} else\n \t\tfprintf_ln(stderr, _(\"Your configuration specifies to merge with the ref '%s'\\n\"\n \t\t\t\"from the remote, but no such ref was fetched.\"),\n-\t\t\t*curr_branch->merge_name);\n+\t\t\tcurr_branch->merge[0]->src);\n \texit(1);\n }\n \ndiff --git a/remote.c b/remote.c\nindex 4099183cacdc8a607a8b5eaec86e456b2ef46b48..ee95126f3f20080a932b82314e8017e277569cc1 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -174,9 +174,15 @@ static void remote_clear(struct remote *remote)\n \n static void add_merge(struct branch *branch, const char *name)\n {\n-\tALLOC_GROW(branch->merge_name, branch->merge_nr + 1,\n+\tstruct refspec_item *merge;\n+\n+\tALLOC_GROW(branch->merge, branch->merge_nr + 1,\n \t\t   branch->merge_alloc);\n-\tbranch->merge_name[branch->merge_nr++] = name;\n+\n+\tmerge = xcalloc(1, sizeof(*merge));\n+\tmerge->src = xstrdup(name);\n+\n+\tbranch->merge[branch->merge_nr++] = merge;\n }\n \n struct branches_hash_key {\n@@ -247,15 +253,23 @@ static struct branch *make_branch(struct remote_state *remote_state,\n \treturn ret;\n }\n \n+static void merge_clear(struct branch *branch)\n+{\n+\tfor (int i = 0; i < branch->merge_nr; i++) {\n+\t\trefspec_item_clear(branch->merge[i]);\n+\t\tfree(branch->merge[i]);\n+\t}\n+\tFREE_AND_NULL(branch->merge);\n+\tbranch->merge_nr = 0;\n+}\n+\n static void branch_release(struct branch *branch)\n {\n \tfree((char *)branch->name);\n \tfree((char *)branch->refname);\n \tfree(branch->remote_name);\n \tfree(branch->pushremote_name);\n-\tfor (int i = 0; i < branch->merge_nr; i++)\n-\t\trefspec_item_clear(branch->merge[i]);\n-\tfree(branch->merge);\n+\tmerge_clear(branch);\n }\n \n static struct rewrite *make_rewrite(struct rewrites *r,\n@@ -429,7 +443,7 @@ static int handle_config(const char *key, const char *value,\n \t\t} else if (!strcmp(subkey, \"merge\")) {\n \t\t\tif (!value)\n \t\t\t\treturn config_error_nonbool(key);\n-\t\t\tadd_merge(branch, xstrdup(value));\n+\t\t\tadd_merge(branch, value);\n \t\t}\n \t\treturn 0;\n \t}\n@@ -692,7 +706,7 @@ char *remote_ref_for_branch(struct branch *branch, int for_push)\n \tif (branch) {\n \t\tif (!for_push) {\n \t\t\tif (branch->merge_nr) {\n-\t\t\t\treturn xstrdup(branch->merge_name[0]);\n+\t\t\t\treturn xstrdup(branch->merge[0]->src);\n \t\t\t}\n \t\t} else {\n \t\t\tchar *dst;\n@@ -1731,32 +1745,30 @@ static void set_merge(struct remote_state *remote_state, struct branch *ret)\n \n \tif (!ret)\n \t\treturn; /* no branch */\n-\tif (ret->merge)\n+\tif (ret->set_merge)\n \t\treturn; /* already run */\n \tif (!ret->remote_name || !ret->merge_nr) {\n \t\t/*\n \t\t * no merge config; let's make sure we don't confuse callers\n \t\t * with a non-zero merge_nr but a NULL merge\n \t\t */\n-\t\tret->merge_nr = 0;\n+\t\tmerge_clear(ret);\n \t\treturn;\n \t}\n+\tret->set_merge = 1;\n \n \tremote = remotes_remote_get(remote_state, ret->remote_name);\n \n-\tCALLOC_ARRAY(ret->merge, ret->merge_nr);\n \tfor (i = 0; i < ret->merge_nr; i++) {\n-\t\tret->merge[i] = xcalloc(1, sizeof(**ret->merge));\n-\t\tret->merge[i]->src = xstrdup(ret->merge_name[i]);\n \t\tif (!remote_find_tracking(remote, ret->merge[i]) ||\n \t\t    strcmp(ret->remote_name, \".\"))\n \t\t\tcontinue;\n-\t\tif (repo_dwim_ref(the_repository, ret->merge_name[i],\n-\t\t\t\t  strlen(ret->merge_name[i]), &oid, &ref,\n+\t\tif (repo_dwim_ref(the_repository, ret->merge[i]->src,\n+\t\t\t\t  strlen(ret->merge[i]->src), &oid, &ref,\n \t\t\t\t  0) == 1)\n \t\t\tret->merge[i]->dst = ref;\n \t\telse\n-\t\t\tret->merge[i]->dst = xstrdup(ret->merge_name[i]);\n+\t\t\tret->merge[i]->dst = xstrdup(ret->merge[i]->src);\n \t}\n }\n \n@@ -1776,7 +1788,7 @@ struct branch *branch_get(const char *name)\n \n int branch_has_merge_config(struct branch *branch)\n {\n-\treturn branch && !!branch->merge;\n+\treturn branch && branch->set_merge;\n }\n \n int branch_merge_matches(struct branch *branch,\n\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"520607","messageId":"20250623-jk-submodule-helper-use-url-v4-3-133ef3d89569@gmail.com","threadId":"63684","inReplyTo":"20250623-jk-submodule-helper-use-url-v4-0-133ef3d89569@gmail.com","subject":"[PATCH v4 3/7] dir: move starts_with_dot(_dot)_slash to dir.h","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-06-23T23:11:31Z","receivedAt":"2025-06-23T23:17:04Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nBoth submodule--helper.c and submodule-config.c have an implementation\nof starts_with_dot_slash and starts_with_dot_dot_slash. The dir.h header\nhas starts_with_dot(_dot)_slash_native, which sets PATH_MATCH_NATIVE.\n\nMove the helpers to dir.h as static inlines. I thought about renaming\nthem to postfix with _platform but that felt too long and ugly. On the\nother hand it might be slightly confusing with _native.\n\nThis simplifies a submodule refactor which wants to use the helpers\nearlier in the submodule--helper.c file.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n dir.h                       | 23 +++++++++++++++++++++++\n builtin/submodule--helper.c | 12 ------------\n submodule-config.c          | 12 ------------\n 3 files changed, 23 insertions(+), 24 deletions(-)\n\ndiff --git a/dir.h b/dir.h\nindex d7e71aa8daa7d833e4c05e6875b997bc321c6070..fc9be7b427a134e46bcd66c8df42375db47727fc 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -676,4 +676,27 @@ static inline int starts_with_dot_dot_slash_native(const char *const path)\n \treturn path_match_flags(path, what | PATH_MATCH_NATIVE);\n }\n \n+/**\n+ * starts_with_dot_slash: convenience wrapper for\n+ * patch_match_flags() with PATH_MATCH_STARTS_WITH_DOT_SLASH and\n+ * PATH_MATCH_XPLATFORM.\n+ */\n+static inline int starts_with_dot_slash(const char *const path)\n+{\n+\tconst enum path_match_flags what = PATH_MATCH_STARTS_WITH_DOT_SLASH;\n+\n+\treturn path_match_flags(path, what | PATH_MATCH_XPLATFORM);\n+}\n+\n+/**\n+ * starts_with_dot_dot_slash: convenience wrapper for\n+ * patch_match_flags() with PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH and\n+ * PATH_MATCH_XPLATFORM.\n+ */\n+static inline int starts_with_dot_dot_slash(const char *const path)\n+{\n+\tconst enum path_match_flags what = PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH;\n+\n+\treturn path_match_flags(path, what | PATH_MATCH_XPLATFORM);\n+}\n #endif\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 53da2116ddf576bc565b29f043e8b703b8b1563b..9e8cdfe1b2a8c2985d9c1b8ad6f1b0d1f9401714 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -438,18 +438,6 @@ static int module_foreach(int argc, const char **argv, const char *prefix,\n \treturn ret;\n }\n \n-static int starts_with_dot_slash(const char *const path)\n-{\n-\treturn path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_SLASH |\n-\t\t\t\tPATH_MATCH_XPLATFORM);\n-}\n-\n-static int starts_with_dot_dot_slash(const char *const path)\n-{\n-\treturn path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH |\n-\t\t\t\tPATH_MATCH_XPLATFORM);\n-}\n-\n struct init_cb {\n \tconst char *prefix;\n \tconst char *super_prefix;\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 8630e27947d3943e1980eb7a53bd41a546842503..d64438b2a18ed2123cc5e18f739539209032d3e9 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -235,18 +235,6 @@ int check_submodule_name(const char *name)\n \treturn 0;\n }\n \n-static int starts_with_dot_slash(const char *const path)\n-{\n-\treturn path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_SLASH |\n-\t\t\t\tPATH_MATCH_XPLATFORM);\n-}\n-\n-static int starts_with_dot_dot_slash(const char *const path)\n-{\n-\treturn path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH |\n-\t\t\t\tPATH_MATCH_XPLATFORM);\n-}\n-\n static int submodule_url_is_relative(const char *url)\n {\n \treturn starts_with_dot_slash(url) || starts_with_dot_dot_slash(url);\n\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"520608","messageId":"20250623-jk-submodule-helper-use-url-v4-2-133ef3d89569@gmail.com","threadId":"63684","inReplyTo":"20250623-jk-submodule-helper-use-url-v4-0-133ef3d89569@gmail.com","subject":"[PATCH v4 2/7] remote: fix tear down of struct remote","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-06-23T23:11:30Z","receivedAt":"2025-06-23T23:17:04Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThe remote_clear() function failed to free the remote->push and\nremote->fetch refspec fields.\n\nThis should be caught by the leak sanitizer. However, for callers which\nuse ``the_repository``, the values never go out of scope and the\nsanitizer doesn't complain.\n\nA future change is going to add a caller of read_config() for a\nsubmodule repository structure, which would result in the leak sanitizer\ncomplaining.\n\nFix remote_clear(), updating it to properly call refspec_clear() for\nboth the push and fetch members.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n remote.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/remote.c b/remote.c\nindex ee95126f3f20080a932b82314e8017e277569cc1..194bb447784ac1f71fb85a9fed3312e7458a9d5d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -165,6 +165,9 @@ static void remote_clear(struct remote *remote)\n \tstrvec_clear(&remote->url);\n \tstrvec_clear(&remote->pushurl);\n \n+\trefspec_clear(&remote->push);\n+\trefspec_clear(&remote->fetch);\n+\n \tfree((char *)remote->receivepack);\n \tfree((char *)remote->uploadpack);\n \tFREE_AND_NULL(remote->http_proxy);\n\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"520609","messageId":"20250623-jk-submodule-helper-use-url-v4-4-133ef3d89569@gmail.com","threadId":"63684","inReplyTo":"20250623-jk-submodule-helper-use-url-v4-0-133ef3d89569@gmail.com","subject":"[PATCH v4 4/7] remote: remove the_repository from some functions","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-06-23T23:11:32Z","receivedAt":"2025-06-23T23:17:05Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThe remotes_remote_get_1 (and its caller, remotes_remote_get, have an\nimplicit dependency on the_repository due to calling\nread_branches_file() and read_remotes_file(), both of which use\nthe_repository. The branch_get() function calls set_merge() which has an\nimplicit dependency on the_repository as well.\n\nBecause of this use of the_repository, the helper functions cannot be\nused in code paths which operate on other repositories. A future\nrefactor of the submodule--helper will want to make use of some of these\nfunctions.\n\nRefactor to break the dependency by passing struct repository *repo\ninstead of struct remote_state *remote_state in a few places.\n\nThe public callers and many other helper functions still depend on\nthe_repository. A repo-aware function will be exposed in a following\nchange for git submodule--helper.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n remote.c | 58 ++++++++++++++++++++++++++++------------------------------\n 1 file changed, 28 insertions(+), 30 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 194bb447784ac1f71fb85a9fed3312e7458a9d5d..e7ff21dc0340fd81b0c0c21c9d2199c3c0d53946 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -334,11 +334,10 @@ static void warn_about_deprecated_remote_type(const char *type,\n \t\ttype, remote->name, remote->name, remote->name);\n }\n \n-static void read_remotes_file(struct remote_state *remote_state,\n-\t\t\t      struct remote *remote)\n+static void read_remotes_file(struct repository *repo, struct remote *remote)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tFILE *f = fopen_or_warn(repo_git_path_append(the_repository, &buf,\n+\tFILE *f = fopen_or_warn(repo_git_path_append(repo, &buf,\n \t\t\t\t\t\t     \"remotes/%s\", remote->name), \"r\");\n \n \tif (!f)\n@@ -354,7 +353,7 @@ static void read_remotes_file(struct remote_state *remote_state,\n \t\tstrbuf_rtrim(&buf);\n \n \t\tif (skip_prefix(buf.buf, \"URL:\", &v))\n-\t\t\tadd_url_alias(remote_state, remote,\n+\t\t\tadd_url_alias(repo->remote_state, remote,\n \t\t\t\t      skip_spaces(v));\n \t\telse if (skip_prefix(buf.buf, \"Push:\", &v))\n \t\t\trefspec_append(&remote->push, skip_spaces(v));\n@@ -367,12 +366,11 @@ static void read_remotes_file(struct remote_state *remote_state,\n \tstrbuf_release(&buf);\n }\n \n-static void read_branches_file(struct remote_state *remote_state,\n-\t\t\t       struct remote *remote)\n+static void read_branches_file(struct repository *repo, struct remote *remote)\n {\n \tchar *frag, *to_free = NULL;\n \tstruct strbuf buf = STRBUF_INIT;\n-\tFILE *f = fopen_or_warn(repo_git_path_append(the_repository, &buf,\n+\tFILE *f = fopen_or_warn(repo_git_path_append(repo, &buf,\n \t\t\t\t\t\t     \"branches/%s\", remote->name), \"r\");\n \n \tif (!f)\n@@ -399,9 +397,9 @@ static void read_branches_file(struct remote_state *remote_state,\n \tif (frag)\n \t\t*(frag++) = '\\0';\n \telse\n-\t\tfrag = to_free = repo_default_branch_name(the_repository, 0);\n+\t\tfrag = to_free = repo_default_branch_name(repo, 0);\n \n-\tadd_url_alias(remote_state, remote, buf.buf);\n+\tadd_url_alias(repo->remote_state, remote, buf.buf);\n \trefspec_appendf(&remote->fetch, \"refs/heads/%s:refs/heads/%s\",\n \t\t\tfrag, remote->name);\n \n@@ -698,7 +696,7 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit)\n \t\t\t\t\t     branch, explicit);\n }\n \n-static struct remote *remotes_remote_get(struct remote_state *remote_state,\n+static struct remote *remotes_remote_get(struct repository *repo,\n \t\t\t\t\t const char *name);\n \n char *remote_ref_for_branch(struct branch *branch, int for_push)\n@@ -717,7 +715,7 @@ char *remote_ref_for_branch(struct branch *branch, int for_push)\n \t\t\t\t\tthe_repository->remote_state, branch,\n \t\t\t\t\tNULL);\n \t\t\tstruct remote *remote = remotes_remote_get(\n-\t\t\t\tthe_repository->remote_state, remote_name);\n+\t\t\t\tthe_repository, remote_name);\n \n \t\t\tif (remote && remote->push.nr &&\n \t\t\t    (dst = apply_refspecs(&remote->push,\n@@ -774,10 +772,11 @@ static void validate_remote_url(struct remote *remote)\n }\n \n static struct remote *\n-remotes_remote_get_1(struct remote_state *remote_state, const char *name,\n+remotes_remote_get_1(struct repository *repo, const char *name,\n \t\t     const char *(*get_default)(struct remote_state *,\n \t\t\t\t\t\tstruct branch *, int *))\n {\n+\tstruct remote_state *remote_state = repo->remote_state;\n \tstruct remote *ret;\n \tint name_given = 0;\n \n@@ -791,9 +790,9 @@ remotes_remote_get_1(struct remote_state *remote_state, const char *name,\n #ifndef WITH_BREAKING_CHANGES\n \tif (valid_remote_nick(name) && have_git_dir()) {\n \t\tif (!valid_remote(ret))\n-\t\t\tread_remotes_file(remote_state, ret);\n+\t\t\tread_remotes_file(repo, ret);\n \t\tif (!valid_remote(ret))\n-\t\t\tread_branches_file(remote_state, ret);\n+\t\t\tread_branches_file(repo, ret);\n \t}\n #endif /* WITH_BREAKING_CHANGES */\n \tif (name_given && !valid_remote(ret))\n@@ -807,35 +806,33 @@ remotes_remote_get_1(struct remote_state *remote_state, const char *name,\n }\n \n static inline struct remote *\n-remotes_remote_get(struct remote_state *remote_state, const char *name)\n+remotes_remote_get(struct repository *repo, const char *name)\n {\n-\treturn remotes_remote_get_1(remote_state, name,\n-\t\t\t\t    remotes_remote_for_branch);\n+\treturn remotes_remote_get_1(repo, name, remotes_remote_for_branch);\n }\n \n struct remote *remote_get(const char *name)\n {\n \tread_config(the_repository, 0);\n-\treturn remotes_remote_get(the_repository->remote_state, name);\n+\treturn remotes_remote_get(the_repository, name);\n }\n \n struct remote *remote_get_early(const char *name)\n {\n \tread_config(the_repository, 1);\n-\treturn remotes_remote_get(the_repository->remote_state, name);\n+\treturn remotes_remote_get(the_repository, name);\n }\n \n static inline struct remote *\n-remotes_pushremote_get(struct remote_state *remote_state, const char *name)\n+remotes_pushremote_get(struct repository *repo, const char *name)\n {\n-\treturn remotes_remote_get_1(remote_state, name,\n-\t\t\t\t    remotes_pushremote_for_branch);\n+\treturn remotes_remote_get_1(repo, name, remotes_pushremote_for_branch);\n }\n \n struct remote *pushremote_get(const char *name)\n {\n \tread_config(the_repository, 0);\n-\treturn remotes_pushremote_get(the_repository->remote_state, name);\n+\treturn remotes_pushremote_get(the_repository, name);\n }\n \n int remote_is_configured(struct remote *remote, int in_repo)\n@@ -1739,7 +1736,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n \t}\n }\n \n-static void set_merge(struct remote_state *remote_state, struct branch *ret)\n+static void set_merge(struct repository *repo, struct branch *ret)\n {\n \tstruct remote *remote;\n \tchar *ref;\n@@ -1760,13 +1757,13 @@ static void set_merge(struct remote_state *remote_state, struct branch *ret)\n \t}\n \tret->set_merge = 1;\n \n-\tremote = remotes_remote_get(remote_state, ret->remote_name);\n+\tremote = remotes_remote_get(repo, ret->remote_name);\n \n \tfor (i = 0; i < ret->merge_nr; i++) {\n \t\tif (!remote_find_tracking(remote, ret->merge[i]) ||\n \t\t    strcmp(ret->remote_name, \".\"))\n \t\t\tcontinue;\n-\t\tif (repo_dwim_ref(the_repository, ret->merge[i]->src,\n+\t\tif (repo_dwim_ref(repo, ret->merge[i]->src,\n \t\t\t\t  strlen(ret->merge[i]->src), &oid, &ref,\n \t\t\t\t  0) == 1)\n \t\t\tret->merge[i]->dst = ref;\n@@ -1785,7 +1782,7 @@ struct branch *branch_get(const char *name)\n \telse\n \t\tret = make_branch(the_repository->remote_state, name,\n \t\t\t\t  strlen(name));\n-\tset_merge(the_repository->remote_state, ret);\n+\tset_merge(the_repository, ret);\n \treturn ret;\n }\n \n@@ -1856,13 +1853,14 @@ static const char *tracking_for_push_dest(struct remote *remote,\n \treturn ret;\n }\n \n-static const char *branch_get_push_1(struct remote_state *remote_state,\n+static const char *branch_get_push_1(struct repository *repo,\n \t\t\t\t     struct branch *branch, struct strbuf *err)\n {\n+\tstruct remote_state *remote_state = repo->remote_state;\n \tstruct remote *remote;\n \n \tremote = remotes_remote_get(\n-\t\tremote_state,\n+\t\trepo,\n \t\tremotes_pushremote_for_branch(remote_state, branch, NULL));\n \tif (!remote)\n \t\treturn error_buf(err,\n@@ -1929,7 +1927,7 @@ const char *branch_get_push(struct branch *branch, struct strbuf *err)\n \n \tif (!branch->push_tracking_ref)\n \t\tbranch->push_tracking_ref = branch_get_push_1(\n-\t\t\tthe_repository->remote_state, branch, err);\n+\t\t\tthe_repository, branch, err);\n \treturn branch->push_tracking_ref;\n }\n \n\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"520610","messageId":"20250623-jk-submodule-helper-use-url-v4-5-133ef3d89569@gmail.com","threadId":"63684","inReplyTo":"20250623-jk-submodule-helper-use-url-v4-0-133ef3d89569@gmail.com","subject":"[PATCH v4 5/7] submodule--helper: improve logic for fallback remote name","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-06-23T23:11:33Z","receivedAt":"2025-06-23T23:17:06Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThe repo_get_default_remote() function in submodule--helper currently\ntries to figure out the proper remote name to use for a submodule based\non a few factors.\n\nFirst, it tries to find the remote for the currently checked out branch.\nThis works if the submodule is configured to checkout to a branch\ninstead of a detached HEAD state.\n\nIn the detached HEAD state, the code calls back to using \"origin\", on\nthe assumption that this is the default remote name. Some users may\nchange this, such as by setting clone.defaultRemoteName, or by changing\nthe remote name manually within the submodule repository.\n\nAs a first step to improving this situation, refactor to reuse the logic\nfrom remotes_remote_for_branch(). This function uses the remote from the\nbranch if it has one. If it doesn't then it checks to see if there is\nexactly one remote. It uses this remote first before attempting to fall\nback to \"origin\".\n\nTo allow using this helper function, introduce a repo_default_remote()\nhelper to remote.c which takes a repository structure. This helper will\nload the remote configuration and get the \"HEAD\" branch. Then it will\ncall remotes_remote_for_branch to find the default remote.\n\nReplace calls of repo_get_default_remote() with the calls to this new\nfunction. To maintain consistency with the existing callers, continue\ncopying the returned string with xstrdup.\n\nThis isn't a perfect solution for users who change remote names, but it\nshould help in cases where the remote name is changed but users haven't\nadded any additional remotes.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n remote.h                    |  3 +++\n builtin/submodule--helper.c | 46 +++++----------------------------------------\n remote.c                    | 25 +++++++++++++++++++-----\n t/t7406-submodule-update.sh | 29 ++++++++++++++++++++++++++++\n 4 files changed, 57 insertions(+), 46 deletions(-)\n\ndiff --git a/remote.h b/remote.h\nindex 76d93bf88d1fb8c0e2cbc2bc99558f23a256155c..8dc5cfa49ef78808348a84c9b3f416b31cd3bbd7 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -9,6 +9,7 @@\n \n struct option;\n struct transport_ls_refs_options;\n+struct repository;\n \n /**\n  * The API gives access to the configuration related to remotes. It handles\n@@ -338,6 +339,8 @@ const char *remote_for_branch(struct branch *branch, int *explicit);\n const char *pushremote_for_branch(struct branch *branch, int *explicit);\n char *remote_ref_for_branch(struct branch *branch, int for_push);\n \n+const char *repo_default_remote(struct repository *repo);\n+\n /* returns true if the given branch has merge configuration given. */\n int branch_has_merge_config(struct branch *branch);\n \ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 9e8cdfe1b2a8c2985d9c1b8ad6f1b0d1f9401714..4aa237033a526fca29cce2926419462179d40ee3 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -41,61 +41,25 @@\n typedef void (*each_submodule_fn)(const struct cache_entry *list_item,\n \t\t\t\t  void *cb_data);\n \n-static int repo_get_default_remote(struct repository *repo, char **default_remote)\n-{\n-\tchar *dest = NULL;\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tstruct ref_store *store = get_main_ref_store(repo);\n-\tconst char *refname = refs_resolve_ref_unsafe(store, \"HEAD\", 0, NULL,\n-\t\t\t\t\t\t      NULL);\n-\n-\tif (!refname)\n-\t\treturn die_message(_(\"No such ref: %s\"), \"HEAD\");\n-\n-\t/* detached HEAD */\n-\tif (!strcmp(refname, \"HEAD\")) {\n-\t\t*default_remote = xstrdup(\"origin\");\n-\t\treturn 0;\n-\t}\n-\n-\tif (!skip_prefix(refname, \"refs/heads/\", &refname))\n-\t\treturn die_message(_(\"Expecting a full ref name, got %s\"),\n-\t\t\t\t   refname);\n-\n-\tstrbuf_addf(&sb, \"branch.%s.remote\", refname);\n-\tif (repo_config_get_string(repo, sb.buf, &dest))\n-\t\t*default_remote = xstrdup(\"origin\");\n-\telse\n-\t\t*default_remote = dest;\n-\n-\tstrbuf_release(&sb);\n-\treturn 0;\n-}\n-\n static int get_default_remote_submodule(const char *module_path, char **default_remote)\n {\n \tstruct repository subrepo;\n-\tint ret;\n \n \tif (repo_submodule_init(&subrepo, the_repository, module_path,\n \t\t\t\tnull_oid(the_hash_algo)) < 0)\n \t\treturn die_message(_(\"could not get a repository handle for submodule '%s'\"),\n \t\t\t\t   module_path);\n-\tret = repo_get_default_remote(&subrepo, default_remote);\n+\n+\t*default_remote = xstrdup(repo_default_remote(&subrepo));\n+\n \trepo_clear(&subrepo);\n \n-\treturn ret;\n+\treturn 0;\n }\n \n static char *get_default_remote(void)\n {\n-\tchar *default_remote;\n-\tint code = repo_get_default_remote(the_repository, &default_remote);\n-\n-\tif (code)\n-\t\texit(code);\n-\n-\treturn default_remote;\n+\treturn xstrdup(repo_default_remote(the_repository));\n }\n \n static char *resolve_relative_url(const char *rel_url, const char *up_path, int quiet)\ndiff --git a/remote.c b/remote.c\nindex e7ff21dc0340fd81b0c0c21c9d2199c3c0d53946..e35cf7ec61ac946d1e84c5a0ae309e63c66d0335 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1772,20 +1772,35 @@ static void set_merge(struct repository *repo, struct branch *ret)\n \t}\n }\n \n-struct branch *branch_get(const char *name)\n+static struct branch *repo_branch_get(struct repository *repo, const char *name)\n {\n \tstruct branch *ret;\n \n-\tread_config(the_repository, 0);\n+\tread_config(repo, 0);\n \tif (!name || !*name || !strcmp(name, \"HEAD\"))\n-\t\tret = the_repository->remote_state->current_branch;\n+\t\tret = repo->remote_state->current_branch;\n \telse\n-\t\tret = make_branch(the_repository->remote_state, name,\n+\t\tret = make_branch(repo->remote_state, name,\n \t\t\t\t  strlen(name));\n-\tset_merge(the_repository, ret);\n+\tset_merge(repo, ret);\n \treturn ret;\n }\n \n+struct branch *branch_get(const char *name)\n+{\n+\treturn repo_branch_get(the_repository, name);\n+}\n+\n+const char *repo_default_remote(struct repository *repo)\n+{\n+\tstruct branch *branch;\n+\n+\tread_config(repo, 0);\n+\tbranch = repo_branch_get(repo, \"HEAD\");\n+\n+\treturn remotes_remote_for_branch(repo->remote_state, branch, NULL);\n+}\n+\n int branch_has_merge_config(struct branch *branch)\n {\n \treturn branch && branch->set_merge;\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex c562bad042ab2d4d0f82cb8b57a1eadbe24044d1..748b529745a5121f121768bb4e0cbc11bc833ea4 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -1134,6 +1134,35 @@ test_expect_success 'setup clean recursive superproject' '\n \tgit clone --recurse-submodules top top-clean\n '\n \n+test_expect_success 'submodule update with renamed remote' '\n+\ttest_when_finished \"rm -fr top-cloned\" &&\n+\tcp -r top-clean top-cloned &&\n+\n+\t# Create a commit in each repo, starting with bottom\n+\ttest_commit -C bottom rename_commit &&\n+\t# Create middle commit\n+\tgit -C middle/bottom fetch &&\n+\tgit -C middle/bottom checkout -f FETCH_HEAD &&\n+\tgit -C middle add bottom &&\n+\tgit -C middle commit -m \"rename_commit\" &&\n+\t# Create top commit\n+\tgit -C top/middle fetch &&\n+\tgit -C top/middle checkout -f FETCH_HEAD &&\n+\tgit -C top add middle &&\n+\tgit -C top commit -m \"rename_commit\" &&\n+\n+\t# rename the submodule remote\n+\tgit -C top-cloned/middle remote rename origin upstream &&\n+\n+\t# Make the update of \"middle\" a no-op, otherwise we error out\n+\t# because of its unmerged state\n+\ttest_config -C top-cloned submodule.middle.update !true &&\n+\tgit -C top-cloned submodule update --recursive 2>actual.err &&\n+\tcat >expect.err <<-\\EOF &&\n+\tEOF\n+\ttest_cmp expect.err actual.err\n+'\n+\n test_expect_success 'submodule update should skip unmerged submodules' '\n \ttest_when_finished \"rm -fr top-cloned\" &&\n \tcp -r top-clean top-cloned &&\n\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"520611","messageId":"20250623-jk-submodule-helper-use-url-v4-6-133ef3d89569@gmail.com","threadId":"63684","inReplyTo":"20250623-jk-submodule-helper-use-url-v4-0-133ef3d89569@gmail.com","subject":"[PATCH v4 6/7] submodule: move get_default_remote_submodule()","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-06-23T23:11:34Z","receivedAt":"2025-06-23T23:17:07Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nA future refactor got get_default_remote_submodule() is going to depend on\nresolve_relative_url(). That function depends on get_default_remote().\n\nMove get_default_remote_submodule() after resolve_relative_url() first\nto make the additional functionality easier to review.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n builtin/submodule--helper.c | 32 ++++++++++++++++----------------\n 1 file changed, 16 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 4aa237033a526fca29cce2926419462179d40ee3..1aa87435c2000e94f43da94c5ef88a307f6f3f4a 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -41,22 +41,6 @@\n typedef void (*each_submodule_fn)(const struct cache_entry *list_item,\n \t\t\t\t  void *cb_data);\n \n-static int get_default_remote_submodule(const char *module_path, char **default_remote)\n-{\n-\tstruct repository subrepo;\n-\n-\tif (repo_submodule_init(&subrepo, the_repository, module_path,\n-\t\t\t\tnull_oid(the_hash_algo)) < 0)\n-\t\treturn die_message(_(\"could not get a repository handle for submodule '%s'\"),\n-\t\t\t\t   module_path);\n-\n-\t*default_remote = xstrdup(repo_default_remote(&subrepo));\n-\n-\trepo_clear(&subrepo);\n-\n-\treturn 0;\n-}\n-\n static char *get_default_remote(void)\n {\n \treturn xstrdup(repo_default_remote(the_repository));\n@@ -86,6 +70,22 @@ static char *resolve_relative_url(const char *rel_url, const char *up_path, int\n \treturn resolved_url;\n }\n \n+static int get_default_remote_submodule(const char *module_path, char **default_remote)\n+{\n+\tstruct repository subrepo;\n+\n+\tif (repo_submodule_init(&subrepo, the_repository, module_path,\n+\t\t\t\tnull_oid(the_hash_algo)) < 0)\n+\t\treturn die_message(_(\"could not get a repository handle for submodule '%s'\"),\n+\t\t\t\t   module_path);\n+\n+\t*default_remote = xstrdup(repo_default_remote(&subrepo));\n+\n+\trepo_clear(&subrepo);\n+\n+\treturn 0;\n+}\n+\n /* the result should be freed by the caller. */\n static char *get_submodule_displaypath(const char *path, const char *prefix,\n \t\t\t\t       const char *super_prefix)\n\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"520612","messageId":"20250623-jk-submodule-helper-use-url-v4-7-133ef3d89569@gmail.com","threadId":"63684","inReplyTo":"20250623-jk-submodule-helper-use-url-v4-0-133ef3d89569@gmail.com","subject":"[PATCH v4 7/7] submodule: look up remotes by URL first","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-06-23T23:11:35Z","receivedAt":"2025-06-23T23:17:07Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThe get_default_remote_submodule() function performs a lookup to find\nthe appropriate remote to use within a submodule. The function first\nchecks to see if it can find the remote for the current branch. If this\nfails, it then checks to see if there is exactly one remote. It will use\nthis, before finally falling back to \"origin\" as the default.\n\nIf a user happens to rename their default remote from origin, either\nmanually or by setting something like clone.defaultRemoteName, this\nfallback will not work.\n\nIn such cases, the submodule logic will try to use a non-existent\nremote. This usually manifests as a failure to trigger the submodule\nupdate.\n\nThe parent project already knows and stores the submodule URL in either\n.gitmodules or its .git/config.\n\nAdd a new repo_remote_from_url() helper which will iterate over all the\nremotes in a repository and return the first remote which has a matching\nURL.\n\nRefactor get_default_remote_submodule to find the submodule and get its\nURL. If a valid URL exists, first try to obtain a remote using the new\nrepo_remote_from_url(). Fall back to the repo_default_remote()\notherwise.\n\nThe fallback logic is kept in case for some reason the user has manually\nchanged the URL within the submodule. Additionally, we still try to use\na remote rather than directly passing the URL in the\nfetch_in_submodule() logic. This ensures that an update will properly\nupdate the remote refs within the submodule as expected, rather than\njust fetching into FETCH_HEAD.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n remote.h                    |  1 +\n builtin/submodule--helper.c | 26 +++++++++++++++++++++++++-\n remote.c                    | 15 +++++++++++++++\n t/t7406-submodule-update.sh | 32 ++++++++++++++++++++++++++++++++\n 4 files changed, 73 insertions(+), 1 deletion(-)\n\ndiff --git a/remote.h b/remote.h\nindex 8dc5cfa49ef78808348a84c9b3f416b31cd3bbd7..0ca399e1835bf1829054d8937d95e5625c38d881 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -340,6 +340,7 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit);\n char *remote_ref_for_branch(struct branch *branch, int for_push);\n \n const char *repo_default_remote(struct repository *repo);\n+const char *repo_remote_from_url(struct repository *repo, const char *url);\n \n /* returns true if the given branch has merge configuration given. */\n int branch_has_merge_config(struct branch *branch);\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 1aa87435c2000e94f43da94c5ef88a307f6f3f4a..84a96d300d9489706fb16280f03aea02f87f1657 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -72,16 +72,40 @@ static char *resolve_relative_url(const char *rel_url, const char *up_path, int\n \n static int get_default_remote_submodule(const char *module_path, char **default_remote)\n {\n+\tconst struct submodule *sub;\n \tstruct repository subrepo;\n+\tconst char *remote_name = NULL;\n+\tchar *url = NULL;\n+\n+\tsub = submodule_from_path(the_repository, null_oid(the_hash_algo), module_path);\n+\tif (sub && sub->url) {\n+\t\turl = xstrdup(sub->url);\n+\n+\t\t/* Possibly a url relative to parent */\n+\t\tif (starts_with_dot_dot_slash(url) ||\n+\t\t    starts_with_dot_slash(url)) {\n+\t\t\tchar *oldurl = url;\n+\n+\t\t\turl = resolve_relative_url(oldurl, NULL, 1);\n+\t\t\tfree(oldurl);\n+\t\t}\n+\t}\n \n \tif (repo_submodule_init(&subrepo, the_repository, module_path,\n \t\t\t\tnull_oid(the_hash_algo)) < 0)\n \t\treturn die_message(_(\"could not get a repository handle for submodule '%s'\"),\n \t\t\t\t   module_path);\n \n-\t*default_remote = xstrdup(repo_default_remote(&subrepo));\n+\t/* Look up by URL first */\n+\tif (url)\n+\t\tremote_name = repo_remote_from_url(&subrepo, url);\n+\tif (!remote_name)\n+\t\tremote_name = repo_default_remote(&subrepo);\n+\n+\t*default_remote = xstrdup(remote_name);\n \n \trepo_clear(&subrepo);\n+\tfree(url);\n \n \treturn 0;\n }\ndiff --git a/remote.c b/remote.c\nindex e35cf7ec61ac946d1e84c5a0ae309e63c66d0335..60b4aec3dee3847b3a7b5be0f1acd9a52c60c71d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1801,6 +1801,21 @@ const char *repo_default_remote(struct repository *repo)\n \treturn remotes_remote_for_branch(repo->remote_state, branch, NULL);\n }\n \n+const char *repo_remote_from_url(struct repository *repo, const char *url)\n+{\n+\tread_config(repo, 0);\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}\n+\treturn NULL;\n+}\n+\n int branch_has_merge_config(struct branch *branch)\n {\n \treturn branch && branch->set_merge;\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 748b529745a5121f121768bb4e0cbc11bc833ea4..c09047b5f441a73a02a9fc4197e9a0ea8f39b529 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -1134,6 +1134,38 @@ test_expect_success 'setup clean recursive superproject' '\n \tgit clone --recurse-submodules top top-clean\n '\n \n+test_expect_success 'submodule update with multiple remotes' '\n+\ttest_when_finished \"rm -fr top-cloned\" &&\n+\tcp -r top-clean top-cloned &&\n+\n+\t# Create a commit in each repo, starting with bottom\n+\ttest_commit -C bottom multiple_remote_commit &&\n+\t# Create middle commit\n+\tgit -C middle/bottom fetch &&\n+\tgit -C middle/bottom checkout -f FETCH_HEAD &&\n+\tgit -C middle add bottom &&\n+\tgit -C middle commit -m \"multiple_remote_commit\" &&\n+\t# Create top commit\n+\tgit -C top/middle fetch &&\n+\tgit -C top/middle checkout -f FETCH_HEAD &&\n+\tgit -C top add middle &&\n+\tgit -C top commit -m \"multiple_remote_commit\" &&\n+\n+\t# rename the submodule remote\n+\tgit -C top-cloned/middle remote rename origin upstream &&\n+\n+\t# Add another remote\n+\tgit -C top-cloned/middle remote add other bogus &&\n+\n+\t# Make the update of \"middle\" a no-op, otherwise we error out\n+\t# because of its unmerged state\n+\ttest_config -C top-cloned submodule.middle.update !true &&\n+\tgit -C top-cloned submodule update --recursive 2>actual.err &&\n+\tcat >expect.err <<-\\EOF &&\n+\tEOF\n+\ttest_cmp expect.err actual.err\n+'\n+\n test_expect_success 'submodule update with renamed remote' '\n \ttest_when_finished \"rm -fr top-cloned\" &&\n \tcp -r top-clean top-cloned &&\n\n-- \n2.48.1.397.gec9d649cc640\n\n"}]}