Re: [PATCH] Handle double slashes in make_relative_path()

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 24, 2010, 18:31 UTC
Message-ID: <7v8wbn8ie2.fsf@alter.siamese.dyndns.org>
In-Reply-To: <201001241744.57139.j6t@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 18 quoted lines

> On Samstag, 23. Januar 2010, Johannes Sixt wrote:
>> On Samstag, 23. Januar 2010, Junio C Hamano wrote:
>> > What happens if you did this?
>> >
>> > 	git --git-dir=//git/repo/repo.git --work-tree=/git/repo
>> >
>> > where "//git/repo" is on the "git server" and you are working in local
>> > hierarchy "/git/repo"?
>>
>> Ah, right, this would not do the right thing. (But I can't verify this
>> claim right now.)
>
> I tested it, and it does the right thing. The reason is that before 
> setup_work_tree() calls make_relative_path(), the --work-tree argument has 
> been processed by make_absolute_path(), which adds the drive prefix.
>
> As long as setup_work_tree() remains the only caller of make_relative_path(), 
> we are safe.

Thanks; I think a more correct description of your findings is:

 - msysgit's make_absolute_path() does the right thing (i.e. adds "drive
   prefix" to "git/repo" given to --work-tree); and

 - as long as the callers feed what the platform considers absolute paths
   in abs and base, make_relative_path() does the right thing.

So I think we are Ok. We _might_ want to add Windows-only test at the beginning of make_relative_path() to make sure that the both inputs have double-slashes at the beginning to catch future broken callers, but I think that is a separate topic, and we don't have to do that as long as setup_work_tree(0 remains the only caller, as you said.