Special parsing for git's not-really-urls containing scp host specifiers

[kornelski]

What git calls a URL is not strictly a URL, but a union of URLs and "scp host specifiers" (host:path). The syntax can't be treated as a regular URL, because hostname of scp hostname specifiers parses as a URL scheme.

This removes the previous workaround (transformUrl, #573), which was transforming scp host specifiers to ssh:// URLs. This wasn't compatible with npm, as it changed semantics of the URL and caused git to ignore SSH host aliases (#1796).

The previous workaround also accidentally disabled hasArchiveCapability check — the regex in hasArchiveCapability expected : as a way to detect scp host specifiers. Since scp goes over SSH, I assume the check can be generalized to check for the SSH protocol, so I've made that explicit.

To make it clear that git "URLs" are not real URLs, I've created a new type for them. I also use the new type to store a couple of precomputed properties that used to be extracted from the URL (the protocol had to be stored explicitly anyway because of the git "URL" parsing ambiguity).

[arcanis]

Do you have an example of a URL that would be incorrectly parsed?

[arcanis]

Can git URLs have a password, ie. foo:bar@example.org? This regex doesn't catch this case

[kornelski]

This is intentional. In scp host specifiers there is no password.

Git manual (man git-clone) specifies it as [user@]host.xz:/~[user]/path/to/repo.git/, although relative paths are also supported (scp man page documents the syntax as [[user@]host1:]file)

[kornelski]

The scp urls are in the test. The format is host:path or user@host:path.

git+ssh://foo:bar should be passed to git as git clone foo:bar, and not as git clone ssh://foo/bar.

[kornelski]

@arcanis Thanks for the review. Is there anything else that needs to be done?

[arcanis]

Just one question: I see you're using npmUrlToGit before each call to a Git static method, except when calling the Git constructor itself. I understand that your methods require a specific type for their arguments to highlight that they aren't actually url - shouldn't the constructor do the same?

[kornelski]

I changed static methods only because it was necessary, but kept constructor as-is to make the change smaller. But it makes sense to have them all consistently take the same type of URL, so I've made that change.

I've rebased on master to avoid a merge conflict.

[arcanis]

Thanks for the rebase 👍