Re: Bug in "svnrdump" ?

[Doug Robinson]

Julian:

No need to drive it further at this time.

Thank you.

Doug

On Thu, Feb 23, 2017 at 9:15 AM, Julian Foad  wrote:

> Doug Robinson wrote:
>
>> https://issues.apache.org/jira/browse/SVN-4672
>>
>
> Thanks, Doug, that's great. Let me know if you need me to drive it
> further; I'm assuming not.
>
> Bert wrote:
>
>> That code is in the backing code for svn_ra_replay(), where it also
>> applies to authz, not on the client.
>>
>
> That makes sense. I believe both svnrdump and svnsync use this. I assumed
> both of them would support converting copies to adds...
>
> @Julian Foad Can we use svnsync in this scenario, or does that break in a
>> similar way?
>>
>
> ... but I haven't tested it.
>
> - Julian
>
>


-- 
*DOUGLAS B. ROBINSON* SENIOR PRODUCT MANAGER

*T *925-396-1125
*E* doug.robin...@wandisco.com

*www.wandisco.com *

-- 


Learn how WANdisco Fusion solves Hadoop data protection and scalability 
challenges 

Listed on the London Stock Exchange: WAND 


THIS MESSAGE AND ANY ATTACHMENTS ARE CONFIDENTIAL, PROPRIETARY, AND MAY BE 
PRIVILEGED.  If this message was misdirected, WANdisco, Inc. and its 
subsidiaries, ("WANdisco") does not waive any confidentiality or privilege. 
 If you are not the intended recipient, please notify us immediately and 
destroy the message without disclosing its contents to anyone.  Any 
distribution, use or copying of this e-mail or the information it contains 
by other than an intended recipient is unauthorized.  The views and 
opinions expressed in this e-mail message are the author's own and may not 
reflect the views and opinions of WANdisco, unless the author is authorized 
by WANdisco to express such views or opinions on its behalf.  All email 
sent to or from this address is subject to electronic storage and review by 
WANdisco.  Although WANdisco operates anti-virus programs, it does not 
accept responsibility for any damage whatsoever caused by viruses being 
passed.

Re: Bug in "svnrdump" ?

[Julian Foad]

Doug Robinson wrote:

https://issues.apache.org/jira/browse/SVN-4672


Thanks, Doug, that's great. Let me know if you need me to drive it 
further; I'm assuming not.


Bert wrote:

That code is in the backing code for svn_ra_replay(), where it also applies to 
authz, not on the client.


That makes sense. I believe both svnrdump and svnsync use this. I 
assumed both of them would support converting copies to adds...



@Julian Foad Can we use svnsync in this scenario, or does that break in a 
similar way?


... but I haven't tested it.

- Julian

RE: Bug in "svnrdump" ?

[bert]

That code is in the backing code for svn_ra_replay(), where it also applies to 
authz, not on the client.

@Julian Foad Can we use svnsync in this scenario, or does that break in a 
similar way?

 Bert

Sent from Mail for Windows 10

From: Julian Foad
Sent: woensdag 22 februari 2017 15:11
To: Doug Robinson
Cc: dev
Subject: Re: Bug in "svnrdump" ?

Doug Robinson wrote:
> This has been reproduced on 1.9.5 and 1.8.16.  I've attached a dump of
> the simple test case repo.

Thanks. I was hasty -- I see what's going on. This is dumping a subtree 
of the repo (/B/Trunk), and r2 was not dumped because it doesn't touch 
the subtree, and it fails on r3 corresponding to step 3. This feature 
(dump a subtree) is not possible with 'svnadmin dump'.

So the bug is that svnrdump doesn't know how to handle a copy source 
that's outside the subtree being dumped.

That's rather poor. I believe the desired behaviour would be to replace 
the 'copy' with a full 'add', and I believe that behaviour is 
implemented somewhere else -- is it in svndumpfilter and/or svnsync?

If you could file a bug report that would be very helpful, thanks.

- Julian


> If so then it has a reproducible bug:
> --
> 1. Add following files into an empty repo.
>
>A /A
>
>A /A/AA
>
>A /A/AA/a.txt
>
>A /A/AA/b.txt
>
> 2. Svn copy folder A to folder B, and then delete /B/AA/a.txt
>
>A /B (from /A:1)
>
>D /B/AA/a.txt
>
> 3. Copy folder B/AA to folder B/Trunk/AA
>
>A /B/Trunk
>
>A /B/Trunk/AA (from /B/AA:2)
>
> 4. run svnrdump command to dump B/Trunk folder.
>
> # svnrdump dump file:///tmp/test/B/Trunk > Trunk.dump
>
> * Dumped revision 0.
>
> * Dumped revision 1.
>
> svnrdump: E160013: File not found: revision 2, path '/B/AA/a.txt'
> --
>
> Is this known already?  Should I file a bug?

Re: Bug in "svnrdump" ?

[Doug Robinson]

Julian:

https://issues.apache.org/jira/browse/SVN-4672

Cheers!

Doug

On Wed, Feb 22, 2017 at 9:11 AM, Julian Foad <julianf...@apache.org> wrote:

> Doug Robinson wrote:
>
>> This has been reproduced on 1.9.5 and 1.8.16.  I've attached a dump of
>> the simple test case repo.
>>
>
> Thanks. I was hasty -- I see what's going on. This is dumping a subtree of
> the repo (/B/Trunk), and r2 was not dumped because it doesn't touch the
> subtree, and it fails on r3 corresponding to step 3. This feature (dump a
> subtree) is not possible with 'svnadmin dump'.
>
> So the bug is that svnrdump doesn't know how to handle a copy source
> that's outside the subtree being dumped.
>
> That's rather poor. I believe the desired behaviour would be to replace
> the 'copy' with a full 'add', and I believe that behaviour is implemented
> somewhere else -- is it in svndumpfilter and/or svnsync?
>
> If you could file a bug report that would be very helpful, thanks.
>
>
> - Julian
>
>
> If so then it has a reproducible bug:
>> --
>> 1. Add following files into an empty repo.
>>
>>A /A
>>
>>A /A/AA
>>
>>A /A/AA/a.txt
>>
>>A /A/AA/b.txt
>>
>> 2. Svn copy folder A to folder B, and then delete /B/AA/a.txt
>>
>>A /B (from /A:1)
>>
>>D /B/AA/a.txt
>>
>> 3. Copy folder B/AA to folder B/Trunk/AA
>>
>>A /B/Trunk
>>
>>A /B/Trunk/AA (from /B/AA:2)
>>
>> 4. run svnrdump command to dump B/Trunk folder.
>>
>> # svnrdump dump file:///tmp/test/B/Trunk > Trunk.dump
>>
>> * Dumped revision 0.
>>
>> * Dumped revision 1.
>>
>> svnrdump: E160013: File not found: revision 2, path '/B/AA/a.txt'
>> --
>>
>> Is this known already?  Should I file a bug?
>>
>


-- 
*DOUGLAS B. ROBINSON* SENIOR PRODUCT MANAGER

*T *925-396-1125
*E* doug.robin...@wandisco.com

*www.wandisco.com <http://www.wandisco.com/>*

-- 


Learn how WANdisco Fusion solves Hadoop data protection and scalability 
challenges <http://www.wandisco.com/hadoop/wd-fusion>

Listed on the London Stock Exchange: WAND 
<http://www.bloomberg.com/quote/WAND:LN>

THIS MESSAGE AND ANY ATTACHMENTS ARE CONFIDENTIAL, PROPRIETARY, AND MAY BE 
PRIVILEGED.  If this message was misdirected, WANdisco, Inc. and its 
subsidiaries, ("WANdisco") does not waive any confidentiality or privilege. 
 If you are not the intended recipient, please notify us immediately and 
destroy the message without disclosing its contents to anyone.  Any 
distribution, use or copying of this e-mail or the information it contains 
by other than an intended recipient is unauthorized.  The views and 
opinions expressed in this e-mail message are the author's own and may not 
reflect the views and opinions of WANdisco, unless the author is authorized 
by WANdisco to express such views or opinions on its behalf.  All email 
sent to or from this address is subject to electronic storage and review by 
WANdisco.  Although WANdisco operates anti-virus programs, it does not 
accept responsibility for any damage whatsoever caused by viruses being 
passed.

Re: Bug in "svnrdump" ?

[Julian Foad]

Doug Robinson wrote:

This has been reproduced on 1.9.5 and 1.8.16.  I've attached a dump of
the simple test case repo.


Thanks. I was hasty -- I see what's going on. This is dumping a subtree 
of the repo (/B/Trunk), and r2 was not dumped because it doesn't touch 
the subtree, and it fails on r3 corresponding to step 3. This feature 
(dump a subtree) is not possible with 'svnadmin dump'.


So the bug is that svnrdump doesn't know how to handle a copy source 
that's outside the subtree being dumped.


That's rather poor. I believe the desired behaviour would be to replace 
the 'copy' with a full 'add', and I believe that behaviour is 
implemented somewhere else -- is it in svndumpfilter and/or svnsync?


If you could file a bug report that would be very helpful, thanks.

- Julian



If so then it has a reproducible bug:
--
1. Add following files into an empty repo.

   A /A

   A /A/AA

   A /A/AA/a.txt

   A /A/AA/b.txt

2. Svn copy folder A to folder B, and then delete /B/AA/a.txt

   A /B (from /A:1)

   D /B/AA/a.txt

3. Copy folder B/AA to folder B/Trunk/AA

   A /B/Trunk

   A /B/Trunk/AA (from /B/AA:2)

4. run svnrdump command to dump B/Trunk folder.

# svnrdump dump file:///tmp/test/B/Trunk > Trunk.dump

* Dumped revision 0.

* Dumped revision 1.

svnrdump: E160013: File not found: revision 2, path '/B/AA/a.txt'
--

Is this known already?  Should I file a bug?

Re: Bug in "svnrdump" ?

[Doug Robinson]

Julian:

This has been reproduced on 1.9.5 and 1.8.16.  I've attached a dump of the
simple test case repo.

Thank you.

Doug

On Wed, Feb 22, 2017 at 8:50 AM, Julian Foad  wrote:

> Doug Robinson wrote:
>
>> Should "svnrdump" be able to dump everything that "svnadmin dump" is
>> capable of?
>>
>
> I'm pretty sure it should, and this isn't a known bug AFAIK (I just
> searched for "svnrdump copy" in Jira).
>
> Thanks for reporting it here.
>
> Can you paste the actual reproduction commands used please. (It isn't
> clear from your description where the commits are, for example. If one
> commit per numbered step, then presumably as it fails on dumping r2 then
> what are steps 3 and 4 for?)
>
> What svn version this is?
>
> - Julian
>
>
>
>> If so then it has a reproducible bug:
>> --
>> 1. Add following files into an empty repo.
>>
>>A /A
>>
>>A /A/AA
>>
>>A /A/AA/a.txt
>>
>>A /A/AA/b.txt
>>
>> 2. Svn copy folder A to folder B, and then delete /B/AA/a.txt
>>
>>A /B (from /A:1)
>>
>>D /B/AA/a.txt
>>
>> 3. Copy folder B/AA to folder B/Trunk/AA
>>
>>A /B/Trunk
>>
>>A /B/Trunk/AA (from /B/AA:2)
>>
>> 4. run svnrdump command to dump B/Trunk folder.
>>
>> # svnrdump dump file:///tmp/test/B/Trunk > Trunk.dump
>>
>> * Dumped revision 0.
>>
>> * Dumped revision 1.
>>
>> svnrdump: E160013: File not found: revision 2, path '/B/AA/a.txt'
>> --
>>
>> Is this known already?  Should I file a bug?
>>
>> Thank you.
>>
>


-- 
*DOUGLAS B. ROBINSON* SENIOR PRODUCT MANAGER

*T *925-396-1125
*E* doug.robin...@wandisco.com

*www.wandisco.com *

-- 


Learn how WANdisco Fusion solves Hadoop data protection and scalability 
challenges 

Listed on the London Stock Exchange: WAND 


THIS MESSAGE AND ANY ATTACHMENTS ARE CONFIDENTIAL, PROPRIETARY, AND MAY BE 
PRIVILEGED.  If this message was misdirected, WANdisco, Inc. and its 
subsidiaries, ("WANdisco") does not waive any confidentiality or privilege. 
 If you are not the intended recipient, please notify us immediately and 
destroy the message without disclosing its contents to anyone.  Any 
distribution, use or copying of this e-mail or the information it contains 
by other than an intended recipient is unauthorized.  The views and 
opinions expressed in this e-mail message are the author's own and may not 
reflect the views and opinions of WANdisco, unless the author is authorized 
by WANdisco to express such views or opinions on its behalf.  All email 
sent to or from this address is subject to electronic storage and review by 
WANdisco.  Although WANdisco operates anti-virus programs, it does not 
accept responsibility for any damage whatsoever caused by viruses being 
passed.


test.dump
Description: Binary data

Re: Bug in "svnrdump" ?

[Julian Foad]

Doug Robinson wrote:

Should "svnrdump" be able to dump everything that "svnadmin dump" is
capable of?


I'm pretty sure it should, and this isn't a known bug AFAIK (I just 
searched for "svnrdump copy" in Jira).


Thanks for reporting it here.

Can you paste the actual reproduction commands used please. (It isn't 
clear from your description where the commits are, for example. If one 
commit per numbered step, then presumably as it fails on dumping r2 then 
what are steps 3 and 4 for?)


What svn version this is?

- Julian




If so then it has a reproducible bug:
--
1. Add following files into an empty repo.

   A /A

   A /A/AA

   A /A/AA/a.txt

   A /A/AA/b.txt

2. Svn copy folder A to folder B, and then delete /B/AA/a.txt

   A /B (from /A:1)

   D /B/AA/a.txt

3. Copy folder B/AA to folder B/Trunk/AA

   A /B/Trunk

   A /B/Trunk/AA (from /B/AA:2)

4. run svnrdump command to dump B/Trunk folder.

# svnrdump dump file:///tmp/test/B/Trunk > Trunk.dump

* Dumped revision 0.

* Dumped revision 1.

svnrdump: E160013: File not found: revision 2, path '/B/AA/a.txt'
--

Is this known already?  Should I file a bug?

Thank you.

Bug in "svnrdump" ?

[Doug Robinson]

Folks:

Should "svnrdump" be able to dump everything that "svnadmin dump" is
capable of?

If so then it has a reproducible bug:
--
1. Add following files into an empty repo.

   A /A

   A /A/AA

   A /A/AA/a.txt

   A /A/AA/b.txt

2. Svn copy folder A to folder B, and then delete /B/AA/a.txt

   A /B (from /A:1)

   D /B/AA/a.txt

3. Copy folder B/AA to folder B/Trunk/AA

   A /B/Trunk

   A /B/Trunk/AA (from /B/AA:2)

4. run svnrdump command to dump B/Trunk folder.

# svnrdump dump file:///tmp/test/B/Trunk > Trunk.dump

* Dumped revision 0.

* Dumped revision 1.

svnrdump: E160013: File not found: revision 2, path '/B/AA/a.txt'
--

Is this known already?  Should I file a bug?

Thank you.

Doug
-- 
*DOUGLAS B. ROBINSON* SENIOR PRODUCT MANAGER

*T *925-396-1125
*E* doug.robin...@wandisco.com

*www.wandisco.com *

-- 


Learn how WANdisco Fusion solves Hadoop data protection and scalability 
challenges 

Listed on the London Stock Exchange: WAND 


THIS MESSAGE AND ANY ATTACHMENTS ARE CONFIDENTIAL, PROPRIETARY, AND MAY BE 
PRIVILEGED.  If this message was misdirected, WANdisco, Inc. and its 
subsidiaries, ("WANdisco") does not waive any confidentiality or privilege. 
 If you are not the intended recipient, please notify us immediately and 
destroy the message without disclosing its contents to anyone.  Any 
distribution, use or copying of this e-mail or the information it contains 
by other than an intended recipient is unauthorized.  The views and 
opinions expressed in this e-mail message are the author's own and may not 
reflect the views and opinions of WANdisco, unless the author is authorized 
by WANdisco to express such views or opinions on its behalf.  All email 
sent to or from this address is subject to electronic storage and review by 
WANdisco.  Although WANdisco operates anti-virus programs, it does not 
accept responsibility for any damage whatsoever caused by viruses being 
passed.

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Justin Erenkrantz]

On Wed, Dec 12, 2012 at 5:06 PM, Daniel Shahaf danie...@elego.de wrote:

 P.S.  This thread was an unusually long one, for a patch that adds about
 a dozen lines of code.


Uh, how is that at all unusual for this crowd?  =)  -- justin

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Ben Reser]

On Tue, Dec 11, 2012 at 5:59 PM, Ben Reser b...@reser.org wrote:
 I'd say that replacing '\r' with a 'space' is wrong.  That would
 change the meaning of some properties.  E.G. svn:ignore, svn:externals
 which use lines to handle individual records within them.

To be more explicit, I think you should change CR or CRLF into LF.

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Daniel Shahaf]

Ben Reser wrote on Wed, Dec 12, 2012 at 13:57:30 -0800:
 On Tue, Dec 11, 2012 at 5:59 PM, Ben Reser b...@reser.org wrote:
  I'd say that replacing '\r' with a 'space' is wrong.  That would
  change the meaning of some properties.  E.G. svn:ignore, svn:externals
  which use lines to handle individual records within them.
 
 To be more explicit, I think you should change CR or CRLF into LF.

I've applied your patch Gabriela, with a tweak to the log message and
with the addition of an expected_dumpfile_path parameter.  Currently the
test checks what Ben said --- which is also consistent with what you and
danielsh suggested in earlier emails.

Naturally we can change the test's expectations if down the road we
decide the correct behaviour is something else.

Thanks for the patch!

Daniel

P.S.  This thread was an unusually long one, for a patch that adds about
a dozen lines of code.

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Gabriela Gibson]

On 11/12/12 00:46, Daniel Shahaf wrote:


Need parentheses around the symbol name.  Lines should be wrapped at 80
characters and subsequent lines indented.


The web page instructions[1] need updating because they doesn't mention 
this and so, I was trying to stay under a 72 character limit for the 
mailing list.



The test needs an @XFail decorator, since it currently FAILs.  And an
@Issue decorator, to associate it with #4263.


I hope to have corrected all outstanding issues in the attached files.


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.


I will attempt to do just this.  Also your tip with the libtool was much 
appreciated, thank you very much :)


regards,

Gabriela
[1] http://subversion.apache.org/docs/community-guide/general.html#patches
Index: subversion/tests/cmdline/svnrdump_tests.py
===
--- subversion/tests/cmdline/svnrdump_tests.py	(revision 1420388)
+++ subversion/tests/cmdline/svnrdump_tests.py	(working copy)
@@ -356,6 +356,12 @@ def copy_bad_line_endings_dump(sbox):
 expected_dumpfile_name=copy-bad-line-endings.expected.dump,
 bypass_prop_validation=True)
 
+@XFail()
+@Issue(4263)
+def copy_bad_line_endings_load(sbox):
+  load: inconsistent line endings in svn:* props
+  run_load_test(sbox, copy-bad-line-endings.dump)
+  
 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 +777,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,
[[[ 
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)
]]]

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Stefan Sperling]

I cannot build/test this right now but both patch and log message
look great to me. Thanks!

On Tue, Dec 11, 2012 at 10:18:54PM +, Gabriela Gibson wrote:
 Index: subversion/tests/cmdline/svnrdump_tests.py
 ===
 --- subversion/tests/cmdline/svnrdump_tests.py(revision 1420388)
 +++ subversion/tests/cmdline/svnrdump_tests.py(working copy)
 @@ -356,6 +356,12 @@ def copy_bad_line_endings_dump(sbox):
  expected_dumpfile_name=copy-bad-line-endings.expected.dump,
  bypass_prop_validation=True)
  
 +@XFail()
 +@Issue(4263)
 +def copy_bad_line_endings_load(sbox):
 +  load: inconsistent line endings in svn:* props
 +  run_load_test(sbox, copy-bad-line-endings.dump)
 +  
  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 +777,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,

 [[[ 
 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)
 ]]]

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Daniel Shahaf]

Gabriela Gibson wrote on Tue, Dec 11, 2012 at 22:18:54 +:
 On 11/12/12 00:46, Daniel Shahaf wrote:

 Need parentheses around the symbol name.  Lines should be wrapped at 80
 characters and subsequent lines indented.

 The web page instructions[1] need updating because they doesn't mention  
 this and so, I was trying to stay under a 72 character limit for the  
 mailing list.


It seems
http://subversion.apache.org/docs/community-guide/conventions#log-messages
doesn't mention that either.  Though I believe we recommend 79 columns
for code.

Anyway, the original issue I saw was that the log message had a ~full
line and the next line was aligned to column zero.  The log message on
this iteration is fine.

 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.

 I will attempt to do just this.  Also your tip with the libtool was much  
 appreciated, thank you very much :)


Welcome.

 Index: subversion/tests/cmdline/svnrdump_tests.py
 ===
 --- subversion/tests/cmdline/svnrdump_tests.py(revision 1420388)
 +++ subversion/tests/cmdline/svnrdump_tests.py(working copy)
 @@ -356,6 +356,12 @@ def copy_bad_line_endings_dump(sbox):
  expected_dumpfile_name=copy-bad-line-endings.expected.dump,
  bypass_prop_validation=True)
  
 +@XFail()
 +@Issue(4263)
 +def copy_bad_line_endings_load(sbox):
 +  load: inconsistent line endings in svn:* props
 +  run_load_test(sbox, copy-bad-line-endings.dump)
 +  

OK, sorry, I missed it yesterday, but there's a problem here.  Looking
at the docstring of run_load_test():

def run_load_test(sbox, dumpfile_name, expected_dumpfile_name = None,
  expect_deltas = True):
  Load a dumpfile using 'svnrdump load', dump it with 'svnadmin
  dump' and check that the same dumpfile is produced

It checks for identity.  However, the problem here is \r in an svn:*
property; as of 1.6, the server doesn't allow any new instances of this
to enter a repository, so the resulting dumpfile won't be equal to the
input one.

I think you need to pass expected_dumpfile_name= to run_load_test().

Does that make sense?

 

Cheers

Daniel

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[C. Michael Pilato]

On 12/11/2012 06:01 PM, Daniel Shahaf wrote:
 Gabriela Gibson wrote on Tue, Dec 11, 2012 at 22:18:54 +:
 On 11/12/12 00:46, Daniel Shahaf wrote:

 Need parentheses around the symbol name.  Lines should be wrapped at 80
 characters and subsequent lines indented.

 The web page instructions[1] need updating because they doesn't mention  
 this and so, I was trying to stay under a 72 character limit for the  
 mailing list.

 
 It seems
 http://subversion.apache.org/docs/community-guide/conventions#log-messages
 doesn't mention that either.  Though I believe we recommend 79 columns
 for code.

http://subversion.apache.org/docs/community-guide/conventions.html#other-conventions
recommends 79 columns for code.  Restrict lines to 79 columns, so that code
will display well in a minimal standard display window.

We should probably link to the Coding Conventions section from the Patch
submission guidelines section just to be thorough.

-- 
C. Michael Pilato cmpil...@collab.net
CollabNet  www.collab.net  Enterprise Cloud Development



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Gabriela Gibson]

On 11/12/12 00:46, Daniel Shahaf wrote:

snip

 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

Thanks for this.  This morphs into:

subversion/svnrdump/svnrdump.c:554: (apr_err=125005)
  This is (load_cmd)
subversion/libsvn_repos/load.c:583: (apr_err=125005)
  (svn_repos_parse_dumpstream3)- (parse_property_block)
subversion/libsvn_repos/load.c:260: (apr_err=125005)
  (parse_property_block)- (parse_fns-set_revision_property)
subversion/svnrdump/load_editor.c:858: (apr_err=125005)
  (set_revision_property)- (svn_repos__validate_prop)
subversion/libsvn_repos/fs-wrap.c:193: (apr_err=125005)
  (svn_repos__validate_prop)

I'm concerned that I shouldn't be altering fs-wrap.c.  So a logical
place to put a fix is probably in (set_revision_property).

I could either hand code a for loop, or call the function
(svn_rdump__normalize_props) in svnrdump/util.c

So, to summarise, my options seem to be:

1.  Alter (svn_repos__validate_prop) to replace '\r' with 'space'.
2.  Hand code a loop at load_editor.c:857
3.  Make a call to (svn_rdump__normalize_props) at load_editor.c:857
4.  Make a call to to (svn_subst_translate_cstring2) at
load_editor.c:857

Which is the preferred option?

Regards

Gabriela

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Gabriela Gibson]

On 11/12/12 23:01, Daniel Shahaf wrote:
 Gabriela Gibson wrote on Tue, Dec 11, 2012 at 22:18:54 +:
 On 11/12/12 00:46, Daniel Shahaf wrote:


 I will attempt to do just this.  Also your tip with the libtool was
 much appreciated, thank you very much :)


 Welcome.

 Index: subversion/tests/cmdline/svnrdump_tests.py
 ===
 --- subversion/tests/cmdline/svnrdump_tests.py   (revision 1420388)
 +++ subversion/tests/cmdline/svnrdump_tests.py   (working copy)
 @@ -356,6 +356,12 @@ def copy_bad_line_endings_dump(sbox):
 
