From: Torsten Bögershausen <tboegi@web.de>
Date: Fri, 03 Jun 2016 05:07:18 GMT
Subject: Re: What's cooking in git.git (Jun 2016, #01; Thu, 2)
Message-ID: <57511086.40206@web.de>
In-Reply-To: <20160602231315.GA22864@glandium.org>

On 06/03/2016 01:13 AM, Mike Hommey wrote:
> On Thu, Jun 02, 2016 at 03:52:41PM -0700, Junio C Hamano wrote:
>> * mh/connect (2016-06-01) 9 commits
>>   - connect: move ssh command line preparation to a separate function
>>   - connect: actively reject git:// urls with a user part
>>   - connect: change the --diag-url output to separate user and host
>>   - connect: make parse_connect_url() return the user part of the url as a separate value
>>   - connect: group CONNECT_DIAG_URL handling code
>>   - connect: make parse_connect_url() return separated host and port
>>   - connect: re-derive a host:port string from the separate host and port variables
>>   - connect: call get_host_and_port() earlier
>>   - connect: document why we sometimes call get_port after get_host_and_port
>>
>>   Needs review.
> I /think/ Torsten reviewed it all, and his last comments are in
> $gmane/295800. It's still not clear to me why he wants to remove the
> comment about [].
There where 2 comments in the review.
The most important thing is that now
git://[example.com:123]/path/to/repo is valid, but it shouldn't.
This patch fixes it:

@@ -673,7 +669,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_user,
          * "host:port" and NULL.
          * To support this undocumented legacy we still need to split the port.
          */
-       if (!port)
+       if (!port && protocol == PROTO_SSH)


The other thing is that I asked for a test case for
git://[example.com:123]/path/to/repo
which shouldn't be hard to do.

If nobody else things that this comment in the code is stale:
-       /*
-        * Don't do destructive transforms as protocol code does
-        * '[]' unwrapping in get_host_and_port()
-        */

then just leave it as it is.
Thanks