Web lists-archives.com

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




On Mon, Apr 17, 2017 at 7:03 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Jacob Keller <jacob.e.keller@xxxxxxxxx> writes:
>
>> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>>
>> 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@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxx>

yes, it's the same broader issue.

>
> 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.

ok, that would have been my understanding as well.

That nearby thread however highlights how it may be a user breaking if
we just fix the paths. The patch here doesn't cd to the root of the repo,
but constructs the correct path, which is correct. It may be considered
clumsy if we decide that cd'ing to the root of the repo is the correct bug fix
for the underlying issue.

Thanks,
Stefan