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

Johannes Sixt [off-list ref] writes:

On Samstag, 23. Januar 2010, Johannes Sixt wrote:

quoted

On Samstag, 23. Januar 2010, Junio C Hamano wrote:

quoted

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.

söndagen den 24 januari 2010 19.31.01 skrev  Junio C Hamano:

Johannes Sixt [off-list ref] writes:

quoted

On Samstag, 23. Januar 2010, Johannes Sixt wrote:

quoted

On Samstag, 23. Januar 2010, Junio C Hamano wrote:

quoted

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.

Separate patch posted since this problem occurs in more than one place. 

-- robin