D2082: wireproto: use maybecapturestdio() for push responses (API)

2018-02-12 Thread indygreg (Gregory Szorc)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGcaca3ac2ac04: wireproto: use maybecapturestdio() for push 
responses (API) (authored by indygreg, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2082?vs=5345=5527

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

AFFECTED FILES
  hgext/largefiles/proto.py
  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
@@ -320,15 +320,13 @@
 req.respond(HTTP_OK, mediatype)
 return gen
 elif isinstance(rsp, wireproto.pushres):
-val = proto.restore()
-rsp = '%d\n%s' % (rsp.res, val)
+rsp = '%d\n%s' % (rsp.res, rsp.output)
 req.respond(HTTP_OK, HGTYPE, body=rsp)
 return []
 elif isinstance(rsp, wireproto.pusherr):
 # This is the httplib workaround documented in _handlehttperror().
 req.drain()
 
-proto.restore()
 rsp = '0\n%s\n' % rsp.res
 req.respond(HTTP_OK, HGTYPE, body=rsp)
 return []
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -510,16 +510,18 @@
 
 The call was successful and returned an integer contained in `self.res`.
 """
-def __init__(self, res):
+def __init__(self, res, output):
 self.res = res
+self.output = output
 
 class pusherr(object):
 """wireproto reply: failure
 
 The call failed. The `self.res` attribute contains the error message.
 """
-def __init__(self, res):
+def __init__(self, res, output):
 self.res = res
+self.output = output
 
 class ooberror(object):
 """wireproto reply: failure of a batch of operation
@@ -997,97 +999,98 @@
 def unbundle(repo, proto, heads):
 their_heads = decodelist(heads)
 
-try:
-proto.redirect()
-
-exchange.check_heads(repo, their_heads, 'preparing changes')
-
-# write bundle data to temporary file because it can be big
-fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
-fp = os.fdopen(fd, pycompat.sysstr('wb+'))
-r = 0
+with proto.mayberedirectstdio() as output:
 try:
-proto.getfile(fp)
-fp.seek(0)
-gen = exchange.readbundle(repo.ui, fp, None)
-if (isinstance(gen, changegroupmod.cg1unpacker)
-and not bundle1allowed(repo, 'push')):
-if proto.name == 'http':
-# need to special case http because stderr do not get to
-# the http client on failed push so we need to abuse some
-# other error type to make sure the message get to the
-# user.
-return ooberror(bundle2required)
-raise error.Abort(bundle2requiredmain,
-  hint=bundle2requiredhint)
+exchange.check_heads(repo, their_heads, 'preparing changes')
 
-r = exchange.unbundle(repo, gen, their_heads, 'serve',
-  proto._client())
-if util.safehasattr(r, 'addpart'):
-# The return looks streamable, we are in the bundle2 case and
-# should return a stream.
-return streamres_legacy(gen=r.getchunks())
-return pushres(r)
-
-finally:
-fp.close()
-os.unlink(tempname)
-
-except (error.BundleValueError, error.Abort, error.PushRaced) as exc:
-# handle non-bundle2 case first
-if not getattr(exc, 'duringunbundle2', False):
+# write bundle data to temporary file because it can be big
+fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
+fp = os.fdopen(fd, pycompat.sysstr('wb+'))
+r = 0
 try:
-raise
-except error.Abort:
-# The old code we moved used util.stderr directly.
-# We did not change it to minimise code change.
-# This need to be moved to something proper.
-# Feel free to do it.
-util.stderr.write("abort: %s\n" % exc)
-if exc.hint is not None:
-util.stderr.write("(%s)\n" % exc.hint)
-return pushres(0)
-except error.PushRaced:
-return pusherr(str(exc))
+proto.getfile(fp)
+fp.seek(0)
+gen = exchange.readbundle(repo.ui, fp, None)
+if (isinstance(gen, changegroupmod.cg1unpacker)
+and not bundle1allowed(repo, 'push')):
+if proto.name == 'http':
+# need to special case http because stderr do not get 
to
+

D2082: wireproto: use maybecapturestdio() for push responses (API)

2018-02-07 Thread indygreg (Gregory Szorc)
indygreg updated this revision to Diff 5345.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2082?vs=5334=5345

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

AFFECTED FILES
  hgext/largefiles/proto.py
  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
@@ -320,15 +320,13 @@
 req.respond(HTTP_OK, mediatype)
 return gen
 elif isinstance(rsp, wireproto.pushres):
-val = proto.restore()
-rsp = '%d\n%s' % (rsp.res, val)
+rsp = '%d\n%s' % (rsp.res, rsp.output)
 req.respond(HTTP_OK, HGTYPE, body=rsp)
 return []
 elif isinstance(rsp, wireproto.pusherr):
 # This is the httplib workaround documented in _handlehttperror().
 req.drain()
 
-proto.restore()
 rsp = '0\n%s\n' % rsp.res
 req.respond(HTTP_OK, HGTYPE, body=rsp)
 return []
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -510,16 +510,18 @@
 
 The call was successful and returned an integer contained in `self.res`.
 """
-def __init__(self, res):
+def __init__(self, res, output):
 self.res = res
+self.output = output
 
 class pusherr(object):
 """wireproto reply: failure
 
 The call failed. The `self.res` attribute contains the error message.
 """
-def __init__(self, res):
+def __init__(self, res, output):
 self.res = res
+self.output = output
 
 class ooberror(object):
 """wireproto reply: failure of a batch of operation
@@ -997,97 +999,98 @@
 def unbundle(repo, proto, heads):
 their_heads = decodelist(heads)
 
-try:
-proto.redirect()
-
-exchange.check_heads(repo, their_heads, 'preparing changes')
-
-# write bundle data to temporary file because it can be big
-fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
-fp = os.fdopen(fd, pycompat.sysstr('wb+'))
-r = 0
+with proto.mayberedirectstdio() as output:
 try:
-proto.getfile(fp)
-fp.seek(0)
-gen = exchange.readbundle(repo.ui, fp, None)
-if (isinstance(gen, changegroupmod.cg1unpacker)
-and not bundle1allowed(repo, 'push')):
-if proto.name == 'http':
-# need to special case http because stderr do not get to
-# the http client on failed push so we need to abuse some
-# other error type to make sure the message get to the
-# user.
-return ooberror(bundle2required)
-raise error.Abort(bundle2requiredmain,
-  hint=bundle2requiredhint)
+exchange.check_heads(repo, their_heads, 'preparing changes')
 
-r = exchange.unbundle(repo, gen, their_heads, 'serve',
-  proto._client())
-if util.safehasattr(r, 'addpart'):
-# The return looks streamable, we are in the bundle2 case and
-# should return a stream.
-return streamres_legacy(gen=r.getchunks())
-return pushres(r)
-
-finally:
-fp.close()
-os.unlink(tempname)
-
-except (error.BundleValueError, error.Abort, error.PushRaced) as exc:
-# handle non-bundle2 case first
-if not getattr(exc, 'duringunbundle2', False):
+# write bundle data to temporary file because it can be big
+fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
+fp = os.fdopen(fd, pycompat.sysstr('wb+'))
+r = 0
 try:
-raise
-except error.Abort:
-# The old code we moved used util.stderr directly.
-# We did not change it to minimise code change.
-# This need to be moved to something proper.
-# Feel free to do it.
-util.stderr.write("abort: %s\n" % exc)
-if exc.hint is not None:
-util.stderr.write("(%s)\n" % exc.hint)
-return pushres(0)
-except error.PushRaced:
-return pusherr(str(exc))
+proto.getfile(fp)
+fp.seek(0)
+gen = exchange.readbundle(repo.ui, fp, None)
+if (isinstance(gen, changegroupmod.cg1unpacker)
+and not bundle1allowed(repo, 'push')):
+if proto.name == 'http':
+# need to special case http because stderr do not get 
to
+# the http client on failed push so we need to abuse
+# some other error type to make sure the message get to
+

D2082: wireproto: use maybecapturestdio() for push responses (API)

2018-02-07 Thread indygreg (Gregory Szorc)
indygreg updated this revision to Diff 5334.
indygreg edited the summary of this revision.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2082?vs=5316=5334

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

AFFECTED FILES
  hgext/largefiles/proto.py
  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
@@ -320,15 +320,13 @@
 req.respond(HTTP_OK, mediatype)
 return gen
 elif isinstance(rsp, wireproto.pushres):
-val = proto.restore()
-rsp = '%d\n%s' % (rsp.res, val)
+rsp = '%d\n%s' % (rsp.res, rsp.output)
 req.respond(HTTP_OK, HGTYPE, body=rsp)
 return []
 elif isinstance(rsp, wireproto.pusherr):
 # This is the httplib workaround documented in _handlehttperror().
 req.drain()
 
-proto.restore()
 rsp = '0\n%s\n' % rsp.res
 req.respond(HTTP_OK, HGTYPE, body=rsp)
 return []
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -510,16 +510,18 @@
 
 The call was successful and returned an integer contained in `self.res`.
 """
-def __init__(self, res):
+def __init__(self, res, output):
 self.res = res
+self.output = output
 
 class pusherr(object):
 """wireproto reply: failure
 
 The call failed. The `self.res` attribute contains the error message.
 """
-def __init__(self, res):
+def __init__(self, res, output):
 self.res = res
+self.output = output
 
 class ooberror(object):
 """wireproto reply: failure of a batch of operation
@@ -997,97 +999,98 @@
 def unbundle(repo, proto, heads):
 their_heads = decodelist(heads)
 
-try:
-proto.redirect()
-
-exchange.check_heads(repo, their_heads, 'preparing changes')
-
-# write bundle data to temporary file because it can be big
-fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
-fp = os.fdopen(fd, pycompat.sysstr('wb+'))
-r = 0
+with proto.mayberedirectstdio() as output:
 try:
-proto.getfile(fp)
-fp.seek(0)
-gen = exchange.readbundle(repo.ui, fp, None)
-if (isinstance(gen, changegroupmod.cg1unpacker)
-and not bundle1allowed(repo, 'push')):
-if proto.name == 'http':
-# need to special case http because stderr do not get to
-# the http client on failed push so we need to abuse some
-# other error type to make sure the message get to the
-# user.
-return ooberror(bundle2required)
-raise error.Abort(bundle2requiredmain,
-  hint=bundle2requiredhint)
+exchange.check_heads(repo, their_heads, 'preparing changes')
 
-r = exchange.unbundle(repo, gen, their_heads, 'serve',
-  proto._client())
-if util.safehasattr(r, 'addpart'):
-# The return looks streamable, we are in the bundle2 case and
-# should return a stream.
-return streamres_legacy(gen=r.getchunks())
-return pushres(r)
-
-finally:
-fp.close()
-os.unlink(tempname)
-
-except (error.BundleValueError, error.Abort, error.PushRaced) as exc:
-# handle non-bundle2 case first
-if not getattr(exc, 'duringunbundle2', False):
+# write bundle data to temporary file because it can be big
+fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
+fp = os.fdopen(fd, pycompat.sysstr('wb+'))
+r = 0
 try:
-raise
-except error.Abort:
-# The old code we moved used util.stderr directly.
-# We did not change it to minimise code change.
-# This need to be moved to something proper.
-# Feel free to do it.
-util.stderr.write("abort: %s\n" % exc)
-if exc.hint is not None:
-util.stderr.write("(%s)\n" % exc.hint)
-return pushres(0)
-except error.PushRaced:
-return pusherr(str(exc))
+proto.getfile(fp)
+fp.seek(0)
+gen = exchange.readbundle(repo.ui, fp, None)
+if (isinstance(gen, changegroupmod.cg1unpacker)
+and not bundle1allowed(repo, 'push')):
+if proto.name == 'http':
+# need to special case http because stderr do not get 
to
+# the http client on failed push so we need to abuse
+# some other error type to make 

D2082: wireproto: use maybecapturestdio() for push responses (API)

2018-02-07 Thread indygreg (Gregory Szorc)
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The "pushres" and "pusherr" response types currently call
  proto.restore() in the HTTP protocol. This completes the pairing
  with proto.redirect() that occurs in the @wireprotocommand
  functions. (But since the SSH protocol has a no-op redirect(),
  it doesn't bother calling restore() because it would also be
  a no-op.)
  
  Having the disconnect between these paired calls is very confusing.
  Knowing that you must use proto.redirect() if returning a "pushres"
  or a "pusherr" is even wonkier.
  
  We replace this confusing code with our new context manager for
  capturing output (maybe).
  
  The "pushres" and "pusherr" types have gained an "output" argument
  to their constructor and an attribute to hold captured data. The
  HTTP protocol now retrieves output from these objects.
  
  .. api::
  
``wireproto.pushres`` and ``wireproto.pusherr`` now explicitly
track stdio output.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/largefiles/proto.py
  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
@@ -320,15 +320,13 @@
 req.respond(HTTP_OK, mediatype)
 return gen
 elif isinstance(rsp, wireproto.pushres):
-val = proto.restore()
-rsp = '%d\n%s' % (rsp.res, val)
+rsp = '%d\n%s' % (rsp.res, rsp.output)
 req.respond(HTTP_OK, HGTYPE, body=rsp)
 return []
 elif isinstance(rsp, wireproto.pusherr):
 # This is the httplib workaround documented in _handlehttperror().
 req.drain()
 
-proto.restore()
 rsp = '0\n%s\n' % rsp.res
 req.respond(HTTP_OK, HGTYPE, body=rsp)
 return []
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -510,16 +510,18 @@
 
 The call was successful and returned an integer contained in `self.res`.
 """
-def __init__(self, res):
+def __init__(self, res, output):
 self.res = res
+self.output = output
 
 class pusherr(object):
 """wireproto reply: failure
 
 The call failed. The `self.res` attribute contains the error message.
 """
-def __init__(self, res):
+def __init__(self, res, output):
 self.res = res
+self.output = output
 
 class ooberror(object):
 """wireproto reply: failure of a batch of operation
@@ -997,97 +999,98 @@
 def unbundle(repo, proto, heads):
 their_heads = decodelist(heads)
 
-try:
-proto.redirect()
-
-exchange.check_heads(repo, their_heads, 'preparing changes')
-
-# write bundle data to temporary file because it can be big
-fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
-fp = os.fdopen(fd, pycompat.sysstr('wb+'))
-r = 0
+with proto.mayberedirectstdio() as output:
 try:
-proto.getfile(fp)
-fp.seek(0)
-gen = exchange.readbundle(repo.ui, fp, None)
-if (isinstance(gen, changegroupmod.cg1unpacker)
-and not bundle1allowed(repo, 'push')):
-if proto.name == 'http':
-# need to special case http because stderr do not get to
-# the http client on failed push so we need to abuse some
-# other error type to make sure the message get to the
-# user.
-return ooberror(bundle2required)
-raise error.Abort(bundle2requiredmain,
-  hint=bundle2requiredhint)
+exchange.check_heads(repo, their_heads, 'preparing changes')
 
-r = exchange.unbundle(repo, gen, their_heads, 'serve',
-  proto._client())
-if util.safehasattr(r, 'addpart'):
-# The return looks streamable, we are in the bundle2 case and
-# should return a stream.
-return streamres_legacy(gen=r.getchunks())
-return pushres(r)
-
-finally:
-fp.close()
-os.unlink(tempname)
-
-except (error.BundleValueError, error.Abort, error.PushRaced) as exc:
-# handle non-bundle2 case first
-if not getattr(exc, 'duringunbundle2', False):
+# write bundle data to temporary file because it can be big
+fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
+fp = os.fdopen(fd, pycompat.sysstr('wb+'))
+r = 0
 try:
-raise
-except error.Abort:
-# The old code we moved used util.stderr directly.
-# We did not change it to minimise code change.
-#