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.
Index: module/posix/test/test_posix2.py
===================================================================
--- module/posix/test/test_posix2.py (revision 69512)
+++ module/posix/test/test_posix2.py (working copy)
@@ -290,6 +290,7 @@
for i in range(5):
stream = os.popen('echo 1')
assert stream.read() == '1\n'
+ stream.close()
if hasattr(__import__(os.name), '_getfullpathname'):
def test__getfullpathname(self):
@@ -393,7 +394,23 @@
def test_os_sysconf_error(self):
os = self.posix
raises(ValueError, os.sysconf, "!...@#$%!#$!@#")
+
+ if hasattr(os, 'wait'):
+ def test_os_wait(self):
+ os = self.posix
+ exit_status = 0x33
+ if not hasattr(os, "fork"):
+ skip("Need fork() to test wait()")
+ child = os.fork()
+ if child == 0: # in child
+ os._exit(exit_status)
+ else:
+ pid, status = os.wait()
+ assert child == pid
+ assert os.WIFEXITED(status)
+ assert os.WEXITSTATUS(status) == exit_status
+
if hasattr(os, 'fsync'):
def test_fsync(self):
os = self.posix
Index: module/posix/__init__.py
===================================================================
--- module/posix/__init__.py (revision 69512)
+++ module/posix/__init__.py (working copy)
@@ -26,6 +26,9 @@
'popen3' : 'app_posix.popen3',
'popen4' : 'app_posix.popen4',
})
+
+ if hasattr(os, 'wait'):
+ appleveldefs['wait'] = 'app_posix.wait'
interpleveldefs = {
'open' : 'interp_posix.open',
Index: module/posix/app_posix.py
===================================================================
--- module/posix/app_posix.py (revision 69512)
+++ module/posix/app_posix.py (working copy)
@@ -153,6 +153,9 @@
try_close(read_end)
raise Exception, e # bare 'raise' does not work here :-(
+ def wait():
+ return posix.waitpid(-1, 0)
+
else:
# Windows implementations
@@ -170,6 +173,7 @@
univ_nl = ('b' not in mode)
import subprocess
+
if mode.startswith('r'):
proc = subprocess.Popen(cmd,
shell=True,
@@ -194,7 +198,6 @@
raise ValueError("invalid mode %r" % (mode,))
import subprocess
-
p = subprocess.Popen(cmd, shell=True, bufsize=bufsize,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
@@ -210,7 +213,6 @@
raise ValueError("invalid mode %r" % (mode,))
import subprocess
-
p = subprocess.Popen(cmd, shell=True, bufsize=bufsize,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
@@ -228,7 +230,6 @@
raise ValueError("invalid mode %r" % (mode,))
import subprocess
-
p = subprocess.Popen(cmd, shell=True, bufsize=bufsize,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
_______________________________________________
[email protected]
http://codespeak.net/mailman/listinfo/pypy-dev
