On Mon, Nov 8, 2010 at 12:05 PM, Paul Burba <ptbu...@gmail.com> wrote:
> 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.
Sorry Julian, scratch that last bit. I'm not sure what I did, but I
obviously ran the wrong thing (too many trunk WCs apparently!).
Running the tests for another change with your patch still in place I
noticed a lot of failures. Reverting my changes and running with just
your patch I still saw lots of problems, most of which are like this
failure of commit_tests.py 41:
[[[
CMD: svnadmin.exe create svn-test-work\repositories\commit_tests-41
--bdb-txn-nosync --fs-type=fsfs
<TIME = 0.094000>
CMD: svnadmin.exe dump svn-test-work\local_tmp\repos | svnadmin.exe
load svn-test-work\repositories\commit_tests-41 --ignore-uuid
<TIME = 0.002000>
CMD: svn.exe co
file:///C:/SVN/src-trunk-2/Release/subversion/tests/cmdline/svn-test-work/repositories/commit_tests-41
svn-test-work\working_copies\commit_tests-41 --config-dir
C:\SVN\src-trunk-2\Release\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-cache --username jrandom
<TIME = 0.197000>
A svn-test-work\working_copies\commit_tests-41\A
A svn-test-work\working_copies\commit_tests-41\A\B
A svn-test-work\working_copies\commit_tests-41\A\B\lambda
A svn-test-work\working_copies\commit_tests-41\A\B\E
A svn-test-work\working_copies\commit_tests-41\A\B\E\alpha
A svn-test-work\working_copies\commit_tests-41\A\B\E\beta
A svn-test-work\working_copies\commit_tests-41\A\B\F
A svn-test-work\working_copies\commit_tests-41\A\mu
A svn-test-work\working_copies\commit_tests-41\A\C
A svn-test-work\working_copies\commit_tests-41\A\D
A svn-test-work\working_copies\commit_tests-41\A\D\gamma
A svn-test-work\working_copies\commit_tests-41\A\D\G
A svn-test-work\working_copies\commit_tests-41\A\D\G\pi
A svn-test-work\working_copies\commit_tests-41\A\D\G\rho
A svn-test-work\working_copies\commit_tests-41\A\D\G\tau
A svn-test-work\working_copies\commit_tests-41\A\D\H
A svn-test-work\working_copies\commit_tests-41\A\D\H\chi
A svn-test-work\working_copies\commit_tests-41\A\D\H\omega
A svn-test-work\working_copies\commit_tests-41\A\D\H\psi
A svn-test-work\working_copies\commit_tests-41\iota
Checked out revision 1.
CMD: svn.exe mkdir -m msg --with-revprop bug=42
file:///C:/SVN/src-trunk-2/Release/subversion/tests/cmdline/svn-test-work/repositories/commit_tests-41/dir
--config-dir
C:\SVN\src-trunk-2\Release\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-cache --username jrandom
<TIME = 0.201000>
Committed revision 2.
UNEXPECTED EXCEPTION:
Traceback (most recent call last):
File "C:\SVN\src-trunk-2\subversion\tests\cmdline\svntest\main.py",
line 1181, in run
rc = self.pred.run(sandbox)
File "C:\SVN\src-trunk-2\subversion\tests\cmdline\svntest\testcase.py",
line 243, in run
return self._delegate.run(sandbox)
File "C:\SVN\src-trunk-2\subversion\tests\cmdline\svntest\testcase.py",
line 170, in run
return self.func(sandbox)
File "C:\SVN\src-trunk-2\subversion/tests/cmdline/commit_tests.py",
line 2192, in mkdir_with_revprop
'--revprop', '-r', 2, sbox.repo_url)
File "C:\SVN\src-trunk-2\subversion\tests\cmdline\svntest\actions.py",
line 264, in run_and_verify_svn
expected_exit, *varargs)
File "C:\SVN\src-trunk-2\subversion\tests\cmdline\svntest\actions.py",
line 302, in run_and_verify_svn2
exit_code, out, err = main.run_svn(want_err, *varargs)
File "C:\SVN\src-trunk-2\subversion\tests\cmdline\svntest\main.py",
line 552, in run_svn
*(_with_auth(_with_config_dir(varargs))))
File "C:\SVN\src-trunk-2\subversion\tests\cmdline\svntest\main.py",
line 336, in run_command
None, *varargs)
File "C:\SVN\src-trunk-2\subversion\tests\cmdline\svntest\main.py",
line 471, in run_command_stdin
*varargs)
File "C:\SVN\src-trunk-2\subversion\tests\cmdline\svntest\main.py",
line 433, in spawn_process
subprocess.list2cmdline(varargs)))
File "C:\Python26\lib\subprocess.py", line 541, in list2cmdline
needquote = (" " in arg) or ("\t" in arg) or ("|" in arg) or not arg
TypeError: argument of type 'int' is not iterable
FAIL: commit_tests.py 41: set revision props during remote mkdir
]]]
No time to look into this further today, but I can check it out tomorrow.
Paul
