[PATCH] chgserver: backport py3 buffered I/O workarounds from procutil

[Augie Fackler]

queued, thanks

> On Nov 17, 2020, at 08:04, Yuya Nishihara <yuya at tcha.org> wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya at 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 at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel