Re: [PATCH v3 0/6] fix repo name when cloning a server's root

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

>  - The naive way of just adding '@' as path separator would break
>    cloning repositories like '/foo/bar@baz.git' (which would
>    currently become 'bar@baz' but would become 'baz' only).
>
>  - Skipping the scheme initially is required because without it we
>    wouldn't be able to scan until the next dir separator in the
>    host part when stripping authentication information.
>
>  - First checking for '/' in the current stripped URI when we
>    want to remove the port is required because we do not want to
>    strip port numbers when cloning from something like
>    '/foo/bar:2222.git' (which would currently become '2222' but
>    would then be stripped of the ':2222' part and instead become
>    'bar'). Still, this breaks on cloning a bare repository in the
>    current dir (e.g. cloning 'bar:2222.git', which should become
>    '2222' because it is not a port number but would become
>    'bar').

This is a very good write-up.

Please make sure all of the above appears in the commit log message
somewhere.

> As you can see, there is a lot of complexity in there and I'm not
> convinced this is better than just exposing
> 'parse_connect_url()', which already handles everything for us.

If the function "handles everything for us", that's fine, but the
primary reason I am hesitant is because parse_connect_url() was
designed specifically not to have to worry about some protocols
(e.g. I think feeding it a "http://" would fail, and more
importantly, its current callers want such a call to fail).  Also it
is meant to handle some non-protocols (e.g. scp style host:path that
does not follow <scheme>://...).

Also does it handle the "2222" case above?  I do not think
parse_connect_url() even calls get_host_and_port() to be able to
tell what "2222" means in these examples.

> Maybe I'm just being blind for the obvious solution here, though.
>
> Patrick Steinhardt (6):
>   tests: fix broken && chains in t1509-root-worktree
>   tests: fix cleanup after tests in t1509-root-worktree
>   clone: do not include authentication data in guessed dir
>   clone: do not use port number as dir name
>   clone: abort if no dir name could be guessed
>   clone: add tests for cloning with empty path
>
>  builtin/clone.c          | 67 ++++++++++++++++++++++++++++++++++++++++--------
>  t/t1509-root-worktree.sh | 51 +++++++++++++++++++++++++++++++++---
>  2 files changed, 103 insertions(+), 15 deletions(-)