Re: svn commit: r1418830 - in /subversion/trunk/subversion/bindings/swig: core.i python/tests/checksum.py python/tests/run_all.py

2012-12-10 Thread Daniel Shahaf
On Sun, Dec 09, 2012 at 08:05:52AM -, bre...@apache.org wrote:
 +class ChecksumTestCases(unittest.TestCase):
 +def test_checksum(self):
 +# Checking primarily the return type for the svn_checksum_create
 +# function
 +val = svn.core.svn_checksum_create(svn.core.svn_checksum_md5)
 +check_val = svn.core.svn_checksum_to_cstring_display(val)
 +
 +# The svn_checksum_to_cstring_display should return a str type object
 +# from the check_val object passed to it
 +if(type(check_val) == str):
 +# The intialized value created from a checksum should be 0

Typo in comment.

 +if(int(check_val) != 0):

It would be better to write:

if check_val == '0'*32

(except that the test shouldn't hardcode 32)

This will catch a digest of the wrong length, and will avoid doing type
equality checking (inheritance checking is preferred).

 +self.assertRaises(AssertionError)

This line does not cause the test to fail.  It returns a context manager ---
the language construct implementing the 'with' statement.

 +else:
 +self.assertRaises(TypeError, test_checksum)

Infinite recursion.


Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)

2012-12-10 Thread Daniel Shahaf
Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100:
 On 10.12.2012 00:08, Johan Corveleyn wrote:
  On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf d...@daniel.shahaf.name 
  wrote:
  Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100:
  2) Am I the only one who wants to protect his repository against this
  corruption? Judging from [1], I don't think so. It doesn't make sense
  that everyone starts writing this pre-commit hook, for something that
  IMHO is an obvious anti-corruption protection. I think every
  repository out there deserves to be protected against this.
 
  FWIW, I suggested a hook because you could implement that in about
  5 minutes, whereas doing a C code change would take at least 10 times
  that (and several weeks if you have to wait for it to appear in a 1.7.x
  release that you can install at $WORK).  I won't object to C code
  verifying the svn:eol-style invariant.
  Thanks. And your pre-commit hook example is much appreciated.
 
  For the moment I get the impression that it's not really doable /
  desirable to implement this in the repository. At least until now
  no-one has suggested how it could be done, and I don't know enough
  myself about the server-side / back-end to figure it out :-).
 
 The first obstacle is that the server does not interpret properties.[1]
 Therefore, you'd have to implement this check at transaction-commit
 time, because there's no earlier point where you're guaranteed to have
 all node properties at their final values. This implies that the time to
 reject a commit would be proportional to the size of the commit (which
 isn't the case when it comes to conflict detection).

Can't you do what stefan2 suggested, but in libsvn_repos?  The repos
layer commit editor would remember for each file whether its textdelta
has added a new 0x0D byte, then at close_edit() it would iterate all
files in the commit --- and for each file only compare its svn:eol-style
property to the by-now-precomputed did it it contain a 0x0D bit.

I'm not sure how efficient it would be to parse for 0x0D's in
libsvn_repos, though.  Maybe we should make this optional.

 
 All properties are interpreted by clients. A buggy client will cause
 cause problems for other users, so the best course of action is to
 report the bug (to SvnKit in this case?).
 
 Also note that you're using a much too strong term when you talk about
 corrupted files in this case. There's nothing corrupted about them.
 

An invariant failed to maintain.  Granted it's not an FS_level
invariant, though.


 
 -- Brane
 
 [1] Not strictly true since mod_dav_svn will look at svn:mime-type when
 serving the content at its default, unversioned URL; but it doesn't
 actually interpret the value.
 
 -- 
 Branko Čibej
 Director of Subversion | WANdisco | www.wandisco.com
 


Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)

2012-12-10 Thread Branko Čibej
On 10.12.2012 11:31, Daniel Shahaf wrote:
 Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100:
 On 10.12.2012 00:08, Johan Corveleyn wrote:
 On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf d...@daniel.shahaf.name 
 wrote:
 Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100:
 2) Am I the only one who wants to protect his repository against this
 corruption? Judging from [1], I don't think so. It doesn't make sense
 that everyone starts writing this pre-commit hook, for something that
 IMHO is an obvious anti-corruption protection. I think every
 repository out there deserves to be protected against this.

 FWIW, I suggested a hook because you could implement that in about
 5 minutes, whereas doing a C code change would take at least 10 times
 that (and several weeks if you have to wait for it to appear in a 1.7.x
 release that you can install at $WORK).  I won't object to C code
 verifying the svn:eol-style invariant.
 Thanks. And your pre-commit hook example is much appreciated.

 For the moment I get the impression that it's not really doable /
 desirable to implement this in the repository. At least until now
 no-one has suggested how it could be done, and I don't know enough
 myself about the server-side / back-end to figure it out :-).
 The first obstacle is that the server does not interpret properties.[1]
 Therefore, you'd have to implement this check at transaction-commit
 time, because there's no earlier point where you're guaranteed to have
 all node properties at their final values. This implies that the time to
 reject a commit would be proportional to the size of the commit (which
 isn't the case when it comes to conflict detection).
 Can't you do what stefan2 suggested, but in libsvn_repos?  The repos
 layer commit editor would remember for each file whether its textdelta
 has added a new 0x0D byte, then at close_edit() it would iterate all
 files in the commit --- and for each file only compare its svn:eol-style
 property to the by-now-precomputed did it it contain a 0x0D bit.

 I'm not sure how efficient it would be to parse for 0x0D's in
 libsvn_repos, though.  Maybe we should make this optional.


It's not enough to just look for \r in the delta stream. Certainly
wouldn't help with historically broken files.

Moreover, if libsvn_repos started looking at svn:eol-style, that'd only
make sense if you verified all normalizations, not just native. And
why should it just be svn:eol-style, when svn:keywords has potentially
the same problem? Eventually you start verifying everything that affects
file contents.

Maybe that's a good idea, but /why/ put it in libsvn_repos when it's
much easier and cleaner to just provide some standard hooks, written in
C and distributed with releases, that the admin plug in if she feels
like it? Surely that's what hooks are for.


-- Brane


-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com



Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)

2012-12-10 Thread Ivan Zhakov
On Mon, Dec 10, 2012 at 2:51 PM, Branko Čibej br...@wandisco.com wrote:
 On 10.12.2012 11:31, Daniel Shahaf wrote:
 Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100:
 On 10.12.2012 00:08, Johan Corveleyn wrote:
 On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf d...@daniel.shahaf.name 
 wrote:
 Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100:
 2) Am I the only one who wants to protect his repository against this
 corruption? Judging from [1], I don't think so. It doesn't make sense
 that everyone starts writing this pre-commit hook, for something that
 IMHO is an obvious anti-corruption protection. I think every
 repository out there deserves to be protected against this.

 FWIW, I suggested a hook because you could implement that in about
 5 minutes, whereas doing a C code change would take at least 10 times
 that (and several weeks if you have to wait for it to appear in a 1.7.x
 release that you can install at $WORK).  I won't object to C code
 verifying the svn:eol-style invariant.
 Thanks. And your pre-commit hook example is much appreciated.

 For the moment I get the impression that it's not really doable /
 desirable to implement this in the repository. At least until now
 no-one has suggested how it could be done, and I don't know enough
 myself about the server-side / back-end to figure it out :-).
 The first obstacle is that the server does not interpret properties.[1]
 Therefore, you'd have to implement this check at transaction-commit
 time, because there's no earlier point where you're guaranteed to have
 all node properties at their final values. This implies that the time to
 reject a commit would be proportional to the size of the commit (which
 isn't the case when it comes to conflict detection).
 Can't you do what stefan2 suggested, but in libsvn_repos?  The repos
 layer commit editor would remember for each file whether its textdelta
 has added a new 0x0D byte, then at close_edit() it would iterate all
 files in the commit --- and for each file only compare its svn:eol-style
 property to the by-now-precomputed did it it contain a 0x0D bit.

 I'm not sure how efficient it would be to parse for 0x0D's in
 libsvn_repos, though.  Maybe we should make this optional.


 It's not enough to just look for \r in the delta stream. Certainly
 wouldn't help with historically broken files.

 Moreover, if libsvn_repos started looking at svn:eol-style, that'd only
 make sense if you verified all normalizations, not just native. And
 why should it just be svn:eol-style, when svn:keywords has potentially
 the same problem? Eventually you start verifying everything that affects
 file contents.

 Maybe that's a good idea, but /why/ put it in libsvn_repos when it's
 much easier and cleaner to just provide some standard hooks, written in
 C and distributed with releases, that the admin plug in if she feels
 like it? Surely that's what hooks are for.


New program 'svnhooks' with set of hooks for standard recommended
polices would be great. svn:eol-style check is good start.


-- 
Ivan Zhakov


Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)

