Jacob Keller <jacob.e.keller@xxxxxxxxx> writes：
>
> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>
> 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?
> This method allows re-using the same existing logic as other flows,
> rather than duplicating it in submodule--helper.c.
>
> This isn't a perfect solution for users who change remote names, but it
> should help in cases where the remote name is changed but users haven't
> added any additional remotes.
>
> Signed-off-by: Jacob Keller <jacob.keller@xxxxxxxxx>
> ---
> remote.h | 2 ++
> builtin/submodule--helper.c | 18 +++---------------
> remote.c | 25 ++++++++++++++++++++-----
> t/t7406-submodule-update.sh | 29 +++++++++++++++++++++++++++++
> 4 files changed, 54 insertions(+), 20 deletions(-)
>
> diff --git a/remote.h b/remote.h
> index 7e4943ae3a70ecefa3332d211084762ca30b59b6..ef0de4aa64e9ccd32cc2eea076c00386dcba1161 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -338,6 +338,8 @@ const char *remote_for_branch(struct branch *branch, int *explicit);
> const char *pushremote_for_branch(struct branch *branch, int *explicit);
> char *remote_ref_for_branch(struct branch *branch, int for_push);
>
> +const char *repo_default_remote(struct repository *repo);
> +
> /* returns true if the given branch has merge configuration given. */
> int branch_has_merge_config(struct branch *branch);
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 9e8cdfe1b2a8c2985d9c1b8ad6f1b0d1f9401714..ef3ff65a80f398c5ac35660288290ad92c7132c7 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -43,8 +43,6 @@ typedef void (*each_submodule_fn)(const struct cache_entry *list_item,
>
> static int repo_get_default_remote(struct repository *repo, char **default_remote)
> {
> - char *dest = NULL;
> - struct strbuf sb = STRBUF_INIT;
> struct ref_store *store = get_main_ref_store(repo);
> const char *refname = refs_resolve_ref_unsafe(store, "HEAD", 0, NULL,
> NULL);
> @@ -52,23 +50,13 @@ static int repo_get_default_remote(struct repository *repo, char **default_remot
> if (!refname)
> return die_message(_("No such ref: %s"), "HEAD");
>
> - /* detached HEAD */
> - if (!strcmp(refname, "HEAD")) {
> - *default_remote = xstrdup("origin");
> - return 0;
> - }
> -
> - if (!skip_prefix(refname, "refs/heads/", &refname))
> + if (strcmp(refname, "HEAD") &&
> + !skip_prefix(refname, "refs/heads/", &refname))
> return die_message(_("Expecting a full ref name, got %s"),
> refname);
>
> - strbuf_addf(&sb, "branch.%s.remote", refname);
> - if (repo_config_get_string(repo, sb.buf, &dest))
> - *default_remote = xstrdup("origin");
> - else
> - *default_remote = dest;
> + *default_remote = xstrdup(repo_default_remote(repo));
>
> - strbuf_release(&sb);
> return 0;
> }
>
> diff --git a/remote.c b/remote.c
> index 1035f0cd32d034cce05bd2a3d829ec90795ff4e2..fcda185ecfab5102afbe8918fed65c74971ef8c2 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1759,20 +1759,35 @@ static void set_merge(struct repository *repo, struct branch *ret)
> }
> }
>
> -struct branch *branch_get(const char *name)
> +static struct branch *repo_branch_get(struct repository *repo, const char *name)
> {
> struct branch *ret;
>
> - read_config(the_repository, 0);
> + read_config(repo, 0);
> if (!name || !*name || !strcmp(name, "HEAD"))
> - ret = the_repository->remote_state->current_branch;
> + ret = repo->remote_state->current_branch;
> else
> - ret = make_branch(the_repository->remote_state, name,
> + ret = make_branch(repo->remote_state, name,
> strlen(name));
> - set_merge(the_repository, ret);
> + set_merge(repo, ret);
> return ret;
> }
>
> +struct branch *branch_get(const char *name)
> +{
> + return repo_branch_get(the_repository, name);
> +}
> +
> +const char *repo_default_remote(struct repository *repo)
> +{
> + struct branch *branch;
> +
> + read_config(repo, 0);
> + branch = repo_branch_get(repo, "HEAD");
> +
> + return remotes_remote_for_branch(repo->remote_state, branch, NULL);
> +}
> +
> int branch_has_merge_config(struct branch *branch)
> {
> return branch && !!branch->merge;
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index c562bad042ab2d4d0f82cb8b57a1eadbe24044d1..748b529745a5121f121768bb4e0cbc11bc833ea4 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -1134,6 +1134,35 @@ test_expect_success 'setup clean recursive superproject' '
> git clone --recurse-submodules top top-clean
> '
>
> +test_expect_success 'submodule update with renamed remote' '
> + test_when_finished "rm -fr top-cloned" &&
> + cp -r top-clean top-cloned &&
> +
> + # Create a commit in each repo, starting with bottom
> + test_commit -C bottom rename_commit &&
> + # Create middle commit
> + git -C middle/bottom fetch &&
> + git -C middle/bottom checkout -f FETCH_HEAD &&
> + git -C middle add bottom &&
> + git -C middle commit -m "rename_commit" &&
> + # Create top commit
> + git -C top/middle fetch &&
> + git -C top/middle checkout -f FETCH_HEAD &&
> + git -C top add middle &&
> + git -C top commit -m "rename_commit" &&
> +
> + # rename the submodule remote
> + git -C top-cloned/middle remote rename origin upstream &&
> +
> + # Make the update of "middle" a no-op, otherwise we error out
> + # because of its unmerged state
> + test_config -C top-cloned submodule.middle.update !true &&
> + git -C top-cloned submodule update --recursive 2>actual.err &&
> + cat >expect.err <<-\EOF &&
> + EOF
> + test_cmp expect.err actual.err
> +'
> +
> test_expect_success 'submodule update should skip unmerged submodules' '
> test_when_finished "rm -fr top-cloned" &&
> cp -r top-clean top-cloned &&
>
> --
> 2.48.1.397.gec9d649cc640
>
>
