This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2081

wireprotoserver: add context manager mechanism for redirecting stdio
ClosedPublic

Authored by indygreg on Feb 7 2018, 7:26 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG2ad145fbde54: wireprotoserver: add context manager mechanism for redirecting stdio
Summary

Today, proto.redirect() sets up redirecting stdio and proto.restore()
undoes that. The API is a bit wonky because restore() is only
implemented on the HTTP protocol. Furthermore, not all calls to
redirect() are obviously paired with calls to restore(). For
example, the call to restore() for "unbundle" requests is handled
by the response handler for the HTTP protocol.

This commit introduces a new method on the protocol handler interface
to maybe capture stdio. It emits a file object or None depending on
whether stdio capture is used by the transport.

To prove it works, the "pushkey" wire protocol command has been
updated to use the new API.

I'm not convinced this is the best mechanism to capture stdio. I may
need to come up with something better once the new wire protocol
emerges into existence. But it is strictly better than before because
it removes variance in the wire protocol handler interface. It
therefore gets us closer to a unified interface between the SSH and
HTTP transports.

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.Feb 7 2018, 7:26 PM
indygreg edited the summary of this revision. (Show Details)Feb 7 2018, 11:32 PM
indygreg updated this revision to Diff 5333.
indygreg updated this revision to Diff 5344.Feb 8 2018, 12:20 AM
durin42 accepted this revision.Feb 12 2018, 4:21 PM
This revision is now accepted and ready to land.Feb 12 2018, 4:21 PM
Closed by commit rHG2ad145fbde54: wireprotoserver: add context manager mechanism for redirecting stdio (authored by indygreg). · Explain WhyFeb 12 2018, 4:33 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/wireproto.py (14 lines)
Mmercurial/wireprotoserver.py (34 lines)

Diff 5528

mercurial/wireproto.py

try: try:
new.decode('utf-8') new.decode('utf-8')
new = encoding.tolocal(new) # but cleanly decodes as UTF-8 new = encoding.tolocal(new) # but cleanly decodes as UTF-8
except UnicodeDecodeError: except UnicodeDecodeError:
pass # binary, leave unmodified pass # binary, leave unmodified
else: else:
new = encoding.tolocal(new) # normal path new = encoding.tolocal(new) # normal path
if util.safehasattr(proto, 'restore'): with proto.mayberedirectstdio() as output:
proto.redirect()
r = repo.pushkey(encoding.tolocal(namespace), encoding.tolocal(key), r = repo.pushkey(encoding.tolocal(namespace), encoding.tolocal(key),
encoding.tolocal(old), new) or False encoding.tolocal(old), new) or False
output = proto.restore() output = output.getvalue() if output else ''
return '%s\n%s' % (int(r), output) return '%s\n%s' % (int(r), output)
r = repo.pushkey(encoding.tolocal(namespace), encoding.tolocal(key),
encoding.tolocal(old), new)
return '%s\n' % int(r)
@wireprotocommand('stream_out') @wireprotocommand('stream_out')
def stream(repo, proto): def stream(repo, proto):
'''If the server supports streaming clone, it advertises the "stream" '''If the server supports streaming clone, it advertises the "stream"
capability with a value representing the version and flags of the repo capability with a value representing the version and flags of the repo
it is serving. Client checks to see if it understands the format. it is serving. Client checks to see if it understands the format.
''' '''
return streamres_legacy(streamclone.generatev1wireproto(repo)) return streamres_legacy(streamclone.generatev1wireproto(repo))

mercurial/wireprotoserver.py

# Copyright 21 May 2005 - (c) 2005 Jake Edge <jake@edge2.net> # Copyright 21 May 2005 - (c) 2005 Jake Edge <jake@edge2.net>
# Copyright 2005-2007 Matt Mackall <mpm@selenic.com> # Copyright 2005-2007 Matt Mackall <mpm@selenic.com>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import abc import abc
import cgi import cgi
import contextlib
import struct import struct
import sys import sys
from .i18n import _ from .i18n import _
from . import ( from . import (
encoding, encoding,
error, error,
hook, hook,
The file is in the form:: The file is in the form::
(<chunk-size>\n<chunk>)+0\n (<chunk-size>\n<chunk>)+0\n
chunk size is the ascii version of the int. chunk size is the ascii version of the int.
""" """
@abc.abstractmethod @abc.abstractmethod
def mayberedirectstdio(self):
"""Context manager to possibly redirect stdio.
The context manager yields a file-object like object that receives
stdout and stderr output when the context manager is active. Or it
yields ``None`` if no I/O redirection occurs.
The intent of this context manager is to capture stdio output
so it may be sent in the response. Some transports support streaming
stdio to the client in real time. For these transports, stdio output
won't be captured.
"""
@abc.abstractmethod
def redirect(self): def redirect(self):
"""may setup interception for stdout and stderr """may setup interception for stdout and stderr
See also the `restore` method.""" See also the `restore` method."""
# If the `redirect` function does install interception, the `restore` # If the `redirect` function does install interception, the `restore`
# function MUST be defined. If interception is not used, this function # function MUST be defined. If interception is not used, this function
# MUST NOT be defined. # MUST NOT be defined.
def getfile(self, fp): def getfile(self, fp):
length = int(self._req.env[r'CONTENT_LENGTH']) length = int(self._req.env[r'CONTENT_LENGTH'])
# If httppostargs is used, we need to read Content-Length # If httppostargs is used, we need to read Content-Length
# minus the amount that was consumed by args. # minus the amount that was consumed by args.
length -= int(self._req.env.get(r'HTTP_X_HGARGS_POST', 0)) length -= int(self._req.env.get(r'HTTP_X_HGARGS_POST', 0))
for s in util.filechunkiter(self._req, limit=length): for s in util.filechunkiter(self._req, limit=length):
fp.write(s) fp.write(s)
@contextlib.contextmanager
def mayberedirectstdio(self):
oldout = self._ui.fout
olderr = self._ui.ferr
out = util.stringio()
try:
self._ui.fout = out
self._ui.ferr = out
yield out
finally:
self._ui.fout = oldout
self._ui.ferr = olderr
def redirect(self): def redirect(self):
self._oldio = self._ui.fout, self._ui.ferr self._oldio = self._ui.fout, self._ui.ferr
self._ui.ferr = self._ui.fout = stringio() self._ui.ferr = self._ui.fout = stringio()
def restore(self): def restore(self):
val = self._ui.fout.getvalue() val = self._ui.fout.getvalue()
self._ui.ferr, self._ui.fout = self._oldio self._ui.ferr, self._ui.fout = self._oldio
return val return val
def getfile(self, fpout): def getfile(self, fpout):
_sshv1respondbytes(self._fout, b'') _sshv1respondbytes(self._fout, b'')
count = int(self._fin.readline()) count = int(self._fin.readline())
while count: while count:
fpout.write(self._fin.read(count)) fpout.write(self._fin.read(count))
count = int(self._fin.readline()) count = int(self._fin.readline())
@contextlib.contextmanager
def mayberedirectstdio(self):
yield None
def redirect(self): def redirect(self):
pass pass
def _client(self): def _client(self):
client = encoding.environ.get('SSH_CLIENT', '').split(' ', 1)[0] client = encoding.environ.get('SSH_CLIENT', '').split(' ', 1)[0]
return 'remote:ssh:' + client return 'remote:ssh:' + client
class sshserver(object): class sshserver(object):