Re: [PATCH v2 2/2] ls-files: fix path used when recursing into submodules

[Jacob Keller <jacob.keller@gmail.com>]

On Mon, Apr 17, 2017 at 7:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Don't assume that the current working directory is the root of the
>> repository. Correctly generate the path for the recursing child
>> processes by building it from the work_tree() root instead. Otherwise if
>> we run ls-files using --git-dir or --work-tree it will not work
>> correctly as it attempts to change directory into a potentially invalid
>> location. Best case, it doesn't exist and we produce an error. Worst
>> case we cd into the wrong location and unknown behavior occurs.
>>
>> Add a new test which highlights this possibility.
>>
>> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
>> ---
>> I'm not sure that I'm convinced by this method of solving the problem as
>> I suspect it has some corner cases (what about when run inside a
>> subdirectory? It seems to work for me but I'm not sure...) Additionally,
>> it felt weird that there's no helper function for creating a toplevel
>> relative path.
>
> Is this a similar issue as discussed in a nearby thread e.g.
>
>   <CACsJy8CLBY22j3EjR4PW3n+K6PWUzb-HCgxTVeCGpwtApZF-6g@mail.gmail.com>
>
> I do think it is a bug that sometimes we do not go to the root of
> the working tree when we know the repository is not a bare one.

Yes and no. I think that this would be a problem in both a bare and
non-bare repo ? I think the correct thing to do here is really to do
what we proposed, and properly lookup the full file name.

I do think it makes the most sense conceptually to always cd into the
top level directory of a non-bare repo though.

Thanks,
Jake