Re: [PATCH 1/3] submodule: use absolute path for computing relative path connecting

From: Stefan Beller <sbeller@google.com>
Date: Nov 22, 2016, 00:04 UTC
Message-ID: <CAGZ79ka60_D8xfQyBegkXxkTGW5YDMuagB7kjhiCR6NriLR8+A@mail.gmail.com>
In-Reply-To: <CAGZ79kY=tzrsn7rS4UsRfSku_pKNNWNDc2OiTO-4-vg5h8NwWQ@mail.gmail.com>

On Mon, Nov 21, 2016 at 1:03 PM, Stefan Beller <sbeller@google.com> wrote:

Show 5 quoted lines

> On Mon, Nov 21, 2016 at 1:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Can the effect of this change demonstrated in a new test?  There
>> must be a scenario where the current behaviour is broken and this
>> change fixes an incorrect computation of relative path, no?

I do not think the current usage exposes this bug in connect_work_tree_and_git_dir. It is only used in builtin/mv.c, which fills the second parameter `git_dir` via a call to read_gitfile, which itself produces an absolute path.

The latest patch of this series however passes in relative path for the respective git directories.

So the commit message of this patch is misleading at least. Maybe:

  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 easier
  to use.

  While at it, change `real_work_tree` to be non const as we own
  the memory.

>
> I found the latest patch of this series broken without this patch.
> I'll try to find existing broken code, though.