[pypy-dev] os.wait() implementation

[Dan Roberts]

Hi Armin,

Thanks for the reply, I've attached a new version of my patch.  At first
glance it would appear I've done little to address your concerns,
however there's good reason for this.  As it turns out, our
implementation of popen() depends on execvp(), which is only implemented
in the os module (specifically, the one borrowed from CPython).  I did
write an interpreter level version of execvp(), which would have allowed
me to remove the imports of os from popen(), however I don't think it's
wise to replace our existing application level implementation with my
interpreter level implementation.  Because of that, I can't remove the
os dependency from popen(). For consistency, i left os in both popen()
and popenfile because they directly interact. I did, however, modify
wait() to use posix instead.

Even though I didn't eliminate the os imports, my test *did* improve a
bit from the old version, so this iteration isn't totally useless. :-)
All posix tests are passing again, including the new os.wait() test.
I'll be happy to address any other concerns you, or anyone else have,
and if you'd like to remove those os imports, I'd like to talk to you
about how to best approach that.

Cheers,
Dan

On Sun, 2009-11-22 at 12:29 +0100, Armin Rigo wrote:
> Hi Dan,
> 
> On Sat, Nov 21, 2009 at 12:00:38PM -0800, Dan Roberts wrote:
> >    I now have an application-level implementation of os.wait() complete.
> > It's built on top of os.waitpid(), but according to the documentation
> > I've found, the behavior should be the same.
> 
> Thank you !
> 
> > +    def wait():
> > +        import os
> > +        return os.waitpid(-1, 0)
> 
> It's more a general issue with app_posix.py, but I think that it should
> avoid these costly "import os" inside application-level functions.  You
> already have the posix module imported (see the start of app_posix.py),
> so you can return posix.waitpid(-1, 0).  The same comment applies to
> other places too, e.g. popenfile.close().
> 
> 
> A bientot,
> 
> Armin.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pypy_os_wait_complete.diff
Type: text/x-patch
Size: 3237 bytes
Desc: not available
URL: <http://mail.python.org/pipermail/pypy-dev/attachments/20091125/e38e6c9f/attachment.bin>