Patchwork D9059: procutil: assign stdio objects if they are None

login
register
mail settings
Submitter phabricator
Date Sept. 19, 2020, 12:07 a.m.
Message ID <differential-rev-PHID-DREV-fnyprtnpo7behqurkmgc-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/47234/
State Superseded
Headers show

Comments

phabricator - Sept. 19, 2020, 12:07 a.m.
quark created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  On Python 3 stdio objects can be None. That causes crashes. Fix it by opening
  devnull automatically.
  
  `stdin` can be `None` by using `0<&-` in bash, or spawning processes less
  carefully, for example, watchman used to cause such `None` stdin [1] (note:
  None is only observable on Python 3).
  
  [1]: https://github.com/facebook/watchman/commit/d241978aaa6b6d7c5b7260bc9e6d699d3a1cea53

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/utils/procutil.py
  tests/test-stdio-missing.t

CHANGE DETAILS




To: quark, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
Yuya Nishihara - Dec. 19, 2020, 2:54 a.m.
As Pulkit pointed out, I and Pulkit made another version in parallel.

https://patchwork.mercurial-scm.org/patch/47937/
https://patchwork.mercurial-scm.org/patch/47938/
https://patchwork.mercurial-scm.org/patch/47939/

`sys.std*` are left unmodified. `procutil.std*` are fixed up to raise EBADF.

Patch

diff --git a/tests/test-stdio-missing.t b/tests/test-stdio-missing.t
new file mode 100644
--- /dev/null
+++ b/tests/test-stdio-missing.t
@@ -0,0 +1,13 @@ 
+  $ cat > prompt.py << 'EOF'
+  > from mercurial import exthelper
+  > eh = exthelper.exthelper()
+  > cmdtable = eh.cmdtable
+  > @eh.command(b'prompt', [], norepo=True)
+  > def prompt(ui):
+  >     chosen = ui.promptchoice(b"is stdin present? (y/N) $$ &Yes $$ &No", default=1)
+  >     ui.write(b"chosen: %d\n" % chosen)
+  > EOF
+
+  $ hg --config extensions.prompt=prompt.py prompt 0<&-
+  is stdin present? (y/N)  n
+  chosen: 1
diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
--- a/mercurial/utils/procutil.py
+++ b/mercurial/utils/procutil.py
@@ -114,6 +114,11 @@ 
 
 
 if pycompat.ispy3:
+    # Stdio objects can be 'None' on Python 3. Most code paths (for example,
+    # dispatch.initstdio) do not expect that. Fix them by opening devnull.
+    sys.stdin = sys.stdin or open(os.devnull, "r")
+    sys.stdout = sys.stdout or open(os.devnull, "w")
+    sys.stderr = sys.stderr or sys.stdout
     # Python 3 implements its own I/O streams.
     # 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.