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 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,
+ iterpool);
+ notify_failure->err = err;
+ notify_func(notify_baton, notify_failure, iterpool);
+ svn_error_clear(err);
+ }
+ else
+ return svn_error_trace(err);
This pattern repeats three times, maybe introduce a macro (like SVN_ERR,
SVN_INT_ERR, etc) to improve readability?
I'd prefer a new helper function instead of a new macro.
Perhaps something like:
if (err&& keep_going)
notify_verification_error(err, rev, pool);
else
SVN_ERR(err);
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 */
@@ -729,6 +734,7 @@
{
svn_stream_t *feedback_stream = baton;
apr_size_t len;
+ const char *revstr;
switch (notify->action)
{
@@ -738,6 +744,17 @@
notify->warning_str));
return;
+ case svn_repos_notify_failure:
+ if (notify->revision != SVN_INVALID_REVNUM)
+ revstr = apr_psprintf(scratch_pool,
+ _("Error verifying revision %ld: "),
+ notify->revision);
+ else
+ revstr = "";
+ svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */,
+ apr_psprintf(scratch_pool, "svnadmin: %s", revstr));
+ return;
+
case svn_repos_notify_dump_rev_end:
svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
_("* Dumped revision %ld.\n"),
@@ -1527,7 +1544,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 +2010,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/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 notifies the error message
+ * 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,
@@ -3333,3 +3360,4 @@
#endif /* __cplusplus */
#endif /* SVN_REPOS_H */
+
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_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);
}
+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)
+{
+ 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, notify_failure, pool);
+}
+
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 +1387,7 @@
svn_revnum_t rev;
apr_pool_t *iterpool = svn_pool_create(pool);
svn_repos_notify_t *notify;
+ svn_error_t *err;
/* Determine the current youngest revision of the filesystem. */
SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
@@ -1397,8 +1411,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));
+ err = svn_fs_verify(svn_fs_path(fs, pool), cancel_func, cancel_baton,
+ start_rev, end_rev, pool);
+ if (err && keep_going)
+ {
+ rev = SVN_INVALID_REVNUM;
+ notify_verification_error(rev, err, notify_func, notify_baton,
+ iterpool);
+ svn_error_clear(err);
+ }
+ else
+ return svn_error_trace(err);
/* Create a notify object that we can reuse within the loop. */
if (notify_func)
@@ -1413,19 +1436,31 @@
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)
+ {
+ notify_verification_error(rev, err, notify_func, notify_baton,
+ 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 +1468,20 @@
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)
+ {
+ notify_verification_error(rev, err, notify_func, notify_baton,
+ 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/notify.c
===================================================================
--- subversion/libsvn_repos/notify.c (revision 1402414)
+++ subversion/libsvn_repos/notify.c (working copy)
@@ -39,6 +39,7 @@
svn_repos_notify_t *notify = apr_pcalloc(result_pool, sizeof(*notify));
notify->action = action;
+ notify->err = SVN_NO_ERROR;
return notify;
}
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 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.
(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.
Patch by: Prabhu Gnana Sundar <prabhugs{_AT_}collab.net>
Reviewed by: Stefan Sperling <stsp{_AT_}elego.de>