On Mon, Nov 8, 2010 at 11:20 AM, Julian Foad <julian.foad_at_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
Received on 2010-11-08 18:06:09 CET