mirror of
https://github.com/Comfy-Org/ComfyUI-Manager.git
synced 2026-06-20 23:09:20 +08:00
fix(git_compat): follow-ups for pygit2 fallback hardening (#2974)
- Route list_remotes() fetch through _fetch_remote so the proxy and SSH-to-HTTPS rewrite apply to every fetch entry point, and provide pull on its proxies (parity with get_remote and the GitPython proxy) via a shared _pull_remote helper - Rework _to_https_url: handle ssh://git@host:port/... URLs (drop the custom SSH port instead of mangling it into the path) and collapse leading slashes so git@host:/abs/path no longer yields a double-slash URL. Behavior narrowing: colon-less git@host/path (not a valid scp-form URL), ssh:// URLs with a port but no path, and IPv6 ssh:// URLs are now returned unchanged instead of being wrongly converted - clone_repo: omit the proxy= kwarg when no proxy is configured, so proxy-less installs keep working on pygit2 older than 1.18 - Simplify redundant except (KeyError, Exception) clause - Require pygit2>=1.18 (clone_repository proxy= parameter) - Add unit tests for _to_https_url (incl. negative cases pinning the narrowing) and a regression test that list_remotes proxies route fetch/pull through their own remote (late-binding guard)
This commit is contained in:
parent
622a7077a5
commit
8b98723b42
@ -31,16 +31,21 @@ _HTTP_PROXY = None
|
|||||||
def _to_https_url(url):
|
def _to_https_url(url):
|
||||||
"""Rewrite an SSH-form git URL to its anonymous HTTPS equivalent.
|
"""Rewrite an SSH-form git URL to its anonymous HTTPS equivalent.
|
||||||
|
|
||||||
Handles `git@host:owner/repo(.git)` and `ssh://git@host/owner/repo(.git)`.
|
Handles `git@host:owner/repo(.git)` and `ssh://git@host[:port]/owner/repo(.git)`.
|
||||||
Returns the URL unchanged if it is not SSH-form, so pygit2 clone/fetch
|
A custom SSH port is dropped — the HTTPS endpoint of a hosting service
|
||||||
of public repos never requires SSH credentials even when a repo's own
|
lives on the standard port regardless of its SSH port. Leading slashes in
|
||||||
config stores an SSH origin.
|
the path part are collapsed so `git@host:/abs/path` does not yield a
|
||||||
|
double-slash URL. Returns the URL unchanged if it is not SSH-form, so
|
||||||
|
pygit2 clone/fetch of public repos never requires SSH credentials even
|
||||||
|
when a repo's own config stores an SSH origin.
|
||||||
"""
|
"""
|
||||||
if not url:
|
if not url:
|
||||||
return url
|
return url
|
||||||
m = re.match(r"^(?:ssh://)?git@([^:/]+)[:/](.+)$", url)
|
m = re.match(r"^ssh://git@([^:/]+)(?::\d+)?/(.+)$", url)
|
||||||
|
if m is None:
|
||||||
|
m = re.match(r"^git@([^:/]+):(.+)$", url)
|
||||||
if m:
|
if m:
|
||||||
return "https://%s/%s" % (m.group(1), m.group(2))
|
return "https://%s/%s" % (m.group(1), m.group(2).lstrip("/"))
|
||||||
return url
|
return url
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@ -80,7 +85,7 @@ if USE_PYGIT2:
|
|||||||
_global_cfg = _pygit2.Config.get_global_config()
|
_global_cfg = _pygit2.Config.get_global_config()
|
||||||
try:
|
try:
|
||||||
_HTTP_PROXY = _global_cfg["http.proxy"] or None
|
_HTTP_PROXY = _global_cfg["http.proxy"] or None
|
||||||
except (KeyError, Exception):
|
except Exception:
|
||||||
_HTTP_PROXY = None
|
_HTTP_PROXY = None
|
||||||
except Exception:
|
except Exception:
|
||||||
_HTTP_PROXY = None
|
_HTTP_PROXY = None
|
||||||
@ -521,24 +526,26 @@ class _Pygit2Repo(GitRepo):
|
|||||||
name = name[len('refs/remotes/'):]
|
name = name[len('refs/remotes/'):]
|
||||||
return name.split('/')[0]
|
return name.split('/')[0]
|
||||||
|
|
||||||
|
def _pull_remote(self, remote):
|
||||||
|
"""Fetch *remote* and fast-forward the active branch onto its upstream."""
|
||||||
|
self._fetch_remote(remote)
|
||||||
|
branch_name = self.active_branch_name
|
||||||
|
branch = self._repo.branches.get(branch_name)
|
||||||
|
if branch and branch.upstream:
|
||||||
|
remote_commit = branch.upstream.peel(_pygit2.Commit)
|
||||||
|
analysis, _ = self._repo.merge_analysis(remote_commit.id)
|
||||||
|
if analysis & _pygit2.GIT_MERGE_ANALYSIS_FASTFORWARD:
|
||||||
|
self._repo.checkout_tree(self._repo.get(remote_commit.id))
|
||||||
|
branch_ref = self._repo.references.get(f'refs/heads/{branch_name}')
|
||||||
|
if branch_ref is not None:
|
||||||
|
branch_ref.set_target(remote_commit.id)
|
||||||
|
self._repo.head.set_target(remote_commit.id)
|
||||||
|
|
||||||
def get_remote(self, name):
|
def get_remote(self, name):
|
||||||
remote = self._repo.remotes[name]
|
remote = self._repo.remotes[name]
|
||||||
|
return _RemoteProxy(remote.name, remote.url,
|
||||||
def _pull():
|
lambda: self._fetch_remote(remote),
|
||||||
self._fetch_remote(remote)
|
lambda: self._pull_remote(remote))
|
||||||
branch_name = self.active_branch_name
|
|
||||||
branch = self._repo.branches.get(branch_name)
|
|
||||||
if branch and branch.upstream:
|
|
||||||
remote_commit = branch.upstream.peel(_pygit2.Commit)
|
|
||||||
analysis, _ = self._repo.merge_analysis(remote_commit.id)
|
|
||||||
if analysis & _pygit2.GIT_MERGE_ANALYSIS_FASTFORWARD:
|
|
||||||
self._repo.checkout_tree(self._repo.get(remote_commit.id))
|
|
||||||
branch_ref = self._repo.references.get(f'refs/heads/{branch_name}')
|
|
||||||
if branch_ref is not None:
|
|
||||||
branch_ref.set_target(remote_commit.id)
|
|
||||||
self._repo.head.set_target(remote_commit.id)
|
|
||||||
|
|
||||||
return _RemoteProxy(remote.name, remote.url, lambda: self._fetch_remote(remote), _pull)
|
|
||||||
|
|
||||||
def has_ref(self, ref_name):
|
def has_ref(self, ref_name):
|
||||||
for prefix in [f'refs/remotes/{ref_name}', f'refs/heads/{ref_name}',
|
for prefix in [f'refs/remotes/{ref_name}', f'refs/heads/{ref_name}',
|
||||||
@ -571,7 +578,9 @@ class _Pygit2Repo(GitRepo):
|
|||||||
def list_remotes(self):
|
def list_remotes(self):
|
||||||
result = []
|
result = []
|
||||||
for r in self._repo.remotes:
|
for r in self._repo.remotes:
|
||||||
result.append(_RemoteProxy(r.name, r.url, r.fetch))
|
result.append(_RemoteProxy(r.name, r.url,
|
||||||
|
lambda r=r: self._fetch_remote(r),
|
||||||
|
lambda r=r: self._pull_remote(r)))
|
||||||
return result
|
return result
|
||||||
|
|
||||||
def get_remote_url(self, index_or_name):
|
def get_remote_url(self, index_or_name):
|
||||||
@ -905,7 +914,13 @@ def clone_repo(url, dest, progress=None):
|
|||||||
(checkout, clear_cache, close, etc.).
|
(checkout, clear_cache, close, etc.).
|
||||||
"""
|
"""
|
||||||
if USE_PYGIT2:
|
if USE_PYGIT2:
|
||||||
_pygit2.clone_repository(_to_https_url(url), dest, proxy=_HTTP_PROXY)
|
# The proxy= kwarg requires pygit2>=1.18; omit it when no proxy is
|
||||||
|
# configured so proxy-less installs keep working on older pygit2.
|
||||||
|
# (A configured proxy still needs >=1.18 — see requirements.txt.)
|
||||||
|
if _HTTP_PROXY is not None:
|
||||||
|
_pygit2.clone_repository(_to_https_url(url), dest, proxy=_HTTP_PROXY)
|
||||||
|
else:
|
||||||
|
_pygit2.clone_repository(_to_https_url(url), dest)
|
||||||
repo = _Pygit2Repo(dest)
|
repo = _Pygit2Repo(dest)
|
||||||
repo.submodule_update()
|
repo.submodule_update()
|
||||||
return repo
|
return repo
|
||||||
|
|||||||
@ -39,7 +39,7 @@ dependencies = [
|
|||||||
]
|
]
|
||||||
|
|
||||||
[project.optional-dependencies]
|
[project.optional-dependencies]
|
||||||
dev = ["pre-commit", "pytest", "ruff", "pytest-cov", "pygit2"]
|
dev = ["pre-commit", "pytest", "ruff", "pytest-cov", "pygit2>=1.18"]
|
||||||
|
|
||||||
[project.urls]
|
[project.urls]
|
||||||
Repository = "https://github.com/ltdrdata/ComfyUI-Manager"
|
Repository = "https://github.com/ltdrdata/ComfyUI-Manager"
|
||||||
|
|||||||
@ -1,5 +1,5 @@
|
|||||||
GitPython
|
GitPython
|
||||||
pygit2
|
pygit2>=1.18 # clone_repository(proxy=...) requires 1.18.0+
|
||||||
PyGithub
|
PyGithub
|
||||||
# matrix-nio
|
# matrix-nio
|
||||||
transformers
|
transformers
|
||||||
|
|||||||
@ -824,6 +824,110 @@ finally:
|
|||||||
self.assertEqual(p2['sha'], self.first_sha)
|
self.assertEqual(p2['sha'], self.first_sha)
|
||||||
self.assertTrue(p2['detached'])
|
self.assertTrue(p2['detached'])
|
||||||
|
|
||||||
|
# === list_remotes fetch routing (pygit2) ===
|
||||||
|
|
||||||
|
def test_list_remotes_fetch_routing_pygit2(self):
|
||||||
|
"""Each list_remotes() proxy must route fetch/pull through
|
||||||
|
_fetch_remote/_pull_remote bound to its OWN remote (pins the
|
||||||
|
``lambda r=r:`` late-binding fix and the pull_fn parity)."""
|
||||||
|
snippet = """
|
||||||
|
import tempfile, shutil
|
||||||
|
dest = tempfile.mkdtemp()
|
||||||
|
try:
|
||||||
|
repo = clone_repo(REPO_PATH, os.path.join(dest, 'cloned'))
|
||||||
|
repo._repo.remotes.create('alt', REPO_PATH)
|
||||||
|
calls = []
|
||||||
|
repo._fetch_remote = lambda remote, refspecs=None: calls.append(remote.name)
|
||||||
|
proxies = repo.list_remotes()
|
||||||
|
for p in proxies:
|
||||||
|
p.fetch()
|
||||||
|
pulled = []
|
||||||
|
for p in proxies:
|
||||||
|
p.pull()
|
||||||
|
pulled.append(p.name)
|
||||||
|
repo.close()
|
||||||
|
print(json.dumps({"names": [p.name for p in proxies],
|
||||||
|
"fetch_calls": calls[:len(proxies)],
|
||||||
|
"pulled": pulled}))
|
||||||
|
finally:
|
||||||
|
shutil.rmtree(dest, ignore_errors=True)
|
||||||
|
"""
|
||||||
|
p2 = _run_snippet(snippet, self.repo_path, use_pygit2=True)
|
||||||
|
self.assertEqual(len(p2['names']), 2)
|
||||||
|
self.assertEqual(p2['fetch_calls'], p2['names'])
|
||||||
|
self.assertEqual(p2['pulled'], p2['names'])
|
||||||
|
|
||||||
|
|
||||||
|
class TestToHttpsUrl(unittest.TestCase):
|
||||||
|
"""Unit tests for the pure SSH→HTTPS URL rewrite helper."""
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def setUpClass(cls):
|
||||||
|
sys.path.insert(0, COMPAT_DIR)
|
||||||
|
from git_compat import _to_https_url
|
||||||
|
cls.convert = staticmethod(_to_https_url)
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def tearDownClass(cls):
|
||||||
|
try:
|
||||||
|
sys.path.remove(COMPAT_DIR)
|
||||||
|
except ValueError:
|
||||||
|
pass
|
||||||
|
|
||||||
|
def test_scp_form(self):
|
||||||
|
self.assertEqual(
|
||||||
|
self.convert('git@github.com:owner/repo.git'),
|
||||||
|
'https://github.com/owner/repo.git')
|
||||||
|
|
||||||
|
def test_scp_form_without_suffix(self):
|
||||||
|
self.assertEqual(
|
||||||
|
self.convert('git@github.com:owner/repo'),
|
||||||
|
'https://github.com/owner/repo')
|
||||||
|
|
||||||
|
def test_ssh_scheme(self):
|
||||||
|
self.assertEqual(
|
||||||
|
self.convert('ssh://git@github.com/owner/repo.git'),
|
||||||
|
'https://github.com/owner/repo.git')
|
||||||
|
|
||||||
|
def test_ssh_scheme_with_port(self):
|
||||||
|
self.assertEqual(
|
||||||
|
self.convert('ssh://git@git.example.com:2222/owner/repo.git'),
|
||||||
|
'https://git.example.com/owner/repo.git')
|
||||||
|
|
||||||
|
def test_https_unchanged(self):
|
||||||
|
url = 'https://github.com/owner/repo.git'
|
||||||
|
self.assertEqual(self.convert(url), url)
|
||||||
|
|
||||||
|
def test_non_git_user_unchanged(self):
|
||||||
|
url = 'ssh://alice@git.example.com/owner/repo.git'
|
||||||
|
self.assertEqual(self.convert(url), url)
|
||||||
|
|
||||||
|
def test_local_path_unchanged(self):
|
||||||
|
url = '/home/user/repos/local-repo'
|
||||||
|
self.assertEqual(self.convert(url), url)
|
||||||
|
|
||||||
|
def test_empty_and_none(self):
|
||||||
|
self.assertEqual(self.convert(''), '')
|
||||||
|
self.assertIsNone(self.convert(None))
|
||||||
|
|
||||||
|
def test_scp_absolute_path_no_double_slash(self):
|
||||||
|
self.assertEqual(
|
||||||
|
self.convert('git@git.example.com:/srv/git/repo.git'),
|
||||||
|
'https://git.example.com/srv/git/repo.git')
|
||||||
|
|
||||||
|
def test_slash_form_without_colon_unchanged(self):
|
||||||
|
# Not a valid scp-form URL (git treats it as a local path).
|
||||||
|
url = 'git@github.com/owner/repo'
|
||||||
|
self.assertEqual(self.convert(url), url)
|
||||||
|
|
||||||
|
def test_ipv6_unchanged(self):
|
||||||
|
url = 'ssh://git@[2001:db8::1]/owner/repo.git'
|
||||||
|
self.assertEqual(self.convert(url), url)
|
||||||
|
|
||||||
|
def test_port_without_path_unchanged(self):
|
||||||
|
url = 'ssh://git@git.example.com:2222'
|
||||||
|
self.assertEqual(self.convert(url), url)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == '__main__':
|
if __name__ == '__main__':
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user