| Submitter | Yuya Nishihara |
|---|---|
| Date | Dec. 19, 2020, 2:49 a.m. |
| Message ID | <09beb9a133f168111fbe.1608346160@lemosa> |
| Download | mbox | patch |
| Permalink | /patch/47939/ |
| State | Accepted |
| Headers | show |
Comments
On 12/19/20 3:49 AM, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1608289751 -32400 > # Fri Dec 18 20:09:11 2020 +0900 > # Node ID 09beb9a133f168111fbef4a729f1c0cc2bfb87ce > # Parent ca07a5705bf78401bf7c910b9be9a6c5a16ceb64 > procutil: assign pseudo file object if sys.stdout/stderr is missing > > This basically simulates the Python 2 behavior. If libc stdio were used, > these file objects would be available and raise EBADF. There is subtle > difference between py2 and py3, but I think py3 behavior (i.e. exit 255) > is more correct. Does this means you are considering moving to the py3 ways once we drop py2 support ?
On Mon, 21 Dec 2020 08:15:29 +0100, Pierre-Yves David wrote: > On 12/19/20 3:49 AM, Yuya Nishihara wrote: > > # HG changeset patch > > # User Yuya Nishihara <yuya@tcha.org> > > # Date 1608289751 -32400 > > # Fri Dec 18 20:09:11 2020 +0900 > > # Node ID 09beb9a133f168111fbef4a729f1c0cc2bfb87ce > > # Parent ca07a5705bf78401bf7c910b9be9a6c5a16ceb64 > > procutil: assign pseudo file object if sys.stdout/stderr is missing > > > > This basically simulates the Python 2 behavior. If libc stdio were used, > > these file objects would be available and raise EBADF. There is subtle > > difference between py2 and py3, but I think py3 behavior (i.e. exit 255) > > is more correct. > > Does this means you are considering moving to the py3 ways once we drop > py2 support ? No for availability of procutil.stdin/out/err. Setting them to None would be source of problems. I just meant exiting with EBADF (and code 255) is more correct if stdout fd is closed. I don't know why it doesn't on Python 2.
Patch
diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py --- a/mercurial/utils/procutil.py +++ b/mercurial/utils/procutil.py @@ -131,17 +131,25 @@ def _make_write_all(stream): if pycompat.ispy3: - # Python 3 implements its own I/O streams. + # Python 3 implements its own I/O streams. Unlike stdio of C library, + # sys.stdin/stdout/stderr may be None if underlying fd is closed. + # TODO: .buffer might not exist if std streams were replaced; we'll need # a silly wrapper to make a bytes stream backed by a unicode one. - # sys.stdin can be None - if sys.stdin: + if sys.stdin is None: + stdin = BadFile() + else: stdin = sys.stdin.buffer + if sys.stdout is None: + stdout = BadFile() else: - stdin = BadFile() - stdout = _make_write_all(sys.stdout.buffer) - stderr = _make_write_all(sys.stderr.buffer) + stdout = _make_write_all(sys.stdout.buffer) + if sys.stderr is None: + stderr = BadFile() + else: + stderr = _make_write_all(sys.stderr.buffer) + if pycompat.iswindows: # Work around Windows bugs. stdout = platform.winstdout(stdout) diff --git a/tests/test-basic.t b/tests/test-basic.t --- a/tests/test-basic.t +++ b/tests/test-basic.t @@ -49,6 +49,31 @@ Writes to stdio succeed and fail appropr [255] #endif +On Python 3, stdio may be None: + + $ hg debuguiprompt --config ui.interactive=true 0<&- + abort: Bad file descriptor + [255] + $ hg version -q 0<&- + Mercurial Distributed SCM * (glob) + +#if py3 + $ hg version -q 1>&- + abort: Bad file descriptor + [255] +#else + $ hg version -q 1>&- +#endif + $ hg unknown -q 1>&- + hg: unknown command 'unknown' + (did you mean debugknown?) + [255] + + $ hg version -q 2>&- + Mercurial Distributed SCM * (glob) + $ hg unknown -q 2>&- + [255] + $ hg commit -m test This command is ancient: