This is an archive of the discontinued Mercurial Phabricator instance.

Differential D331

sshpeer: make instance attributes and methods internal
ClosedPublic

Authored by indygreg on Aug 11 2017, 12:20 AM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG82d564d5ac4f: sshpeer: make instance attributes and methods internal
Summary

Peer types are supposed to conform to a formal interface defined by
peer.peerrepository and wireproto.wirepeer. Every "public" attribute on
*peer types makes it harder to understand what attributes are part
of the interface and what are instance specific.

This commit converts a number of "public" instance attributes and
methods on sshpeer to internal so they can't be confused to be part of
the peer API.

The URL-related instance attributes were introduced in 876333a295ff
in 2005. AFAICT most of them aren't used and could potentially be
removed. But I kept them around anyway.

I also reorded some code to make things slightly easier to read.

.. api::

Rename attributes on sshpeer to reflect peer API

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Aug 11 2017, 12:20 AM
lothiraldan added a subscriber: lothiraldan.Aug 11 2017, 3:35 AM

Silly question, what is the deprecation policy for such internal classes?

indygreg added a comment.Aug 11 2017, 11:43 AM

The deprecation policy isn't super well defined. Essentially, every change we ask ourselves how common we think out-of-repo consumers are. "Internal" attributes we typically don't even warn about changing them. For something like @command, we make noise and set a long deprecation policy (a few releases to a year).

durin42 accepted this revision.Aug 11 2017, 2:24 PM
durin42 added a subscriber: durin42.

I've queued up to this point in the series.

This revision is now accepted and ready to land.Aug 11 2017, 2:24 PM
Closed by commit rHG82d564d5ac4f: sshpeer: make instance attributes and methods internal (authored by indygreg). · Explain WhyAug 11 2017, 2:52 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/sshpeer.py (83 lines)

Diff 795

mercurial/sshpeer.py

def flush(self): def flush(self):
return self._main.flush() return self._main.flush()
class sshpeer(wireproto.wirepeer): class sshpeer(wireproto.wirepeer):
def __init__(self, ui, path, create=False): def __init__(self, ui, path, create=False):
self._url = path self._url = path
self.ui = ui self.ui = ui
self.pipeo = self.pipei = self.pipee = None self._pipeo = self._pipei = self._pipee = None
u = util.url(path, parsequery=False, parsefragment=False) u = util.url(path, parsequery=False, parsefragment=False)
if u.scheme != 'ssh' or not u.host or u.path is None: if u.scheme != 'ssh' or not u.host or u.path is None:
self._abort(error.RepoError(_("couldn't parse location %s") % path)) self._abort(error.RepoError(_("couldn't parse location %s") % path))
util.checksafessh(path) util.checksafessh(path)
self.user = u.user
if u.passwd is not None: if u.passwd is not None:
self._abort(error.RepoError(_("password in URL not supported"))) self._abort(error.RepoError(_("password in URL not supported")))
self.host = u.host
self.port = u.port self._user = u.user
self.path = u.path or "." self._host = u.host
self._port = u.port
self._path = u.path or '.'
sshcmd = self.ui.config("ui", "ssh") sshcmd = self.ui.config("ui", "ssh")
remotecmd = self.ui.config("ui", "remotecmd") remotecmd = self.ui.config("ui", "remotecmd")
args = util.sshargs(sshcmd, self.host, self.user, self.port) args = util.sshargs(sshcmd, self._host, self._user, self._port)
if create: if create:
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(self.path)))) (_serverquote(remotecmd), _serverquote(self._path))))
ui.debug('running %s\n' % cmd) ui.debug('running %s\n' % cmd)
res = ui.system(cmd, blockedtag='sshpeer') res = ui.system(cmd, blockedtag='sshpeer')
if res != 0: if res != 0:
self._abort(error.RepoError(_("could not create remote repo"))) self._abort(error.RepoError(_("could not create remote repo")))
self._validaterepo(sshcmd, args, remotecmd) self._validaterepo(sshcmd, args, remotecmd)
def url(self): def url(self):
return self._url return self._url
def _validaterepo(self, sshcmd, args, remotecmd): def _validaterepo(self, sshcmd, args, remotecmd):
# cleanup up previous run # cleanup up previous run
self.cleanup() self._cleanup()
cmd = '%s %s %s' % (sshcmd, args, cmd = '%s %s %s' % (sshcmd, args,
util.shellquote("%s -R %s serve --stdio" % util.shellquote("%s -R %s serve --stdio" %
(_serverquote(remotecmd), _serverquote(self.path)))) (_serverquote(remotecmd), _serverquote(self._path))))
self.ui.debug('running %s\n' % cmd) self.ui.debug('running %s\n' % cmd)
cmd = util.quotecommand(cmd) cmd = util.quotecommand(cmd)
# while self.subprocess isn't used, having it allows the subprocess to # while self._subprocess isn't used, having it allows the subprocess to
# to clean up correctly later # to clean up correctly later
# #
# no buffer allow the use of 'select' # no buffer allow the use of 'select'
# feel free to remove buffering and select usage when we ultimately # feel free to remove buffering and select usage when we ultimately
# move to threading. # move to threading.
sub = util.popen4(cmd, bufsize=0) sub = util.popen4(cmd, bufsize=0)
self.pipeo, self.pipei, self.pipee, self.subprocess = sub self._pipeo, self._pipei, self._pipee, self._subprocess = sub
self.pipei = util.bufferedinputpipe(self.pipei) self._pipei = util.bufferedinputpipe(self._pipei)
self.pipei = doublepipe(self.ui, self.pipei, self.pipee) self._pipei = doublepipe(self.ui, self._pipei, self._pipee)
self.pipeo = doublepipe(self.ui, self.pipeo, self.pipee) self._pipeo = doublepipe(self.ui, self._pipeo, self._pipee)
# skip any noise generated by remote shell # skip any noise generated by remote shell
self._callstream("hello") self._callstream("hello")
r = self._callstream("between", pairs=("%s-%s" % ("0"*40, "0"*40))) r = self._callstream("between", pairs=("%s-%s" % ("0"*40, "0"*40)))
lines = ["", "dummy"] lines = ["", "dummy"]
max_noise = 500 max_noise = 500
while lines[-1] and max_noise: while lines[-1] and max_noise:
l = r.readline() l = r.readline()
self.readerr() self._readerr()
if lines[-1] == "1\n" and l == "\n": if lines[-1] == "1\n" and l == "\n":
break break
if l: if l:
self.ui.debug("remote: ", l) self.ui.debug("remote: ", l)
lines.append(l) lines.append(l)
max_noise -= 1 max_noise -= 1
else: else:
self._abort(error.RepoError(_('no suitable response from ' self._abort(error.RepoError(_('no suitable response from '
'remote hg'))) 'remote hg')))
self._caps = set() self._caps = set()
for l in reversed(lines): for l in reversed(lines):
if l.startswith("capabilities:"): if l.startswith("capabilities:"):
self._caps.update(l[:-1].split(":")[1].split()) self._caps.update(l[:-1].split(":")[1].split())
break break
def _capabilities(self): def _capabilities(self):
return self._caps return self._caps
def readerr(self): def _readerr(self):
_forwardoutput(self.ui, self.pipee) _forwardoutput(self.ui, self._pipee)
def _abort(self, exception): def _abort(self, exception):
self.cleanup() self._cleanup()
raise exception raise exception
def cleanup(self): def _cleanup(self):
if self.pipeo is None: if self._pipeo is None:
return return
self.pipeo.close() self._pipeo.close()
self.pipei.close() self._pipei.close()
try: try:
# read the error descriptor until EOF # read the error descriptor until EOF
for l in self.pipee: for l in self._pipee:
self.ui.status(_("remote: "), l) self.ui.status(_("remote: "), l)
except (IOError, ValueError): except (IOError, ValueError):
pass pass
self.pipee.close() self._pipee.close()
__del__ = cleanup __del__ = _cleanup
def _submitbatch(self, req): def _submitbatch(self, req):
rsp = self._callstream("batch", cmds=wireproto.encodebatchcmds(req)) rsp = self._callstream("batch", cmds=wireproto.encodebatchcmds(req))
available = self._getamount() available = self._getamount()
# TODO this response parsing is probably suboptimal for large # TODO this response parsing is probably suboptimal for large
# batches with large responses. # batches with large responses.
toread = min(available, 1024) toread = min(available, 1024)
work = rsp.read(toread) work = rsp.read(toread)
available -= toread available -= toread
chunk = work chunk = work
while chunk: while chunk:
while ';' in work: while ';' in work:
one, work = work.split(';', 1) one, work = work.split(';', 1)
yield wireproto.unescapearg(one) yield wireproto.unescapearg(one)
toread = min(available, 1024) toread = min(available, 1024)
chunk = rsp.read(toread) chunk = rsp.read(toread)
available -= toread available -= toread
work += chunk work += chunk
yield wireproto.unescapearg(work) yield wireproto.unescapearg(work)
def _callstream(self, cmd, **args): def _callstream(self, cmd, **args):
args = pycompat.byteskwargs(args) args = pycompat.byteskwargs(args)
self.ui.debug("sending %s command\n" % cmd) self.ui.debug("sending %s command\n" % cmd)
self.pipeo.write("%s\n" % cmd) self._pipeo.write("%s\n" % cmd)
_func, names = wireproto.commands[cmd] _func, names = wireproto.commands[cmd]
keys = names.split() keys = names.split()
wireargs = {} wireargs = {}
for k in keys: for k in keys:
if k == '*': if k == '*':
wireargs['*'] = args wireargs['*'] = args
break break
else: else:
wireargs[k] = args[k] wireargs[k] = args[k]
del args[k] del args[k]
for k, v in sorted(wireargs.iteritems()): for k, v in sorted(wireargs.iteritems()):
self.pipeo.write("%s %d\n" % (k, len(v))) self._pipeo.write("%s %d\n" % (k, len(v)))
if isinstance(v, dict): if isinstance(v, dict):
for dk, dv in v.iteritems(): for dk, dv in v.iteritems():
self.pipeo.write("%s %d\n" % (dk, len(dv))) self._pipeo.write("%s %d\n" % (dk, len(dv)))
self.pipeo.write(dv) self._pipeo.write(dv)
else: else:
self.pipeo.write(v) self._pipeo.write(v)
self.pipeo.flush() self._pipeo.flush()
return self.pipei return self._pipei
def _callcompressable(self, cmd, **args): def _callcompressable(self, cmd, **args):
return self._callstream(cmd, **args) return self._callstream(cmd, **args)
def _call(self, cmd, **args): def _call(self, cmd, **args):
self._callstream(cmd, **args) self._callstream(cmd, **args)
return self._recv() return self._recv()
def _calltwowaystream(self, cmd, fp, **args): def _calltwowaystream(self, cmd, fp, **args):
r = self._call(cmd, **args) r = self._call(cmd, **args)
if r: if r:
# XXX needs to be made better # XXX needs to be made better
raise error.Abort(_('unexpected remote reply: %s') % r) raise error.Abort(_('unexpected remote reply: %s') % r)
for d in iter(lambda: fp.read(4096), ''): for d in iter(lambda: fp.read(4096), ''):
self._send(d) self._send(d)
self._send("", flush=True) self._send("", flush=True)
return self.pipei return self._pipei
def _getamount(self): def _getamount(self):
l = self.pipei.readline() l = self._pipei.readline()
if l == '\n': if l == '\n':
self.readerr() self._readerr()
msg = _('check previous remote output') msg = _('check previous remote output')
self._abort(error.OutOfBandError(hint=msg)) self._abort(error.OutOfBandError(hint=msg))
self.readerr() self._readerr()
try: try:
return int(l) return int(l)
except ValueError: except ValueError:
self._abort(error.ResponseError(_("unexpected response:"), l)) self._abort(error.ResponseError(_("unexpected response:"), l))
def _recv(self): def _recv(self):
return self.pipei.read(self._getamount()) return self._pipei.read(self._getamount())
def _send(self, data, flush=False): def _send(self, data, flush=False):
self.pipeo.write("%d\n" % len(data)) self._pipeo.write("%d\n" % len(data))
if data: if data:
self.pipeo.write(data) self._pipeo.write(data)
if flush: if flush:
self.pipeo.flush() self._pipeo.flush()
self.readerr() self._readerr()
instance = sshpeer instance = sshpeer