logtoprocess: connect all fds to /dev/null to avoid bad interaction with pager
authorBoris Feld <boris.feld@octobus.net>
Fri, 03 Nov 2017 21:11:07 +0100
changeset 39926 c4a3d3c67c4f
parent 39925 dfca83594145
child 39927 59592ac26f85
logtoprocess: connect all fds to /dev/null to avoid bad interaction with pager We detected that pager is waiting for log-to-process script to finish, which is annoying when adding a script on commandfinish that does an HTTP push. There seems to be no workaround on the script side and it will make the behavior on Linux/MacOS closer to the Windows behavior. The drawback is that it makes the related tests more flaky as log-to-process outputs are now really asynchronous. If it's considered a BC change, another option would be to add a config option for this new behavior. I personally think that the different behavior between Windows and Linux is confusing and that it's a bug I would be fine with a new config option. Differential Revision: https://phab.mercurial-scm.org/D4816
hgext/logtoprocess.py
tests/test-logtoprocess.t
--- a/hgext/logtoprocess.py	Fri Nov 03 21:35:36 2017 +0100
+++ b/hgext/logtoprocess.py	Fri Nov 03 21:11:07 2017 +0100
@@ -83,11 +83,16 @@
             else:
                 newsession = {'start_new_session': True}
             try:
-                # connect stdin to devnull to make sure the subprocess can't
-                # muck up that stream for mercurial.
+                # connect std* to devnull to make sure the subprocess can't
+                # muck up these stream for mercurial.
+                # Connect all the streams to be more close to Windows behavior
+                # and pager will wait for scripts to end if we don't do that
+                nullrfd = open(os.devnull, 'r')
+                nullwfd = open(os.devnull, 'w')
                 subprocess.Popen(
                     procutil.tonativestr(script),
-                    shell=True, stdin=open(os.devnull, 'r'),
+                    shell=True, stdin=nullrfd,
+                    stdout=nullwfd, stderr=nullwfd,
                     env=procutil.tonativeenv(env),
                     close_fds=True, **newsession)
             finally:
--- a/tests/test-logtoprocess.t	Fri Nov 03 21:35:36 2017 +0100
+++ b/tests/test-logtoprocess.t	Fri Nov 03 21:11:07 2017 +0100
@@ -121,4 +121,4 @@
   $ touch $TESTTMP/wait-for-touched
   $ sleep 0.2
   $ test -f $TESTTMP/touched && echo "SUCCESS Pager is not waiting on ltp" || echo "FAIL Pager is waiting on ltp"
-  FAIL Pager is waiting on ltp
+  SUCCESS Pager is not waiting on ltp