2012-12-10 Thread Daniel Shahaf
Branko Čibej wrote on Mon, Dec 10, 2012 at 11:51:08 +0100:
 On 10.12.2012 11:31, Daniel Shahaf wrote:
  Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100:
  On 10.12.2012 00:08, Johan Corveleyn wrote:
  On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf d...@daniel.shahaf.name 
  wrote:
  Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100:
  2) Am I the only one who wants to protect his repository against this
  corruption? Judging from [1], I don't think so. It doesn't make sense
  that everyone starts writing this pre-commit hook, for something that
  IMHO is an obvious anti-corruption protection. I think every
  repository out there deserves to be protected against this.
 
  FWIW, I suggested a hook because you could implement that in about
  5 minutes, whereas doing a C code change would take at least 10 times
  that (and several weeks if you have to wait for it to appear in a 1.7.x
  release that you can install at $WORK).  I won't object to C code
  verifying the svn:eol-style invariant.
  Thanks. And your pre-commit hook example is much appreciated.
 
  For the moment I get the impression that it's not really doable /
  desirable to implement this in the repository. At least until now
  no-one has suggested how it could be done, and I don't know enough
  myself about the server-side / back-end to figure it out :-).
  The first obstacle is that the server does not interpret properties.[1]
  Therefore, you'd have to implement this check at transaction-commit
  time, because there's no earlier point where you're guaranteed to have
  all node properties at their final values. This implies that the time to
  reject a commit would be proportional to the size of the commit (which
  isn't the case when it comes to conflict detection).
  Can't you do what stefan2 suggested, but in libsvn_repos?  The repos
  layer commit editor would remember for each file whether its textdelta
  has added a new 0x0D byte, then at close_edit() it would iterate all
  files in the commit --- and for each file only compare its svn:eol-style
  property to the by-now-precomputed did it it contain a 0x0D bit.
 
  I'm not sure how efficient it would be to parse for 0x0D's in
  libsvn_repos, though.  Maybe we should make this optional.
 
 
 It's not enough to just look for \r in the delta stream. Certainly
 wouldn't help with historically broken files.

I wasn't trying to fix historically broken files.  (That would require
a fulltext scan --- that's not a cheap computation.)  Do you see another
problem?

 Maybe that's a good idea, but /why/ put it in libsvn_repos when it's
 much easier and cleaner to just provide some standard hooks, written in
 C and distributed with releases, that the admin plug in if she feels
 like it? Surely that's what hooks are for.

The argument is that a Subversion server should be enforcing
Subversion's invariants.

That said, I'm not opposed to doing it via standard hooks.  It's a good
way to introduce the feature in a way that allows more easily changing
it before it hits the APIs and their strict compatibility rules.

 
 
 -- Brane
 
 
 -- 
 Branko Čibej
 Director of Subversion | WANdisco | www.wandisco.com
 


[PATCH] Implement svnadmin verify --keep-going

2012-12-10 Thread Prabhu Gnana Sundar

Hi,

This patch is a follow up of the long discussion we had in thread [1]. 
This patch implements a new switch --keep-going to svnadmin verify.


If --keep-going is set(True), svnadmin verify would continue to run 
till verifying all the revisions i.e, it would not stop at the very 
first occurance of a corruption/error found in the repo and would report 
corruptions wherever found.



I have worked on your suggestions and inputs for this patch. Kindly 
share your thoughts. Attaching the patch and the log message with this mail.



Thanks and regards
Prabhu

[1] http://svn.haxx.se/dev/archive-2012-10/0271.shtml

Implement svnadmin verify --keep-going, which would continue the verification
even if there is some corruption, after printing the errors to stderr.

* subversion/svnadmin/svnadmin.c
  (svnadmin__cmdline_options_t): Add keep-going option.
  (svnadmin_opt_state): Add keep-going option.
  (subcommand_verify): Switch to the new svn_repos_verify_fs3 function instead
   of svn_repos_verify_fs2, and pass the keep-going option.
  (repos_notify_handler): Handle svn_repos_notify_failure notification by
   printing warnings to stderr with the respective revision number.

* subversion/include/svn_repos.h
  (svn_repos_notify_action_t): Add svn_repos_notify_failure to notify failure.
  (svn_repos_verify_fs3): Newly added to handle keep-going option.
  (svn_repos_notify_t): Add err, the error chain which indicates what went
   wrong during verification.

* subversion/libsvn_repos/dump.c
  (svn_repos_verify_fs3): Handle keep-going. If keep-going is TRUE, the
   error message is notified and verification process continues.
  When a repository fails to verify, return an error SVN_ERR_REPOS_CORRUPTED.
  (notify_verification_error): New function to notify the verification
  failure error message.

* subversion/libsvn_repos/deprecated.c
  (svn_repos_verify_fs2): Deprecate. Call svn_repos_verify_fs3 with
   keep_going as FALSE by default to keep the old default implementation.

* subversion/libsvn_repos/notify.c
  (svn_repos_notify_create): Initialise the error chain err to SVN_NO_ERROR.

* subversion/tests/cmdline/svnadmin_tests.py
  (verify_keep_going): New test case to test svnadmin verify and the new
  switch --keep-going.

Patch by: Prabhu Gnana Sundar prabhugs{_AT_}collab.net
Index: subversion/tests/cmdline/svnadmin_tests.py
===
--- subversion/tests/cmdline/svnadmin_tests.py	(revision 1411074)
+++ subversion/tests/cmdline/svnadmin_tests.py	(working copy)
@@ -1835,6 +1835,114 @@
   svntest.main.run_svnadmin(recover, sbox.repo_dir)
 
 
+def verify_keep_going(sbox):
+  svnadmin verify --keep-going test
+  test_create(sbox)
+  dumpfile_skeleton = open(os.path.join(os.path.dirname(sys.argv[0]),
+'svnadmin_tests_data',
+'skeleton_repos.dump')).read()
+  load_dumpstream(sbox, dumpfile_skeleton, '--ignore-uuid')
+
+  r2 = fsfs_file(sbox.repo_dir, 'revs', '6')
+  fp = open(r2, 'wb')
+  fp.write(id: 3-6.0.r6/0
+type: file
+count: 0
+text: 2 0 60 48 02d086e41b03058c5f1af6282c1f483f cc67e4dd7cd8ca83095c8b95f65b6698b39cb263 5-5/_5
+props: 2 73 34 0 25e6c2f7558b7484000d4d090dea5b92
+cpath: /Projects/docs/README
+copyroot: 0 /
+
+PLAIN
+K 6
+README
+V 15
+file 3-6.0.r6/0
+END
+ENDREP
+id: 1-6.0.r6/275
+type: dir
+count: 0
+text: 6 226 36 0 2c3f6410944c6ff8667fa1b3e78f45a2
+cpath: /Projects/docs
+copyroot: 0 /
+
+PLAIN
+K 9
+Project-X
+V 14
+dir 1-3.0.r3/0
+K 9
+Project-Y
+V 14
+dir 1-4.0.r4/0
+K 9
+Project-Z
+V 14
+dir 1-5.0.r5/0
+K 4
+docs
+V 16
+dir 1-6.0.r6/275
+END
+ENDREP
+id: 0-1.0.r6/548
+type: dir
+pred: 0-1.0.r5/195
+count: 4
+text: 6 398 137 0 c758f548da93cc57d68af0610766b549
+cpath: /Projects
+copyroot: 0 /
+
+PLAIN
+K 8
+Projects
+V 16
+dir 0-1.0.r6/548
+K 6
+README
+V 17
+file 0-2.0.r2/120
+END
+ENDREP
+id: 0.0.r6/772
+type: dir
+pred: 0.0.r5/418
+count: 6
+text: 6 686 73 0 353c6bbf43b0f2ae474d85e206337bbd
+cpath: /
+copyroot: 0 /
+
+_1.0.t5-5 add-dir false false /Projects/docs
+
+_3.0.t5-5 add-file true true /Projects/docs/README
+
+
+)
+  fp.close()
+  exit_code, output, errput = svntest.main.run_svnadmin(verify,
+sbox.repo_dir)
+  exit_code, output, errput2 = svntest.main.run_svnadmin(verify,
+ --keep-going,
+ sbox.repo_dir)
+
+  if svntest.verify.verify_outputs(Unexpected error while running 'svnadmin verify'.,
+ [], errput, None, .*svnadmin: E165005: .*):
+raise svntest.Failure
+
+  if svntest.verify.verify_outputs(Unexpected error while running 'svnadmin verify'.,
+   [], errput2, None,
+   [* Verified revision 0.\n,
+   * Verified 

Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065)

2012-12-10 Thread Johan Corveleyn
On Mon, Dec 10, 2012 at 12:47 PM, Daniel Shahaf d...@daniel.shahaf.name wrote:
 Branko Čibej wrote on Mon, Dec 10, 2012 at 11:51:08 +0100:
 On 10.12.2012 11:31, Daniel Shahaf wrote:
  Branko Čibej wrote on Mon, Dec 10, 2012 at 00:26:20 +0100:
  On 10.12.2012 00:08, Johan Corveleyn wrote:
  On Sun, Dec 9, 2012 at 11:43 PM, Daniel Shahaf d...@daniel.shahaf.name 
  wrote:
  Johan Corveleyn wrote on Sun, Dec 09, 2012 at 21:15:24 +0100:
  2) Am I the only one who wants to protect his repository against this
  corruption? Judging from [1], I don't think so. It doesn't make sense
  that everyone starts writing this pre-commit hook, for something that
  IMHO is an obvious anti-corruption protection. I think every
  repository out there deserves to be protected against this.
 
  FWIW, I suggested a hook because you could implement that in about
  5 minutes, whereas doing a C code change would take at least 10 times
  that (and several weeks if you have to wait for it to appear in a 1.7.x
  release that you can install at $WORK).  I won't object to C code
  verifying the svn:eol-style invariant.
  Thanks. And your pre-commit hook example is much appreciated.
 
  For the moment I get the impression that it's not really doable /
  desirable to implement this in the repository. At least until now
  no-one has suggested how it could be done, and I don't know enough
  myself about the server-side / back-end to figure it out :-).
  The first obstacle is that the server does not interpret properties.[1]
  Therefore, you'd have to implement this check at transaction-commit
  time, because there's no earlier point where you're guaranteed to have
  all node properties at their final values. This implies that the time to
  reject a commit would be proportional to the size of the commit (which
  isn't the case when it comes to conflict detection).
  Can't you do what stefan2 suggested, but in libsvn_repos?  The repos
  layer commit editor would remember for each file whether its textdelta
  has added a new 0x0D byte, then at close_edit() it would iterate all
  files in the commit --- and for each file only compare its svn:eol-style
  property to the by-now-precomputed did it it contain a 0x0D bit.
 
  I'm not sure how efficient it would be to parse for 0x0D's in
  libsvn_repos, though.  Maybe we should make this optional.


 It's not enough to just look for \r in the delta stream. Certainly
 wouldn't help with historically broken files.

 I wasn't trying to fix historically broken files.  (That would require
 a fulltext scan --- that's not a cheap computation.)  Do you see another
 problem?

Indeed, historically broken files are not the focus here. They're
broken in the history of the repository anyway, so that's not really
fixable anymore. But if we can prevent new broken files from entering
the repository, that would be great.

 Maybe that's a good idea, but /why/ put it in libsvn_repos when it's
 much easier and cleaner to just provide some standard hooks, written in
 C and distributed with releases, that the admin plug in if she feels
 like it? Surely that's what hooks are for.

 The argument is that a Subversion server should be enforcing
 Subversion's invariants.

 That said, I'm not opposed to doing it via standard hooks.  It's a good
 way to introduce the feature in a way that allows more easily changing
 it before it hits the APIs and their strict compatibility rules.

Yes, that would be fine I think. I like Ivan's suggestion of creating
a new standard 'svnhooks' program or something like that, which can
then be part of standard distributions.

-- 
Johan


RE: enforcing LF-normalization for svn:eol-style=native files (issue #4065)

2012-12-10 Thread Bert Huijben


 -Original Message-
 From: Branko Čibej [mailto:br...@wandisco.com]
 Sent: maandag 10 december 2012 8:04
 To: dev@subversion.apache.org
 Subject: Re: enforcing LF-normalization for svn:eol-style=native files (issue
 #4065)
 
 On 10.12.2012 07:35, Bert Huijben wrote:
  I don’t think you have to wait until commit time: You could verify the
  commit base revision’s properties + changes. In the cases where the
  properties change before commit, the commit would fail for being out of
  date.
 
 That would imply you can't change properties and contents in the same
 commit, wouldn't it. Which I suspect is not the case? :)
 
 The problem is worse, you can theoretically reopen and modify a
 transaction any number of times before committing it, so you don't
 really know until txn commit time which properties actually apply to
 which file contents.

On the fs layer this is probably true, but in a normal editor drive through the 
ra layers this should never happen.

On the ra layer the final set of properties is valid in the file_close() 
callback. I'm just not sure if this is really an event that the server sees for 
ra_dav, or if this is implied by the committing.

Bert 




Re: [PATCH] Implement svnadmin verify --keep-going

2012-12-10 Thread Julian Foad
Prabhu Gnana Sundar

 This patch is a follow up of the long discussion we had in thread [1]. This 
 patch implements a new switch --keep-going to svnadmin verify.
 
 If --keep-going is set(True), svnadmin verify would continue to run 
 till verifying all the revisions i.e, it would not stop at the very first 
 occurance of a corruption/error found in the repo and would report 
 corruptions 
 wherever found.
 
 
 I have worked on your suggestions and inputs for this patch. Kindly share 
 your 
 thoughts. Attaching the patch and the log message with this mail.

Thank you for the patch.

Please will you summarize the issues that were raised in the previous 
discussion and how you have chosen to proceed.  I'm thinking in particular of 
the discussion about how the output is notified -- how to display each error, 
on what output stream, and what to print at the end of the whole verification 
and what exit code to return.  There may have been other issues too.

I scanned quickly through your patch and I noticed one place where you declare 
a local function without the 'static' keyword.  I expect this should give a 
warning when you compile it, so please will you compile with and without your 
patch applied and check for any additional compiler warnings that your patch 
adds.

- Julian


Re: [PATCH] Implement svnadmin verify --keep-going

2012-12-10 Thread Daniel Shahaf
 Index: subversion/tests/cmdline/svnadmin_tests.py
 ===
 --- subversion/tests/cmdline/svnadmin_tests.py(revision 1411074)
 +++ subversion/tests/cmdline/svnadmin_tests.py(working copy)
 @@ -1835,6 +1835,114 @@
svntest.main.run_svnadmin(recover, sbox.repo_dir)
  
  
 +def verify_keep_going(sbox):
 +  svnadmin verify --keep-going test
 +  test_create(sbox)
 +  dumpfile_skeleton = open(os.path.join(os.path.dirname(sys.argv[0]),
 +
 'svnadmin_tests_data',
 +
 'skeleton_repos.dump')).read()
 +  load_dumpstream(sbox, dumpfile_skeleton, '--ignore-uuid')
 +
 +  r2 = fsfs_file(sbox.repo_dir, 'revs', '6')
 +  fp = open(r2, 'wb')
 +  fp.write(id: 3-6.0.r6/0

This test will fail when building with
-DSVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR=3 -DPACK_AFTER_EVERY_COMMIT
.  Can it recognise the situation and Skip?

(It's fine if it can't: svnadmin_tests 17 has always FAILed with those flags.)

 +)
 +  fp.close()
 +  exit_code, output, errput = svntest.main.run_svnadmin(verify,
 +sbox.repo_dir)
 +  exit_code, output, errput2 = svntest.main.run_svnadmin(verify,
 + --keep-going,
 + sbox.repo_dir)
 +
 +  if svntest.verify.verify_outputs(Unexpected error while running 'svnadmin 
 verify'.,
 + [], errput, None, .*svnadmin: E165005: 
 .*):
 +raise svntest.Failure
 +
 +  if svntest.verify.verify_outputs(Unexpected error while running 'svnadmin 
 verify'.,
 +   [], errput2, None,
 +   [* Verified revision 0.\n,
 +   * Verified revision 1.\n,
 +   * Verified revision 2.\n,
 +   * Verified revision 3.\n,

Please avoid testing the literal output.  It means every single change
to the progress reporting or error reporting will need the test to be
updated.

 +   * Verified revision 4.\n,
 +   * Verified revision 5.\n,
 +   * Error verifying revision 6.\n,
 +   svnadmin: E24: Could not convert '' 
 into a number\n,

It might be useful to add a comment here explaining the error --- it's
because the last line in the revision file is blank.  Alternatively, you
could make that line contain a sentinel string and then check that the
sentinel appears in the error message; that would be self-documenting.

 +   svnadmin: E165005: Repository 
 'svn-test-work/repositories/svnadmin_tests-31' failed to verify\n]):
 +  raise svntest.Failure
 +

 +{keep-going,svnadmin__keep_going, 0,
 + N_(continue verifying even if there is a corruption)},

s/even if there is/after detecting/
?

 @@ -744,6 +749,21 @@
  notify-warning_str));
return;
  
 +case svn_repos_notify_failure:
 +  if (notify-revision != SVN_INVALID_REVNUM)
 +svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
 +  (* Error verifying revision 
 %ld.\n),
 +  notify-revision));
 +/*svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
 +  _(svnadmin: E%d: Error verifying 
 revision %ld\n),
 +  notify-err-apr_err,
 +  notify-revision));
 +*/  

