Use of popen2 deprecated in py2.6

[thijs]

Trial reports DeprecationWarnings for  popen2 in trunk/twisted/test/test_process.py.  Deprecated in py2.6.

DeprecationWarning: The popen2 module is deprecated.  Use the subprocess module.

[exarkun]

We could drop this entirely in favor of running a program with known output. Of course, knowing the stderr of a Python program is pretty hard... Or maybe impossible. Maybe it could be /bin/sh, though.

[thijs]

#3986 was a duplicate of this.

[Markon]

Uhm, I've edited this line:

lsOut = popen2.popen3("/bin/ls ZZXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX")[2].read()

to:

    lsOut = subprocess.Popen("/bin/ls ZZXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX",
                             shell=True, stderr=subprocess.PIPE).communicate()[1]

And tried to run the tests using trial test_process.py. It passes all the tests (except SKIPPED tests). But I've used Python 2.5. Can someone test this new line with Py 2.6? ( you've to just add import subprocess, and go to the lines >= 2280, where you can see lsOut) Bye!

[khorn]

this is a duplicate of #3900

[thijs]

Un-assigning.

[gas]

Have copied over kelly patch from #3900. It appears to work running trial in no extra errors running trial against test_process.py

[exarkun]

Thanks. Note that the keywords field is case sensitive, unfortunately.

[exarkun]

(In [27975]) Branching to 'remove-popen2-3431'

[thijs]

I guess the branch is ready for review so there we go.

• I know it's for internal use but twisted/python/_release.py (and it's tests) still uses popen2, bonus points for fixing that one as well..

Modified files need to have an updated copyright header (2010).

[exarkun]

The branch has another issue as well. Somehow it fails a test in another module:

===============================================================================
[FAIL]: twisted.test.test_tcp_internals.PlatformAssumptionsTestCase.test_acceptOutOfFiles

Traceback (most recent call last):
  File "/var/lib/buildbot/bigdog24-twisted/ubuntu64-py2.5-select/Twisted/twisted/test/test_tcp_internals.py", line 97, in test_acceptOutOfFiles
    self.assertIn(exc.args[0], (EMFILE, ENOBUFS))
twisted.trial.unittest.FailTest: 9 not in (24, 105)
-------------------------------------------------------------------------------

[exarkun]

The test failure comes from a pipe leak in some versions of pygtk. It's only related to the change in this branch in that somehow this change disturbs things slightly and causes the existing problem to become visible.

There is a problem with test_acceptOutOfFiles that should be fixed - it shouldn't close its port if it can't create any other sockets - but I think that will just re-mask the underlying problem.

[exarkun]

I replaced the other usage of popen2 as well. However, merging this would still turn 3 supported builders red.

Arguably they shouldn't really be supported - they have a version of pygtk which leaks file descriptors. What we can do about this, though, isn't obvious to me.

[thijs]

#4294 was a duplicate of this.

[thijs]

Replying to exarkun:

I replaced the other usage of popen2 as well. However, merging this would still turn 3 supported builders red. Arguably they shouldn't really be supported - they have a version of pygtk which leaks file descriptors. What we can do about this, though, isn't obvious to me.

Let's contact the maintainers of those buildslaves so they can upgrade their version of pygtk? I think those issues shouldn't block this ticket though and I'm putting it back up for review.

[TimAllen]

Looks good to me.

[exarkun]

(In [29042]) Branching to 'remove-popen2-3431-2'

[exarkun]

(In [29075]) Raise the artificial limit to 512 to work-around the pygtk fd leak

refs #3431

See also  https://bugs.launchpad.net/ubuntu/+source/gtk+2.0/+bug/363245

[exarkun]

Made a few final changes. Nothing really huge. Would be nice for someone to take one last look, though. Most recent  build results.

[glyph]

Just a few nits to pick, then merge:

1. testStderr should be renamed test_stderr as long as you're touching it.
2. "produces the correct result", really? testStderr's docstring should phrase that in a way that doesn't leave the "correct result" up to the imagination.

Other than that, looks good, merge.

[exarkun]

(In [29077]) Fix testStderr a bit

refs #3431

[exarkun]

(In [29078]) Merge remove-popen2-3431-2

Author: kelly, exarkun Reviewer: thijs, TimAllen, glyph Fixes: #3431

Remove the uses of the deprecated popen2 module, replacing them with the subprocess module.

Also, raise the fd limit set by a test in test_tcp_internals. This is really an unrelated change, but the popen2 changes in this branch shuffle things around just enough to make a separate existing issue more likely to arise. Raising this limit gives us some more time to fix the real cause of these other failures (pygtk file descriptor leak, see  https://bugs.launchpad.net/ubuntu/+source/gtk+2.0/+bug/363245 for further details).