On Mon, Nov 8, 2010 at 11:20 AM, Julian Foad <julian.f...@wandisco.com> wrote: > Here's a patch to fix the Windows buildbot failures that started at my > r1031610. The problem is our _quote_arg() function doesn't do the right > thing with a trailing backslash. For a solution it seems better to > simply pass the args separately to subprocess.Popen() and let it do the > quoting, than to try to fix and maintain our own quoting function. And > we can use the undocumented subprocess.list2cmdline() function for > generating a command-line string for display purposes. (We could also > use that for generating a single-string argument to Popen, but we don't > need to because it does it for us if we pass a list of arguments.) > > I tested this in a WinXP VM, and it seemed to work properly with Python > 2.4.1 and 2.4.3 and 2.7. (I ran 1.7-trunk's prop_tests.py 7 against an > installed svn 1.6.13, and it passed.) > > [[[ > Fix Windows command-line argument quoting in the Python test harness. > Arguments ending with a backslash were not correctly quoted. > This reverts r875257. > > * subversion/tests/cmdline/svntest/main.py > (_safe_arg_re, _quote_arg): Delete, as this didn't quote properly. > (open_pipe): Pass the list of arguments directly to subprocess.Popen() > instead of trying to quote it ourselves. Use subprocess.list2cmdline() > to generate a command line for display. > (spawn_process): Use subprocess.list2cmdline() instead of _quote_arg() > to generate a command line for display. > ]]] > > It reverts Paul's r875257, which was: > > [[[ > Fix test suite's use of subprocess on earlier versions of Python and thus > fix the djh-xp-vse2005 buildbot. > > * subversion/tests/cmdline/svntest/main.py > (open_pipe2): Quote arguments to subprocess.Popen(). Later versions of > subprocess (2.5.2+ that I know of for sure) don't require this quoting, but > versions < 2.4.3 do. > ]]] > > I was unable to see exactly what the problem was that r875257 fixed. I > would like to be able to say why it's OK to revert it, but I don't yet > know. > > Can Paul or anyone comment or test or review? > > Or shall I just commit this current patch and see if anyone has > problems?
Hi Julian, +1 on "commit this current patch and see if anyone has problems". You've confirmed it works on Windows with 2.4.1, 2.4.3 and 2.7 (and 2.4 is the minimum we claim to support). I can confirm it works with 2.6.5. Paul