Patchwork [3,of,5] histedit: do not close stdin

login
register
mail settings
Submitter Jun Wu
Date March 15, 2016, 9:58 a.m.
Message ID <c769f8196ccfdfa54686.1458035914@x1c>
Download mbox | patch
Permalink /patch/13893/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 15, 2016, 9:58 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1458002553 0
#      Tue Mar 15 00:42:33 2016 +0000
# Node ID c769f8196ccfdfa546862b0c4f608b78ca80a1c7
# Parent  cab663c8ba601c8ed617d18344ed8e232f86284b
histedit: do not close stdin

Closing stdin is unexpected by chgserver and is not a good idea generally.
This patch refactors related code a bit and make sure stdin is not closed.
It will make chg much happier on test-histedit*.t.
Yuya Nishihara - March 17, 2016, 2:09 p.m.
On Tue, 15 Mar 2016 09:58:34 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1458002553 0
> #      Tue Mar 15 00:42:33 2016 +0000
> # Node ID c769f8196ccfdfa546862b0c4f608b78ca80a1c7
> # Parent  cab663c8ba601c8ed617d18344ed8e232f86284b
> histedit: do not close stdin
> 
> Closing stdin is unexpected by chgserver and is not a good idea generally.

Sure, queued 2 and 3 to the clowncopter, thanks.

> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -998,6 +998,13 @@
>          return goaleditplan
>      return goalnew
>  
> +def _readfile(path):
> +    if path == '-':
> +        return sys.stdin.read()
> +    else:
> +        with open(path, 'rb') as f:
> +            return f.read()

OT: reading sys.stdin doesn't work in pure command server. it should be
ui.fin.read().

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -998,6 +998,13 @@ 
         return goaleditplan
     return goalnew
 
+def _readfile(path):
+    if path == '-':
+        return sys.stdin.read()
+    else:
+        with open(path, 'rb') as f:
+            return f.read()
+
 def _validateargs(ui, repo, state, freeargs, opts, goal, rules, revs):
     # TODO only abort if we try to histedit mq patches, not just
     # blanket if mq patches are applied somewhere
@@ -1190,12 +1197,7 @@ 
                                  node.short(state.topmost))
         rules = ruleeditor(repo, ui, state.actions, comment)
     else:
-        if rules == '-':
-            f = sys.stdin
-        else:
-            f = open(rules)
-        rules = f.read()
-        f.close()
+        rules = _readfile(rules)
     actions = parserules(rules, state)
     ctxs = [repo[act.nodetoverify()] \
             for act in state.actions if act.nodetoverify()]
@@ -1236,12 +1238,7 @@ 
         actions = [pick(state, r) for r in revs]
         rules = ruleeditor(repo, ui, actions, comment)
     else:
-        if rules == '-':
-            f = sys.stdin
-        else:
-            f = open(rules)
-        rules = f.read()
-        f.close()
+        rules = _readfile(rules)
     actions = parserules(rules, state)
     warnverifyactions(ui, repo, actions, state, ctxs)