Re: [PATCHv6 1/7] submodule: use absolute path for computing relative path connecting

From: Duy Nguyen <pclouds@gmail.com>
Date: Dec 8, 2016, 09:45 UTC
Message-ID: <CACsJy8ChzxZCygrh6eNjqCkwv+Z43FmJh2T_6u6_qNE9Byq+sQ@mail.gmail.com>
In-Reply-To: <20161208014623.7588-2-sbeller@google.com>

On Thu, Dec 8, 2016 at 8:46 AM, Stefan Beller <sbeller@google.com> wrote:

Show 26 quoted lines

> The current caller of connect_work_tree_and_git_dir passes
> an absolute path for the `git_dir` parameter. In the future patch
> we will also pass in relative path for `git_dir`. Extend the functionality
> of connect_work_tree_and_git_dir to take relative paths for parameters.
>
> We could work around this in the future patch by computing the absolute
> path for the git_dir in the calling site, however accepting relative
> paths for either parameter makes the API for this function much harder
> to misuse.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  submodule.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 6f7d883de9..66c5ce5a24 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1227,23 +1227,25 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
>  {
>         struct strbuf file_name = STRBUF_INIT;
>         struct strbuf rel_path = STRBUF_INIT;
> -       const char *real_work_tree = xstrdup(real_path(work_tree));
> +       char *real_git_dir = xstrdup(real_path(git_dir));

Is it a bad idea to rename the argument git_dir to git_dir_input, or something, then have a new "git_dir" variable here? It certainly helps reduce diff size, but not sure if the end result looks better or worse.

Show 28 quoted lines

> +       char *real_work_tree = xstrdup(real_path(work_tree));
>
>         /* Update gitfile */
>         strbuf_addf(&file_name, "%s/.git", work_tree);
>         write_file(file_name.buf, "gitdir: %s",
> -                  relative_path(git_dir, real_work_tree, &rel_path));
> +                  relative_path(real_git_dir, real_work_tree, &rel_path));
>
>         /* Update core.worktree setting */
>         strbuf_reset(&file_name);
> -       strbuf_addf(&file_name, "%s/config", git_dir);
> +       strbuf_addf(&file_name, "%s/config", real_git_dir);
>         git_config_set_in_file(file_name.buf, "core.worktree",
> -                              relative_path(real_work_tree, git_dir,
> +                              relative_path(real_work_tree, real_git_dir,
>                                              &rel_path));
>
>         strbuf_release(&file_name);
>         strbuf_release(&rel_path);
> -       free((void *)real_work_tree);
> +       free(real_work_tree);
> +       free(real_git_dir);
>  }
>
>  int parallel_submodules(void)
> --
> 2.11.0.rc2.30.gc512cbd.dirty
>

-- 
Duy