changeset 35975:00b9e26d727b

sshpeer: establish SSH connection before class instantiation We want to move the handshake to before peers are created so we can instantiate a different peer class depending on the results of the handshake. This necessitates moving the SSH process invocation to outside the peer class. As part of the code move, some variables were renamed for clarity. util.popen4() returns stdin, stdout, and stderr in their typical file descriptor order. However, stdin and stdout were being mapped to "pipeo" and "pipei" respectively. "o" for "stdin" and "i" for "stdout" is a bit confusing. Although it does make sense for "output" and "input" from the perspective of the client. But in the context of the new function, it makes sense to refer to these as their file descriptor names. In addition, the last use of self._path disappeared, so we stop setting that attribute and we can delete the redundant URL parsing necessary to set it. Differential Revision: https://phab.mercurial-scm.org/D2031
author Gregory Szorc <gregory.szorc@gmail.com>
date Mon, 05 Feb 2018 14:05:59 -0800
parents 94ba29934f00
children f8f034344b39
files mercurial/sshpeer.py tests/test-check-interfaces.py
diffstat 2 files changed, 36 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/sshpeer.py	Sun Feb 04 11:40:13 2018 -0800
+++ b/mercurial/sshpeer.py	Mon Feb 05 14:05:59 2018 -0800
@@ -131,16 +131,40 @@
 
         pipee.close()
 
+def _makeconnection(ui, sshcmd, args, remotecmd, path, sshenv=None):
+    """Create an SSH connection to a server.
+
+    Returns a tuple of (process, stdin, stdout, stderr) for the
+    spawned process.
+    """
+    cmd = '%s %s %s' % (
+        sshcmd,
+        args,
+        util.shellquote('%s -R %s serve --stdio' % (
+            _serverquote(remotecmd), _serverquote(path))))
+
+    ui.debug('running %s\n' % cmd)
+    cmd = util.quotecommand(cmd)
+
+    # no buffer allow the use of 'select'
+    # feel free to remove buffering and select usage when we ultimately
+    # move to threading.
+    stdin, stdout, stderr, proc = util.popen4(cmd, bufsize=0, env=sshenv)
+
+    stdout = doublepipe(ui, util.bufferedinputpipe(stdout), stderr)
+    stdin = doublepipe(ui, stdin, stderr)
+
+    return proc, stdin, stdout, stderr
+
 class sshpeer(wireproto.wirepeer):
     def __init__(self, ui, path, create=False, sshstate=None):
         self._url = path
         self._ui = ui
-        self._pipeo = self._pipei = self._pipee = None
+        # self._subprocess is unused. Keeping a handle on the process
+        # holds a reference and prevents it from being garbage collected.
+        self._subprocess, self._pipei, self._pipeo, self._pipee = sshstate
 
-        u = util.url(path, parsequery=False, parsefragment=False)
-        self._path = u.path or '.'
-
-        self._validaterepo(*sshstate)
+        self._validaterepo()
 
     # Begin of _basepeer interface.
 
@@ -172,28 +196,7 @@
 
     # End of _basewirecommands interface.
 
-    def _validaterepo(self, sshcmd, args, remotecmd, sshenv=None):
-        assert self._pipei is None
-
-        cmd = '%s %s %s' % (sshcmd, args,
-            util.shellquote("%s -R %s serve --stdio" %
-                (_serverquote(remotecmd), _serverquote(self._path))))
-        self.ui.debug('running %s\n' % cmd)
-        cmd = util.quotecommand(cmd)
-
-        # while self._subprocess isn't used, having it allows the subprocess to
-        # to clean up correctly later
-        #
-        # no buffer allow the use of 'select'
-        # feel free to remove buffering and select usage when we ultimately
-        # move to threading.
-        sub = util.popen4(cmd, bufsize=0, env=sshenv)
-        self._pipeo, self._pipei, self._pipee, self._subprocess = sub
-
-        self._pipei = util.bufferedinputpipe(self._pipei)
-        self._pipei = doublepipe(self.ui, self._pipei, self._pipee)
-        self._pipeo = doublepipe(self.ui, self._pipeo, self._pipee)
-
+    def _validaterepo(self):
         def badresponse():
             msg = _("no suitable response from remote hg")
             hint = self.ui.config("ui", "ssherrorhint")
@@ -380,6 +383,9 @@
         if res != 0:
             raise error.RepoError(_('could not create remote repo'))
 
-    sshstate = (sshcmd, args, remotecmd, sshenv)
+    proc, stdin, stdout, stderr = _makeconnection(ui, sshcmd, args, remotecmd,
+                                                  remotepath, sshenv)
+
+    sshstate = (proc, stdout, stdin, stderr)
 
     return sshpeer(ui, path, create=create, sshstate=sshstate)
--- a/tests/test-check-interfaces.py	Sun Feb 04 11:40:13 2018 -0800
+++ b/tests/test-check-interfaces.py	Mon Feb 05 14:05:59 2018 -0800
@@ -69,7 +69,8 @@
     checkobject(badpeer())
     checkobject(httppeer.httppeer(ui, 'http://localhost'))
     checkobject(localrepo.localpeer(dummyrepo()))
-    checkobject(testingsshpeer(ui, 'ssh://localhost/foo', False, ()))
+    checkobject(testingsshpeer(ui, 'ssh://localhost/foo', False,
+                               (None, None, None, None)))
     checkobject(bundlerepo.bundlepeer(dummyrepo()))
     checkobject(statichttprepo.statichttppeer(dummyrepo()))
     checkobject(unionrepo.unionpeer(dummyrepo()))