Gabriela Gibson wrote on Tue, Dec 11, 2012 at 00:21:46 +0000: > Thanks for the patch, a few quick comments:
> [[[ > Test for issue #4263: svnrdump: E125005: Cannot accept non-LF line > endings in 'svn:log' property > > * subversion/tests/cmdline/svnrdump_tests.py > copy_bad_line_endings_load: Test for \r line ending bug in svnrdump > (issue 4263) Need parentheses around the symbol name. Lines should be wrapped at 80 characters and subsequent lines indented. > ]]] > > > > Index: svnrdump_tests.py > =================================================================== > --- svnrdump_tests.py (revision 1419536) > +++ svnrdump_tests.py (working copy) > @@ -356,6 +356,10 @@ def copy_bad_line_endings_dump(sbox): > expected_dumpfile_name="copy-bad-line-endings.expected.dump", > bypass_prop_validation=True) > > +def copy_bad_line_endings_load(sbox): The test needs an @XFail decorator, since it currently FAILs. And an @Issue decorator, to associate it with #4263. > + "load: inconsistent line endings in svn:* props" > + run_load_test(sbox, "copy-bad-line-endings.dump") FTR, when run over svn://, this currently errors as: subversion/svnrdump/svnrdump.c:554: (apr_err=125005) subversion/libsvn_repos/load.c:583: (apr_err=125005) subversion/libsvn_repos/load.c:260: (apr_err=125005) subversion/svnrdump/load_editor.c:858: (apr_err=125005) subversion/libsvn_repos/fs-wrap.c:193: (apr_err=125005) svnrdump: E125005: Cannot accept non-LF line endings in 'svn:log' property > + > def copy_bad_line_endings2_dump(sbox): > "dump: non-LF line endings in svn:* props" > run_dump_test(sbox, "copy-bad-line-endings2.dump", > @@ -771,6 +775,7 @@ test_list = [ None, > move_and_modify_in_the_same_revision_dump, > move_and_modify_in_the_same_revision_load, > copy_bad_line_endings_dump, > + copy_bad_line_endings_load, > copy_bad_line_endings2_dump, > commit_a_copy_of_root_dump, > commit_a_copy_of_root_load, > In summary, thanks for this contribution. I guess it's correct but I don't want to make that judgement at 2am, so I'll probably apply the patch Wed or Thu after reviewing the relevant parts of the C code too. Re your other mail about OPW, you shouldn't let yourself be blocked by this --- while this patch is outstanding, you should feel free to work on another patch. The natural choice would be the C patch that turns this test from XFAIL to PASS. Cheers, Daniel
