Re: [PATCH 1/2] submodule: ignore trailing slash on superproject URL

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 17, 2016, 18:28 UTC
Message-ID: <xmqqh98avnhh.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kYW2qK1GKxoKy_mkVkjjqEUzkh5aPSzDEfRd6U=PYDdzw@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 14 quoted lines

>> In any case, I find it more disturbing that we somehow ended up with
>> a system where these three things are expected to behave differently:
>>
>>         A - path/to/dir
>>         B - path/to/dir/
>>         C - path/to/dir/.
>>
>> Is that something we can fix?
>
> Well A, B are the same.
> C is "obviously" different, when it comes to counting slashes for relative
> path/url purposes, in the way that there are characters after the last slash
> and just by coincidence '.' refers to the directory itself, C behaving like
> 'path/to/dir/sub' seems right to me.

It doesn't look right to me at all.  If you were contrasting

	cd path/to/dir/sub && cd ..
	cd path/to/dir/bus && cd ..

then I would understand, but why should these two

	cd path/to/dir/. && cd ..
	cd path/to/dir/sub && cd ..

behave the same?

> So how do you imagine this fix going forward?
> * Breaking existing users with /. at the end? by treating it the same as A,B
> * Do some check based on time/version of Git and cover the old data?
> * Forbid /. at the end from now on?

Where at the end-user facing level does this trailing "/." surface and how does the difference appear to them? I think that is the crucial question.

Unless there is some convincing argument why "." is not special (i.e. counter-argument to the above "bus vs sub" and ". vs sub" example), I would think "existing users with /." does not matter. If they are "relying" on the behaviour, I would think it is not because they find that behaviour intuitive, but only because they learned to live with it. IOW, treating all of A/B/C the same way would appear to them a strict bugfix, I would think.

It is totally a different matter if OUR code that consumes the output from the submodule-helper --resolve-relative" internally is confused and relies on "../. relative to path/to/dir/. is the same as ../. relative to path/to/dir/sub" for whatever reason. Without fixing that, I would not surprised if fixing things to treat A/B/C the same way would surface differences in the end-user observable behaviour in a negative way.