We are shipping a fairly old version of subprocess.py. Some development has happened since I was involved. Some things are nice (such as restarting read upon EINTR). Others are not so nice (for example, the stupid getstatusoutput which is available in the 3.0 version (about to be removed though, see http://bugs.python.org/issue9922). In any case, we need to ship a version which is compatible with Python 2.3.
One alternative is raising our requirements to Python 2.4 and use the system's subprocess.
On bug 4213, we are considering upgrading to Python 2.4. In that case, we want to look out for possible subprocess.py bugs in 2.4.0. It was released 2004-11-30. This is what I've found: * check_call was added in 2.5 * check_output was added in 2.7 * Popen.send_signal(), Popen.terminate(), Popen.kill() was added in 2.6 The version we are currently shipping was added 2007-07-12, from Python release25-maint branch. Thus, we do not need to consider features added in later versions; we are not using them. Python 2.5.0 was released 2006-10-11. I've checked our tree, and the only place where check_call is used is: /home/astrand/ctc/vsm/test-tlsession ie not in the runtime product. As when it comes to important bug fixes between 2004-11-30 and 2007-07-12, I've found: ------------------------------------------------------------------------ r38169 | astrand | 2005-01-01 10:38:57 +0100 (lör, 01 jan 2005) | 3 lines On UNIX, when the execution of the child fails, we must waitpid() to prevent leaving zombies. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r38559 | astrand | 2005-03-03 22:10:23 +0100 (tor, 03 mar 2005) | 2 lines Corrected bug in list2cmdline wrt backslashes. Fixes #1083306. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r45234 | martin.v.loewis | 2006-04-10 17:55:37 +0200 (mån, 10 apr 2006) | 5 lines Patch #1467770: Add Popen objects to _active only in __del__. Introduce _child_active member to keep track on whether a child needs to be waited for. Backport candidate. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r46651 | georg.brandl | 2006-06-05 00:15:37 +0200 (mån, 05 jun 2006) | 2 lines Bug #1500293: fix memory leaks in _subprocess module. ------------------------------------------------------------------------ r47077 | peter.astrand | 2006-06-22 22:21:26 +0200 (tor, 22 jun 2006) | 1 line Applied patch #1506758: Prevent MemoryErrors with large MAXFD. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r50638 | peter.astrand | 2006-07-14 16:04:45 +0200 (fre, 14 jul 2006) | 1 line Bug #1223937: CalledProcessError.errno -> CalledProcessError.returncode. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r50720 | georg.brandl | 2006-07-20 18:28:39 +0200 (tor, 20 jul 2006) | 3 lines Guard for _active being None in __del__ method. ------------------------------------------------------------------------ r51758 | gustavo.niemeyer | 2006-09-06 03:58:52 +0200 (ons, 06 sep 2006) | 3 lines Fixing #1531862: Do not close standard file descriptors in the subprocess module. ------------------------------------------------------------------------ Plus more. IOW, I don't think we want to use subprocess.py from 2.4.0 with TL. One other idea is to require, say, 2.4.3, released 2006-04-25. Critical bug fixes between that and what we are using now are: ------------------------------------------------------------------------ r50638 | peter.astrand | 2006-07-14 16:04:45 +0200 (fre, 14 jul 2006) | 1 line Bug #1223937: CalledProcessError.errno -> CalledProcessError.returncode. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r50720 | georg.brandl | 2006-07-20 18:28:39 +0200 (tor, 20 jul 2006) | 3 lines Guard for _active being None in __del__ method. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r51758 | gustavo.niemeyer | 2006-09-06 03:58:52 +0200 (ons, 06 sep 2006) | 3 lines Fixing #1531862: Do not close standard file descriptors in the subprocess module. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r53295 | peter.astrand | 2007-01-07 15:34:16 +0100 (sön, 07 jan 2007) | 1 line Avoid O(N**2) bottleneck in _communicate_(). Fixes #1598181. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r53624 | peter.astrand | 2007-02-02 20:06:36 +0100 (fre, 02 feb 2007) | 1 line We had several if statements checking the value of a fd. This is unsafe, since valid fds might be zero. We should check for not None instead. ------------------------------------------------------------------------ ------------------------------------------------------------------------ r54918 | georg.brandl | 2007-04-21 22:35:38 +0200 (lör, 21 apr 2007) | 3 lines Bug #1704790: bind name "sys" locally in __del__ method so that it is not cleared before __del__ is run. ------------------------------------------------------------------------ In other wors, it seems we need to continue shipping our own subprocess.py a little bit longer.
Bug 4213 has been solved and we're now requiring Python 2.4, but we're still shipping our own subprocess module. What are the remaining obstacles for starting to use the system subprocess?
We could move the imports in a lot of files to before we modify sys.path now, but I'm not sure it's worth the noise.
Upgrade works fine and can't see any issues with callers of subprocess. Should be good to go.
> * subprocess.py should no longer be included in any thinlinc package Yup. > # dpkg -L $(dpkg -l | grep thinlinc | awk '{print $2}') | grep subprocess > # > * subprocess.py (and .pyc) should be gone after an upgrade Yup. Tested on a 32-bit Debian 9 VM with tl-4.9.0->4.9.0post-5867.
As a note, it's unclear why a different approach to removing the .pyc file was taken compared to the removal of xmlrpclib, where tl-setup removes the .pyc file. I would have preferred a single approach, but I'm not going to be picky about it and reopen the bug. It's fine for now.
