Skip to content

Commit 6f9622f

Browse files
eshjordanLUCI
authored andcommitted
sync: Remove dependency on ssh if not needed
When running on a machine without the `ssh` command, repo sync would fail even if no ssh or ssh proxy was required. Use exception handling inside ssh.ProxyManager to more gracefully handle the case where ssh is not installed. Bug: 467714011 Change-Id: I602a0819638ead4d02de88b750839bc3d70549ce Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/535141 Reviewed-by: Mike Frysinger <vapier@google.com> Reviewed-by: Gavin Mak <gavinmak@google.com> Tested-by: Jordan Esh <esh.jordan@gmail.com> Commit-Queue: Jordan Esh <esh.jordan@gmail.com>
1 parent 5cb0251 commit 6f9622f

2 files changed

Lines changed: 33 additions & 11 deletions

File tree

ssh.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ def _parse_ssh_version(ver_str=None):
5252

5353
@functools.lru_cache(maxsize=None)
5454
def version():
55-
"""return ssh version as a tuple"""
55+
"""Return ssh version as a tuple.
56+
57+
If ssh is not available, a FileNotFoundError will be raised.
58+
"""
5659
try:
5760
return _parse_ssh_version()
58-
except FileNotFoundError:
59-
print("fatal: ssh not installed", file=sys.stderr)
60-
sys.exit(1)
6161
except subprocess.CalledProcessError as e:
6262
print(
6363
"fatal: unable to detect ssh version"
@@ -102,9 +102,18 @@ def __init__(self, manager):
102102
self._clients = manager.list()
103103
# Path to directory for holding master sockets.
104104
self._sock_path = None
105+
# See if ssh is usable.
106+
self._ssh_installed = False
105107

106108
def __enter__(self):
107109
"""Enter a new context."""
110+
# Check which version of ssh is available.
111+
try:
112+
version()
113+
self._ssh_installed = True
114+
except FileNotFoundError:
115+
self._ssh_installed = False
116+
108117
return self
109118

110119
def __exit__(self, exc_type, exc_value, traceback):
@@ -282,6 +291,9 @@ def _open(self, host, port=None):
282291

283292
def preconnect(self, url):
284293
"""If |uri| will create a ssh connection, setup the ssh master for it.""" # noqa: E501
294+
if not self._ssh_installed:
295+
return False
296+
285297
m = URI_ALL.match(url)
286298
if m:
287299
scheme = m.group(1)
@@ -306,6 +318,9 @@ def sock(self, create=True):
306318
307319
This has all the master sockets so clients can talk to them.
308320
"""
321+
if not self._ssh_installed:
322+
return None
323+
309324
if self._sock_path is None:
310325
if not create:
311326
return None

tests/test_ssh.py

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
class SshTests(unittest.TestCase):
2626
"""Tests the ssh functions."""
2727

28+
def setUp(self) -> None:
29+
super().setUp()
30+
ssh.version.cache_clear()
31+
2832
def test_parse_ssh_version(self):
2933
"""Check _parse_ssh_version() handling."""
3034
ver = ssh._parse_ssh_version("Unknown\n")
@@ -56,11 +60,12 @@ def test_context_manager_empty(self):
5660
def test_context_manager_child_cleanup(self):
5761
"""Verify orphaned clients & masters get cleaned up."""
5862
with multiprocessing.Manager() as manager:
59-
with ssh.ProxyManager(manager) as ssh_proxy:
60-
client = subprocess.Popen(["sleep", "964853320"])
61-
ssh_proxy.add_client(client)
62-
master = subprocess.Popen(["sleep", "964853321"])
63-
ssh_proxy.add_master(master)
63+
with mock.patch("ssh.version", return_value=(1, 2)):
64+
with ssh.ProxyManager(manager) as ssh_proxy:
65+
client = subprocess.Popen(["sleep", "964853320"])
66+
ssh_proxy.add_client(client)
67+
master = subprocess.Popen(["sleep", "964853321"])
68+
ssh_proxy.add_master(master)
6469
# If the process still exists, these will throw timeout errors.
6570
client.wait(0)
6671
master.wait(0)
@@ -72,9 +77,11 @@ def test_ssh_sock(self):
7277
with mock.patch("tempfile.mkdtemp", return_value="/tmp/foo"):
7378
# Old ssh version uses port.
7479
with mock.patch("ssh.version", return_value=(6, 6)):
75-
self.assertTrue(proxy.sock().endswith("%p"))
80+
with proxy as ssh_proxy:
81+
self.assertTrue(ssh_proxy.sock().endswith("%p"))
7682

7783
proxy._sock_path = None
7884
# New ssh version uses hash.
7985
with mock.patch("ssh.version", return_value=(6, 7)):
80-
self.assertTrue(proxy.sock().endswith("%C"))
86+
with proxy as ssh_proxy:
87+
self.assertTrue(ssh_proxy.sock().endswith("%C"))

0 commit comments

Comments
 (0)