Re: [PATCH] Implement svnadmin verify --force

2012-11-05 Thread Prabhu Gnana Sundar
Folks, I seriously think that it is time for me to take a break from sending iterative patches and just sit back and analyse the whole problem from first. I value all your precious times spent on this and I wish to come back with a much cleaner work which would abide by the coding standards of

Re: [PATCH] Implement svnadmin verify --force

2012-11-05 Thread Julian Foad
Prabhu Gnana Sundar wrote: > Julian Foad wrote: >> [...]  We should choose either "notification" style >> (that is, messages that are not error messages), such as [...] >> or "error messages" style, in which case the messages should be >> formatted like all svn err msgs, for example: [...] >>

Re: [PATCH] Implement svnadmin verify --force

2012-11-05 Thread Daniel Shahaf
Stefan Sperling wrote on Mon, Nov 05, 2012 at 12:37:19 +0100: > On Mon, Nov 05, 2012 at 02:42:29PM +0530, Prabhu Gnana Sundar wrote: > > I guess, this also solves the layer violation problem. Correct me if > > am wrong. > > Daniel suggested to use svn_error_quick_wrap() which you aren't using. > S

Re: [PATCH] Implement svnadmin verify --force

2012-11-05 Thread Stefan Sperling
On Mon, Nov 05, 2012 at 02:42:29PM +0530, Prabhu Gnana Sundar wrote: > On 11/04/2012 06:14 AM, Daniel Shahaf wrote: > > > >>Index: subversion/svnadmin/main.c > >>=== > >>--- subversion/svnadmin/main.c (revision 1402414) > >>+++ su

Re: [PATCH] Implement svnadmin verify --force

2012-11-05 Thread Prabhu Gnana Sundar
On 11/04/2012 06:14 AM, Daniel Shahaf wrote: Index: subversion/svnadmin/main.c === --- subversion/svnadmin/main.c (revision 1402414) +++ subversion/svnadmin/main.c (working copy) @@ -738,6 +743,16 @@

Re: [PATCH] Implement svnadmin verify --force

2012-11-03 Thread Daniel Shahaf
> Index: subversion/libsvn_repos/dump.c > === > --- subversion/libsvn_repos/dump.c(revision 1402414) > +++ subversion/libsvn_repos/dump.c(working copy) > @@ -1413,19 +1443,31 @@ >void *cancel_edit_baton; >svn_fs

Re: [PATCH] Implement svnadmin verify --force

2012-11-03 Thread Daniel Shahaf
> Index: subversion/libsvn_repos/dump.c > === > --- subversion/libsvn_repos/dump.c(revision 1402414) > +++ subversion/libsvn_repos/dump.c(working copy) > @@ -1459,5 +1512,8 @@ >/* Per-backend verification. */ >svn_pool_

Re: [PATCH] Implement svnadmin verify --force

2012-11-03 Thread Prabhu Gnana Sundar
But the code moves the E160004 away from its current location immediately after the "svnadmin:" prefix. I am not sure I like that; is there a good reason not to have the message be of the form svnadmin: E160004: %s in the interest of parseability? I agree that would be better: the prefix

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Daniel Shahaf
Prabhu Gnana Sundar wrote on Thu, Nov 01, 2012 at 23:59:44 +0530: > > > Julian Foad wrote: > > >Prabhu Gnana Sundar wrote: > > > >> Julian Foad wrote: > >>> At the very least there must be agreement on how it's going to exit > >and > >>> commitment to make it happen.  I already asked the ques

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Julian Foad
Prabhu, some more review comments on your last patch. Instead of checking for errors in several places within the main loop, put the verification code in a wrapper function and check for errors exactly once where you call that function.  That way you won't miss any.  In your last patch, there i

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Prabhu Gnana Sundar
Julian Foad wrote: >Prabhu Gnana Sundar wrote: > >> Julian Foad wrote: >>> At the very least there must be agreement on how it's going to exit >and >>> commitment to make it happen.  I already asked the question several >>> messages back.  Prabhu, please share your thoughts. >> >> Yeah Julian

