Re: svn commit: r1653988 - in /subversion/trunk/subversion: svnrdump/load_editor.c tests/cmdline/svnrdump_tests.py
From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 23 Jan 2015 14:50:28 +0000
I (Julian Foad) wrote:
RA-serf accepts the calls but commits an unexpected result because it processes the non-deleting prop changes first and then the prop deletes.
For example (as in svnrdump_tests.py 52), to change properties from (p=v, q=v) to (q=v), svnrdump calls:
which results in libsvn_ra_serf/commit.c sending:
PROPPATCH: pname="q", pval="v"
resulting in no properties.
> I should be able to test for this bug independently of issue #4551.
r1654186 adds such a regression test. I'm keeping it within issue #4551, but it's independent of *copying* a node which was where that issue showed up until now.
Bert's commit http://svn.apache.org/r1654194 "Simplify the ra_serf proppatch code ..." changes RA-serf to work the same way as the other RA layers and thus avoids the problem here. The tests for this issue now pass, although svndumpfilter is still violating the editor rules.
Why haven't we found this before? Because we haven't tested it, is the easy answer. But we already had a test that exercises this kind of scenario (svnrdump_tests.py 49 -- but it's only looking to see if the commit fails, not checking the results). If we had generic editor call checking in place then maybe we would have found this driving order violation earlier.
Maybe we should introduce an editor validator, even at this late stage. Note that Ev2 attempts to provide one -- see ENABLE_ORDERING_CHECK in libsvn_delta/editor.c.
I also noticed that configuring with '--enable-ev2-shims' creates the same driving order violation, so that several tests would fail against RA-serf, but since r1654194 they no longer fail.
Wondering what to do next... I think we should still fix the driving order in svndumpfilter.
We should definitely backport the issue #4551 fixes when they're ready.
This is an archived mail posted to the Subversion Dev mailing list.