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 patch.
Thanks and regards
Prabhu
On 10/29/2012 05:01 PM, Stefan Sperling wrote:
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
updated patch with this mail. Please share your reviews.
More review below (mostly stylistic nits).
Index: subversion/libsvn_repos/dump.c
===================================================================
--- subversion/libsvn_repos/dump.c (revision 1402414)
+++ subversion/libsvn_repos/dump.c (working copy)
@@ -1360,9 +1360,10 @@
}
svn_error_t *
-svn_repos_verify_fs2(svn_repos_t *repos,
+svn_repos_verify_fs3(svn_repos_t *repos,
svn_revnum_t start_rev,
svn_revnum_t end_rev,
+ svn_boolean_t keep_going,
svn_repos_notify_func_t notify_func,
void *notify_baton,
svn_cancel_func_t cancel_func,
@@ -1398,7 +1399,7 @@
/* Verify global/auxiliary data and backend-specific data first. */
SVN_ERR(svn_fs_verify(svn_fs_path(fs, pool), cancel_func, cancel_baton,
- start_rev, end_rev, pool));
+ start_rev, end_rev, keep_going, pool));
/* Create a notify object that we can reuse within the loop. */
if (notify_func)
@@ -1413,11 +1414,12 @@
void *cancel_edit_baton;
svn_fs_root_t *to_root;
apr_hash_t *props;
+ svn_error_t *err;
svn_pool_clear(iterpool);
/* Get cancellable dump editor, but with our close_directory handler. */
- SVN_ERR(get_dump_editor(&dump_editor,&dump_edit_baton,
+ err = get_dump_editor(&dump_editor,&dump_edit_baton,
fs, rev, "",
svn_stream_empty(iterpool),
NULL, NULL,
Please also change indentation of function parameters on additional
lines to align at the opening bracket like this:
err = get_dump_editor(&dump_editor,&dump_edit_baton,
fs, rev, "",
svn_stream_empty(iterpool),
NULL, NULL,
With your patch it would look like this, which is wrong indentation
for our code base:
err = get_dump_editor(&dump_editor,&dump_edit_baton,
fs, rev, "",
svn_stream_empty(iterpool),
NULL, NULL,
@@ -1425,7 +1427,19 @@
notify_func, notify_baton,
start_rev,
FALSE, TRUE, /* use_deltas, verify */
- iterpool));
+ iterpool);
+
+ if (err&& keep_going)
+ {
+ svn_repos_notify_t *notify2 =
svn_repos_notify_create(svn_repos_notify_failure, iterpool);
Why call the variable 'notify2', not just 'notify'?
Also, the above line doesn't wrap at column 78.
I'd suggest using 'notify' as variable name and indenting like this:
svn_repos_notify_t *notify;
notify = svn_repos_notify_create(svn_repos_notify_failure,
iterpool);
+ notify2->err = err;
+ notify_func(notify_baton, notify2, iterpool);
+ svn_error_clear(err);
+ continue;
+ }
+ else
+ SVN_ERR(err);
+
SVN_ERR(svn_delta_get_cancellation_editor(cancel_func, cancel_baton,
dump_editor, dump_edit_baton,
&cancel_editor,
@@ -1433,9 +1447,21 @@
iterpool));
SVN_ERR(svn_fs_revision_root(&to_root, fs, rev, iterpool));
- SVN_ERR(svn_repos_replay2(to_root, "", SVN_INVALID_REVNUM, FALSE,
+ err = svn_repos_replay2(to_root, "", SVN_INVALID_REVNUM, FALSE,
cancel_editor, cancel_edit_baton,
Again, please adjust indentation of function parameters.
- NULL, NULL, iterpool));
+ NULL, NULL, iterpool);
+
+ if (err&& keep_going)
+ {
+ svn_repos_notify_t *notify2 =
svn_repos_notify_create(svn_repos_notify_failure, iterpool);
Same as above suggestion for 'notify' + indentation.
+ notify2->err = err;
+ notify_func(notify_baton, notify2, iterpool);
+ svn_error_clear(err);
+ continue;
+ }
+ else
+ SVN_ERR(err);
+
/* While our editor close_edit implementation is a no-op, we still
do this for completeness. */
SVN_ERR(cancel_editor->close_edit(cancel_edit_baton, iterpool));
Index: subversion/libsvn_repos/deprecated.c
===================================================================
--- subversion/libsvn_repos/deprecated.c (revision 1402414)
+++ subversion/libsvn_repos/deprecated.c (working copy)
@@ -728,6 +728,27 @@
}
svn_error_t *
+svn_repos_verify_fs2(svn_repos_t *repos,
+ svn_revnum_t start_rev,
+ svn_revnum_t end_rev,
+ svn_repos_notify_func_t notify_func,
+ void *notify_baton,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
+ apr_pool_t *pool)
+{
+ return svn_error_trace(svn_repos_verify_fs3(repos,
+ start_rev,
+ end_rev,
+ FALSE,
+ notify_func,
+ notify_baton,
+ cancel_func,
+ cancel_baton,
+ pool));
+}
+
+svn_error_t *
svn_repos_verify_fs(svn_repos_t *repos,
svn_stream_t *feedback_stream,
svn_revnum_t start_rev,
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 1402414)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -10117,6 +10117,7 @@
void *cancel_baton,
svn_revnum_t start,
svn_revnum_t end,
+ svn_boolean_t keep_going,
apr_pool_t *pool)
{
fs_fs_data_t *ffd = fs->fsap_data;
@@ -10151,6 +10152,7 @@
commit-time checks in validate_root_noderev(). */
{
svn_revnum_t i;
+ svn_error_t *err;
We usually add a blank line between variable declarations and code.
So you should add a blank here.
for (i = start; i<= end; i++)
{
svn_fs_root_t *root;
@@ -10162,7 +10164,15 @@
When this code is called in the library, we want to ensure we
use the on-disk data --- rather than some data that was read
in the possibly-distance past and cached since. */
- SVN_ERR(svn_fs_fs__revision_root(&root, fs, i, iterpool));
+ err = svn_fs_fs__revision_root(&root, fs, i, iterpool);
+ if (err&& keep_going)
+ {
+ svn_error_clear(err);
No notification about this error?
Instead of pushing the repos-layer notifier down into the FS-layer,
I'd suggest catching this error thrown from svn_fs_fs__verify() in the
repos-layer caller and notifying there if --keep-going was used.
And then this function won't even need a keep_going parameter and all
the FS-layer bits can be left unchanged, making your patch a bit smaller.
+ return SVN_NO_ERROR;
+ }
+ else
+ SVN_ERR(err);
+
SVN_ERR(svn_fs_fs__verify_root(root, iterpool));
}
}
Index: subversion/libsvn_fs_fs/fs_fs.h
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.h (revision 1402414)
+++ subversion/libsvn_fs_fs/fs_fs.h (working copy)
@@ -38,12 +38,14 @@
svn_error_t *svn_fs_fs__upgrade(svn_fs_t *fs,
apr_pool_t *pool);
-/* Verify the fsfs filesystem FS. Use POOL for temporary allocations. */
+/* Verify the fsfs filesystem FS. Use POOL for temporary allocations.
+ * If KEEP_GOING is TRUE, do not stop upon failure. */
svn_error_t *svn_fs_fs__verify(svn_fs_t *fs,
svn_cancel_func_t cancel_func,
void *cancel_baton,
svn_revnum_t start,
svn_revnum_t end,
+ svn_boolean_t keep_going,
apr_pool_t *pool);
/* Copy the fsfs filesystem SRC_FS at SRC_PATH into a new copy DST_FS at
Index: subversion/libsvn_fs_fs/fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs.c (revision 1402414)
+++ subversion/libsvn_fs_fs/fs.c (working copy)
@@ -287,6 +287,7 @@
void *cancel_baton,
svn_revnum_t start,
svn_revnum_t end,
+ svn_boolean_t keep_going,
apr_pool_t *pool,
apr_pool_t *common_pool)
{
@@ -295,7 +296,8 @@
SVN_ERR(svn_fs_fs__open(fs, path, pool));
SVN_ERR(svn_fs_fs__initialize_caches(fs, pool));
SVN_ERR(fs_serialized_init(fs, common_pool, pool));
- return svn_fs_fs__verify(fs, cancel_func, cancel_baton, start, end, pool);
+ return svn_fs_fs__verify(fs, cancel_func, cancel_baton, start, end,
+ keep_going, pool);
}
static svn_error_t *
Index: subversion/libsvn_fs/fs-loader.c
===================================================================
--- subversion/libsvn_fs/fs-loader.c (revision 1402414)
+++ subversion/libsvn_fs/fs-loader.c (working copy)
@@ -487,6 +487,7 @@
void *cancel_baton,
svn_revnum_t start,
svn_revnum_t end,
+ svn_boolean_t keep_going,
apr_pool_t *pool)
{
fs_library_vtable_t *vtable;
@@ -497,7 +498,7 @@
SVN_MUTEX__WITH_LOCK(common_pool_lock,
vtable->verify_fs(fs, path, cancel_func, cancel_baton,
- start, end, pool, common_pool));
+ start, end, keep_going, pool,
common_pool));
return SVN_NO_ERROR;
}
Index: subversion/libsvn_fs/fs-loader.h
===================================================================
--- subversion/libsvn_fs/fs-loader.h (revision 1402414)
+++ subversion/libsvn_fs/fs-loader.h (working copy)
@@ -92,6 +92,7 @@
svn_cancel_func_t cancel_func, void *cancel_baton,
svn_revnum_t start,
svn_revnum_t end,
+ svn_boolean_t keep_going,
apr_pool_t *pool,
apr_pool_t *common_pool);
svn_error_t *(*delete_fs)(const char *path, apr_pool_t *pool);
Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c (revision 1402414)
+++ subversion/svnadmin/main.c (working copy)
@@ -176,6 +176,7 @@
{
svnadmin__version = SVN_OPT_FIRST_LONGOPT_ID,
svnadmin__incremental,
+ svnadmin__keep_going,
svnadmin__deltas,
svnadmin__ignore_uuid,
svnadmin__force_uuid,
@@ -288,6 +289,9 @@
N_("use format compatible with Subversion versions\n"
" earlier than 1.8")},
+ {"keep-going", svnadmin__keep_going, 0,
+ N_("continue verifying even if there is a corruption")},
+
{"memory-cache-size", 'M', 1,
N_("size of the extra in-memory cache in MB used to\n"
" minimize redundant operations. Default:
16.\n"
@@ -475,7 +479,7 @@
{"verify", subcommand_verify, {0}, N_
("usage: svnadmin verify REPOS_PATH\n\n"
"Verifies the data stored in the repository.\n"),
- {'r', 'q', 'M'} },
+ {'r', 'q', svnadmin__keep_going, 'M'} },
{ NULL, NULL, {0}, NULL, {0} }
};
@@ -505,6 +509,7 @@
svn_boolean_t clean_logs; /* --clean-logs */
svn_boolean_t bypass_hooks; /* --bypass-hooks */
svn_boolean_t wait; /* --wait */
+ svn_boolean_t keep_going; /* --keep-going */
svn_boolean_t bypass_prop_validation; /*
--bypass-prop-validation */
enum svn_repos_load_uuid uuid_action; /* --ignore-uuid,
--force-uuid */
@@ -738,6 +743,10 @@
notify->warning_str));
return;
+ case svn_repos_notify_failure:
+ svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */, "svnadmin:
");
Another overlong line. I'd suggest:
svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */,
"svnadmin: ");
+ return;
+
case svn_repos_notify_dump_rev_end:
svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
_("* Dumped revision %ld.\n"),
@@ -1527,7 +1536,8 @@
if (! opt_state->quiet)
progress_stream = recode_stream_create(stderr, pool);
- return svn_repos_verify_fs2(repos, lower, upper,
+ return svn_repos_verify_fs3(repos, lower, upper,
+ opt_state->keep_going,
!opt_state->quiet
? repos_notify_handler : NULL,
progress_stream, check_cancel, NULL, pool);
@@ -1992,6 +2002,9 @@
case svnadmin__pre_1_8_compatible:
opt_state.pre_1_8_compatible = TRUE;
break;
+ case svnadmin__keep_going:
+ opt_state.keep_going = TRUE;
+ break;
case svnadmin__fs_type:
SVN_INT_ERR(svn_utf_cstring_to_utf8(&opt_state.fs_type, opt_arg,
pool));
break;
Index: subversion/libsvn_fs_base/fs.c
===================================================================
--- subversion/libsvn_fs_base/fs.c (revision 1402414)
+++ subversion/libsvn_fs_base/fs.c (working copy)
@@ -896,6 +896,7 @@
void *cancel_baton,
svn_revnum_t start,
svn_revnum_t end,
+ svn_boolean_t keep_going,
apr_pool_t *pool,
apr_pool_t *common_pool)
{
Index: subversion/include/svn_fs.h
===================================================================
--- subversion/include/svn_fs.h (revision 1402414)
+++ subversion/include/svn_fs.h (working copy)
@@ -280,6 +280,7 @@
void *cancel_baton,
svn_revnum_t start,
svn_revnum_t end,
+ svn_boolean_t keep_going,
apr_pool_t *scratch_pool);
/**
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 1402414)
+++ subversion/include/svn_repos.h (working copy)
@@ -193,6 +193,9 @@
/** A warning message is waiting. */
svn_repos_notify_warning = 0,
+ /** A revision is found with corruption/errors. */
+ svn_repos_notify_failure,
+
This enum has already been released in 1.7!
We must add new values at the end to preserve binary compatibility.
So please put svn_repos_notify_failure after svn_repos_notify_load_skipped_rev.
And please add "@since New in 1.8" to the docstring, like this:
/** A revision is found with corruption/errors. @since New in 1.8. */
/** A revision has finished being dumped. */
svn_repos_notify_dump_rev_end,
@@ -318,9 +321,13 @@
/** For #svn_repos_notify_load_node_start, the path of the node. */
const char *path;
+ /** The error chain. */
Please expand this comment to explain the intended use of this field.
I'd suggest:
/** For #svn_repos_notify_failure, this error chain indicates what
* went wrong during verification. */
+ svn_error_t *err;
+
/* NOTE: Add new fields at the end to preserve binary compatibility.
Also, if you add fields here, you have to update
svn_repos_notify_create(). */
+
Why add a blank line here?
} svn_repos_notify_t;
/** Callback for providing notification from the repository.
@@ -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 prints the error message
+ * to the stderr and continues.
+ *
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_repos_verify_fs3(svn_repos_t *repos,
+ svn_revnum_t start_rev,
+ svn_revnum_t end_rev,
+ svn_boolean_t keep_going,
+ svn_repos_notify_func_t notify_func,
+ void *notify_baton,
+ svn_cancel_func_t cancel,
+ void *cancel_baton,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Similar to svn_repos_verify_fs3(), but without force.
It's now called 'keep_going', not 'force'.
I'd suggest to say:
* Like svn_repos_verify_fs3(), but with @a keep_going set to @c FALSE.
+ *
* @since New in 1.7.
+ * @deprecated Provided for backward compatibility with the 1.7 API.
*/
+SVN_DEPRECATED
svn_error_t *
svn_repos_verify_fs2(svn_repos_t *repos,
svn_revnum_t start_rev,
Implement svnadmin verify --keep-going, which would continue the verification
even if there is some corruption, after printing the errors to stderr.
* subversion/svnadmin/main.c
(svnadmin__cmdline_options_t):
Missing change description? Or has svnadmin__cmdline_options_t not changed?
(svnadmin_opt_state) : add keep-going option.
The indentation style of the log message differs from the style
we usually use. I'd suggest formatting log entries like this:
* subversion/svnadmin/main.c
(svnadmin_opt_state): add keep-going option.
(subcommand_verify): uses the new svn_repos_verify_fs3 function.
(repos_notify_handler): svn_repos_notify_failure handles failure and
prints warning to stderr.
I'd also suggest using proper capitalisation.
* subversion/svnadmin/main.c
(svnadmin_opt_state): Add keep-going option.
What follows are stylistic suggestions. Log messages all differ based on
personal style and preferences. There are no strict guidelines and what
follows is just my personal opinion.
(subcommand_verify) : uses the new svn_repos_verify_fs3 function.
The above doesn't describe the change itself but the result of the change.
Also, third-person simple present verbs like "X uses Y" makes it sounds as
if you're stating a fact, something that is always true and independent of
your change, rather than describing a change you are making.
Compare to this:
(subcommand_verify): Switch to the new svn_repos_verify_fs3 function
instead of svn_repos_verify_fs2, and pass the keep-going option.
I've added a minor detail by mentioning the keep-going option.
When describing changes I often try to describe each chunk of the diff
in a way that allows the reader of the log message to imagine what the
chunk might look like. This might seem overly detailed and it's not a
strict requirement of the project. But I found that this helps me with
writing log messages that I can make sense of myself when I read them
again half a year later :)
(repos_notify_handler) : svn_repos_notify_failure handles failure and
prints warning to stderr.
When expressing code changes in English, I prefer to phrase data structures
(in this case, svn_repos_notify_failure) as grammatical objects and executing
code acting on the data (in this case, repos_notify_handler) as the
grammatical subject acting on the object.
Your wording suggests that svn_repos_notify_failure (the data), and not
repos_notify_handler (the executing code), prints the warning to stderr.
Compare to phrasing it like this:
(repos_notify_handler): Handle svn_repos_notify_failure notification
by printing warnings to stderr.
That's all for now. I haven't tried to compile and run the patch yet.
You might also want to add a regression test for your change, by the way,
to make sure the --keep-going option doesn't break when we make further
changes in the future. But that can be done as a separate patch.
Thanks!
* 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.
* subversion/include/svn_fs.h
(svn_fs_verify) : handles keep-going. If keep-going is TRUE, svn_fs_verify
would return no error.
* subversion/libsvn_fs_fs/fs_fs.h
(svn_fs_fs__verify) : added the keep_going parameter.
* subversion/libsvn_fs_fs/fs_fs.c
(svn_fs_fs__verify) : handles keep_going. If keep_going is TRUE, SVN_NO_ERROR
is returned upon failure too.
* subversion/libsvn_fs/fs-loader.h
(fs_library_vtable_t) : added keep_going parameter.
* subversion/libsvn_fs/fs-loader.c
(svn_fs_verify) : handles keep_going, which is also passed on to
vtable->verify_fs.
* subversion/libsvn_fs_base/fs.c
(base_verify) : handles keep_going parameter.
* subversion/libsvn_fs_fs/fs.c
(fs_verify) : send keep_going to svn_fs_fs__verify function.
* subversion/libsvn_repos/dump.c
(svn_repos_verify_fs3): now handles "keep-going".
If "keep-going" is TRUE, then the error message is
written to the stderr and verify process continues.
* subversion/libsvn_repos/deprecated.c
(svn_repos_verify_fs2) : deprecated.
Now calls svn_repos_verify_fs3 with keep-going as
FALSE.
Patch by: Prabhu Gnana Sundar<prabhugs{_AT_}collab.net>
Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c (revision 1402414)
+++ subversion/svnadmin/main.c (working copy)
@@ -176,6 +176,7 @@
{
svnadmin__version = SVN_OPT_FIRST_LONGOPT_ID,
svnadmin__incremental,
+ svnadmin__keep_going,
svnadmin__deltas,
svnadmin__ignore_uuid,
svnadmin__force_uuid,
@@ -288,6 +289,9 @@
N_("use format compatible with Subversion versions\n"
" earlier than 1.8")},
+ {"keep-going", svnadmin__keep_going, 0,
+ N_("continue verifying even if there is a corruption")},
+
{"memory-cache-size", 'M', 1,
N_("size of the extra in-memory cache in MB used to\n"
" minimize redundant operations. Default: 16.\n"
@@ -475,7 +479,7 @@
{"verify", subcommand_verify, {0}, N_
("usage: svnadmin verify REPOS_PATH\n\n"
"Verifies the data stored in the repository.\n"),
- {'r', 'q', 'M'} },
+ {'r', 'q', svnadmin__keep_going, 'M'} },
{ NULL, NULL, {0}, NULL, {0} }
};
@@ -505,6 +509,7 @@
svn_boolean_t clean_logs; /* --clean-logs */
svn_boolean_t bypass_hooks; /* --bypass-hooks */
svn_boolean_t wait; /* --wait */
+ svn_boolean_t keep_going; /* --keep-going */
svn_boolean_t bypass_prop_validation; /* --bypass-prop-validation */
enum svn_repos_load_uuid uuid_action; /* --ignore-uuid,
--force-uuid */
@@ -738,6 +743,11 @@
notify->warning_str));
return;
+ case svn_repos_notify_failure:
+ svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */,
+ "svnadmin: ");
+ return;
+
case svn_repos_notify_dump_rev_end:
svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
_("* Dumped revision %ld.\n"),
@@ -1527,7 +1537,8 @@
if (! opt_state->quiet)
progress_stream = recode_stream_create(stderr, pool);
- return svn_repos_verify_fs2(repos, lower, upper,
+ return svn_repos_verify_fs3(repos, lower, upper,
+ opt_state->keep_going,
!opt_state->quiet
? repos_notify_handler : NULL,
progress_stream, check_cancel, NULL, pool);
@@ -1992,6 +2003,9 @@
case svnadmin__pre_1_8_compatible:
opt_state.pre_1_8_compatible = TRUE;
break;
+ case svnadmin__keep_going:
+ opt_state.keep_going = TRUE;
+ break;
case svnadmin__fs_type:
SVN_INT_ERR(svn_utf_cstring_to_utf8(&opt_state.fs_type, opt_arg, pool));
break;
Index: subversion/libsvn_repos/dump.c
===================================================================
--- subversion/libsvn_repos/dump.c (revision 1402414)
+++ subversion/libsvn_repos/dump.c (working copy)
@@ -1360,9 +1360,10 @@
}
svn_error_t *
-svn_repos_verify_fs2(svn_repos_t *repos,
+svn_repos_verify_fs3(svn_repos_t *repos,
svn_revnum_t start_rev,
svn_revnum_t end_rev,
+ svn_boolean_t keep_going,
svn_repos_notify_func_t notify_func,
void *notify_baton,
svn_cancel_func_t cancel_func,
@@ -1374,6 +1375,7 @@
svn_revnum_t rev;
apr_pool_t *iterpool = svn_pool_create(pool);
svn_repos_notify_t *notify;
+ svn_error_t *error;
/* Determine the current youngest revision of the filesystem. */
SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
@@ -1397,9 +1399,17 @@
end_rev, youngest);
/* Verify global/auxiliary data and backend-specific data first. */
- SVN_ERR(svn_fs_verify(svn_fs_path(fs, pool), cancel_func, cancel_baton,
- start_rev, end_rev, pool));
+ error = svn_fs_verify(svn_fs_path(fs, pool), cancel_func, cancel_baton,
+ start_rev, end_rev, pool);
+ if(error && !keep_going)
+ SVN_ERR(error);
+ /* We don't have to notify this failure when keep_going is TRUE since
+ * that is taken care by the svn_repos_notify_failure notification.
+ * Notifying it here too would make it notified twice when keep_going
+ * is TRUE. */
+ svn_error_clear(error);
+
/* Create a notify object that we can reuse within the loop. */
if (notify_func)
notify = svn_repos_notify_create(svn_repos_notify_verify_rev_end,
@@ -1413,19 +1423,34 @@
void *cancel_edit_baton;
svn_fs_root_t *to_root;
apr_hash_t *props;
+ svn_error_t *err;
svn_pool_clear(iterpool);
/* Get cancellable dump editor, but with our close_directory handler. */
- SVN_ERR(get_dump_editor(&dump_editor, &dump_edit_baton,
- fs, rev, "",
- svn_stream_empty(iterpool),
- NULL, NULL,
- verify_close_directory,
- notify_func, notify_baton,
- start_rev,
- FALSE, TRUE, /* use_deltas, verify */
- iterpool));
+ err = get_dump_editor(&dump_editor, &dump_edit_baton,
+ fs, rev, "",
+ svn_stream_empty(iterpool),
+ NULL, NULL,
+ verify_close_directory,
+ notify_func, notify_baton,
+ start_rev,
+ FALSE, TRUE, /* use_deltas, verify */
+ iterpool);
+
+ if (err && keep_going)
+ {
+ svn_repos_notify_t *notify_failure;
+ notify_failure = svn_repos_notify_create(svn_repos_notify_failure,
+ iterpool);
+ notify_failure->err = err;
+ notify_func(notify_baton, notify_failure, iterpool);
+ svn_error_clear(err);
+ continue;
+ }
+ else
+ SVN_ERR(err);
+
SVN_ERR(svn_delta_get_cancellation_editor(cancel_func, cancel_baton,
dump_editor, dump_edit_baton,
&cancel_editor,
@@ -1433,9 +1458,23 @@
iterpool));
SVN_ERR(svn_fs_revision_root(&to_root, fs, rev, iterpool));
- SVN_ERR(svn_repos_replay2(to_root, "", SVN_INVALID_REVNUM, FALSE,
- cancel_editor, cancel_edit_baton,
- NULL, NULL, iterpool));
+ err = svn_repos_replay2(to_root, "", SVN_INVALID_REVNUM, FALSE,
+ cancel_editor, cancel_edit_baton,
+ NULL, NULL, iterpool);
+
+ if (err && keep_going)
+ {
+ svn_repos_notify_t *notify_failure;
+ notify_failure = svn_repos_notify_create(svn_repos_notify_failure,
+ iterpool);
+ notify_failure->err = err;
+ notify_func(notify_baton, notify_failure, iterpool);
+ svn_error_clear(err);
+ continue;
+ }
+ else
+ SVN_ERR(err);
+
/* While our editor close_edit implementation is a no-op, we still
do this for completeness. */
SVN_ERR(cancel_editor->close_edit(cancel_edit_baton, iterpool));
Index: subversion/libsvn_repos/deprecated.c
===================================================================
--- subversion/libsvn_repos/deprecated.c (revision 1402414)
+++ subversion/libsvn_repos/deprecated.c (working copy)
@@ -728,6 +728,27 @@
}
svn_error_t *
+svn_repos_verify_fs2(svn_repos_t *repos,
+ svn_revnum_t start_rev,
+ svn_revnum_t end_rev,
+ svn_repos_notify_func_t notify_func,
+ void *notify_baton,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
+ apr_pool_t *pool)
+{
+ return svn_error_trace(svn_repos_verify_fs3(repos,
+ start_rev,
+ end_rev,
+ FALSE,
+ notify_func,
+ notify_baton,
+ cancel_func,
+ cancel_baton,
+ pool));
+}
+
+svn_error_t *
svn_repos_verify_fs(svn_repos_t *repos,
svn_stream_t *feedback_stream,
svn_revnum_t start_rev,
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 1402414)
+++ subversion/include/svn_repos.h (working copy)
@@ -245,8 +245,11 @@
svn_repos_notify_upgrade_start,
/** A revision was skipped during loading. @since New in 1.8. */
- svn_repos_notify_load_skipped_rev
+ svn_repos_notify_load_skipped_rev,
+ /** A revision is found with corruption/errors. @since New in 1.8. */
+ svn_repos_notify_failure
+
} svn_repos_notify_action_t;
/** The type of error occurring.
@@ -318,6 +321,10 @@
/** 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. */
+ svn_error_t *err;
+
/* NOTE: Add new fields at the end to preserve binary compatibility.
Also, if you add fields here, you have to update
svn_repos_notify_create(). */
@@ -2517,8 +2524,28 @@
* cancel_baton as argument to see if the caller wishes to cancel the
* verification.
*
+ * If @a keep_going is @c TRUE, the verify process prints the error message
+ * to the stderr and continues.
+ *
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_repos_verify_fs3(svn_repos_t *repos,
+ svn_revnum_t start_rev,
+ svn_revnum_t end_rev,
+ svn_boolean_t keep_going,
+ svn_repos_notify_func_t notify_func,
+ void *notify_baton,
+ svn_cancel_func_t cancel,
+ void *cancel_baton,
+ apr_pool_t *scratch_pool);
+
+/**
+ * Like svn_repos_verify_fs3(), but with @a keep_going set to @c FALSE.
* @since New in 1.7.
+ * @deprecated Provided for backward compatibility with the 1.7 API.
*/
+SVN_DEPRECATED
svn_error_t *
svn_repos_verify_fs2(svn_repos_t *repos,
svn_revnum_t start_rev,
Implement svnadmin verify --keep-going, which would continue the verification
even if there is some corruption, after printing the errors to stderr.
* subversion/svnadmin/main.c
(svnadmin__cmdline_options_t):
(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.
* 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 written to the stderr and verify
process continues.
* subversion/libsvn_repos/deprecated.c
(svn_repos_verify_fs2): Deprecated.
Call svn_repos_verify_fs3 with keep-going as FALSE.
Patch by: Prabhu Gnana Sundar <prabhugs{_AT_}collab.net>
Reviewed by: Stefan Sperling <stsp{_AT_}elego.de>