RE: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Bert Huijben
Huijben Cc: Daniel Shahaf; Michael Pilato; Stefan Sperling; Prabhu Gnana Sundar Ponnarasu; dev@subversion.apache.org Subject: Re: [PATCH] Implement svnadmin verify --force Bert Huijben wrote: >> +1 to having a non-zero exit code if there was any error throughout. > > In that case: w

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Julian Foad
Prabhu Gnana Sundar wrote: > Julian Foad wrote: >> At the very least there must be agreement on how it's going to exit and >> commitment to make it happen.  I already asked the question several >> messages back.  Prabhu, please share your thoughts. > > Yeah Julian, once an error is encountered,

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Prabhu Gnana Sundar
Julian Foad wrote: > >At the very least there must be agreement on how it's going to exit and >commitment to make it happen.  I already asked the question several >messages back.  Prabhu, please share your thoughts. > Yeah Julian, once an error is encountered, the error message is notified an

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Prabhu Gnana Sundar
>Near the start of this thread we mentioned the main use cases.  There >are basically two: > >* Confirm that the repository is in a good state (e.g. before/after a >backup); > >* Diagnostic mode for use when a problem has been encountered. > >Keep-going mode is more useful for the latter; stop-q

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Julian Foad
Bert Huijben wrote: >> +1 to having a non-zero exit code if there was any error throughout. > > In that case: why do we add --force? The option is called --keep-going. > Maybe we should default to this new behavior and *add* a quick exit on error > for whoever needs it. > > I think the total

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Mark Phippard
On Thu, Nov 1, 2012 at 1:22 PM, Stefan Sperling wrote: > On Thu, Nov 01, 2012 at 04:53:41PM +0100, Bert Huijben wrote: >> Maybe we should default to this new behavior and *add* a quick exit on error >> for whoever needs it. > > That's what I've been arguing all along. I think the new behaviour > s

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Stefan Sperling
On Thu, Nov 01, 2012 at 04:53:41PM +0100, Bert Huijben wrote: > Maybe we should default to this new behavior and *add* a quick exit on error > for whoever needs it. That's what I've been arguing all along. I think the new behaviour should be the default. > I think the total report on which revisi

