On 2025.08.17 00:36, Adrian Ratiu wrote:
> While testing submodule gitdir path encoding, I noticed submodule--helper
> is still using a hardcoded name-based path leading to test failures, so
> convert it to the common helper function introduced by commit ce125d431a
> ("submodule: extract path to submodule gitdir func") and used in other
> locations accross the source tree.
>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@xxxxxxxxxxxxx>
> ---
> builtin/submodule--helper.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 07a1935cbe..7243429c6f 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -3213,10 +3213,11 @@ static int add_submodule(const struct add_data *add_data)
> free(submod_gitdir_path);
> } else {
> struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf submod_gitdir = STRBUF_INIT;
>
> - submod_gitdir_path = xstrfmt(".git/modules/%s", add_data->sm_name);
> + submodule_name_to_gitdir(&submod_gitdir, the_repository, add_data->sm_name);
I believe submod_gitdir_path is now only used in the `if (...) {...}`
side corresponding to this `else` branch, so perhaps we should make it
local to that block?
> - if (is_directory(submod_gitdir_path)) {
> + if (is_directory(submod_gitdir.buf)) {
> if (!add_data->force) {
> struct strbuf msg = STRBUF_INIT;
> char *die_msg;
> @@ -3225,8 +3226,8 @@ static int add_submodule(const struct add_data *add_data)
> "locally with remote(s):\n"),
> add_data->sm_name);
>
> - append_fetch_remotes(&msg, submod_gitdir_path);
> - free(submod_gitdir_path);
> + append_fetch_remotes(&msg, submod_gitdir.buf);
> + strbuf_release(&submod_gitdir);
>
> strbuf_addf(&msg, _("If you want to reuse this local git "
> "directory instead of cloning again from\n"
> @@ -3244,7 +3245,7 @@ static int add_submodule(const struct add_data *add_data)
> "submodule '%s'\n"), add_data->sm_name);
> }
> }
> - free(submod_gitdir_path);
> + strbuf_release(&submod_gitdir);
>
> clone_data.prefix = add_data->prefix;
> clone_data.path = add_data->sm_path;
> --
> 2.50.1.679.gbf363a8fbb.dirty
>
>
