[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