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 should remain just
"svnadmin: " and the error message should be adjusted instead.
Does this look fine ? I personally feel this is easily readable.
* Verified revision 0.
* Verified revision 1.
* Verified revision 2.
Error verifying revision 3. svnadmin: E160004: Missing node-id in node-rev
at r3 (offset 787)
Error verifying revision 4. svnadmin: E140001: zlib (uncompress): corrupt
data: Decompression of svndiff data failed
* Verified revision 5.
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 revision 2.
* Error verifying revision 3: Missing node-id in node-rev at r3 (offset 787)
* Error verifying revision 4: zlib (uncompress): corrupt data: Decompression of
svndiff data failed
* Verified revision 5.
or "error messages" style, in which case the messages should be formatted like
all svn err msgs, for example:
* Verified revision 0.
* Verified revision 1.
* Verified revision 2.
svnadmin: E199999: Error verifying revision 3
svnadmin: E160004: Missing node-id in node-rev at r3 (offset 787)
svnadmin: E199999: Error verifying revision 4
svnadmin: E140001: zlib (uncompress): corrupt data: Decompression of svndiff
data failed
* Verified revision 5.
But the output-style design here is linked to the question of what we're going
to print at the end of the verification. Prabhu, what's the plan for when the
command exits? Print a summary or an error message? Exit with non-zero return
code (I hope)?
I made the output to be exactly like the one you have suggested above.
But, I am also adding one more error message as follows
svnadmin: E165005: Repository has corruptions.
and then exit with exit code 1. The default behaviour is not at all
affected. This new error message is effected only when --keep-going is
true. Is that fine ? Now all the test cases pass.
Attaching the patch for you to have a look at it.
Not attaching the log message since I am yet to work on sending the
"Verified revision x" to the stdout. I am facing a few issues in it.
Like, a few tests fail because the expected output don't match with the
actual output. So need suggestions on that too. Is it fine to tweak the
already available test case's expected output based on the output of
what we send to stdout and stderr ?
Thanks and regards
Prabhu
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,28 @@
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)
+{
+ 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, 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 +1392,8 @@
svn_revnum_t rev;
apr_pool_t *iterpool = svn_pool_create(pool);
svn_repos_notify_t *notify;
+ svn_error_t *err;
+ svn_boolean_t found_corruption = FALSE;
/* Determine the current youngest revision of the filesystem. */
SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
@@ -1397,8 +1417,18 @@
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);
+ found_corruption = TRUE;
+ svn_error_clear(err);
+ }
+ else
+ SVN_ERR(err);
/* Create a notify object that we can reuse within the loop. */
if (notify_func)
@@ -1413,19 +1443,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 +1475,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));
@@ -1459,5 +1512,8 @@
/* Per-backend verification. */
svn_pool_destroy(iterpool);
+ if (found_corruption)
+ return svn_error_createf(SVN_ERR_REPOS_CORRUPTED, NULL,
+ _("Repository has corruptions."));
return SVN_NO_ERROR;
}
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;
}
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,16 @@
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,
+ _("svnadmin: E%d: Error verifying revision %ld\n"),
+ notify->err->apr_err,
+ notify->revision));
+ 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,10 +1542,12 @@
if (! opt_state->quiet)
progress_stream = recode_stream_create(stderr, pool);
- return svn_repos_verify_fs2(repos, lower, upper,
- !opt_state->quiet
- ? repos_notify_handler : NULL,
- progress_stream, check_cancel, NULL, pool);
+ return svn_error_trace(svn_repos_verify_fs3(repos, lower, upper,
+ opt_state->keep_going,
+ !opt_state->quiet
+ ? repos_notify_handler : NULL,
+ progress_stream, check_cancel,
+ NULL, pool));
}
/* This implements `svn_opt_subcommand_t'. */
@@ -1992,6 +2009,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_error_codes.h
===================================================================
--- subversion/include/svn_error_codes.h (revision 1402414)
+++ subversion/include/svn_error_codes.h (working copy)
@@ -809,6 +809,10 @@
SVN_ERR_REPOS_CATEGORY_START + 4,
"Bogus revision report")
+ SVN_ERRDEF(SVN_ERR_REPOS_CORRUPTED,
+ SVN_ERR_REPOS_CATEGORY_START + 5,
+ "Repository has corruptions")
+
/* This is analogous to SVN_ERR_FS_UNSUPPORTED_FORMAT. To avoid
* confusion with "versions" (i.e., releases) of Subversion, we
* started using the word "format" instead of "version". However,
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,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 verification failure is
+ * not notified.
+ *
+ * @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,