RE: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Bert Huijben
> -Original Message- > From: Daniel Shahaf [mailto:d...@daniel.shahaf.name] > Sent: donderdag 1 november 2012 16:22 > To: C. Michael Pilato > Cc: Julian Foad; Stefan Sperling; Prabhu Gnana Sundar; > dev@subversion.apache.org > Subject: Re: [PATCH] Implement svnadmin

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Stefan Sperling
On Thu, Nov 01, 2012 at 05:40:02PM +0200, Daniel Shahaf wrote: > Daniel Shahaf wrote on Thu, Nov 01, 2012 at 17:37:53 +0200: > > That discussion is perfectly within the scope of the current patch: > > notifying the errors and clearing them _causes_ svn to return non-zero > > exit code when it encou

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Julian Foad
Stefan Sperling wrote: > On Thu, Nov 01, 2012 at 05:22:14PM +0200, Daniel Shahaf wrote: >> +1 to having a non-zero exit code if there was any error throughout. > > I think we should try to keep this discussion within the scope > of the current patch, which is about adding a --keep-going option >

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Daniel Shahaf
Daniel Shahaf wrote on Thu, Nov 01, 2012 at 17:37:53 +0200: > Stefan Sperling wrote on Thu, Nov 01, 2012 at 16:31:24 +0100: > > On Thu, Nov 01, 2012 at 05:22:14PM +0200, Daniel Shahaf wrote: > > > +1 to having a non-zero exit code if there was any error throughout. > > > > I think we should try to

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Daniel Shahaf
Stefan Sperling wrote on Thu, Nov 01, 2012 at 16:31:24 +0100: > On Thu, Nov 01, 2012 at 05:22:14PM +0200, Daniel Shahaf wrote: > > +1 to having a non-zero exit code if there was any error throughout. > > I think we should try to keep this discussion within the scope > of the current patch, which i

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Stefan Sperling
On Thu, Nov 01, 2012 at 05:22:14PM +0200, Daniel Shahaf wrote: > +1 to having a non-zero exit code if there was any error throughout. I think we should try to keep this discussion within the scope of the current patch, which is about adding a --keep-going option and providing sensible output when

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Daniel Shahaf
Stefan Sperling wrote on Thu, Nov 01, 2012 at 11:59:43 +0100: > On Thu, Nov 01, 2012 at 02:21:48PM +0530, Prabhu Gnana Sundar wrote: > > On 10/31/2012 11:31 PM, Julian Foad wrote: > > >Daniel Shahaf wrote: > > > > > >>Julian Foad wrote on Wed, Oct 31, 2012 at 17:36:09 +: > > >>> Prabhu Gnana S

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Daniel Shahaf
C. Michael Pilato wrote on Thu, Nov 01, 2012 at 10:56:59 -0400: > On 11/01/2012 10:33 AM, Julian Foad wrote: > >> Agreed. And for what it's worth, I like the second form, especially if > >> the errorful lines go to stderr. > > > > Hmm, it's also reasonable to consider a combination of both: print

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread C. Michael Pilato
On 11/01/2012 10:33 AM, Julian Foad wrote: >> Agreed. And for what it's worth, I like the second form, especially if >> the errorful lines go to stderr. > > Hmm, it's also reasonable to consider a combination of both: print a > notification for every revision ("Verified rX" or "FAILED to verify r

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread C. Michael Pilato
On 11/01/2012 10:17 AM, Julian Foad wrote: > That's easily readable, but I don't like it: it's a funny mixture of styles. > We should choose either "notification" style (that is, messages that are not > error messages), such as > > * Verified revision 0. > * Verified revision 1. > * Verified re

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Julian Foad
Stefan Sperling wrote: > On Thu, Nov 01, 2012 at 02:21:48PM +0530, Prabhu Gnana Sundar wrote: >> On 10/31/2012 11:31 PM, Julian Foad wrote: >>> Daniel Shahaf wrote: The code will print    svnadmin: E160004: Corrupt filesystem or    svnadmin: Error verifying revision 42: E160004:

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Stefan Sperling
On Thu, Nov 01, 2012 at 02:21:48PM +0530, Prabhu Gnana Sundar wrote: > On 10/31/2012 11:31 PM, Julian Foad wrote: > >Daniel Shahaf wrote: > > > >>Julian Foad wrote on Wed, Oct 31, 2012 at 17:36:09 +: > >>> Prabhu Gnana Sundar wrote: > >>> > +revstr = ""; > >>> > + svn_handle_error2(n

Re: [PATCH] Implement svnadmin verify --force

2012-11-01 Thread Prabhu Gnana Sundar
On 10/31/2012 11:31 PM, Julian Foad wrote: Daniel Shahaf wrote: Julian Foad wrote on Wed, Oct 31, 2012 at 17:36:09 +: Prabhu Gnana Sundar wrote: > +revstr = ""; > + svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */, > +apr_psprintf(scratch_po

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Julian Foad
Daniel Shahaf wrote: > Julian Foad wrote on Wed, Oct 31, 2012 at 17:36:09 +: >> Prabhu Gnana Sundar wrote: >> > +    revstr = ""; >> > +  svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */, >> > +    apr_psprintf(scratch_pool, "svnadmin: %s", revstr)); >> > +  r

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Daniel Shahaf
Julian Foad wrote on Wed, Oct 31, 2012 at 17:36:09 +: > Prabhu Gnana Sundar wrote: > > +    revstr = ""; > > +  svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */, > > +    apr_psprintf(scratch_pool, "svnadmin: %s", > > revstr)); > > +  return; >

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Julian Foad
Prabhu Gnana Sundar wrote: > Index: subversion/svnadmin/main.c [...] > @@ -729,6 +734,7 @@ >  { >    svn_stream_t *feedback_stream = baton; >    apr_size_t len; > +  const char *revstr; You can put the variables in a local-scope block within the switch case...   >    switch (notify->actio

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Julian Foad
Daniel Shahaf wrote: > Julian Foad wrote on Wed, Oct 31, 2012 at 17:14:02 +: >> Prabhu Gnana Sundar wrote: >> > +notify_verification_error(svn_revnum_t rev, svn_error_t *err, >> > +  svn_repos_notify_func_t notify_func, >> > +  void *notify_

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Daniel Shahaf
Julian Foad wrote on Wed, Oct 31, 2012 at 17:14:02 +: > Prabhu Gnana Sundar wrote: > > +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) Anothe

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Julian Foad
Prabhu Gnana Sundar wrote: > Index: subversion/libsvn_repos/dump.c [...] > > +void Doc string? > +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)

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Julian Foad
Prabhu Gnana Sundar wrote: > @@ -2517,8 +2524,29 @@ >   * cancel_baton as argument to see if the caller wishes to cancel the >   * verification. >   * > + * If @a keep_going is @c TRUE, the verify process notifies the error message > + * and continues. If @a notify_func is @c NULL, the verificatio

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Prabhu Gnana Sundar
Thanks Daniel, Attaching the patch that would fix the suggestions given by you. Thanks and regards Prabhu On 10/31/2012 08:28 PM, Daniel Shahaf wrote: Index: subversion/libsvn_repos/dump.c === --- subversion/libsvn_repos/dump.

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Daniel Shahaf
> Index: subversion/libsvn_repos/dump.c > === > --- subversion/libsvn_repos/dump.c(revision 1402414) > +++ subversion/libsvn_repos/dump.c(working copy) > @@ -1359,10 +1359,23 @@ >return close_directory(dir_baton, pool); >

Re: [PATCH] Implement svnadmin verify --force

2012-10-31 Thread Prabhu Gnana Sundar
Thanks Stefan, Addressed the suggestions in this patch. Attaching the log message too with this mail. Please share your thoughts... Thanks and regards Prabhu On 10/30/2012 08:38 PM, Stefan Sperling wrote: On Tue, Oct 30, 2012 at 04:07:49PM +0200, Daniel Shahaf wrote: Prabhu Gnana Sundar w

Re: [PATCH] Implement svnadmin verify --force

2012-10-30 Thread Stefan Sperling
On Tue, Oct 30, 2012 at 04:07:49PM +0200, Daniel Shahaf wrote: > Prabhu Gnana Sundar wrote on Tue, Oct 30, 2012 at 19:22:31 +0530: > > + if (err && keep_going) > > +{ > > + svn_repos_notify_t *notify_failure; > > + notify_failure = svn_repos_notify_create(svn_repos_notify_failure, >

Re: [PATCH] Implement svnadmin verify --force

2012-10-30 Thread Stefan Sperling
On Tue, Oct 30, 2012 at 07:22:31PM +0530, Prabhu Gnana Sundar wrote: > Index: subversion/svnadmin/main.c > === > --- subversion/svnadmin/main.c(revision 1402414) > +++ subversion/svnadmin/main.c(working copy) > @@ -738

Re: [PATCH] Implement svnadmin verify --force

2012-10-30 Thread Daniel Shahaf
Prabhu Gnana Sundar wrote on Tue, Oct 30, 2012 at 19:22:31 +0530: > > Thanks to Stefan and Daniel. > > Attaching a new patch addressing the suggestions given by Stefan and > Daniel. Hope this is good :) > Edited the log message also. > Index: subversion/libsvn_repos/dump.c >

Re: [PATCH] Implement svnadmin verify --force

2012-10-30 Thread Prabhu Gnana Sundar
Thanks to Stefan and Daniel. Attaching a new patch addressing the suggestions given by Stefan and Daniel. Hope this is good :) Edited the log message also. Thanks and regards Prabhu On 10/29/2012 11:53 PM, Stefan Sperling wrote: On Mon, Oct 29, 2012 at 10:45:19PM +0530, Prabhu Gnana Sunda

Re: [PATCH] Implement svnadmin verify --force

2012-10-29 Thread Daniel Shahaf
Stefan Sperling wrote on Mon, Oct 29, 2012 at 19:23:46 +0100: > Looks great otherwise! The docstring for svn_repos_fs_verify3() refers to stderr. (It should talk about "making a notification" instead - the library never touches stderr)

Re: [PATCH] Implement svnadmin verify --force

2012-10-29 Thread Stefan Sperling
On Mon, Oct 29, 2012 at 10:45:19PM +0530, Prabhu Gnana Sundar wrote: > > Thank you so much Stefan for your patience in reviewing and guiding > me through. > > I have addressed your points in the patch attached in this mail. I > hope I addressed all the suggestions given by you :) > Please share y

Re: [PATCH] Implement svnadmin verify --force

2012-10-29 Thread Prabhu Gnana Sundar
Thank you so much Stefan for your patience in reviewing and guiding me through. I have addressed your points in the patch attached in this mail. I hope I addressed all the suggestions given by you :) Please share your views. I will come up with a test case for this implementation in a new p

