Merge pull request #2058 from timbrel/fix-2057

Author: kaste

github/github.py

@@ -5,13 +5,15 @@
 from __future__ import annotations
 import os
 import re
-from webbrowser import open as open_in_browser
-from functools import partial
+import subprocess
+from functools import lru_cache, partial
 from typing import NamedTuple
+from webbrowser import open as open_in_browser
 
 from ..common import interwebs
 from ..core.exceptions import FailedGithubRequest
 from ..core.settings import GitSavvySettings
+from ..core.utils import STARTUPINFO
 
 
 GITHUB_PER_PAGE_MAX = 100
@@ -33,14 +35,15 @@ class GitHubRepo(NamedTuple):
     token: str | None
 
 
+@lru_cache(maxsize=128)
 def remote_to_url(remote_url: str) -> str:
     """
     Parse out a Github HTTP URL from a remote URI:
 
     r1 = remote_to_url("git://github.com/timbrel/GitSavvy.git")
     assert r1 == "https://github.com/timbrel/GitSavvy"
 
-    r2 = remote_to_url("git@github.com:divmain/GitSavvy.git")
+    r2 = remote_to_url("git@github.com:timbrel/GitSavvy.git")
     assert r2 == "https://github.com/timbrel/GitSavvy"
 
     r3 = remote_to_url("https://github.com/timbrel/GitSavvy.git")
@@ -50,14 +53,14 @@ def remote_to_url(remote_url: str) -> str:
     if remote_url.endswith(".git"):
         remote_url = remote_url[:-4]
 
-    if remote_url.startswith("git@"):
-        return remote_url.replace(":", "/").replace("git@", "https://")
-    elif remote_url.startswith("git://"):
+    if remote_url.startswith("git://"):
         return remote_url.replace("git://", "https://")
-    elif remote_url.startswith("http"):
+    if remote_url.startswith("http"):
         return remote_url
-    else:
-        raise ValueError('Cannot parse remote "{}" and transform to url'.format(remote_url))
+    if url := _transform_ssh_remote_to_https(remote_url):
+        return url
+
+    raise ValueError('Cannot parse remote "{}" and transform to url'.format(remote_url))
 
 
 def parse_remote(remote_url: str) -> GitHubRepo:
@@ -77,6 +80,51 @@ def parse_remote(remote_url: str) -> GitHubRepo:
     return GitHubRepo(url, fqdn, owner, repo, token)
 
 
+def _transform_ssh_remote_to_https(remote_url: str) -> str | None:
+    match = re.match(r"^ssh://(?:[^@/]+@)?(?P<host>[^/]+)/(?P<path>.+)$", remote_url)
+    if not match:
+        if "://" in remote_url:
+            return None
+        match = re.match(r"^(?:[^@/:]+@)?(?P<host>[^:]+):(?P<path>.+)$", remote_url)
+        if not match:
+            return None
+
+    host, path = match.group("host"), match.group("path").lstrip("/")
+    if "/" not in path:
+        return None
+
+    return "https://{host}/{path}".format(host=_resolve_ssh_hostname(host), path=path)
+
+
+@lru_cache(maxsize=128)
+def _resolve_ssh_hostname(hostname: str) -> str:
+    return _read_ssh_config_hostname(hostname) or hostname
+
+
+def _read_ssh_config_hostname(hostname: str) -> str | None:
+    try:
+        output = subprocess.check_output(
+            ["ssh", "-G", hostname],
+            stderr=subprocess.DEVNULL,
+            text=True,
+            timeout=1.0,
+            startupinfo=STARTUPINFO,
+        )
+    except (OSError, subprocess.SubprocessError):
+        return None
+
+    for line in output.splitlines():
+        parts = line.split(maxsplit=1)
+        if len(parts) != 2:
+            continue
+
+        key, value = parts
+        if key.lower() == "hostname" and value:
+            return value.strip()
+
+    return None
+
+
 def construct_github_file_url(rel_path, remote_url, commit_hash, start_line=None, end_line=None) -> str:
     """
     Open the URL corresponding to the provided `rel_path` on `remote_url`.

tests/test_github.py

@@ -0,0 +1,54 @@
+from unittesting import DeferrableTestCase
+
+from GitSavvy.github import github
+from GitSavvy.tests.mockito import unstub, when
+from GitSavvy.tests.parameterized import parameterized as p
+
+
+class TestGitHubRemoteParsing(DeferrableTestCase):
+    def tearDown(self):
+        unstub()
+        github.remote_to_url.cache_clear()
+        github._resolve_ssh_hostname.cache_clear()
+
+    @p.expand([
+        ("git://github.com/timbrel/GitSavvy.git", "https://github.com/timbrel/GitSavvy"),
+        ("https://github.com/timbrel/GitSavvy.git", "https://github.com/timbrel/GitSavvy"),
+        ("https://github.com/timbrel/GitSavvy", "https://github.com/timbrel/GitSavvy"),
+    ])
+    def test_remote_to_url_non_ssh_remotes(self, remote_url, expected):
+        self.assertEqual(expected, github.remote_to_url(remote_url))
+
+    def test_remote_to_url_resolves_host_alias_for_scp_like_ssh_remote(self):
+        when(github)._resolve_ssh_hostname("my-github").thenReturn("github.com")
+
+        actual = github.remote_to_url("git@my-github:me/my-project.git")
+
+        self.assertEqual("https://github.com/me/my-project", actual)
+
+    def test_remote_to_url_resolves_host_alias_for_ssh_scheme_remote(self):
+        when(github)._resolve_ssh_hostname("my-github").thenReturn("github.com")
+
+        actual = github.remote_to_url("ssh://git@my-github/me/my-project.git")
+
+        self.assertEqual("https://github.com/me/my-project", actual)
+
+    def test_read_ssh_config_hostname(self):
+        when(github.subprocess).check_output(
+            ["ssh", "-G", "my-github"],
+            stderr=github.subprocess.DEVNULL,
+            text=True,
+            timeout=1.0,
+            startupinfo=github.STARTUPINFO,
+        ).thenReturn("host my-github\nhostname github.com\n")
+
+        actual = github._read_ssh_config_hostname("my-github")
+
+        self.assertEqual("github.com", actual)
+
+    def test_resolve_ssh_hostname_falls_back_to_input_value(self):
+        when(github)._read_ssh_config_hostname("my-github").thenReturn(None)
+
+        actual = github._resolve_ssh_hostname("my-github")
+
+        self.assertEqual("my-github", actual)

unittesting.json

@@ -1,6 +1,5 @@
 {
     "deferred": true,
     "capture_console": true,
-    "legacy_runner": false,
-    "failfast": true
+    "legacy_runner": false
 }