Re: [PATCH 4/6] submodule--helper: improve logic for fallback remote name
- From
Jacob Keller <jacob.e.keller@intel.com>
- Date
- Jun 17, 2025, 17:53 UTC
- Message-ID
- <974293a0-6f5f-4077-9977-84c07fe431ca@intel.com>
- In-Reply-To
- <90B8957C-9469-4D46-9026-46407AD2EC1B@gmail.com>
On 6/16/2025 7:58 PM, Lidong Yan wrote:
Show 32 quoted lines
> Jacob Keller <jacob.e.keller@intel.com> writes： >> >> From: Jacob Keller <jacob.keller@gmail.com> >> >> 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.