Patchwork [4,of,4,V2] procutil: assign pseudo file object if sys.stdout/stderr is missing

login
register
mail settings
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

Yuya Nishihara - Dec. 19, 2020, 2:49 a.m.
# 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.

"if" conditions are adjust so that they look similar to
dispatch.initstdio().
Pierre-Yves David - Dec. 21, 2020, 7:15 a.m.
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 ?
Yuya Nishihara - Dec. 21, 2020, 7:45 a.m.
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: