I've taken a shot at a regression test in r1086497.
However, I think we may have a larger problem here: The svn_delta_editor_t API allows change_dir_prop() calls to be transmitted only after all directory-children of a directory have been transmitted [1]. How can we generate a dumpfile given that input? On the one hand, we don't want to buffer entire subtrees rooted in D while we're waiting for D's properties. On the other hand, can we dump the properties of D only after dumping the entire subtree rooted in D? I expect the 'svnadmin load parser to want to see an entry for D before seeing an entry for any of its children. Which, yes, leaves the option to dump first a skeleton entry for D, then dump D's subtree, and finally dump *another* "Node-path: D" entry --- but that smells ugly, if not in outright violation of the dumpfile format. Thoughts? Daniel [1] * 4. A producer must call @c change_dir_prop on a directory either * before opening any of the directory's subdirs or after closing * them, but not in the middle. neil.win...@bt.com wrote on Mon, Mar 28, 2011 at 13:26:46 +0100: > On 25 March 2011 at 19:19, Hyrum K Wright wrote > > On Fri, Mar 25, 2011 at 2:11 PM, C. Michael Pilato <cmpil...@collab.net> > > wrote: > >> On 03/25/2011 11:31 AM, Hyrum K Wright wrote: > >>> Neil, > >>> Thanks for the report. In attempting to reproduce, I wasn't even able > >>> to get as far as comparing the dumpfiles, as svnrdump asserted > >>> somewhere in the process. I opened issue 3844[1] to track this bug, > >>> and committed an XFailing test in r1085428. I also marked the issue > >>> as important for a 1.7.0 release. > >>> > >>> -Hyrum > >>> > >>> [1] http://subversion.tigris.org/issues/show_bug.cgi?id=3844 > >> > >> See r1085523. > > > > Thanks, Mike. > > > > Neil, when you get a chance, could you test a r1085523 and see if it > > fixes your problem? > > > > Cheers, > > -Hyrum > > Hi guys, > > Sorry, but no cigar :( This probably fixes the assertion found by Hyrum, but > doesn't address the original issue. The current test case added to the test > suite addresses loading multiple propedits. That's good, but it's not > sufficient. The heart of the issue is that a transaction with multiple > propedits to a single path is *dumped* incorrectly. So for an original dump > representation like this: > > --- > Node-path: > Node-kind: dir > Node-action: change > Prop-content-length: 42 > Content-length: 42 > > K 3 > foo > V 3 > bar > K 3 > bar > V 3 > baz > PROPS-END > --- > > The svnrdump output for the same transaction comes out like this: > > --- > Node-path: > Node-kind: dir > Node-action: change > Prop-delta: true > Prop-content-length: 26 > Content-length: 26 > > K 3 > foo > V 3 > bar > PROPS-END > > > Prop-delta: true > Prop-content-length: 26 > Content-length: 26 > > K 3 > bar > V 3 > baz > PROPS-END > --- > > Note the second "orphaned" prop-delta. > > It would be great if svnrdump produced exactly the same output as svnadmin > (it'd make the test assertions nice and easy), but from a bit of > experimentation (I haven't read the svnadmin code here) I don't believe that > it's necessary for the two prop-deltas actually to be merged for the dumpfile > to be valid. So if it's easier to repeat the preceding node-path information > for each prop-delta then I guess that would be OK, as long as the net result > is the same. > > Thanks, > Neil