expected_dumpfile_name=copy-bad-line-endings.expected.dump,

   bypass_prop_validation=True)

 +@XFail()
 +@Issue(4263)
 +def copy_bad_line_endings_load(sbox):
 +  load: inconsistent line endings in svn:* props
 +  run_load_test(sbox, copy-bad-line-endings.dump)
 +

 OK, sorry, I missed it yesterday, but there's a problem here.  Looking
 at the docstring of run_load_test():

  def run_load_test(sbox, dumpfile_name, expected_dumpfile_name = None,
expect_deltas = True):
   Load a dumpfile using 'svnrdump load', dump it with 'svnadmin
dump' and check that the same dumpfile is produced

 It checks for identity.  However, the problem here is \r in an svn:*
 property; as of 1.6, the server doesn't allow any new instances of
 this to enter a repository, so the resulting dumpfile won't be
 equal to the input one.

 I think you need to pass expected_dumpfile_name= to run_load_test().

 Does that make sense?

Yes.

There is a candidate for expected_dumpfile_name already in the tree:
/tests/cmdline/svnrdump_tests/copy-bad-line-endings.expected.dump

However, it's not clear that this defines the desired behaviour when
loading.

The differences between copy-bad-line-endings.expected.dump and
copy-bad-line-endings.dump appear to be:

1.  '\r' in the middle of a line is replaced by '\n'.
2.  '\r' at the end of a line is deleted.

Let's call this option 1.

I had in mind to replace '\r' with 'space'.
This would be option 2.

Which is the prefered option?

Regards

Gabriela

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Ben Reser]

On Tue, Dec 11, 2012 at 4:50 PM, Gabriela Gibson
gabriela.gib...@gmail.com wrote:
 I'm concerned that I shouldn't be altering fs-wrap.c.  So a logical
 place to put a fix is probably in (set_revision_property).

 I could either hand code a for loop, or call the function
 (svn_rdump__normalize_props) in svnrdump/util.c

 So, to summarise, my options seem to be:

 1.  Alter (svn_repos__validate_prop) to replace '\r' with 'space'.
 2.  Hand code a loop at load_editor.c:857
 3.  Make a call to (svn_rdump__normalize_props) at load_editor.c:857
 4.  Make a call to to (svn_subst_translate_cstring2) at
 load_editor.c:857

 Which is the preferred option?

Option 5.

Refactor the svn_rdump__normalize_props to use a function that you can
also use from the load editor to normalize a single property.

I think what was done to svnsync in r877869 should be instructive to you.

I tracked that down by looking at the issue that's referenced in the
issue you're looking at, which then says it is fixed in r37795.  When
we migrated from tigris.org to apache.org for our repo hosting our
revision numbers changed.  Fortunately our bot on IRC is helpful with
this:
17:22 [msg(wayita)] #svn r37795
17:22 [wayita(~way...@svn-qavm.apache.org )] breser: r37795 - r877869

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Daniel Shahaf]

Ben Reser wrote on Tue, Dec 11, 2012 at 17:44:59 -0800:
 I tracked that down by looking at the issue that's referenced in the
 issue you're looking at, which then says it is fixed in r37795.  When
 we migrated from tigris.org to apache.org for our repo hosting our
 revision numbers changed.  Fortunately our bot on IRC is helpful with
 this:
 17:22 [msg(wayita)] #svn r37795
 17:22 [wayita(~way...@svn-qavm.apache.org )] breser: r37795 - r877869

This is documented in ^/subversion/README.  The transition happened 2-3
years ago (so we run into the need for translation proportionally
rarely).  

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Daniel Shahaf]

Gabriela Gibson wrote on Wed, Dec 12, 2012 at 01:16:34 +:
 The differences between copy-bad-line-endings.expected.dump and
 copy-bad-line-endings.dump appear to be:

 1.  '\r' in the middle of a line is replaced by '\n'.
 2.  '\r' at the end of a line is deleted.


Actually what happens in (2) is that a CRLF at the end of the line
becomes LF.  (How to tell?  Look at svn_hash_write2() for the
serialisation format description, then count the bytes within the field.
The second \r is byte 48 in a 49-byte field.  Byte 49 is a \n.)

 Let's call this option 1.

 I had in mind to replace '\r' with 'space'.
 This would be option 2.

 Which is the prefered option?


Whatever is consistent with how we handle \r elsewhere --- eg, in
svnsync, 'svnadmin dump' (which I think dumps \r literally), and
'svnrdump dump'.

We might just need to reuse copy-bad-line-endings.expected.dump --- but
you're correct that that is not a priori obvious.

Cheers

Daniel


 Regards

 Gabriela

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Ben Reser]

On Tue, Dec 11, 2012 at 5:16 PM, Gabriela Gibson
gabriela.gib...@gmail.com wrote:
 The differences between copy-bad-line-endings.expected.dump and
 copy-bad-line-endings.dump appear to be:

 1.  '\r' in the middle of a line is replaced by '\n'.
 2.  '\r' at the end of a line is deleted.

 Let's call this option 1.

 I had in mind to replace '\r' with 'space'.
 This would be option 2.

 Which is the prefered option?

I'd say that replacing '\r' with a 'space' is wrong.  That would
change the meaning of some properties.  E.G. svn:ignore, svn:externals
which use lines to handle individual records within them.

Your \r at the end of a line being deleted is in a log message.  I
suspect we have some code someplace that removes trailing new lines
from svn:log.  But I haven't dug too far on that.

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Daniel Shahaf]

Ben Reser wrote on Tue, Dec 11, 2012 at 17:59:47 -0800:
 Your \r at the end of a line being deleted is in a log message.  I
 suspect we have some code someplace that removes trailing new lines
 from svn:log.  But I haven't dug too far on that.

We have code on the client side that removes \r from the log message
supplied by the user.  (I don't really what that is in 'svn' (the
cmdline client) or in libsvn_client.)

But I guess you meant, code on the server side?

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Daniel Shahaf]

On Wed, Dec 12, 2012 at 04:02:01AM +0200, Daniel Shahaf wrote:
 Ben Reser wrote on Tue, Dec 11, 2012 at 17:59:47 -0800:
  Your \r at the end of a line being deleted is in a log message.  I
  suspect we have some code someplace that removes trailing new lines
  from svn:log.  But I haven't dug too far on that.
 
 We have code on the client side that removes \r from the log message
 supplied by the user.  (I don't really what that is in 'svn' (the

s/what/remember whether/

 cmdline client) or in libsvn_client.)
 
 But I guess you meant, code on the server side?

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Daniel Shahaf]

Gabriela Gibson wrote on Wed, Dec 12, 2012 at 00:50:41 +:
 On 11/12/12 00:46, Daniel Shahaf wrote:

 snip
 
  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
 
 Thanks for this.  This morphs into:


I got the stack trace by building --enable-maintainer-mode (which you
should be using) and then running just the individual test (r1420511
extends the documentation on this).

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Ben Reser]

On Tue, Dec 11, 2012 at 6:02 PM, Daniel Shahaf danie...@elego.de wrote:
 We have code on the client side that removes \r from the log message
 supplied by the user.  (I don't really remember whether that is in 'svn' (the
 cmdline client) or in libsvn_client.)

That would be the svn_subst_translate_string2() call in
svn_cl__get_log_message() which is part of the cmdline client.

I was thinking we had some code to remove trailing whitespace from
logs, but I just looked and we don't.

I'd missed that copy-bad-line-endings.dump had a trailing CRLF not
just a trailing CR.  The CRLF is being converted to just a LF because
we are passing TRUE as the repair argument to the
svn_subst_translate_string2() call in svnsync.

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Ben Reser]

On Tue, Dec 11, 2012 at 3:14 PM, C. Michael Pilato cmpil...@collab.net wrote:
 We should probably link to the Coding Conventions section from the Patch
 submission guidelines section just to be thorough.

Done in r1420516.

[PATCH] Test for line ending bug in svnrdump (issue 4263)

[Gabriela Gibson]

[[[
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)
]]]



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):
+  load: inconsistent line endings in svn:* props
+  run_load_test(sbox, copy-bad-line-endings.dump)
+  
 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,

Re: [PATCH] Test for line ending bug in svnrdump (issue 4263)

[Daniel Shahaf]

Gabriela Gibson wrote on Tue, Dec 11, 2012 at 00:21:46 +:


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

[PATCH] Test for line ending bug in svnrdump (issue 4263)

[Gabriela Gibson]

[[[
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)

]]]
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):
+  load: inconsistent line endings in svn:* props
+  run_load_test(sbox, copy-bad-line-endings.dump)
+  
 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,

RE: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties

[neil.winton]

 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

[Hyrum K Wright]

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

[Daniel Shahaf]

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

[C. Michael Pilato]

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

[neil.winton]

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

[neil.winton]

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

[Hyrum K Wright]

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

[C. Michael Pilato]

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

[Hyrum K Wright]

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