| Message ID | 623b0b8a-a5b3-408c-b924-9f88d9763b0e@app.fastmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use correct default remote for fetching in submodule. | expand |
On Mon, Feb 13, 2023 at 8:31 AM Mara <mara@marabos.nl> wrote: > "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... > 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.
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);
"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(-) -- 2.39.0