[PATCH 4 of 5 STABLE] mail: use popen4 instead of popen for portability
Yuya Nishihara
yuya at tcha.org
Fri Oct 28 23:22:56 EDT 2016
On Sat, 29 Oct 2016 03:16:49 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy at 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.
I don't think this change is good for stable considering how many people
would actually use the sendmail method on Windows.
> --- 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
Maybe we can factor out a helper to run an external command with ui.fout and
ui.ferr. util.system() does a similar thing in a slightly different way. It
would be a bit unfortunate we have many variants of process handling.
More information about the Mercurial-devel
mailing list