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

[Jeff King <peff@peff.net>]

On Wed, Aug 05, 2015 at 10:34:34AM -0700, Junio C Hamano wrote:

> > 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>://...).

True, but the transport code _is_ handling that at some point. It makes
me wonder if it would be possible to push the call to transport_get
further up inside cmd_clone(), and then provide some way to query the
remote path and hostname from the transport code. Then guess_dir_name
could just go away entirely, in favor of something like:

  dir_name = transport_get_path(transport);
  if (!*dir_name)
	dir_name = transport_get_host(transport);

That may be overly simplistic or unworkable, though. I haven't dug into
the code.

> 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.

Speaking of which, has anyone tested whether the old or new code handles
external remote helpers? Certainly:

  foo::https://host/repo.git

should still use repo.git. But technically the string handed to
git-remote-foo does not have to look anything like a URL. In those cases
neither guess_dir_name nor the transport code have any idea what anything
to the right of the "::" means; we probably have to resort to blind
guessing based on characters like colon and slash.

-Peff