This debris doesn't belong in the patch. :)

/** For #svn_repos_notify_load_node_start, the path of the node. */
const char *path;
  
 +  /** For #svn_repos_notify_failure, this error chain indicates what
 +  went wrong during verification. */

Add @since please to the docstring.

 +  svn_error_t *err;
 +
 Index: subversion/libsvn_repos/dump.c
 ===
 --- subversion/libsvn_repos/dump.c(revision 1411074)
 +++ subversion/libsvn_repos/dump.c(working copy)
 @@ -1359,10 +1359,28 @@
return close_directory(dir_baton, pool);
  }
  
 +void

static void

 +notify_verification_error(svn_revnum_t rev,
 +  svn_error_t *err,
 +  svn_repos_notify_func_t notify_func,
 +  void *notify_baton,
 +  apr_pool_t *pool)
 +{
 +  if (notify_func)
 +{
 +  svn_repos_notify_t *notify_failure;
 +  notify_failure = svn_repos_notify_create(svn_repos_notify_failure, 
 pool);
 +  notify_failure-err = err;
 +  notify_failure-revision = rev;
 +  notify_func(notify_baton, 

Re: [PATCH] Implement svnadmin verify --keep-going

2012-12-10 Thread Daniel Shahaf
Julian Foad wrote on Mon, Dec 10, 2012 at 14:45:57 +:
 I scanned quickly through your patch and I noticed one place where you 
 declare a local function without the 'static' keyword.  I expect this should 
 give a warning when you compile it, so please will you compile with and 
 without your patch applied and check for any additional compiler warnings 
 that your patch adds.

Or you can cheat by using compiler flags that build trunk by default
with no warnings:

'./configure'  '--enable-maintainer-mode' '-C' 'CFLAGS= -DSVN_DEPRECATED= 
-Wformat=0 -Wno-unreachable-code -g' 'CC=x86_64-linux-gnu-gcc' $@ 


Re: Buildbot screen per branch?

2012-12-10 Thread Daniel Shahaf
Filed a ticket:
https://issues.apache.org/jira/browse/INFRA-5623

Daniel Shahaf wrote on Wed, Nov 21, 2012 at 11:42:26 +0200:
 Currently http://subversion.apache.org/buildbot contains a mixture of
 builds for all active branches (trunk, 1.6.x, 1.7.x).  That makes it
 unfeasible to track the status of, say, the 1.6.x branch (which had
 a failure tonight - but that will be washed out soon by trunk commits
 and 1.7.x backport merges).
 
 We could split up each builder (column in
 http://subversion.apache.org/buildbot) into N builders, one per branch
 --- that adds no maintenance overhead (according to Gavin), and will
 allow us to easily monitor the status (and history) of each branch
 by adding a http://subversion.apache.org/buildbot-17x redirect that
 shows only the 1.7.x builders (at least N recent builds of each).
 
 WDYT?
 
 Daniel
 
 (Implementation-wise --- see mkcassbuilder() in cassandra.conf for an
 example.)


RE: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c

2012-12-10 Thread Bert Huijben


 -Original Message-
 From: julianf...@apache.org [mailto:julianf...@apache.org]
 Sent: maandag 10 december 2012 17:29
 To: comm...@subversion.apache.org
 Subject: svn commit: r1419560 -
 /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
 
 Author: julianfoad
 Date: Mon Dec 10 16:28:34 2012
 New Revision: 1419560
 
 URL: http://svn.apache.org/viewvc?rev=1419560view=rev
 Log:
 * subversion/libsvn_wc/wc_db_update_move.c
   (update_working_file): Update a comment.
 
 Modified:
 subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
 
 Modified: subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
 URL:
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_
 db_update_move.c?rev=1419560r1=1419559r2=1419560view=diff
 ==
 
 --- subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
 (original)
 +++ subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c Mon
 Dec 10 16:28:34 2012
 @@ -247,8 +247,8 @@ update_working_file(svn_skel_t **work_it
old_version-props, old_version-props,
actual_props, propchanges,
scratch_pool, scratch_pool));
 -  /* ### TODO: Make a WQ item in WORK_ITEMS to set new_actual_props
 ... */
 -  /* ### Not a direct DB op like this... */
 +  /* Install the new actual props. Don't set the conflict_skel yet, because
 + we might need to add a text conflict to it as well. */
SVN_ERR(svn_wc__db_op_set_props(db, local_abspath,
new_actual_props,
svn_wc__has_magic_property(propchanges),

This tells me that the change is not atomic, so this really needs some fix-me 
comment after all.

The text and properties should be modified in one sqlite operation. 
(Or in other words: the db should be updated to its final state, with the 
installing of the wq items in a single transaction)

I think the standard svn_wc_mergesomething() function should handle this for 
you in one step, so this function should do something similar.


It is not a problem to install a conflict skel and then to reinstall it later 
with more details. But it would be a problem for the client to crash between 
updating the state and installing the conflict. The sqlite transactions should 
catch that.

Bert



Re: svn commit: r1417639 - in /subversion/trunk/subversion/mod_dav_svn: dav_svn.h mod_dav_svn.c reports/update.c

2012-12-10 Thread C. Michael Pilato
On 12/07/2012 02:39 AM, Lieven Govaerts wrote:
 AFAIK, it is only dump where the svnrdump tool fails when using Serf.
 Because of the Ev1 stuff.  Using bulk-updates ought to avoid that
 issue.
 
 Actually, my proposed hack of forcing ra_serf to use bulk update mode
 is not going to work. If the server has SVNAllowBulkUpdates set to
 Off, bulk update mode isn't available, the server will ignore any
 requests from the client and force skelta mode.
 
 If I remember correctly the issue here was responses arriving out of
 order due to multiple parallel connections. So the fix until svnrdump
 has moved to Ev2 is to stick to one extra connection for all the file
 content requests, so all of those responses arrive in sync.
 Don't know how easy that is to do, I wonder if we have a simple
 mechanism that allows a client to pass ra implementation specific
 options.

IIUC, the problem with svnrdump occurs only for the single revision at the
start of a non-incremental dump.  svnrdump runs svn_ra_do_update2() in such
a way as to pretend that it's doing a checkout.  After that one revision, it
runs a loop around svn_ra_replay_range().

Perhaps there's an opportunity here to solve two problems at once.

What if we revved the svn_ra_replay_range() API in such a way that it could
now handle this initial revision scenario?  We might add an 'incremental'
flag that parallels what 'svnadmin dump' and 'svnrdump dump' do.  This would
get us to a single RA API used for revision replays (replay_range, instead
of a combination of do_update + replay_range), which simplifies how replays
are done from an API perspective.  Of course, under the hood, the RA
implementations of the new replay_range2() would probably just use the same
update mechanics to handle that initial revision when not in incremental
mode, but ra_serf's particular implementation would be free to choose
send-all mode in this one scenario to do exactly that.

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



signature.asc
Description: OpenPGP digital signature


Re: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c

2012-12-10 Thread Julian Foad
Bert Huijben wrote:

  URL: http://svn.apache.org/viewvc?rev=1419560view=rev
  Log:
  * subversion/libsvn_wc/wc_db_update_move.c
    (update_working_file): Update a comment.

  @@ -247,8 +247,8 @@ update_working_file(svn_skel_t **work_it
                                 old_version-props, old_version-props,
                                 actual_props, propchanges,
                                 scratch_pool, scratch_pool));
  -  /* ### TODO: Make a WQ item in WORK_ITEMS to set new_actual_props
  ... */
  -  /* ### Not a direct DB op like this... */
  +  /* Install the new actual props. Don't set the conflict_skel yet, 
  +     because we might need to add a text conflict to it as well. */
     SVN_ERR(svn_wc__db_op_set_props(db, local_abspath,
                                     new_actual_props,
                                     svn_wc__has_magic_property(propchanges),
 
 This tells me that the change is not atomic, so this really needs some fix-me 
 comment after all.
 
 The text and properties should be modified in one sqlite operation. 
 (Or in other words: the db should be updated to its final state, with the 
 installing of the wq items in a single transaction)

The db_op_set_property() here is just one of a series of DB modifications being 
made here, all within a single DB txn.

svn_wc__db_op_set_property() uses a txn internally, but is being called within 
a larger 'outer txn' (sorry if the terminology is wrong) set up by the 
top-level function, svn_wc__db_update_moved_away_conflict_victim(), which 
executes the whole call tree in this file (wc_db_update_move.c) within 
svn_wc__db_with_txn().

- Julian


 I think the standard svn_wc_mergesomething() function should handle this 
 for you in one step, so this function should do something similar.
 
 
 It is not a problem to install a conflict skel and then to reinstall it later 
 with more details. But it would be a problem for the client to crash between 
 updating the state and installing the conflict. The sqlite transactions 
 should 
 catch that.


Re: Literals in wc SQLite queries

2012-12-10 Thread Daniel Shahaf
As I said on IRC, happy to look into this, my main question is how
transform_sql.py would know what files to scan for the MAP_DELETED -
'base-deleted' mappings.

Do we want a header file with a well-known name
(subversion/include/private/)?  Maybe in the same directory as the
source .sql file?  Maybe the .sql file should have a directive pointing
to the map file?

Among these I prefer the second one, i.e., 
subversion/libsvn_wc/wc-queries.sql - subversion/libsvn_wc/token-maps.h

Philip - perhaps you can move the relevant definitions to that header?
Then I'll follow up with a transform_sql.py patch.

Philip Martin wrote on Fri, Dec 07, 2012 at 17:54:16 +:
 Columns such as nodes.kind, nodes.presence, etc. have strings that
 should be one of a discrete set of values.  When we bind these columns
 in C code we use something like:
 
 svn_sqlite__bindf(t, presence_map, svn_wc__db_status_normal);
 
 This means we only use known values (svn_wc__db_status_normal) and the
 map converts it to the correct discrete string.  This checking happens
 at build time.
 
 We also have queries where the strings are defined as literals in
 wc-queries.sql like:
 
 DELETE FROM nodes
 WHERE wc_id = ?1
   AND IS_STRICT_DESCENDANT_OF(local_relpath, ?2)
   AND (op_depth  ?3
OR (op_depth = ?3 AND presence = 'base-deleted'))
 
 There is no checking of these literals to catch errors such as
 'base-delete'.
 
 I've been thinking that transform_sql.py should do some checking.
 Perhaps we could move the maps into a know header, annotate them:
 
 { base-deleted, svn_wc__db_status_base_deleted },  /* MAP_DELETED */
 
 and then have transform_sql.py parse the header and convert:
 
   OR (op_depth = ?3 AND presence = MAP_DELETED))
 
 into
 
   OR (op_depth = ?3 AND presence = 'base-deleted'))
 
 -- 
 Philip


Re: Literals in wc SQLite queries

2012-12-10 Thread Philip Martin
Daniel Shahaf danie...@elego.de writes:

 Philip - perhaps you can move the relevant definitions to that header?
 Then I'll follow up with a transform_sql.py patch.

OK, I've done that.  I didn't annotate the maps or elements in any way.

-- 
Certified  Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download


Re: svn commit: r1419560 - /subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c

2012-12-10 Thread Daniel Shahaf
Julian Foad wrote on Mon, Dec 10, 2012 at 16:57:26 +:
 svn_wc__db_op_set_property() uses a txn internally, but is being
 called within a larger 'outer txn' (sorry if the terminology is wrong)

Savepoint? 

http://www.sqlite.org/lang_savepoint.html


svnhooks program (was Re: enforcing LF-normalization for svn:eol-style=native files (issue #4065))

2012-12-10 Thread Ivan Zhakov
On Mon, Dec 10, 2012 at 3:53 PM, Johan Corveleyn jcor...@gmail.com wrote:
 On Mon, Dec 10, 2012 at 12:47 PM, Daniel Shahaf d...@daniel.shahaf.name 
 wrote:
[...]

 The argument is that a Subversion server should be enforcing
 Subversion's invariants.

 That said, I'm not opposed to doing it via standard hooks.  It's a good
 way to introduce the feature in a way that allows more easily changing
 it before it hits the APIs and their strict compatibility rules.

 Yes, that would be fine I think. I like Ivan's suggestion of creating
 a new standard 'svnhooks' program or something like that, which can
 then be part of standard distributions.

I'd like to share my thoughts about 'svnhooks' if someone is going to
implement it.

Introduce svnhooks program with the separate subcommand for each hook
kind: 'pre-commit', 'post-comit', 'pre-lock', 'pre-unlock and etc.
The polices for svnhooks program defined in conf\svnhooks.conf file.
This makes very easy for user to use svnhooks: just add line 'svnhooks
HOOK-TYPE $*' in each hook and tweak svnhooks.conf configuration file.

svnhooks configuration file has separate section for each policy. For example:
[[[
[check-eol-normalization]
enable = yes

[rev-propchange]
enable = yes
users = svnsync

[edit-log-message]
enable = yes
users = admin, @author
]]]

-- 
Ivan Zhakov


Re: SQLite vacuum or auto_vacuum?

2012-12-10 Thread C. Michael Pilato
On 12/03/2012 07:39 AM, Hyrum K Wright wrote:
 I think adding vacuum to cleanup is a reasonable first step.  cleanup is
 an explicit operation that a user could reasonably expect to take some
 non-trivial amount of time.  We already remove unneeded pristines during
 cleanup, might as well do the same thing with wc.db space.

Yup, agreed.  +1.

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



signature.asc
Description: OpenPGP digital signature


Re: svn commit: r1415864 - /subversion/trunk/subversion/libsvn_ra_serf/update.c

2012-12-10 Thread Lieven Govaerts
Hi Markus,

On Mon, Dec 3, 2012 at 10:43 AM, Markus Schaber m.scha...@codesys.com wrote:
 Hi,

 Just another crazy idea:

 The main problem with the parallelization in ra_serf seems to be the number 
 of http requests (which potentially causes a high number of authentications 
 and tcp connections).

 Maybe we could add some partitioned send-all request:

 When the client decides to use 4 connections, it could send 4 requests, with 
 some parameter like send-all(1/4), send-all(2/4), ..., send-all(4/4).

 Then the server can send one quarter of the complete response on each 
 connection.

 An advanced server could even share the common state of those 4 requests 
 through some shared memory / caching scheme, to avoid doing the same work 
 multiple times.

 Years ago, I implemented a similar scheme between caching GIS web frontend 
 servers, and the rendering backend server, in the protocol for fetching and 
 rendering the map tiles. It gave a nearly linear speedup with the number of 
 connections, up to the point where the CPUs were saturated.


the concept implemented in ra_serf is to parallelize individual GET
requests, so that these can be cached by proxies either on the client
or on the server side. So we want to avoid using send-all as much as
possible, as this creates always one large uncacheable response.

I've made a mental note of your idea though, if we need to stick with
send-all and further improve it, your suggestion is one way to do
this.


 Best regards

 Markus Schaber


thanks,

Lieven
[..]


[PATCH] Improve svn_checksum_t bindings in SWIG

2012-12-10 Thread Shivani Poddar
Log Message:

Improve support for svn_checksum.h in SWIG bindings
* subversion/bindings/swig/python/tests/checksum.py: Improved test_checksum

Review by: danielsh

Modified:
subversion/trunk/subversion/bindings/swig/python/tests/checksum.py


Regards,
Shivani Poddar
Bachelors in Computer Sciences and MS in Exact Humanities, Sophomore
International Institute of Information Technology, Hyderabad
Index: subversion/bindings/swig/python/tests/checksum.py
===
--- subversion/bindings/swig/python/tests/checksum.py   (revision 1419694)
+++ subversion/bindings/swig/python/tests/checksum.py   (working copy)
@@ -20,22 +20,25 @@
 #
 import unittest, setup_path
 import svn.core
-
+LENGTH = 32 or 40;
 class ChecksumTestCases(unittest.TestCase):
 def test_checksum(self):
 # Checking primarily the return type for the svn_checksum_create
 # function
 val = svn.core.svn_checksum_create(svn.core.svn_checksum_md5)
 check_val = svn.core.svn_checksum_to_cstring_display(val)
-
 # The svn_checksum_to_cstring_display should return a str type object
 # from the check_val object passed to it
 if(type(check_val) == str):
-# The intialized value created from a checksum should be 0
-if(int(check_val) != 0):
-self.assertRaises(AssertionError)
+#Check the length of the string check_val
+if(len(check_val) == LENGTH):
+# The intialized value created from a checksum should be 0
+if(int(check_val) != 0):
+raise
+else:
+ raise
 else:
-self.assertRaises(TypeError, test_checksum)
+raise
 
 def suite():
 return unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases)


1.7.8 up for testing/signing

2012-12-10 Thread Ben Reser
The 1.7.8 release artifacts are now available for testing/signing.
Please get the tarballs from
  https://dist.apache.org/repos/dist/dev/subversion
and add your signatures there.

Thanks!


Re: [PATCH] Improve svn_checksum_t bindings in SWIG

2012-12-10 Thread Daniel Shahaf
Shivani Poddar wrote on Tue, Dec 11, 2012 at 02:22:28 +0530:
 Log Message:
 
 Improve support for svn_checksum.h in SWIG bindings
 * subversion/bindings/swig/python/tests/checksum.py: Improved test_checksum
 

Need a blank line before the * line, and to use the * file\n  (symbol)
syntax --- 'test_checksum' is a symbol.

 Review by: danielsh
 

That's inappropriate; I haven't reviewed the patch yet.  You might add
this field after I review it, not before.

 Index: subversion/bindings/swig/python/tests/checksum.py
 ===
 --- subversion/bindings/swig/python/tests/checksum.py (revision 1419694)
 +++ subversion/bindings/swig/python/tests/checksum.py (working copy)
 @@ -20,22 +20,25 @@
  #
  import unittest, setup_path
  import svn.core
 -
 +LENGTH = 32 or 40;

This is wrong in two different ways:

- 32 or 40 is a constant expression that evaluates to 32.

- You hardcode two values, when you should be hardcoding neither of
  them.  (The bindings shouldn't need to change if the C library grows
  a third svn_checksum_kind_t.)

  class ChecksumTestCases(unittest.TestCase):
  def test_checksum(self):
  # Checking primarily the return type for the svn_checksum_create
  # function
  val = svn.core.svn_checksum_create(svn.core.svn_checksum_md5)
  check_val = svn.core.svn_checksum_to_cstring_display(val)
 -
  # The svn_checksum_to_cstring_display should return a str type object
  # from the check_val object passed to it
  if(type(check_val) == str):
 -# The intialized value created from a checksum should be 0
 -if(int(check_val) != 0):
 -self.assertRaises(AssertionError)
 +#Check the length of the string check_val
 +if(len(check_val) == LENGTH):
 +# The intialized value created from a checksum should be 0
 +if(int(check_val) != 0):
 +raise

This bare raise statement without arguments is itself an error.

See for yourself:

% python -c 'raise'
TypeError: exceptions must be old-style classes or derived from 
BaseException, not NoneType

This exception signifies a bug in your program.  It has become
a RuntimeError in recent Pythons (and, frankly, could become
a compile-time error as well --- the compiler knows there's no except:
block surrounding this statement).  It might work, but not because it's
correct.

Daniel

 +else:
 + raise
  else:
 -self.assertRaises(TypeError, test_checksum)
 +raise
  
  def suite():
  return 
 unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases)



Re: Literals in wc SQLite queries

2012-12-10 Thread Daniel Shahaf
Philip Martin wrote on Mon, Dec 10, 2012 at 17:49:44 +:
 Daniel Shahaf danie...@elego.de writes:
 
  Philip - perhaps you can move the relevant definitions to that header?
  Then I'll follow up with a transform_sql.py patch.
 
 OK, I've done that.  I didn't annotate the maps or elements in any way.

Implemented.  Please note the question embedded in the r1419771 log
message.  (The code added in r1419817 might be relevant, too, in case
both the node-kind and the depth maps have an unknown map member.)


OWP: Introduction for Gabriela Gibson

2012-12-10 Thread Gabriela Gibson

Dear All,

My name is Gabriela Gibson and I've applied for the internship with
Subversion under the Gnome Outreach for Women Program.

I am an ex-wizardess who used to haunt a long-lost MUD.  I've not
programmed in a while, although I've *intended* to join more than one
open-source project in the past.  I've been using linux since 1997.

For my initial submission I have written a test for issue 4263 which
I'll mail shortly.  I admit that I struggled to figure out how to
connect gdb to svnrdump in my build tree (since svnrdump is actually a
shell script, and svnrdump reads from stdin) but I am now working on a
fix for 4263.

BTW, I noticed that the python tests in subversion/tests/cmdline/
don't support a list parameter as suggested in
subversion/tests/README.  Is there a reason for this?  Otherwise, this
might something I could fix.

I don't know what I would be doing for my project yet, because I
realise that the Subversion codebase is large and complicated - my
learning curve has been both long and steep.  However, I feel that I'm
reaching the point where I can contribute to the project.

Regards

Gabriela




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

2012-12-10 Thread 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: OWP: Introduction for Gabriela Gibson

2012-12-10 Thread Daniel Shahaf
Gabriela Gibson wrote on Tue, Dec 11, 2012 at 00:21:19 +:
 For my initial submission I have written a test for issue 4263 which
 I'll mail shortly.  I admit that I struggled to figure out how to
 connect gdb to svnrdump in my build tree (since svnrdump is actually a
 shell script, and svnrdump reads from stdin) but I am now working on a
 fix for 4263.


libtool --mode=execute gdb -args subversion/svnrdump/svnrdump

(documentation patches welcome...)

 BTW, I noticed that the python tests in subversion/tests/cmdline/
 don't support a list parameter as suggested in
 subversion/tests/README.  Is there a reason for this?  Otherwise, this
 might something I could fix.

The parameter has been renamed to --list.  foo_tests.py bar now runs
the bar function defined in foo_tests.py (usually one of the functions
enumerated in test_list).


 I don't know what I would be doing for my project yet, because I
 realise that the Subversion codebase is large and complicated - my
 learning curve has been both long and steep.  However, I feel that I'm
 reaching the point where I can contribute to the project.


Great!

 Regards

 Gabriela




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

2012-12-10 Thread 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


Re: svn_client operations start failing - unable to resolve host name

2012-12-10 Thread Robert Turner
So I managed to figure out the problem. It was a socket leak, caused by me
failing to destroy a pool inside a function called in a loop.
My apologies for disturbing people.



On 2012-11-27 9:33 AM, Robert Turner rtur...@qnx.com wrote:

Thanks for the reply.

So I spent ages digging into this (and related issues) last night, and
that is correct. gethostbyname() is indeed failing, spontaneously it
seems. However, as to why, I don't know yet. I'm suspecting something is
getting messed up in the local memory space that is affecting it. Other
processes do not seem to be affected.


I also tried using 1.7.7 client libraries (which I eventually got to
compile and more or less work from source once I stopped using a custom
built APR and used the one Apple provides on the platform). However, I was
running into stack corruption issues during repeated svn_client_cat2()
calls (the revision was getting overwritten, or the pointer was getting
changed and ended up being the URL string). I haven't yet figured these
issues out, or tried more current code than 1.7.7.

Robert

On 2012-11-27 9:14 AM, Daniel Shahaf d...@daniel.shahaf.name wrote:

Robert Turner wrote on Mon, Nov 26, 2012 at 19:49:35 +:
 I'm in the process of making some updates to a FUSE svnfs, and while
the process is merrily chugging away pulling file information, it starts
getting lots of errors from the svn_client commands (not just the one
below, but that's an example):
 
 DEBUG : [00] svnclient_cache(): svn_client_cat(), err-message='OPTIONS
of 'pathname removed': Could not resolve hostname `host name
removed': Host not found (host name removed)'
err-apr_err=175002,0x2ab9a 'RA layer request failed'
 
 There is nothing of note configured related to proxies, or anything. As
mentioned above, the operations are performing correctly for a while,
then stop, and no further operations succeed until the process exits and
is restarted. My process seems to behaving well from a memory
perspective, and static analysis tools are coming up clean.
 
 I'm suspecting that something is getting confused in the internal state
of the SVN client libraries.
 

And I'm suspecting your C library DNS resolver function is returning an
error.  Have you ruled that out yet?

 Any suggestions on where to look further, or what might be the problem?
 
 
 
 (I'm using the 1.6.18 client libraries from wanDisco, compiled for OSX
-- I have been unable to get locally compiled and working copies of the
SVN libraries myself when trying to compile from source, so I have no
easy way to debug through the SVN libraries)
 
 Thanks for any ideas anyone might have,
 
 Robert




OWP: Introduction for Gabriela Gibson

2012-12-10 Thread Gabriela Gibson

Dear All,

My name is Gabriela Gibson and I've applied for the internship with
Subversion under the Gnome Outreach for Women Program.

I am an ex-wizardess who used to haunt a long-lost MUD.  I've not
programmed in a while, although I've *intended* to join more than one
open-source project in the past.  I've been using linux since 1997.

For my initial submission I have written a test for issue 4263 which
I'll mail shortly.  I admit that I struggled to figure out how to
connect gdb to svnrdump in my build tree (since svnrdump is actually a
shell script, and svnrdump reads from stdin) but I am now working on a 
fix for 4263.


BTW, I noticed that the python tests in subversion/tests/cmdline/
don't support a list parameter as suggested in
subversion/tests/README.  Is there a reason for this?  Otherwise, this
might something I could fix.

I don't know what I would be doing for my project yet, because I
realise that the Subversion codebase is large and complicated - my
learning curve has been both long and steep.  However, I feel that I'm
reaching the point where I can contribute to the project.

Regards

Gabriela


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

2012-12-10 Thread 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: 1.7.8 up for testing/signing

2012-12-10 Thread Paul Burba
On Mon, Dec 10, 2012 at 5:07 PM, Ben Reser b...@reser.org wrote:
 The 1.7.8 release artifacts are now available for testing/signing.
 Please get the tarballs from
   https://dist.apache.org/repos/dist/dev/subversion
 and add your signatures there.

 Thanks!

SUMMARY:
-
+1 to release

VERIFIED:
-
Other than the expected differences in
subversion/include/svn_version.h,
https://dist.apache.org/repos/dist/dev/subversion/subversion-1.7.8.zip
is identical to
https://svn.apache.org/repos/asf/subversion/branches/1.7.x@1419691.

SHA1 checksum of
https://dist.apache.org/repos/dist/dev/subversion/subversion-1.7.8.zip
is 65985725f8138cc18993a9088d4ad70df6c0d816

TESTED:
---
[Release-Build] x[ fsfs | bdb ] x [ file | svn | http (neon) | http (serf) ]
Ruby bindings (patched as described here:
http://svn.haxx.se/dev/archive-2011-06/0682.shtml)
JavaHL Bindings

RESULTS:

All Tests Pass

PLATFORM:
-
MS Windows 7 Home Premium Service Pack 1
Microsoft Visual Studio 2008 Version 9.0.30729.1 SP

DEPENDENCIES:
-
APR: 1.4.6
APR-UTIL: 1.4.1
Neon: 0.29.5
zlib: 1.2.4
OpenSSL: 0.9.8q
Apache: 2.2.22
BDB: 4.8.30
sqlite: 3.7.7.1
Python: 2.7.2 (ActivePython 2.7.2.5)
Perl: 5.14.2
Ruby: ruby 1.8.7
java: 1.6.0_21
junit: 4.8.2
swig: 1.3.40
serf: 1.1.1

SIGNATURE:
--
https://dist.apache.org/repos/dist/dev/subversion/subversion-1.7.8.zip:

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.5 (MingW32)

iD8DBQBQxnLZ2RaJMFP83FURAhdgAJ4uhjXgujUQK5MCObZWanCv0y06dgCfcip4
9wYkft1k86LMlDaXYBRZfvk=
=EbgM
-END PGP SIGNATURE-

-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba


Re: OWP: Introduction for Gabriela Gibson

2012-12-10 Thread C. Michael Pilato
On 12/10/2012 07:32 PM, Daniel Shahaf wrote:
 Gabriela Gibson wrote on Tue, Dec 11, 2012 at 00:21:19 +:
 For my initial submission I have written a test for issue 4263 which
 I'll mail shortly.  I admit that I struggled to figure out how to
 connect gdb to svnrdump in my build tree (since svnrdump is actually a
 shell script, and svnrdump reads from stdin) but I am now working on a
 fix for 4263.

 
 libtool --mode=execute gdb -args subversion/svnrdump/svnrdump
 
 (documentation patches welcome...)

I often run gdb using subversion/svnrdump/.libs/lt-svnrdump.  But note that
that program doesn't exist until after the first post-compilation invocation
of subversion/svnrdump/svnrdump itself, so I'm thankful to also learn of the
method Daniel reports here.

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



signature.asc
Description: OpenPGP digital signature


Re: [PATCH] Improve svn_checksum_t bindings in SWIG

2012-12-10 Thread Shivani Poddar
On Tue, Dec 11, 2012 at 4:08 AM, Daniel Shahaf danie...@elego.de wrote:

 Shivani Poddar wrote on Tue, Dec 11, 2012 at 02:22:28 +0530:
  Log Message:
 
  Improve support for svn_checksum.h in SWIG bindings
  * subversion/bindings/swig/python/tests/checksum.py: Improved
 test_checksum
 

 Need a blank line before the * line, and to use the * file\n  (symbol)
 syntax --- 'test_checksum' is a symbol.

  Review by: danielsh
 

 That's inappropriate; I haven't reviewed the patch yet.  You might add
 this field after I review it, not before.

Since you reviewed the last patch because of which i wrote this 1 i thought
that i needed to attribute you at review


  Index: subversion/bindings/swig/python/tests/checksum.py
  ===
  --- subversion/bindings/swig/python/tests/checksum.py (revision 1419694)
  +++ subversion/bindings/swig/python/tests/checksum.py (working copy)
  @@ -20,22 +20,25 @@
   #
   import unittest, setup_path
   import svn.core
  -
  +LENGTH = 32 or 40;

 This is wrong in two different ways:

 - 32 or 40 is a constant expression that evaluates to 32.

 - You hardcode two values, when you should be hardcoding neither of
   them.  (The bindings shouldn't need to change if the C library grows
   a third svn_checksum_kind_t.)

 the symbolic constants in python are declared as this one. However in this
test, since we are checking by only svn_checksum_md5 , we only need the
length to be = 32, i dont know why we would want to include 40 in the
length here , since atleast in this test length should always be 32.
so maybe
LENGTH =
svn.core.svn_checksum_to_cstring_display(svn_checksum_create(svn_checksum_md5))
would have been a better thing to do

  class ChecksumTestCases(unittest.TestCase):
   def test_checksum(self):
   # Checking primarily the return type for the svn_checksum_create
   # function
   val = svn.core.svn_checksum_create(svn.core.svn_checksum_md5)
   check_val = svn.core.svn_checksum_to_cstring_display(val)
  -
   # The svn_checksum_to_cstring_display should return a str type
 object
   # from the check_val object passed to it
   if(type(check_val) == str):
  -# The intialized value created from a checksum should be 0
  -if(int(check_val) != 0):
  -self.assertRaises(AssertionError)
  +#Check the length of the string check_val
  +if(len(check_val) == LENGTH):
  +# The intialized value created from a checksum should
 be 0
  +if(int(check_val) != 0):
  +raise

 This bare raise statement without arguments is itself an error.

 See for yourself:

 % python -c 'raise'
 TypeError: exceptions must be old-style classes or derived from
 BaseException, not NoneType

 This exception signifies a bug in your program.  It has become
 a RuntimeError in recent Pythons (and, frankly, could become
 a compile-time error as well --- the compiler knows there's no except:
 block surrounding this statement).  It might work, but not because it's
 correct.

 Yes it was working for me in the program, will check how i can fix this one


 Daniel

  +else:
  + raise
   else:
  -self.assertRaises(TypeError, test_checksum)
  +raise
 
   def suite():
   return
 unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases)




-- 
Shivani Poddar,
Bachelors in Computer Sciences and MS in Exact Humanities, Sophomore
International Institute of Information Technology, Hyderabad