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
Received on 2012-12-11 01:47:45 CET