[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix Windows test failures - Python test suite bug in quoting a trailing backslash

From: Paul Burba <ptburba_at_gmail.com>
Date: Mon, 8 Nov 2010 12:05:33 -0500

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

This is an archived mail posted to the Subversion Dev mailing list.