[PATCH V3] posix: update os.popen with subprocess (issue4746)

Yuya Nishihara yuya at tcha.org
Sat Mar 4 22:43:48 EST 2017


On Sat, 4 Mar 2017 12:40:54 +0530, Rishabh Madan wrote:
> > Oops, I noticed this wasn't that simple. The issue 4746 says the exit code
> > is wrongly interpreted, which is a return value of fp.close(), where fp
> > would
> > be closed by pclose(). However, a file created by subprocess isn't
> > special, so
> > its close() would return no value.
> 
> So is there any other way we could resolve this issue

Some ideas:

 a. reintroduce old version of explainexit() by different name
 b. stop using util.popen(), and add new utility function that constructs
    subprocess.Popen() in friendlier way
 c. add wrapper class to emulate behavior of popen

(a) is simple, though it might not be nice in API point of view. The simplicity
should sell. (b) seems also good as it would eventually get rid of util.popen*()
family, which appear to rely on GC to wait child processes. (c) would work but
seems cumbersome so I don't like it.

> or should we just
> stick with no return value?

No. It's a bug. We'll need a test to cover a failure of external patch command.


More information about the Mercurial-devel mailing list