Now with the patch attached.
And here's the email thread about the original change r875257 (aka
r35183): <http://svn.haxx.se/dev/archive-2009-01/0316.shtml>.
On Mon, 2010-11-08 at 16:20 +0000, Julian Foad 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?
>
> - Julian
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.
--This line, and those below, will be ignored--
Index: subversion/tests/cmdline/svntest/main.py
===================================================================
--- subversion/tests/cmdline/svntest/main.py (revision 1032502)
+++ subversion/tests/cmdline/svntest/main.py (working copy)
@@ -332,38 +332,12 @@ def run_command(command, error_expected,
as lists of lines (including line terminators). See run_command_stdin()
for details. If ERROR_EXPECTED is None, any stderr also will be printed."""
return run_command_stdin(command, error_expected, 0, binary_mode,
None, *varargs)
-# A regular expression that matches arguments that are trivially safe
-# to pass on a command line without quoting on any supported operating
-# system:
-_safe_arg_re = re.compile(r'^[A-Za-z\d\.\_\/\-\:\...@]+$')
-
-def _quote_arg(arg):
- """Quote ARG for a command line.
-
- Simply surround every argument in double-quotes unless it contains
- only universally harmless characters.
-
- WARNING: This function cannot handle arbitrary command-line
- arguments. It can easily be confused by shell metacharacters. A
- perfect job would be difficult and OS-dependent (see, for example,
- http://msdn.microsoft.com/library/en-us/vccelng/htm/progs_12.asp).
- In other words, this function is just good enough for what we need
- here."""
-
- arg = str(arg)
- if _safe_arg_re.match(arg):
- return arg
- else:
- if os.name != 'nt':
- arg = arg.replace('$', '\$')
- return '"%s"' % (arg,)
-
def open_pipe(command, bufsize=0, stdin=None, stdout=None, stderr=None):
"""Opens a subprocess.Popen pipe to COMMAND using STDIN,
STDOUT, and STDERR. BUFSIZE is passed to subprocess.Popen's
argument of the same name.
Returns (infile, outfile, errfile, waiter); waiter
@@ -372,21 +346,13 @@ def open_pipe(command, bufsize=0, stdin=
# On Windows subprocess.Popen() won't accept a Python script as
# a valid program to execute, rather it wants the Python executable.
if (sys.platform == 'win32') and (command[0].endswith('.py')):
command.insert(0, sys.executable)
- # Quote only the arguments on Windows. Later versions of subprocess,
- # 2.5.2+ confirmed, don't require this quoting, but versions < 2.4.3 do.
- if sys.platform == 'win32':
- args = command[1:]
- args = ' '.join([_quote_arg(x) for x in args])
- command = command[0] + ' ' + args
- command_string = command
- else:
- command_string = ' '.join(command)
+ command_string = command[0] + ' ' + subprocess.list2cmdline(command[1:])
if not stdin:
stdin = subprocess.PIPE
if not stdout:
stdout = subprocess.PIPE
if not stderr:
@@ -461,13 +427,13 @@ def spawn_process(command, bufsize=0, bi
if stdin_lines and not isinstance(stdin_lines, list):
raise TypeError("stdin_lines should have list type")
# Log the command line
if options.verbose and not command.endswith('.py'):
sys.stdout.write('CMD: %s %s\n' % (os.path.basename(command),
- ' '.join([_quote_arg(x) for x in varargs])))
+ subprocess.list2cmdline(varargs)))
sys.stdout.flush()
infile, outfile, errfile, kid = open_pipe([command] + list(varargs), bufsize)
if stdin_lines:
for x in stdin_lines:
