RE: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
I committed a patch which fixes issue #3847 in r1092783. Neil, if you want to get this revision to validate the fix in your environment, I'd be much obliged. I've tested this out locally with dumps/restores from a number of our repositories and it all looks good. Thanks guys! Neil
Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
On Tue, Mar 29, 2011 at 4:06 PM, C. Michael Pilato cmpil...@collab.net wrote: On 03/28/2011 08:26 AM, neil.win...@bt.com wrote: Sorry, but no cigar :( This probably fixes the assertion found by Hyrum, but doesn't address the original issue. To simplify things, I've repurposed issue #3844[1] to track the problem that Hyrum was solving, and opened a new issue #3847[2] for this issue. -- C-Mike [1] http://subversion.tigris.org/issues/show_bug.cgi?id=3844 [2] http://subversion.tigris.org/issues/show_bug.cgi?id=3847 I committed a patch which fixes issue #3847 in r1092783. Neil, if you want to get this revision to validate the fix in your environment, I'd be much obliged. Best, -Hyrum
Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
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
Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
On 03/28/2011 08:26 AM, neil.win...@bt.com wrote: Sorry, but no cigar :( This probably fixes the assertion found by Hyrum, but doesn't address the original issue. To simplify things, I've repurposed issue #3844[1] to track the problem that Hyrum was solving, and opened a new issue #3847[2] for this issue. -- C-Mike [1] http://subversion.tigris.org/issues/show_bug.cgi?id=3844 [2] http://subversion.tigris.org/issues/show_bug.cgi?id=3847 -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
RE: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
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
Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
Hi, There appears to be a problem with the new svnrdump command handling revisions where multiple simultaneous property changes occur on a single file or directory. I couldn't see any mention of this in the user or dev lists elsewhere. To recreate to bug, do the following: svnadmin create test-original svnadmin load test-original original.dump svnrdump dump file://$PWD/test-original new.dump svnadmin create test-new ln -s /bin/true test-new/hooks/pre-revprop-change svnrdump load file://$PWD/test-new new.dump The result of the load (also using svnadmin instead of svnrdump) is: * Loaded revision 0. svnrdump: E140001: Unrecognised record type in stream The problem appears to be that multiple property changes are not consolidated into a single set of changes associated with a node, but instead appear as orphaned Prop-delta sections. So in the original dump you see: 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 But in the output from svnrdump this appears as follows: 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 Sorry, but I'm not quite up to providing a fix as yet, but I hope this helps someone to figure it out. Regards, Neil Neil Winton BT Innovate and Design, Tools Platform Tel: +44 (0)1473 651976, E-Mail: neil.win...@bt.commailto:neil.win...@bt.com This email contains information from British Telecommunications plc which may be confidential or privileged. The information is intended to be for the use of the individuals or the entities named above. If you are not the intended recipient be aware that any disclosure, copying, distribution or use of the contents of this information is prohibited. If you have received this electronic message in error, please notify us by telephone or email (to the numbers or address above) immediately.
Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
On Fri, Mar 25, 2011 at 9:50 AM, neil.win...@bt.com wrote: Hi, There appears to be a problem with the new svnrdump command handling revisions where multiple simultaneous property changes occur on a single file or directory. I couldn’t see any mention of this in the user or dev lists elsewhere. To recreate to bug, do the following: svnadmin create test-original svnadmin load test-original original.dump svnrdump dump file://$PWD/test-original new.dump svnadmin create test-new ln -s /bin/true test-new/hooks/pre-revprop-change svnrdump load file://$PWD/test-new new.dump The result of the load (also using svnadmin instead of svnrdump) is: * Loaded revision 0. svnrdump: E140001: Unrecognised record type in stream The problem appears to be that multiple property changes are not consolidated into a single set of changes associated with a node, but instead appear as “orphaned” Prop-delta sections. So in the original dump you see: 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 But in the output from svnrdump this appears as follows: 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 Sorry, but I’m not quite up to providing a fix as yet, but I hope this helps someone to figure it out. 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
Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
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. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
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
