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

From: Jeff King <peff@peff.net>
Date: Aug 5, 2015, 21:19 UTC
Message-ID: <20150805211947.GC21134@sigill.intra.peff.net>
In-Reply-To: <xmqq4mkd4p1x.fsf@gitster.dls.corp.google.com>

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

Show 11 quoted lines

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