Re: [PATCH 4/6] submodule--helper: improve logic for fallback remote name
From: Jacob Keller <jacob.e.keller@intel.com>
Date: 2025-06-17 17:53:28
On 6/16/2025 7:58 PM, Lidong Yan wrote:
Jacob Keller [off-list ref] writes:quoted
From: Jacob Keller <redacted> The repo_get_default_remote() function in submodule--helper currently tries to figure out the proper remote name to use for a submodule based on a few factors. First, it tries to find the remote for the currently checked out branch. This works if the submodule is configured to checkout to a branch instead of a detached HEAD state. In the detached HEAD state, the code calls back to using "origin", on the assumption that this is the default remote name. Some users may change this, such as by setting clone.defaultRemoteName, or by changing the remote name manually within the submodule repository. As a first step to improving this situation, refactor to reuse the logic from remotes_remote_for_branch(). This function uses the remote from the branch if it has one. If it doesn't then it checks to see if there is exactly one remote. It uses this remote first before attempting to fall back to "origin". To allow using this helper function, introduce a repo_default_remote() helper to remote.c which takes a repository structure. This helper will load the remote configuration and get the "HEAD" branch. Then it will call remotes_remote_for_branch to find the default remote.Just a thought: since repo_default_remote() is only used within repo_get_default_remote(), and the two have very similar names, do you think it might be clearer to inline the former into the latter?
Probably, but I can't inline it within submodule--helper.c because it needs access to static functions in remote.c which I don't think make sense to export. I could also either move repo_get_default_remote() from submodule--helper.c into remote.c, or try to eliminate it entirely. My original thought was that it does some submodule-specific checks which I don't want to remove but which don't seem to make sense in-context with how the other remote.c helpers were implemented. Thus, it didn't make sense to move it into remote.c, but I can't put repo_default_remote() contents into submodule--helper.c without exposing the read_config() and other static functions in remote.c I'll think about this while resolving the memory issues and working on the next version.