threads / patch / 59234

patchUse correct default remote for fetching in submodule.

Subject: [PATCH] Use correct default remote for fetching in submodule.

## tl;dr

2 messages between Feb 13, 2023 and Feb 13, 2023. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Mara· Feb 13, 2023, 13:14 UTC · lore

"git submodule update" first tries a regular "git fetch" to fetch the commit, but when that doesn't retrieve the commit it wants, it tries "git fetch <remote> <commit>". For <remote>, it used the wrong default remote: the default remote of the outer repository, rather than the default remote of the submodule.

Signed-off-by: Mara Bos <mara@marabos.nl>
---
 builtin/submodule--helper.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Show changes to builtin/submodule--helper.c +4 −2
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4c173d8b37..50b96e0b9d 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2225,7 +2225,10 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,
 		strvec_pushf(&cp.args, "--depth=%d", depth);
 	if (oid) {
 		char *hex = oid_to_hex(oid);
-		char *remote = get_default_remote();
+		char *remote;
+		int code = get_default_remote_submodule(module_path, &remote);
+		if (code)
+			return code;

 		strvec_pushl(&cp.args, remote, hex, NULL);
 		free(remote);
--
2.39.0
Eric Sunshine· Feb 13, 2023, 13:45 UTC · re: Mara · lore

Re: [PATCH] Use correct default remote for fetching in submodule.

On Mon, Feb 13, 2023 at 8:31 AM Mara <mara@marabos.nl> wrote:
Show 9 quoted lines
> "git submodule update" first tries a regular "git fetch"
> to fetch the commit, but when that doesn't retrieve the commit
> it wants, it tries "git fetch <remote> <commit>".
> For <remote>, it used the wrong default remote: the default
> remote of the outer repository, rather than the default remote
> of the submodule.
>
> Signed-off-by: Mara Bos <mara@marabos.nl>
> ---

I'm not a submodule user and I don't have any particular familiarity with this code, so I may be wrong, but...

Show 12 quoted lines
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 4c173d8b37..50b96e0b9d 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2225,7 +2225,10 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet,
> -               char *remote = get_default_remote();
> +               char *remote;
> +               int code = get_default_remote_submodule(module_path, &remote);
> +               if (code)
> +                       return code;
>                 strvec_pushl(&cp.args, remote, hex, NULL);
>                 free(remote);

... it looks like this change may introduce a memory leak. Digging down through get_default_remote_submodule() and the functions it calls, it appears that repo_get_default_remote() can return a non-zero code _after_ it has allocated memory for `remote`. If I'm reading this correctly, then the above should probably be:

    char *remote = NULL;
    int code = get_default_remote_submodule(module_path, &remote);
    if (code) {
        free(remote);
        return code;
    }

Also, if possible, add a new test, perhaps to t/t7406-submodule-update.sh, demonstrating that this change fixes the problem and to ensure that it doesn't get broken again. If you have a minimal-reproduction recipe which exhibits the problem, then you may be able to turn it into an actual test.

← back to recent threads