[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