Re: [PATCH] Implement svnadmin verify --force

2012-10-29 Thread Stefan Sperling
On Mon, Oct 29, 2012 at 03:26:36PM +0530, Prabhu Gnana Sundar wrote: > Thanks much Stefan, > > Now, I have changed the code in libsvn_fs/fs-loader.c to handle the > error. Passed the keep_going to the verify_fs so that it can be > handled in both libsvn_fs_fs and libsvn_fs_base. I have attached th

Re: [PATCH] Implement svnadmin verify --force

2012-10-29 Thread Prabhu Gnana Sundar
On 10/29/2012 02:51 PM, Stefan Sperling wrote: On Mon, Oct 29, 2012 at 12:27:32PM +0530, Prabhu Gnana Sundar wrote: On 10/28/2012 01:54 AM, Daniel Shahaf wrote: Index: subversion/libsvn_fs/fs-loader.c === --- subversion/libsvn_fs/f

Re: [PATCH] Implement svnadmin verify --force

2012-10-29 Thread Stefan Sperling
On Mon, Oct 29, 2012 at 12:27:32PM +0530, Prabhu Gnana Sundar wrote: > On 10/28/2012 01:54 AM, Daniel Shahaf wrote: > >>Index: subversion/libsvn_fs/fs-loader.c > >>=== > >>--- subversion/libsvn_fs/fs-loader.c(revision 1402414)

Re: [PATCH] Implement svnadmin verify --force

2012-10-29 Thread Prabhu Gnana Sundar
On 10/28/2012 01:54 AM, Daniel Shahaf wrote: Index: subversion/libsvn_fs/fs-loader.c === --- subversion/libsvn_fs/fs-loader.c(revision 1402414) +++ subversion/libsvn_fs/fs-loader.c(working copy) @@ -487,17 +487,21 @@

Re: [PATCH] Implement svnadmin verify --force

2012-10-27 Thread Daniel Shahaf
> Index: subversion/libsvn_fs/fs-loader.c > === > --- subversion/libsvn_fs/fs-loader.c (revision 1402414) > +++ subversion/libsvn_fs/fs-loader.c (working copy) > @@ -487,17 +487,21 @@ >void *cancel_baton, >

Re: [PATCH] Implement svnadmin verify --force

2012-10-27 Thread Prabhu Gnana Sundar
Thank you so much Stefan and Daniel. I learnt so much from you guys during the course of this patch. I attached a new patch and a log message with this mail. Please let me know if I missed out on something. Thanks and regards Prabhu On 10/27/2012 09:59 PM, Daniel Shahaf wrote: Stefan Spe

Re: [PATCH] Implement svnadmin verify --force

2012-10-27 Thread Daniel Shahaf
Stefan Sperling wrote on Sat, Oct 27, 2012 at 10:46:59 +0200: > I don't think this is a problem at all. If revision A has a broken > rep and revision B refers to that rep, then both A and B are broken. > Even if the source of the corruption is in A only I want to know that > B is affected by it. Be

Re: [PATCH] Implement svnadmin verify --force

2012-10-27 Thread Stefan Sperling
On Fri, Oct 26, 2012 at 07:07:45PM +0200, Daniel Shahaf wrote: > Stefan Sperling wrote on Fri, Oct 26, 2012 at 12:45:49 +0200: > > I'd like to have this feature because it will make my work a little > > easier should I ever be tasked again with repairing broken repositories. > > When I last had to

Re: [PATCH] Implement svnadmin verify --force

2012-10-26 Thread Daniel Shahaf
Stefan Sperling wrote on Fri, Oct 26, 2012 at 12:45:49 +0200: > On Fri, Oct 26, 2012 at 11:57:12AM +0200, Daniel Shahaf wrote: > > Using terminology from : if X is "I am > > not comfortable writings loops in DOS batch", I don't think applying > > your patch to trunk

Re: [PATCH] Implement svnadmin verify --force

2012-10-26 Thread Stefan Sperling
On Thu, Oct 25, 2012 at 08:33:17PM +0530, Prabhu Gnana Sundar wrote: > > Thanks Daniel, these are really good pointers for me. Shall I > continue with this and submit a patch > with the changes. In my opinion, yes, please do so. Thanks!

Re: [PATCH] Implement svnadmin verify --force

2012-10-26 Thread Stefan Sperling
On Fri, Oct 26, 2012 at 11:57:12AM +0200, Daniel Shahaf wrote: > Using terminology from : if X is "I am > not comfortable writings loops in DOS batch", I don't think applying > your patch to trunk is the correct Y. I still don't get why you're so much objected to th

Re: [PATCH] Implement svnadmin verify --force

2012-10-26 Thread Daniel Shahaf
Using terminology from : if X is "I am not comfortable writings loops in DOS batch", I don't think applying your patch to trunk is the correct Y. Prabhu Gnana Sundar wrote on Thu, Oct 25, 2012 at 20:33:17 +0530: > > Thanks Daniel, these are really good pointers for

Re: [PATCH] Implement svnadmin verify --force

2012-10-26 Thread Prabhu Gnana Sundar
Thanks Daniel, these are really good pointers for me. Shall I continue with this and submit a patch with the changes. Regards Prabhu On 10/21/2012 10:42 PM, Daniel Shahaf wrote: BTW, I'd like to point out a few further problems with the patch; even if it doesn't get applied, you might find

Re: [PATCH] Implement svnadmin verify --force

2012-10-21 Thread Daniel Shahaf
BTW, I'd like to point out a few further problems with the patch; even if it doesn't get applied, you might find them useful for your next patch. Prabhu Gnana Sundar wrote on Sun, Oct 21, 2012 at 00:57:47 +0530: > Index: subversion/libsvn_repos/dump.c >

Re: [PATCH] Implement svnadmin verify --force

2012-10-21 Thread Daniel Shahaf
Stefan Sperling wrote on Sun, Oct 21, 2012 at 18:08:27 +0200: > On Sun, Oct 21, 2012 at 05:47:28PM +0200, Daniel Shahaf wrote: > > Please address my earlier review question, asking why this needs to be > > implemented in the library and can't be done satisfactorily in user > > (C or Python) code. >

Re: [PATCH] Implement svnadmin verify --force

2012-10-21 Thread Prabhu Gnana Sundar
> >I might think differently if Prabhu says he already wrote >a verify-keep-going.py a year ago, has been using it since, and decided >it's so useful it belongs in the core. But I don't know that to be the >case. Ok. Let me tell you how I started writing this. I had a repo on a windows serve

Re: [PATCH] Implement svnadmin verify --force

2012-10-21 Thread Daniel Shahaf
Stefan Sperling wrote on Sun, Oct 21, 2012 at 18:08:27 +0200: > On Sun, Oct 21, 2012 at 05:47:28PM +0200, Daniel Shahaf wrote: > > Please address my earlier review question, asking why this needs to be > > implemented in the library and can't be done satisfactorily in user > > (C or Python) code. >

Re: [PATCH] Implement svnadmin verify --force

2012-10-21 Thread Stefan Sperling
On Sun, Oct 21, 2012 at 05:47:28PM +0200, Daniel Shahaf wrote: > Please address my earlier review question, asking why this needs to be > implemented in the library and can't be done satisfactorily in user > (C or Python) code. Well... let me turn this around on you and ask: Why should it not be i

Re: [PATCH] Implement svnadmin verify --force

2012-10-21 Thread Daniel Shahaf
Please address my earlier review question, asking why this needs to be implemented in the library and can't be done satisfactorily in user (C or Python) code. Prabhu Gnana Sundar wrote on Sun, Oct 21, 2012 at 00:57:47 +0530: > Thanks Stefan, > >I have written a new patch as per your suggestion

Re: [PATCH] Implement svnadmin verify --force

