Re: [PATCHv3] submodule--helper: normalize funny urls

From: Stefan Beller <sbeller@google.com>
Date: Oct 20, 2016, 19:34 UTC
Message-ID: <CAGZ79kZyCXKnt9kTbnD8KrnsjJVb_7juW2epmdP9XqcRGfg=GA@mail.gmail.com>
In-Reply-To: <xmqqzilyg6tk.fsf@gitster.mtv.corp.google.com>

On Thu, Oct 20, 2016 at 12:26 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> Do we actually want to fix git-clone as well?
>
> If I understand correctly, the point of this fix is to make it not
> to matter whether the original URL the end user gives or recorded as
> the remote by "git clone" in the repository is any one of:
>
>         $any_leading_part/path/to/dir
>         $any_leading_part/path/to/dir/
>         $any_leading_part/path/to/dir/.
>
> So I do not think there is anything to "fix", as long as "git clone"

Yes "git clone" works with any of the three above.

My thought was to fix it nevertheless, such that the url recorded as remote.origin.url is always the first case (no l or /. at the end).

If we were to add this fix to clone, then it may be easier to debug submodule url schemes for users as the submodule url would then be a concatenation of remote.origin.url and the relative part.

That seems easier to understand than ${remote.origin.url%%/.} + relative path, maybe? (Because then the user doesn't need to guess or remember historical behavior that is wrong on how this)

Show 10 quoted lines

> that is given any one of the above three records any one of the
> above three as the result.  It _may_ be desirable if the result is
> identical what was given as input, but I do not offhand think that
> is required.
>
>> I tried and then I see breakage in 5603-clone-dirname
>> as ssh://host seems to be an invalid url; it has to end with a slash?
>
> That is a separate issue, isn't it?  We shouldn't be touching the
> leading "<scheme>://<host>/" part, I would think.

I agree, So I'll first fix the submodule parts only.

>
> For example, a URL "../another" relative to "ssh://host/path" may be
> "ssh://host/another", but shouldn't it be an error to take
> "../../outside" relative to "ssh://host/path"?

That is correct. I'll stop looking at clone code.