On 19/05/2021 11:49, Atharva Raykar wrote: > If I understood you correctly, you'd prefer that the updating of the > submodule should be independent of the ref that is checked out in the > submodule's directory. > > While I am not sure of the reason why the design of 'update > --remote' uses the remote-tracking branch of the submodule, I can > imagine adding a switch like 'submodule.<name>.remote' that defaults > to 'origin'. Then the behaviour could be changed such that it always > pulls from the remote specified in that option. > > This would help make the behaviour consistent in all the cases you > mentioned, while also giving the option for a user to update the > submodule from the remote of their choice (which may not be origin). I like that solution. Although, I should note that if the user has set submodule.<name>.remote to something other than 'origin', they will need to ensure that submodule.<name>.branch is also set, or they will still hit the "Unable to find current <remote>/HEAD revision in submodule" error that I initially stumbled on. How about an implementation like the following? I introduced a new "git submodule--helper" command rather than modify "print-default-remote" for a couple of reasons: 1) "print-default-remote" is also used for "git submodule sync" (I'm not sure if we should change its behaviour too) 2) "print-default-remote" needs to be executed from within the submodule, and takes no arguments, whereas I need to parse the superproject's .git/config so need to be executed from the superproject and take the submodule path as an argument The two functions I added are heavily based on "git submodule--helper remote-branch". However: * Unlike with the branch name, I don't fall back to using the name for the remote cached from when we read the .gitmodules file, if it isn't found in .git/config. It doesn't make sense to me for the .gitmodules file to include this information, as any new clones will only contain "origin" remotes anyway. * I removed "struct strbuf sb" since I don't think it's used. Ben diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 9d505a6329..25ce3c8a1d 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2444,6 +2444,41 @@ static int resolve_remote_submodule_branch(int argc, const char **argv, return 0; } +static const char *remote_submodule_remote(const char *path) +{ + const struct submodule *sub; + const char *remote = NULL; + char *key; + + sub = submodule_from_path(the_repository, &null_oid, path); + if (!sub) + return NULL; + + key = xstrfmt("submodule.%s.remote", sub->name); + repo_config_get_string_tmp(the_repository, key, &remote); + free(key); + + if (!remote) + return "origin"; + + return remote; +} + +static int resolve_remote_submodule_remote(int argc, const char **argv, + const char *prefix) +{ + const char *ret; + if (argc != 2) + die("submodule--helper remote-remote takes exactly one arguments, got %d", argc); + + ret = remote_submodule_remote(argv[1]); + if (!ret) + die("submodule %s doesn't exist", argv[1]); + + printf("%s", ret); + return 0; +} + static int push_check(int argc, const char **argv, const char *prefix) { struct remote *remote; @@ -2770,6 +2805,7 @@ static struct cmd_struct commands[] = { {"deinit", module_deinit, 0}, {"summary", module_summary, SUPPORT_SUPER_PREFIX}, {"remote-branch", resolve_remote_submodule_branch, 0}, + {"remote-remote", resolve_remote_submodule_remote, 0}, {"push-check", push_check, 0}, {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX}, {"is-active", is_active, 0}, diff --git a/git-submodule.sh b/git-submodule.sh index eb90f18229..4d0df1cf5a 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -577,7 +577,7 @@ cmd_update() fetch_in_submodule "$sm_path" $depth || die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" fi - remote_name=$(sanitize_submodule_env; cd "$sm_path" && git submodule--helper print-default-remote) + remote_name=$(git submodule--helper remote-remote "$sm_path") sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify "${remote_name}/${branch}") || die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"