# HG changeset patch
# User FUJIWARA Katsunori <fo...@lares.dti.ne.jp>
# Date 1477678088 -32400
#      Sat Oct 29 03:08:08 2016 +0900
# Branch stable
# Node ID 51b14d2127fbd214c5ec57f9c1ef17457c61f93d
# Parent  141cb12c0175d9e4fbdab1f69d99be24d50ce3f4
mail: use popen4 instead of popen for portability

On Windows platform, the process spawned by util.popen("w") doesn't
work as expected, if it writes anything into stdout of it. In such
case, spawned child process terminates with status code 68864 (at
least, simple .bat script does so).

Maybe, this is a variant of python issue below, which is handled in
popen() in windows.py of Mercurial.

    http://bugs.python.org/issue1366

This patch shows stderr output of spawned process at first, to prevent
users from overlooking serious information after long stdout output.

This ui.warn() invocation causes flushing messages buffered by ui
before popen4(). Therefore, this patch also moves ui.note() output in
test-patchbomb.t.

This patch chooses util.popen4(), because other util.popen* family
doesn't provide the way to get exit code of spawned process.

BTW, other clients except for spawning pager process in patchbomb use
util.popen() only in "read" mode. They (spawning pager, too) should be
safe and portable enough, even though discarding stderr on Windows
might have to be fixed for safety.

diff --git a/mercurial/mail.py b/mercurial/mail.py
--- a/mercurial/mail.py
+++ b/mercurial/mail.py
@@ -160,9 +160,15 @@ def _sendmail(ui, sender, recipients, ms
     cmdline = '%s -f %s %s' % (program, util.email(sender),
                                ' '.join(map(util.email, recipients)))
     ui.note(_('sending mail: %s\n') % cmdline)
-    fp = util.popen(cmdline, 'w')
-    fp.write(msg)
-    ret = fp.close()
+    p = util.popen4(cmdline)[3]
+    outdata, errdata = p.communicate(msg)
+    errdata = errdata.rstrip()
+    if errdata:
+        ui.warn(errdata + '\n')
+    outdata = outdata.rstrip()
+    if outdata:
+        ui.write(outdata + '\n')
+    ret = p.returncode
     if ret:
         raise error.Abort('%s %s' % (
             os.path.basename(program.split(None, 1)[0]),
diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
--- a/tests/test-patchbomb.t
+++ b/tests/test-patchbomb.t
@@ -2831,6 +2831,9 @@ single rev
   
   warning: invalid patchbomb.intro value "mpmwearaclownnose"
   (should be one of always, never, auto)
+  
+  sending [PATCH] test ...
+  sending mail: $TESTTMP/t2/pretendmail.sh -f test foo
   -f test foo
   Content-Type: text/plain; charset="us-ascii"
   MIME-Version: 1.0
@@ -2861,9 +2864,6 @@ single rev
   @@ -1,1 +1,2 @@
    d
   +d
-  
-  sending [PATCH] test ...
-  sending mail: $TESTTMP/t2/pretendmail.sh -f test foo
 
 Test pull url header
 =================================
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to