D2081: wireprotoserver: add context manager mechanism for redirecting stdio

[indygreg (Gregory Szorc)]

indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION 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 and
  gets us closer to a unified interface between the SSH and HTTP
  transports.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/wireproto.py
  mercurial/wireprotoserver.py

CHANGE DETAILS

diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -8,6 +8,7 @@
 
 import abc
 import cgi
+import contextlib
 import struct
 import sys
 
@@ -74,6 +75,20 @@
         """
 
     @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):
         """may setup interception for stdout and stderr
 
@@ -151,6 +166,21 @@
         for s in util.filechunkiter(self._req, limit=length):
             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):
         self._oldio = self._ui.fout, self._ui.ferr
         self._ui.ferr = self._ui.fout = stringio()
@@ -374,6 +404,10 @@
             fpout.write(self._fin.read(count))
             count = int(self._fin.readline())
 
+    @contextlib.contextmanager
+    def mayberedirectstdio(self):
+        yield None
+
     def redirect(self):
         pass
 
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -978,20 +978,12 @@
     else:
         new = encoding.tolocal(new) # normal path
 
-    if util.safehasattr(proto, 'restore'):
-
-        proto.redirect()
-
+    with proto.mayberedirectstdio() as output:
         r = repo.pushkey(encoding.tolocal(namespace), encoding.tolocal(key),
                          encoding.tolocal(old), new) or False
 
-        output = proto.restore()
-
-        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)
+    output = output.getvalue() if output else ''
+    return '%s\n%s' % (int(r), output)
 
 @wireprotocommand('stream_out')
 def stream(repo, proto):



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