( )⚙ D2031 sshpeer: establish SSH connection before class instantiation

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2031

sshpeer: establish SSH connection before class instantiation
ClosedPublic

Authored by indygreg on Feb 4 2018, 10:35 PM.

Details

Reviewers
lothiraldan
yuja
Group Reviewers
hg-reviewers
Commits
rHG00b9e26d727b: sshpeer: establish SSH connection before class instantiation
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 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.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Feb 4 2018, 10:35 PM
lothiraldan accepted this revision.Feb 5 2018, 8:12 AM
indygreg edited the summary of this revision. (Show Details)Feb 5 2018, 5:21 PM
indygreg updated this revision to Diff 5232.
yuja accepted this revision.Feb 6 2018, 8:07 AM
This revision is now accepted and ready to land.Feb 6 2018, 8:07 AM
Closed by commit rHG00b9e26d727b: sshpeer: establish SSH connection before class instantiation (authored by indygreg). · Explain WhyFeb 6 2018, 8:24 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/sshpeer.py (62 lines)
Mtests/test-check-interfaces.py (3 lines)

Diff 5232

mercurial/sshpeer.py

try: try:
for l in pipee: for l in pipee:
ui.status(_('remote: '), l) ui.status(_('remote: '), l)
except (IOError, ValueError): except (IOError, ValueError):
pass pass
pipee.close() 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): class sshpeer(wireproto.wirepeer):
def __init__(self, ui, path, create=False, sshstate=None): def __init__(self, ui, path, create=False, sshstate=None):
self._url = path self._url = path
self._ui = ui 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._validaterepo()
self._path = u.path or '.'
self._validaterepo(*sshstate)
# Begin of _basepeer interface. # Begin of _basepeer interface.
@util.propertycache @util.propertycache
def ui(self): def ui(self):
return self._ui return self._ui
def url(self): def url(self):
# Begin of _basewirecommands interface. # Begin of _basewirecommands interface.
def capabilities(self): def capabilities(self):
return self._caps return self._caps
# End of _basewirecommands interface. # End of _basewirecommands interface.
def _validaterepo(self, sshcmd, args, remotecmd, sshenv=None): def _validaterepo(self):
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 badresponse(): def badresponse():
msg = _("no suitable response from remote hg") msg = _("no suitable response from remote hg")
hint = self.ui.config("ui", "ssherrorhint") hint = self.ui.config("ui", "ssherrorhint")
self._abort(error.RepoError(msg, hint=hint)) self._abort(error.RepoError(msg, hint=hint))
try: try:
# skip any noise generated by remote shell # skip any noise generated by remote shell
self._callstream("hello") self._callstream("hello")
cmd = '%s %s %s' % (sshcmd, args, cmd = '%s %s %s' % (sshcmd, args,
util.shellquote('%s init %s' % util.shellquote('%s init %s' %
(_serverquote(remotecmd), _serverquote(remotepath)))) (_serverquote(remotecmd), _serverquote(remotepath))))
ui.debug('running %s\n' % cmd) ui.debug('running %s\n' % cmd)
res = ui.system(cmd, blockedtag='sshpeer', environ=sshenv) res = ui.system(cmd, blockedtag='sshpeer', environ=sshenv)
if res != 0: if res != 0:
raise error.RepoError(_('could not create remote repo')) 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) return sshpeer(ui, path, create=create, sshstate=sshstate)

tests/test-check-interfaces.py

pass pass
def main(): def main():
ui = uimod.ui() ui = uimod.ui()
checkobject(badpeer()) checkobject(badpeer())
checkobject(httppeer.httppeer(ui, 'http://localhost')) checkobject(httppeer.httppeer(ui, 'http://localhost'))
checkobject(localrepo.localpeer(dummyrepo())) 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(bundlerepo.bundlepeer(dummyrepo()))
checkobject(statichttprepo.statichttppeer(dummyrepo())) checkobject(statichttprepo.statichttppeer(dummyrepo()))
checkobject(unionrepo.unionpeer(dummyrepo())) checkobject(unionrepo.unionpeer(dummyrepo()))
main() main()