| msg171856 - (view) |
Author: Chris Jerdonek (chris.jerdonek) *  |
Date: 2012-10-03 06:19 |
| The executable argument to Popen() takes precedence over args[0] when the executable argument is provided. The test suite should include a test of this that runs on all systems. The test suite does not currently include such a test. This test is a precursor to more specific, platform-specific tests that will be added for issue 16114. |
|
|
| msg171919 - (view) |
Author: Kushal Das (kushal.das) * |
Date: 2012-10-04 05:09 |
| Attached patch which has a new test called test_executable_precedence to test precedence of executable argument over args[0]. |
|
|
| msg172123 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-10-05 19:53 |
| New changeset 0df5aeab229f by Andrew Svetlov in branch '3.3': Issue #16115: Add test for check that executable arg to Popen() takes precedence over args[0] arg\n \n Patch by Kushal Das http://hg.python.org/cpython/rev/0df5aeab229f New changeset 0fcfb6066e17 by Andrew Svetlov in branch 'default': Merge issue #16115: Add test for check that executable arg to Popen() takes precedence over args[0] arg\n \n Patch by Kushal Das http://hg.python.org/cpython/rev/0fcfb6066e17 |
|
|
| msg172124 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-05 20:01 |
| Committed. Thank you, Kushal Das. BTW, please fill http://www.python.org/psf/contrib/ as contributor of Python project. We would to get that agreement from everybody who has pushed any patch. Thanks again. |
|
|
| msg172127 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-05 20:11 |
| Actually, I was still working on this. I had assigned it to myself. + p = subprocess.Popen(["nonexistent","-c",'import sys; sys.exit(42)'], + executable=sys.executable, cwd=python_dir) The test for the executable argument should really test the executable argument independent of the cwd argument. Also, it would be better if the test uses a valid args[0] to demonstrate precedence. I had prepared a test but did not upload it yet, because I also wanted to add an accompanying clarification to the docs which I am still preparing. Reopening. |
|
|
| msg172132 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-05 20:32 |
| ok, it will be better. |
|
|
| msg172133 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-05 20:35 |
| Thanks, Andrew. I'm hoping to finish preparing the patch sometime this weekend. |
|
|
| msg172194 - (view) |
Author: Kushal Das (kushal.das) * |
Date: 2012-10-06 12:58 |
| On Saturday, October 6, 2012, Andrew Svetlov wrote: > > Andrew Svetlov added the comment: > > Committed. Thank you, Kushal Das. > BTW, please fill http://www.python.org/psf/contrib/ as contributor of > Python project. > We would to get that agreement from everybody who has pushed any patch. > Thanks again. Hi, I already filled and sent it. Thanks for the commit. Kushal |
|
|
| msg172366 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-08 08:27 |
| Proposed patch attached. I verified that the tests included in the patch pass on 2.7, 3.2, 3.3, and default (replacing FileNotFoundError with OSError as necessary). I also verified that the tests pass on Windows for default. The patch also includes improvements to the parts of Popen()'s documentation that relate to the added tests. The improvements simplify the documentation by more clearly separating the argument-specific documentation from the default/general behavior. Previously, certain general information was under various argument-specific paragraphs which made the documentation less clear. |
|
|
| msg172370 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-08 09:44 |
| I also checked the tests in the patch on Windows for 2.7. |
|
|
| msg172380 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-08 13:39 |
| Hmm, I dislike "Normally, *args* should be a sequence." From my perspective better to say something like: *args* should be a sequence if *shell* is *False* or string of *shell* is *True* or something like this. I would to directly recommend reader to follow that schema, see also http://bugs.python.org/issue7839 |
|
|
| msg172393 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-08 16:13 |
| Thanks for the good suggestion and pointer to the relevant discussion, Andrew. I agree re: the sentence beginning with "normally." Incidentally, that was existing language that I had preserved. I'm attaching an updated patch that incorporates your suggestion. |
|
|
| msg172394 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-08 16:22 |
| LGTM. Thanks, Chris. |
|
|
| msg172424 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2012-10-08 23:02 |
| New changeset 862430c68fec by Chris Jerdonek in branch '3.3': Issue #16115: Improve testing of the executable argument to subprocess.Popen(). http://hg.python.org/cpython/rev/862430c68fec New changeset 1b37fc50dc1b by Chris Jerdonek in branch 'default': Issue #16115: Merge test improvements from 3.3. http://hg.python.org/cpython/rev/1b37fc50dc1b |
|
|
| msg172425 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-08 23:03 |
| I am pushing the documentation changes separately (which will include 2.7 and 3.2). |
|
|
| msg172426 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-08 23:19 |
| The test_executable test fails on AMD64 Ubuntu LTS. For example: http://buildbot.python.org/all/builders/AMD64%20Ubuntu%20LTS%203.3/builds/38/steps/test/logs/stdio FAIL: test_executable (test.test_subprocess.ProcessTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_subprocess.py", line 205, in test_executable self._assert_python(["doesnotexist", "-c"], executable=sys.executable) File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_subprocess.py", line 201, in _assert_python self.assertEqual(47, p.returncode) AssertionError: 47 != -6 ====================================================================== FAIL: test_executable (test.test_subprocess.ProcessTestCaseNoPoll) ---------------------------------------------------------------------- Traceback (most recent call last): File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_subprocess.py", line 205, in test_executable self._assert_python(["doesnotexist", "-c"], executable=sys.executable) File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_subprocess.py", line 201, in _assert_python self.assertEqual(47, p.returncode) AssertionError: 47 != -6 This test passed locally (at least for me) on Mac OS X and Windows. |
|
|
| msg172429 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2012-10-09 00:55 |
| New changeset ef90c5e482f4 by Chris Jerdonek in branch '3.3': Issue #16115: Skip a newly added subprocess.Popen() test on Linux. http://hg.python.org/cpython/rev/ef90c5e482f4 New changeset 96b8eb3d3208 by Chris Jerdonek in branch 'default': Issue #16115: Merge test skip from 3.3. http://hg.python.org/cpython/rev/96b8eb3d3208 |
|
|
| msg172430 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-09 01:03 |
| More information on the failure above (from the same buildbot link): test_executable (test.test_subprocess.ProcessTestCase) ... Could not find platform independent libraries Could not find platform dependent libraries <exec_prefix> Consider setting $PYTHONHOME to [:<exec_prefix>] Fatal Python error: Py_Initialize: Unable to get the locale encoding ImportError: No module named 'encodings' FAIL |
|
|
| msg172433 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-09 01:43 |
| I created issue 16170 to track fixing the Linux-skip for the test_executable() test. |
|
|
| msg172451 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2012-10-09 06:19 |
| New changeset 3dd012397b11 by Chris Jerdonek in branch '3.2': Issue #16115: Improve subprocess.Popen() documentation around args, shell, and executable arguments. http://hg.python.org/cpython/rev/3dd012397b11 New changeset 0ef3b801ccbc by Chris Jerdonek in branch '3.3': Issue #16115: Merge subprocess.Popen() documentation improvements from 3.2. http://hg.python.org/cpython/rev/0ef3b801ccbc New changeset 91d4593ac974 by Chris Jerdonek in branch 'default': Issue #16115: Merge subprocess.Popen() documentation improvements from 3.3. http://hg.python.org/cpython/rev/91d4593ac974 New changeset 11171bf6a6bd by Chris Jerdonek in branch '2.7': Issue #16115: Backport subprocess.Popen() documentation improvements from 3.2. http://hg.python.org/cpython/rev/11171bf6a6bd |
|
|
| msg172550 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-10 03:26 |
| I would like to make a few more miscellaneous improvements to the Popen() docs, including a clarification resulting from issue #16170, which stemmed from this issue. The proposed patch is attached. |
|
|
| msg172611 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2012-10-11 00:58 |
| New changeset 41ffed580eef by Chris Jerdonek in branch '3.2': Issue #16115: Make further improvements to subprocess.Popen() documentation. http://hg.python.org/cpython/rev/41ffed580eef New changeset bae644e46556 by Chris Jerdonek in branch '3.3': Issue #16115: Merge subprocess.Popen() documentation improvements from 3.2. http://hg.python.org/cpython/rev/bae644e46556 New changeset 178ba363af1c by Chris Jerdonek in branch 'default': Issue #16115: Merge subprocess.Popen() documentation improvements from 3.3. http://hg.python.org/cpython/rev/178ba363af1c New changeset f051e37ac11d by Chris Jerdonek in branch '2.7': Issue #16115: Backport subprocess.Popen() documentation improvements from 3.2. http://hg.python.org/cpython/rev/f051e37ac11d |
|
|