Patchwork D2031: sshpeer: establish SSH connection before class instantiation

login
register
mail settings
Submitter phabricator
Date Feb. 5, 2018, 3:35 a.m.
Message ID <differential-rev-PHID-DREV-bybjgunof6hmamluekp3-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/27290/
State Superseded
Headers show

Comments

phabricator - Feb. 5, 2018, 3:35 a.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  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 this, 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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2031

AFFECTED FILES
  mercurial/sshpeer.py
  tests/test-check-interfaces.py

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/tests/test-check-interfaces.py b/tests/test-check-interfaces.py
--- a/tests/test-check-interfaces.py
+++ b/tests/test-check-interfaces.py
@@ -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()))
diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -131,16 +131,41 @@ 
 
         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.
+    pipeo, pipei, pipee, proc = util.popen4(cmd, bufsize=0, env=sshenv)
+
+    pipei = util.bufferedinputpipe(pipei)
+    pipei = doublepipe(ui, pipei, pipee)
+    pipeo = doublepipe(ui, pipeo, pipee)
+
+    return proc, pipei, pipeo, pipee
+
 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 +197,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 +384,9 @@ 
         if res != 0:
             raise error.RepoError(_('could not create remote repo'))
 
-    sshstate = (sshcmd, args, remotecmd, sshenv)
+    proc, pipei, pipeo, pipee = _makeconnection(ui, sshcmd, args, remotecmd,
+                                                remotepath, sshenv)
+
+    sshstate = (proc, pipei, pipeo, pipee)
 
     return sshpeer(ui, path, create=create, sshstate=sshstate)