Submitter | Yuya Nishihara |
---|---|
Date | Nov. 17, 2020, 1:04 p.m. |
Message ID | <0748354881c942727aef.1605618293@lemosa> |
Download | mbox | patch |
Permalink | /patch/47612/ |
State | Accepted |
Headers | show |
Comments
queued, thanks > On Nov 17, 2020, at 08:04, Yuya Nishihara <yuya@tcha.org> wrote: > > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1605608948 -32400 > # Tue Nov 17 19:29:08 2020 +0900 > # Node ID 0748354881c942727aef46945d721bc51d213e9d > # Parent d68618954adef9c2cbec868c1af0e01f67288cb8 > chgserver: backport py3 buffered I/O workarounds from procutil > > I've recently switched to new machine and I found chg's stdout is fully > buffered. > > Even though chg server is a daemon process, it inherits the environment > where the chg client originally forked the server. This means the server's > stdout might have been wrapped by LineBufferedWrapper. That's why we need > to do wrap/unwrap in both ways. > > The "if" condition in _restoreio() looks weird, but I'm not willing to > clean things up because stdio behavior is fundamentally different between > py2 and py3, and py2 support will be dropped anyway. > > diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py > --- a/mercurial/chgserver.py > +++ b/mercurial/chgserver.py > @@ -409,14 +409,23 @@ class chgcmdserver(commandserver.server) > # be unbuffered no matter if it is a tty or not. > if fn == b'ferr': > newfp = fp > + elif pycompat.ispy3: > + # On Python 3, the standard library doesn't offer line-buffered > + # binary streams, so wrap/unwrap it. > + if fp.isatty(): > + newfp = procutil.make_line_buffered(fp) > + else: > + newfp = procutil.unwrap_line_buffered(fp) > else: > - # make it line buffered explicitly because the default is > - # decided on first write(), where fout could be a pager. > + # Python 2 uses the I/O streams provided by the C library, so > + # make it line-buffered explicitly. Otherwise the default would > + # be decided on first write(), where fout could be a pager. > if fp.isatty(): > bufsize = 1 # line buffered > else: > bufsize = -1 # system default > newfp = os.fdopen(fp.fileno(), mode, bufsize) > + if newfp is not fp: > setattr(ui, fn, newfp) > setattr(self, cn, newfp) > > @@ -440,13 +449,16 @@ class chgcmdserver(commandserver.server) > ui = self.ui > for (ch, fp, fd), (cn, fn, mode) in zip(self._oldios, _iochannels): > newfp = getattr(ui, fn) > - # close newfp while it's associated with client; otherwise it > - # would be closed when newfp is deleted > - if newfp is not fp: > + # On Python 2, newfp and fp may be separate file objects associated > + # with the same fd, so we must close newfp while it's associated > + # with the client. Otherwise the new associated fd would be closed > + # when newfp gets deleted. On Python 3, newfp is just a wrapper > + # around fp even if newfp is not fp, so deleting newfp is safe. > + if not (pycompat.ispy3 or newfp is fp): > newfp.close() > # restore original fd: fp is open again > try: > - if newfp is fp and 'w' in mode: > + if (pycompat.ispy3 or newfp is fp) and 'w' in mode: > # Discard buffered data which couldn't be flushed because > # of EPIPE. The data should belong to the current session > # and should never persist. > diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py > --- a/mercurial/utils/procutil.py > +++ b/mercurial/utils/procutil.py > @@ -80,6 +80,13 @@ def make_line_buffered(stream): > return LineBufferedWrapper(stream) > > > +def unwrap_line_buffered(stream): > + if isinstance(stream, LineBufferedWrapper): > + assert not isinstance(stream.orig, LineBufferedWrapper) > + return stream.orig > + return stream > + > + > class WriteAllWrapper(object): > def __init__(self, orig): > self.orig = orig > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py --- a/mercurial/chgserver.py +++ b/mercurial/chgserver.py @@ -409,14 +409,23 @@ class chgcmdserver(commandserver.server) # be unbuffered no matter if it is a tty or not. if fn == b'ferr': newfp = fp + elif pycompat.ispy3: + # On Python 3, the standard library doesn't offer line-buffered + # binary streams, so wrap/unwrap it. + if fp.isatty(): + newfp = procutil.make_line_buffered(fp) + else: + newfp = procutil.unwrap_line_buffered(fp) else: - # make it line buffered explicitly because the default is - # decided on first write(), where fout could be a pager. + # Python 2 uses the I/O streams provided by the C library, so + # make it line-buffered explicitly. Otherwise the default would + # be decided on first write(), where fout could be a pager. if fp.isatty(): bufsize = 1 # line buffered else: bufsize = -1 # system default newfp = os.fdopen(fp.fileno(), mode, bufsize) + if newfp is not fp: setattr(ui, fn, newfp) setattr(self, cn, newfp) @@ -440,13 +449,16 @@ class chgcmdserver(commandserver.server) ui = self.ui for (ch, fp, fd), (cn, fn, mode) in zip(self._oldios, _iochannels): newfp = getattr(ui, fn) - # close newfp while it's associated with client; otherwise it - # would be closed when newfp is deleted - if newfp is not fp: + # On Python 2, newfp and fp may be separate file objects associated + # with the same fd, so we must close newfp while it's associated + # with the client. Otherwise the new associated fd would be closed + # when newfp gets deleted. On Python 3, newfp is just a wrapper + # around fp even if newfp is not fp, so deleting newfp is safe. + if not (pycompat.ispy3 or newfp is fp): newfp.close() # restore original fd: fp is open again try: - if newfp is fp and 'w' in mode: + if (pycompat.ispy3 or newfp is fp) and 'w' in mode: # Discard buffered data which couldn't be flushed because # of EPIPE. The data should belong to the current session # and should never persist. diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py --- a/mercurial/utils/procutil.py +++ b/mercurial/utils/procutil.py @@ -80,6 +80,13 @@ def make_line_buffered(stream): return LineBufferedWrapper(stream) +def unwrap_line_buffered(stream): + if isinstance(stream, LineBufferedWrapper): + assert not isinstance(stream.orig, LineBufferedWrapper) + return stream.orig + return stream + + class WriteAllWrapper(object): def __init__(self, orig): self.orig = orig