On 01.05.16 08:02, Mike Hommey wrote:
> The CONNECT_DIAG_URL code for PROTO_GIT and PROTO_SSH were different in
> subtle ways.
Yes, and there (historical) reasons for that.
The first implementation did support IPV6 with SSH:
commit 5ba884483fe1a5f9ce1ce5e3c5e1c37c0fd296c4
[PATCH] GIT: Try all addresses for given remote name
Try all addresses for given remote name until it succeeds. Also
supports IPv6.
This lead to a regression, which was fixed here:
commit ce6f8e7ec2bbebe2472e23b684cae0a4adf325ad
Fix git protocol connection 'port' override
It was broken by the IPv6 patches - we need to remove the ":" part from
the hostname for a successful name lookup.
Later the ssh:// syntax was added:
commit c05186cc38ca4605bff1f275619d7d0faeaf2fa5
Support git+ssh:// and ssh+git:// URL
Later support for [] was added:
commit 356bece0a2725818191b12f6e991490d52baa1d1
GIT: Support [address] in URLs
Allow IPv6address/IPvFuture enclosed by [] in URLs, like:
git push '[3ffe:ffff:...:1]:GIT/git'
or
git push 'ssh://[3ffe:ffff:...:1]/GIT/git'
Later, support for a trailing ':' was added:
commit 608d48b2207a6152839a9762c7a66f217bceb440
Fix "getaddrinfo()" buglet
So when somebody passes me a "please pull" request pointing to something
like the following
git://git.kernel.org:/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git
(note the extraneous colon at the end of the host name), git would happily
try to connect to port 0, which would generally just cause the remote to
not even answer, and the "connect()" will take a long time to time out.
Then it was possible to use ssh:// with a port number:
commit 2e7766655abd0312a6bf4db6a6ff141e7e4ac8b6
URL: allow port specification in ssh:// URLs
Parsing of the port number was improved here:
commit 8f1482536ad680fcd738158e76e254a534f2e690
connect.c: stricter port validation, silence compiler warning
Then, it was possible to pass the port number to putty or plink:
commit 36ad53ffee6ed5b7c277cde660f526fd8ce3d68f
connect.c: Support PuTTY plink and TortoisePlink as SSH on Windows
This caused a regression, see below.
Later, there was a fix for [] together with a port number:
commit 7acf438215d1b0e8e47a707f3585de8486a2e5fe
git: Wrong parsing of ssh urls with IPv6 literals ignores port
A better way to distinguish "local file system" URLS from scp like URLs,
(None of them has a scheme, but they are nice to use)
commit 60003340cda05f5ecd79ee8522b21eda038b994b
clone: allow cloning local paths with colons in them
This fix lead to another regression, which was partly resolved here
(And that's where test cases came in)
commit 8d3d28f5dba94a15a79975e4adc909c295c80d80
clone: tighten "local paths with colons" check a bit
In order to allow diagnostics in the parser, the diag-url feature was added:
commit 5610b7c0c6957cf0b236b6fac087c1f4dc209376
git fetch-pack: add --diag-url
The the parser was improved:
commit 6a59974869c0a6e9399101bc02223b4c00a8aff2
git fetch: support host:/~repo
and refactored:
commit 83b058752707a6ba4af51ebc98c47913bc7d2d25
git_connect(): refactor the port handling for ssh
And more refactored:
commit c59ab2e52a64abd7fded97e0983a9b7f3d0508a0
connect.c: refactor url parsing
And more refactored:
commit a2036d7e00ad8aa16ba010a80078e10f0e4568a3
git_connect(): use common return point
Later the parser was improved:
commit 86ceb337ec340c7db9b060b90bfab05a08b8251b
connect.c: allow ssh://user@[2001:db8::1]/repo.git
commit 3f55ccab8e0fec73c8e38b909e9bb4963bfb8f6a
t5500: show user name and host in diag-url
However, one of the refactoring broke a use case,
which was valid before, leading to regressions around the world:
commit 6b6c5f7a2f66751a93afce54277a1f30ab0dc521
connect.c: ignore extra colon after hostname
Now back to the regression introduced by supporting plink/putty with
ports, it was improved by a preparing commit, followed by the fix:
commit baaf233755f71c057d28b9e8692e24d4fca7d22f
connect: improve check for plink to reduce false positives
commit 37ee646e72d7f39d61a538e21a4c2721e32cb444
connect: simplify SSH connection code path
Later IPV6/IPV4 only connections had been supported:
commit c915f11eb4922e154e29cf62d3b549d65c06a170
connect & http: support -4 and -6 switches for remote operations
I skipped the dates and names (I was responsible for one regression)
I hope this gives a half-correct overview,
why I am reluctant to change any code in connect.c
unless there is a fix for a real world problem.
And even here, the test cases should be changed first (and reviewed) in an own commit.
Marked as test_must_failure.
The c-code can be changed in the next commit, and the TC are marked as "test_expect_success"
Back to another topic:
If you want to support the native Git-support for native HG via hg-serve,
I will be happy to assist with reviews.
Please, if possible, don't touch connect.c at all.
(Beside the memory leak fix).
It may be better to start with a real remote-helper and copy the code from connect.c
And, later, if there are common code paths, refactor stuff.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html