2012-10-20 Thread Prabhu Gnana Sundar
Thanks Stefan, I have written a new patch as per your suggestions and Julian's suggestions. Attaching the new patch and the log message with this mail. Please share your thoughts. Thanks and regards Prabhu On 10/18/2012 09:26 PM, Stefan Sperling wrote: On Thu, Oct 18, 2012 at 08:35:03P

Re: [PATCH] Implement svnadmin verify --force

2012-10-18 Thread Daniel Shahaf
Philip Martin wrote on Thu, Oct 18, 2012 at 23:32:15 +0100: > Stefan Fuhrmann writes: > > >> Why is > >> > >> r=0 > >> HEAD=$(svnlook youngest $repos) > >> while [ $r -le $HEAD ]; do > >> svnadmin verify -r$r $repos > >> r=$(($r + 1)) > >> done > >> > >> not an option? > >> > > >

Re: [PATCH] Implement svnadmin verify --force

2012-10-18 Thread Philip Martin
Stefan Fuhrmann writes: >> Why is >> >> r=0 >> HEAD=$(svnlook youngest $repos) >> while [ $r -le $HEAD ]; do >> svnadmin verify -r$r $repos >> r=$(($r + 1)) >> done >> >> not an option? >> > > It will not report multiple issues with the same revision > (e.g. predecessor revision m

Re: [PATCH] Implement svnadmin verify --force

2012-10-18 Thread Daniel Shahaf
Stefan Fuhrmann wrote on Thu, Oct 18, 2012 at 23:20:45 +0200: > On Thu, Oct 18, 2012 at 11:00 PM, Daniel Shahaf > wrote: > > > Prabhu Gnana Sundar wrote on Thu, Oct 18, 2012 at 20:35:03 +0530: > > > Hi all, > > > > > > Currently svnadmin verify would stop verification process once an > > > error/

Re: [PATCH] Implement svnadmin verify --force

2012-10-18 Thread Stefan Fuhrmann
On Thu, Oct 18, 2012 at 11:00 PM, Daniel Shahaf wrote: > Prabhu Gnana Sundar wrote on Thu, Oct 18, 2012 at 20:35:03 +0530: > > Hi all, > > > > Currently svnadmin verify would stop verification process once an > > error/corruption is found in the repo. It does not go till the HEAD of > > the repo t

Re: [PATCH] Implement svnadmin verify --force

2012-10-18 Thread Daniel Shahaf
Prabhu Gnana Sundar wrote on Thu, Oct 18, 2012 at 20:35:03 +0530: > Hi all, > > Currently svnadmin verify would stop verification process once an > error/corruption is found in the repo. It does not go till the HEAD of > the repo to see if there are further corruptions/errors. > Why is r=0

Re: [PATCH] Implement svnadmin verify --force

2012-10-18 Thread Julian Foad
Stefan Sperling wrote: > Prabhu Gnana Sundar wrote: >> Currently svnadmin verify would stop verification process once an >> error/corruption is found in the repo. It does not go till the HEAD >> of the repo to see if there are further corruptions/errors. >> >> It would be helpful if "--force"

Re: [PATCH] Implement svnadmin verify --force

2012-10-18 Thread Stefan Sperling
On Thu, Oct 18, 2012 at 08:35:03PM +0530, Prabhu Gnana Sundar wrote: > Hi all, > > Currently svnadmin verify would stop verification process once an > error/corruption is found in the repo. It does not go till the HEAD > of the repo to see if there are further corruptions/errors. > > It would be

RE: [PATCH] Implement svnadmin verify --force

2012-10-18 Thread Bert Huijben
> -Original Message- > From: Prabhu Gnana Sundar [mailto:prabh...@collab.net] > Sent: donderdag 18 oktober 2012 17:05 > To: dev@subversion.apache.org > Subject: [PATCH] Implement svnadmin verify --force > > Hi all, > > Currently svnadmin verify would stop v

[PATCH] Implement svnadmin verify --force

2012-10-18 Thread Prabhu Gnana Sundar
Hi all, Currently svnadmin verify would stop verification process once an error/corruption is found in the repo. It does not go till the HEAD of the repo to see if there are further corruptions/errors. It would be helpful if "--force" switch would do this. Attaching a patch and the log messa