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 @@
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)
+++
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.
So I
Prabhu Gnana Sundar prabh...@collab.net 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:
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
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
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. */
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 Sundarprabh...@collab.net wrote:
+revstr = ;
+ svn_handle_error2(notify-err, stderr, FALSE /* non-fatal */,
+
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 Sundarprabh...@collab.net wrote:
+revstr = ;
+
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 rX
on
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 a
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 is
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 keep
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
and
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 encounters
-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 verify --force
C. Michael
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-quickly
Julian Foad julianf...@btopenworld.com 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
Prabhu Gnana Sundar prabh...@collab.net wrote:
Julian Foad julianf...@btopenworld.com 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
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: why do we add --force
Julian Foad julianf...@btopenworld.com wrote:
Prabhu Gnana Sundar prabh...@collab.net wrote:
Julian Foad julianf...@btopenworld.com 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
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
Prabhu Gnana Sundar wrote on Thu, Nov 01, 2012 at 23:59:44 +0530:
Julian Foad julianf...@btopenworld.com wrote:
Prabhu Gnana Sundar prabh...@collab.net wrote:
Julian Foad julianf...@btopenworld.com wrote:
At the very least there must be agreement on how it's going to exit
and
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
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);
}
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
===
---
Prabhu Gnana Sundar prabh...@collab.net 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,
Prabhu Gnana Sundar prabh...@collab.net 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,
Julian Foad wrote on Wed, Oct 31, 2012 at 17:14:02 +:
Prabhu Gnana Sundar prabh...@collab.net 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
Daniel Shahaf wrote:
Julian Foad wrote on Wed, Oct 31, 2012 at 17:14:02 +:
Prabhu Gnana Sundar prabh...@collab.net wrote:
+notify_verification_error(svn_revnum_t rev, svn_error_t *err,
+ svn_repos_notify_func_t notify_func,
+ void
Prabhu Gnana Sundar prabh...@collab.net 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
Julian Foad wrote on Wed, Oct 31, 2012 at 17:36:09 +:
Prabhu Gnana Sundar prabh...@collab.net wrote:
+ revstr = ;
+ svn_handle_error2(notify-err, stderr, FALSE /* non-fatal */,
+ apr_psprintf(scratch_pool, svnadmin: %s,
revstr));
+
Daniel Shahaf wrote:
Julian Foad wrote on Wed, Oct 31, 2012 at 17:36:09 +:
Prabhu Gnana Sundar prabh...@collab.net wrote:
+ revstr = ;
+ svn_handle_error2(notify-err, stderr, FALSE /* non-fatal */,
+ apr_psprintf(scratch_pool, svnadmin: %s, revstr));
+
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
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
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,6
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,
+
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 @@
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)
+++
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
===
---
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 the
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
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 your
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)
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 repair
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.
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
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,
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
Using terminology from http://s.apache.org/xy-problem: 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 me.
On Fri, Oct 26, 2012 at 11:57:12AM +0200, Daniel Shahaf wrote:
Using terminology from http://s.apache.org/xy-problem: 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 this.
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 http://s.apache.org/xy-problem: if X is I am
not comfortable writings loops in DOS batch, I don't think applying
your patch to trunk is the
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 suggestions
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 in
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.
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
server.
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.
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
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
-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 verification process once an
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
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 switch
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
On Thu, Oct 18, 2012 at 11:00 PM, Daniel Shahaf d...@daniel.shahaf.namewrote:
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
Stefan Fuhrmann wrote on Thu, Oct 18, 2012 at 23:20:45 +0200:
On Thu, Oct 18, 2012 at 11:00 PM, Daniel Shahaf
d...@daniel.shahaf.namewrote:
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
Stefan Fuhrmann stefan.fuhrm...@wandisco.com 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
Philip Martin wrote on Thu, Oct 18, 2012 at 23:32:15 +0100:
Stefan Fuhrmann stefan.fuhrm...@wandisco.com 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
67 matches
Mail list logo
