Index: subversion/include/svn_error_codes.h =================================================================== --- subversion/include/svn_error_codes.h (revision 1687456) +++ subversion/include/svn_error_codes.h (working copy) @@ -922,11 +922,6 @@ SVN_ERROR_START SVN_ERR_REPOS_CATEGORY_START + 10, "Repository upgrade is not supported") - /** @since New in 1.9. */ - SVN_ERRDEF(SVN_ERR_REPOS_VERIFY_FAILED, - SVN_ERR_REPOS_CATEGORY_START + 11, - "Repository verification failed") - /* generic RA errors */ SVN_ERRDEF(SVN_ERR_RA_ILLEGAL_URL, @@ -1497,6 +1492,11 @@ SVN_ERROR_START SVN_ERR_CL_CATEGORY_START + 11, "Failed processing one or more externals definitions") + /** @since New in 1.9. */ + SVN_ERRDEF(SVN_ERR_CL_REPOS_VERIFY_FAILED, + SVN_ERR_CL_CATEGORY_START + 12, + "Repository verification failed") + /* ra_svn errors */ SVN_ERRDEF(SVN_ERR_RA_SVN_CMD_ERR, Index: subversion/include/svn_repos.h =================================================================== --- subversion/include/svn_repos.h (revision 1687456) +++ subversion/include/svn_repos.h (working copy) @@ -234,9 +234,6 @@ typedef enum svn_repos_notify_action_t /** The structure of a revision is being verified. @since New in 1.8. */ svn_repos_notify_verify_rev_structure, - /** A revision is found with corruption/errors. @since New in 1.9. */ - svn_repos_notify_failure, - /** A revprop shard got packed. @since New in 1.9. */ svn_repos_notify_pack_revprops, @@ -348,11 +345,6 @@ typedef struct svn_repos_notify_t /** 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. - @since New in 1.9. */ - svn_error_t *err; - /** For #svn_repos_notify_hotcopy_rev_range, the start of the copied revision range. @since New in 1.9. */ @@ -2825,6 +2817,22 @@ enum svn_repos_load_uuid svn_repos_load_uuid_force }; +/** Callback type for use with svn_repos_verify_fs3(). @a revision + * and @a verify_err are the details of a single verification failure + * that occurred during the svn_repos_verify_fs3() call. @a baton is + * the same baton given to svn_repos_verify_fs3(). @a scratch_pool is + * provided for the convenience of the implementor, who should not + * expect it to live longer than a single callback call. + * + * @see svn_repos_verify_fs3 + * + * @since New in 1.9. + */ +typedef svn_error_t *(*svn_repos_verify_callback_t)(void *baton, + svn_revnum_t revision, + svn_error_t *verify_err, + apr_pool_t *scratch_pool); + /** * Verify the contents of the file system in @a repos. * @@ -2835,9 +2843,6 @@ enum svn_repos_load_uuid * range, then also verify "global invariants" of the repository, as * described in svn_fs_verify(). * - * When a failure is found, if @a keep_going is @c TRUE then continue - * verification from the next revision, otherwise stop. - * * If @a check_normalization is @c TRUE, report any name collisions * within the same directory or svn:mergeinfo property where the names * differ only in character representation, but are otherwise @@ -2848,25 +2853,31 @@ enum svn_repos_load_uuid * file context reconstruction and verification. For FSFS format 7+ and * FSX, this allows for a very fast check against external corruption. * + * If @a verify_callback is not @c NULL, call it with @a verify_baton upon + * receiving an FS-specific structure failure or a revision verification + * failure. Set @c revision callback argument to #SVN_INVALID_REVNUM or + * to the revision number respectively. Set @c verify_err to svn_error_t + * describing the reason of the failure. @c verify_err will be cleared + * after the callback returns, use svn_error_dup() to preserve the error. + * If @a verify_callback returns an error different from #SVN_NO_ERROR, + * stop verifying the repository and immediately return the error from + * @a verify_callback. + * + * If @a verify_callback is @c NULL, this function returns the first + * encountered verification error or #SVN_NO_ERROR if there were no failures + * during the verification. Errors that prevent the verification process + * from continuing, such as #SVN_ERR_CANCELLED, are returned immediately + * and do not trigger an invocation of @a verify_callback. + * * If @a notify_func is not null, then call it with @a notify_baton and * with a notification structure in which the fields are set as follows. - * (For a warning or error notification that does not apply to a specific - * revision, the revision number is #SVN_INVALID_REVNUM.) + * (For a warning that does not apply to a specific revision, the revision + * number is #SVN_INVALID_REVNUM.) * * For each FS-specific structure warning: * @c action = svn_repos_notify_verify_rev_structure * @c revision = the revision or #SVN_INVALID_REVNUM * - * For a FS-specific structure failure: - * @c action = #svn_repos_notify_failure - * @c revision = #SVN_INVALID_REVNUM - * @c err = the corresponding error chain - * - * For each revision verification failure: - * @c action = #svn_repos_notify_failure - * @c revision = the revision - * @c err = the corresponding error chain - * * For each revision verification warning: * @c action = #svn_repos_notify_warning * @c warning and @c warning_str fields set accordingly @@ -2888,11 +2899,6 @@ enum svn_repos_load_uuid * * Use @a scratch_pool for temporary allocation. * - * Return an error if there were any failures during verification, or - * #SVN_NO_ERROR if there were no failures. A failure means an event that, - * if a notification callback were provided, would send a notification - * with @c action = #svn_repos_notify_failure. - * * @since New in 1.9. */ svn_error_t * @@ -2899,18 +2905,20 @@ 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_boolean_t check_normalization, svn_boolean_t metadata_only, svn_repos_notify_func_t notify_func, void *notify_baton, + svn_repos_verify_callback_t verify_callback, + void *verify_baton, svn_cancel_func_t cancel, void *cancel_baton, apr_pool_t *scratch_pool); /** - * Like svn_repos_verify_fs3(), but with @a keep_going, - * @a check_normalization and @a metadata_only set to @c FALSE. + * Like svn_repos_verify_fs3(), but with @a verify_callback and + * @a verify_baton set to @c NULL and with @a check_normalization + * and @a metadata_only set to @c FALSE. * * @since New in 1.7. * @deprecated Provided for backward compatibility with the 1.8 API. Index: subversion/libsvn_repos/deprecated.c =================================================================== --- subversion/libsvn_repos/deprecated.c (revision 1687456) +++ subversion/libsvn_repos/deprecated.c (working copy) @@ -774,9 +774,9 @@ svn_repos_verify_fs2(svn_repos_t *repos, end_rev, FALSE, FALSE, - FALSE, notify_func, notify_baton, + NULL, NULL, cancel_func, cancel_baton, pool)); Index: subversion/libsvn_repos/dump.c =================================================================== --- subversion/libsvn_repos/dump.c (revision 1687456) +++ subversion/libsvn_repos/dump.c (working copy) @@ -2265,24 +2265,6 @@ verify_close_directory(void *dir_baton, apr_pool_t return close_directory(dir_baton, pool); } -static 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; - - if (notify_func == NULL) - return; - - 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); -} - /* Verify revision REV in file system FS. */ static svn_error_t * verify_one_revision(svn_fs_t *fs, @@ -2359,15 +2341,42 @@ verify_fs_notify_func(svn_revnum_t revision, notify_baton->notify, pool); } +static svn_error_t * +report_error(svn_revnum_t revision, + svn_error_t *verify_err, + svn_repos_verify_callback_t verify_callback, + void *verify_baton, + apr_pool_t *pool) +{ + if (verify_callback) + { + svn_error_t *cb_err; + + /* The caller provided us with a callback, so make him responsible + for what's going to happen with the error. */ + cb_err = verify_callback(verify_baton, revision, verify_err, pool); + svn_error_clear(verify_err); + SVN_ERR(cb_err); + + return SVN_NO_ERROR; + } + else + { + /* No callback -- no second guessing. Just return the error. */ + return svn_error_trace(verify_err); + } +} + 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_boolean_t check_normalization, svn_boolean_t metadata_only, svn_repos_notify_func_t notify_func, void *notify_baton, + svn_repos_verify_callback_t verify_callback, + void *verify_baton, svn_cancel_func_t cancel_func, void *cancel_baton, apr_pool_t *pool) @@ -2380,8 +2389,6 @@ svn_repos_verify_fs3(svn_repos_t *repos, svn_fs_progress_notify_func_t verify_notify = NULL; struct verify_fs_notify_func_baton_t *verify_notify_baton = NULL; svn_error_t *err; - svn_boolean_t failed_metadata = FALSE; - svn_revnum_t failed_revisions = 0; /* Determine the current youngest revision of the filesystem. */ SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool)); @@ -2430,20 +2437,8 @@ svn_repos_verify_fs3(svn_repos_t *repos, } else if (err) { - notify_verification_error(SVN_INVALID_REVNUM, err, notify_func, - notify_baton, iterpool); - - if (!keep_going) - { - /* Return the error, the caller doesn't want us to continue. */ - return svn_error_trace(err); - } - else - { - /* Clear the error and keep going. */ - failed_metadata = TRUE; - svn_error_clear(err); - } + SVN_ERR(report_error(SVN_INVALID_REVNUM, err, verify_callback, + verify_baton, iterpool)); } if (!metadata_only) @@ -2463,20 +2458,8 @@ svn_repos_verify_fs3(svn_repos_t *repos, } else if (err) { - notify_verification_error(rev, err, notify_func, notify_baton, - iterpool); - - if (!keep_going) - { - /* Return the error, the caller doesn't want us to continue. */ - return svn_error_trace(err); - } - else - { - /* Clear the error and keep going. */ - ++failed_revisions; - svn_error_clear(err); - } + SVN_ERR(report_error(rev, err, verify_callback, verify_baton, + iterpool)); } else if (notify_func) { @@ -2495,40 +2478,5 @@ svn_repos_verify_fs3(svn_repos_t *repos, svn_pool_destroy(iterpool); - /* Summarize the results. */ - if (failed_metadata || 0 != failed_revisions) - { - const char *const repos_path = - svn_dirent_local_style(svn_repos_path(repos, pool), pool); - - if (0 == failed_revisions) - { - return svn_error_createf( - SVN_ERR_REPOS_VERIFY_FAILED, NULL, - _("Metadata verification failed on repository '%s'"), - repos_path); - } - else - { - const char* format_string; - - if (failed_metadata) - format_string = apr_psprintf( - pool, _("Verification of metadata and" - " %%%s out of %%%s revisions" - " failed on repository '%%s'"), - SVN_REVNUM_T_FMT, SVN_REVNUM_T_FMT); - else - format_string = apr_psprintf( - pool, _("Verification of %%%s out of %%%s revisions" - " failed on repository '%%s'"), - SVN_REVNUM_T_FMT, SVN_REVNUM_T_FMT); - - return svn_error_createf( - SVN_ERR_REPOS_VERIFY_FAILED, NULL, format_string, - failed_revisions, end_rev - start_rev + 1, repos_path); - } - } - return SVN_NO_ERROR; } Index: subversion/svnadmin/svnadmin.c =================================================================== --- subversion/svnadmin/svnadmin.c (revision 1687456) +++ subversion/svnadmin/svnadmin.c (working copy) @@ -864,25 +864,60 @@ err_cleanup(void *data) return APR_SUCCESS; } -struct repos_notify_handler_baton { - /* Stream to write progress and other non-error output to. */ - svn_stream_t *feedback_stream; +struct repos_verify_callback_baton +{ + /* Should we continue after receiving a first verification error? */ + svn_boolean_t keep_going; - /* Suppress notifications that are neither errors nor warnings. */ - svn_boolean_t silent_running; - - /* Whether errors contained in notifications should be printed along - with the notification. If FALSE, any errors will only be - summarized. */ - svn_boolean_t silent_errors; - /* List of errors encountered during 'svnadmin verify --keep-going'. */ apr_array_header_t *error_summary; - /* Pool for data collected during notifications. */ + /* Pool for data collected during callback invocations. */ apr_pool_t *result_pool; }; +/* Implementation of svn_repos_verify_callback_t to handle errors coming + from svn_repos_verify_fs3(). */ +static svn_error_t * +repos_verify_callback(void *baton, + svn_revnum_t revision, + svn_error_t *verify_err, + apr_pool_t *scratch_pool) +{ + struct repos_verify_callback_baton *b = baton; + + if (revision == SVN_INVALID_REVNUM) + { + SVN_ERR(svn_cmdline_fputs(_("* Error verifying repository metadata.\n"), + stderr, scratch_pool)); + } + else + { + SVN_ERR(svn_cmdline_fprintf(stderr, scratch_pool, + _("* Error verifying revision %ld.\n"), + revision)); + } + + if (b->keep_going) + { + struct verification_error *verr; + + svn_handle_error2(verify_err, stderr, FALSE, "svnadmin: "); + + /* Remember the error in B->ERROR_SUMMARY. */ + verr = apr_palloc(b->result_pool, sizeof(*verr)); + verr->rev = revision; + verr->err = svn_error_dup(verify_err); + apr_pool_cleanup_register(b->result_pool, verr->err, err_cleanup, + apr_pool_cleanup_null); + APR_ARRAY_PUSH(b->error_summary, struct verification_error *) = verr; + + return SVN_NO_ERROR; + } + else + return svn_error_trace(svn_error_dup(verify_err)); +} + /* Implementation of svn_repos_notify_func_t to wrap the output to a response stream for svn_repos_dump_fs2(), svn_repos_verify_fs(), svn_repos_hotcopy3() and others. */ @@ -891,17 +926,8 @@ repos_notify_handler(void *baton, const svn_repos_notify_t *notify, apr_pool_t *scratch_pool) { - struct repos_notify_handler_baton *b = baton; - svn_stream_t *feedback_stream = b->feedback_stream; + svn_stream_t *feedback_stream = baton; - /* Don't print anything if the feedback stream isn't provided. - Only print errors and warnings in silent mode. */ - if (!feedback_stream - || (b->silent_running - && notify->action != svn_repos_notify_warning - && notify->action != svn_repos_notify_failure)) - return; - switch (notify->action) { case svn_repos_notify_warning: @@ -910,32 +936,6 @@ repos_notify_handler(void *baton, 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, - _("* Error verifying revision %ld.\n"), - notify->revision)); - if (notify->err) - { - if (!b->silent_errors) - svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */, - "svnadmin: "); - - if (b->error_summary && notify->revision != SVN_INVALID_REVNUM) - { - struct verification_error *verr; - - verr = apr_palloc(b->result_pool, sizeof(*verr)); - verr->rev = notify->revision; - verr->err = svn_error_dup(notify->err); - apr_pool_cleanup_register(b->result_pool, verr->err, err_cleanup, - apr_pool_cleanup_null); - APR_ARRAY_PUSH(b->error_summary, - struct verification_error *) = verr; - } - } - return; - case svn_repos_notify_dump_rev_end: svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool, _("* Dumped revision %ld.\n"), @@ -1183,7 +1183,7 @@ subcommand_dump(apr_getopt_t *os, void *baton, apr svn_stream_t *stdout_stream; svn_revnum_t lower = SVN_INVALID_REVNUM, upper = SVN_INVALID_REVNUM; svn_revnum_t youngest; - struct repos_notify_handler_baton notify_baton = { 0 }; + svn_stream_t *feedback_stream = NULL; /* Expect no more arguments. */ SVN_ERR(parse_args(NULL, os, 0, 0, pool)); @@ -1217,12 +1217,12 @@ subcommand_dump(apr_getopt_t *os, void *baton, apr /* Progress feedback goes to STDERR, unless they asked to suppress it. */ if (! opt_state->quiet) - notify_baton.feedback_stream = recode_stream_create(stderr, pool); + feedback_stream = recode_stream_create(stderr, pool); SVN_ERR(svn_repos_dump_fs3(repos, stdout_stream, lower, upper, opt_state->incremental, opt_state->use_deltas, !opt_state->quiet ? repos_notify_handler : NULL, - ¬ify_baton, check_cancel, NULL, pool)); + feedback_stream, check_cancel, NULL, pool)); return SVN_NO_ERROR; } @@ -1372,7 +1372,7 @@ subcommand_load(apr_getopt_t *os, void *baton, apr svn_repos_t *repos; svn_revnum_t lower = SVN_INVALID_REVNUM, upper = SVN_INVALID_REVNUM; svn_stream_t *stdin_stream; - struct repos_notify_handler_baton notify_baton = { 0 }; + svn_stream_t *feedback_stream = NULL; /* Expect no more arguments. */ SVN_ERR(parse_args(NULL, os, 0, 0, pool)); @@ -1406,7 +1406,7 @@ subcommand_load(apr_getopt_t *os, void *baton, apr /* Progress feedback goes to STDOUT, unless they asked to suppress it. */ if (! opt_state->quiet) - notify_baton.feedback_stream = recode_stream_create(stdout, pool); + feedback_stream = recode_stream_create(stdout, pool); err = svn_repos_load_fs5(repos, stdin_stream, lower, upper, opt_state->uuid_action, opt_state->parent_dir, @@ -1415,7 +1415,7 @@ subcommand_load(apr_getopt_t *os, void *baton, apr !opt_state->bypass_prop_validation, opt_state->ignore_dates, opt_state->quiet ? NULL : repos_notify_handler, - ¬ify_baton, check_cancel, NULL, pool); + feedback_stream, check_cancel, NULL, pool); if (err && err->apr_err == SVN_ERR_BAD_PROPERTY_VALUE) return svn_error_quick_wrap(err, _("Invalid property value found in " @@ -1462,12 +1462,12 @@ subcommand_recover(apr_getopt_t *os, void *baton, svn_repos_t *repos; svn_error_t *err; struct svnadmin_opt_state *opt_state = baton; - struct repos_notify_handler_baton notify_baton = { 0 }; + svn_stream_t *feedback_stream = NULL; /* Expect no more arguments. */ SVN_ERR(parse_args(NULL, os, 0, 0, pool)); - SVN_ERR(svn_stream_for_stdout(¬ify_baton.feedback_stream, pool)); + SVN_ERR(svn_stream_for_stdout(&feedback_stream, pool)); /* Restore default signal handlers until after we have acquired the * exclusive lock so that the user interrupt before we actually @@ -1475,7 +1475,7 @@ subcommand_recover(apr_getopt_t *os, void *baton, setup_cancellation_signals(SIG_DFL); err = svn_repos_recover4(opt_state->repository_path, TRUE, - repos_notify_handler, ¬ify_baton, + repos_notify_handler, feedback_stream, check_cancel, NULL, pool); if (err) { @@ -1493,7 +1493,7 @@ subcommand_recover(apr_getopt_t *os, void *baton, " another process has it open?\n"))); SVN_ERR(svn_cmdline_fflush(stdout)); SVN_ERR(svn_repos_recover4(opt_state->repository_path, FALSE, - repos_notify_handler, ¬ify_baton, + repos_notify_handler, feedback_stream, check_cancel, NULL, pool)); } @@ -1779,7 +1779,7 @@ subcommand_pack(apr_getopt_t *os, void *baton, apr { struct svnadmin_opt_state *opt_state = baton; svn_repos_t *repos; - struct repos_notify_handler_baton notify_baton = { 0 }; + svn_stream_t *feedback_stream = NULL; /* Expect no more arguments. */ SVN_ERR(parse_args(NULL, os, 0, 0, pool)); @@ -1788,11 +1788,11 @@ subcommand_pack(apr_getopt_t *os, void *baton, apr /* Progress feedback goes to STDOUT, unless they asked to suppress it. */ if (! opt_state->quiet) - notify_baton.feedback_stream = recode_stream_create(stdout, pool); + feedback_stream = recode_stream_create(stdout, pool); return svn_error_trace( svn_repos_fs_pack2(repos, !opt_state->quiet ? repos_notify_handler : NULL, - ¬ify_baton, check_cancel, NULL, pool)); + feedback_stream, check_cancel, NULL, pool)); } @@ -1804,10 +1804,8 @@ subcommand_verify(apr_getopt_t *os, void *baton, a svn_repos_t *repos; svn_fs_t *fs; svn_revnum_t youngest, lower, upper; - struct repos_notify_handler_baton notify_baton = { 0 }; - struct repos_notify_handler_baton *notify_baton_p = ¬ify_baton; - svn_repos_notify_func_t notify_func = repos_notify_handler; - svn_error_t *verify_err; + svn_stream_t *feedback_stream = NULL; + struct repos_verify_callback_baton verify_baton = { 0 }; /* Expect no more arguments. */ SVN_ERR(parse_args(NULL, os, 0, 0, pool)); @@ -1851,42 +1849,27 @@ subcommand_verify(apr_getopt_t *os, void *baton, a upper = lower; } - /* Set up the notification handler. */ - if (!opt_state->quiet || opt_state->keep_going) - { - if (opt_state->quiet) - { - notify_baton.silent_running = TRUE; - notify_baton.feedback_stream = recode_stream_create(stderr, pool); - } - else - notify_baton.feedback_stream = recode_stream_create(stdout, pool); + if (!opt_state->quiet) + feedback_stream = recode_stream_create(stdout, pool); - if (opt_state->keep_going) - notify_baton.error_summary = - apr_array_make(pool, 0, sizeof(struct verification_error *)); - else - notify_baton.silent_errors = TRUE; + verify_baton.keep_going = opt_state->keep_going; + verify_baton.error_summary = + apr_array_make(pool, 0, sizeof(struct verification_error *)); + verify_baton.result_pool = pool; - notify_baton.result_pool = pool; - } - else - { - notify_func = NULL; - notify_baton_p = NULL; - } + SVN_ERR(svn_repos_verify_fs3(repos, lower, upper, + opt_state->check_normalization, + opt_state->metadata_only, + !opt_state->quiet + ? repos_notify_handler : NULL, + feedback_stream, + repos_verify_callback, &verify_baton, + check_cancel, NULL, pool)); - verify_err = svn_repos_verify_fs3(repos, lower, upper, - opt_state->keep_going, - opt_state->check_normalization, - opt_state->metadata_only, - notify_func, notify_baton_p, - check_cancel, NULL, pool); - /* Show the --keep-going error summary. */ if (!opt_state->quiet && opt_state->keep_going - && notify_baton.error_summary->nelts > 0) + && verify_baton.error_summary->nelts > 0) { int rev_maxlength; svn_revnum_t end_revnum; @@ -1894,7 +1877,7 @@ subcommand_verify(apr_getopt_t *os, void *baton, a int i; svn_error_clear( - svn_stream_puts(notify_baton.feedback_stream, + svn_stream_puts(feedback_stream, _("\n-----Summary of corrupt revisions-----\n"))); /* The standard column width for the revision number is 6 characters. @@ -1901,8 +1884,8 @@ subcommand_verify(apr_getopt_t *os, void *baton, a If the revision number can potentially be larger (i.e. if end_revnum is larger than 1000000), we increase the column width as needed. */ rev_maxlength = 6; - end_revnum = APR_ARRAY_IDX(notify_baton.error_summary, - notify_baton.error_summary->nelts - 1, + end_revnum = APR_ARRAY_IDX(verify_baton.error_summary, + verify_baton.error_summary->nelts - 1, struct verification_error *)->rev; while (end_revnum >= 1000000) { @@ -1911,7 +1894,7 @@ subcommand_verify(apr_getopt_t *os, void *baton, a } iterpool = svn_pool_create(pool); - for (i = 0; i < notify_baton.error_summary->nelts; i++) + for (i = 0; i < verify_baton.error_summary->nelts; i++) { struct verification_error *verr; svn_error_t *err; @@ -1919,22 +1902,25 @@ subcommand_verify(apr_getopt_t *os, void *baton, a svn_pool_clear(iterpool); - verr = APR_ARRAY_IDX(notify_baton.error_summary, i, + verr = APR_ARRAY_IDX(verify_baton.error_summary, i, struct verification_error *); - rev_str = apr_psprintf(iterpool, "r%ld", verr->rev); - rev_str = apr_psprintf(iterpool, "%*s", rev_maxlength, rev_str); - for (err = svn_error_purge_tracing(verr->err); - err != SVN_NO_ERROR; err = err->child) + + if (verr->rev != SVN_INVALID_REVNUM) { - char buf[512]; - const char *message; + rev_str = apr_psprintf(iterpool, "r%ld", verr->rev); + rev_str = apr_psprintf(iterpool, "%*s", rev_maxlength, rev_str); + for (err = svn_error_purge_tracing(verr->err); + err != SVN_NO_ERROR; err = err->child) + { + char buf[512]; + const char *message; - message = svn_err_best_message(err, buf, sizeof(buf)); - svn_error_clear(svn_stream_printf(notify_baton.feedback_stream, - iterpool, - "%s: E%06d: %s\n", - rev_str, err->apr_err, - message)); + message = svn_err_best_message(err, buf, sizeof(buf)); + svn_error_clear(svn_stream_printf(feedback_stream, iterpool, + "%s: E%06d: %s\n", + rev_str, err->apr_err, + message)); + } } } @@ -1941,7 +1927,15 @@ subcommand_verify(apr_getopt_t *os, void *baton, a svn_pool_destroy(iterpool); } - return svn_error_trace(verify_err); + if (verify_baton.error_summary->nelts > 0) + { + return svn_error_createf(SVN_ERR_CL_REPOS_VERIFY_FAILED, NULL, + _("Failed to verify repository '%s'"), + svn_dirent_local_style( + opt_state->repository_path, pool)); + } + + return SVN_NO_ERROR; } /* This implements `svn_opt_subcommand_t'. */ @@ -1949,7 +1943,7 @@ svn_error_t * subcommand_hotcopy(apr_getopt_t *os, void *baton, apr_pool_t *pool) { struct svnadmin_opt_state *opt_state = baton; - struct repos_notify_handler_baton notify_baton = { 0 }; + svn_stream_t *feedback_stream = NULL; apr_array_header_t *targets; const char *new_repos_path; @@ -1960,12 +1954,12 @@ subcommand_hotcopy(apr_getopt_t *os, void *baton, /* Progress feedback goes to STDOUT, unless they asked to suppress it. */ if (! opt_state->quiet) - notify_baton.feedback_stream = recode_stream_create(stdout, pool); + feedback_stream = recode_stream_create(stdout, pool); return svn_repos_hotcopy3(opt_state->repository_path, new_repos_path, opt_state->clean_logs, opt_state->incremental, !opt_state->quiet ? repos_notify_handler : NULL, - ¬ify_baton, check_cancel, NULL, pool); + feedback_stream, check_cancel, NULL, pool); } svn_error_t * @@ -2349,18 +2343,18 @@ subcommand_upgrade(apr_getopt_t *os, void *baton, { svn_error_t *err; struct svnadmin_opt_state *opt_state = baton; - struct repos_notify_handler_baton notify_baton = { 0 }; + svn_stream_t *feedback_stream = NULL; /* Expect no more arguments. */ SVN_ERR(parse_args(NULL, os, 0, 0, pool)); - SVN_ERR(svn_stream_for_stdout(¬ify_baton.feedback_stream, pool)); + SVN_ERR(svn_stream_for_stdout(&feedback_stream, pool)); /* Restore default signal handlers. */ setup_cancellation_signals(SIG_DFL); err = svn_repos_upgrade2(opt_state->repository_path, TRUE, - repos_notify_handler, ¬ify_baton, pool); + repos_notify_handler, feedback_stream, pool); if (err) { if (APR_STATUS_IS_EAGAIN(err->apr_err)) @@ -2378,7 +2372,7 @@ subcommand_upgrade(apr_getopt_t *os, void *baton, " another process has it open?\n"))); SVN_ERR(svn_cmdline_fflush(stdout)); SVN_ERR(svn_repos_upgrade2(opt_state->repository_path, FALSE, - repos_notify_handler, ¬ify_baton, + repos_notify_handler, feedback_stream, pool)); } else if (err->apr_err == SVN_ERR_FS_UNSUPPORTED_UPGRADE) Index: subversion/tests/cmdline/svnadmin_tests.py =================================================================== --- subversion/tests/cmdline/svnadmin_tests.py (revision 1687456) +++ subversion/tests/cmdline/svnadmin_tests.py (working copy) @@ -2070,8 +2070,6 @@ def verify_keep_going(sbox): exp_out = svntest.verify.RegexListOutput([".*Verified revision 0.", ".*Verified revision 1.", - ".*Error verifying revision 2.", - ".*Error verifying revision 3.", ".*", ".*Summary.*", ".*r2: E160004:.*", @@ -2082,8 +2080,18 @@ def verify_keep_going(sbox): if (svntest.main.fs_has_rep_sharing()): exp_out.insert(0, ".*Verifying.*metadata.*") - exp_err = svntest.verify.RegexListOutput(["svnadmin: E160004:.*", - "svnadmin: E165011:.*"], False) + exp_err = svntest.verify.RegexListOutput([".*Error verifying revision 2.", + "svnadmin: E160004:.*", + "svnadmin: E160004:.*", + ".*Error verifying revision 3.", + "svnadmin: E160004:.*", + "svnadmin: E160004:.*", + "svnadmin: E205012:.*"], False) + + if (svntest.main.is_fs_log_addressing()): + exp_err.insert(0, ".*Error verifying repository metadata.") + exp_err.insert(1, "svnadmin: E160004:.*") + if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin verify'.", output, errput, exp_out, exp_err): raise svntest.Failure @@ -2095,12 +2103,19 @@ def verify_keep_going(sbox): exp_out = svntest.verify.RegexListOutput([".*Verifying metadata at revision 0"]) else: exp_out = svntest.verify.RegexListOutput([".*Verified revision 0.", - ".*Verified revision 1.", - ".*Error verifying revision 2."]) + ".*Verified revision 1."]) if (svntest.main.fs_has_rep_sharing()): exp_out.insert(0, ".*Verifying repository metadata.*") - exp_err = svntest.verify.RegexListOutput(["svnadmin: E160004:.*"], False) + if (svntest.main.is_fs_log_addressing()): + exp_err = svntest.verify.RegexListOutput([ + ".*Error verifying repository metadata.", + "svnadmin: E160004:.*"], False) + else: + exp_err = svntest.verify.RegexListOutput([".*Error verifying revision 2.", + "svnadmin: E160004:.*", + "svnadmin: E160004:.*"], False) + if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin verify'.", output, errput, exp_out, exp_err): raise svntest.Failure @@ -2110,8 +2125,17 @@ def verify_keep_going(sbox): "--quiet", sbox.repo_dir) + if (svntest.main.is_fs_log_addressing()): + exp_err = svntest.verify.RegexListOutput([ + ".*Error verifying repository metadata.", + "svnadmin: E160004:.*"], False) + else: + exp_err = svntest.verify.RegexListOutput([".*Error verifying revision 2.", + "svnadmin: E160004:.*", + "svnadmin: E160004:.*"], False) + if svntest.verify.verify_outputs("Output of 'svnadmin verify' is unexpected.", - None, errput, None, "svnadmin: E160004:.*"): + None, errput, None, exp_err): raise svntest.Failure # Don't leave a corrupt repository @@ -2152,11 +2176,12 @@ def verify_keep_going_quiet(sbox): ".*Error verifying revision 3.", "svnadmin: E160004:.*", "svnadmin: E160004:.*", - "svnadmin: E165011:.*"], False) + "svnadmin: E205012:.*"], False) # Insert another expected error from checksum verification if (svntest.main.is_fs_log_addressing()): - exp_err.insert(0, "svnadmin: E160004:.*") + exp_err.insert(0, ".*Error verifying repository metadata.") + exp_err.insert(1, "svnadmin: E160004:.*") if svntest.verify.verify_outputs( "Unexpected error while running 'svnadmin verify'.", @@ -2231,23 +2256,15 @@ def verify_invalid_path_changes(sbox): exp_out = svntest.verify.RegexListOutput([".*Verified revision 0.", ".*Verified revision 1.", - ".*Error verifying revision 2.", ".*Verified revision 3.", - ".*Error verifying revision 4.", ".*Verified revision 5.", - ".*Error verifying revision 6.", ".*Verified revision 7.", ".*Verified revision 8.", ".*Verified revision 9.", - ".*Error verifying revision 10.", ".*Verified revision 11.", - ".*Error verifying revision 12.", ".*Verified revision 13.", - ".*Error verifying revision 14.", ".*Verified revision 15.", - ".*Error verifying revision 16.", ".*Verified revision 17.", - ".*Error verifying revision 18.", ".*Verified revision 19.", ".*", ".*Summary.*", @@ -2271,9 +2288,30 @@ def verify_invalid_path_changes(sbox): if svntest.main.is_fs_log_addressing(): exp_out.insert(1, ".*Verifying.*metadata.*") - exp_err = svntest.verify.RegexListOutput(["svnadmin: E160020:.*", + exp_err = svntest.verify.RegexListOutput([".*Error verifying revision 2.", + "svnadmin: E160020:.*", + "svnadmin: E160020:.*", + ".*Error verifying revision 4.", + "svnadmin: E160013:.*", + ".*Error verifying revision 6.", + "svnadmin: E160013:.*", + "svnadmin: E160013:.*", + ".*Error verifying revision 10.", + "svnadmin: E160013:.*", + "svnadmin: E160013:.*", + ".*Error verifying revision 12.", "svnadmin: E145001:.*", - "svnadmin: E160013:.*"], False) + "svnadmin: E145001:.*", + ".*Error verifying revision 14.", + "svnadmin: E160013:.*", + "svnadmin: E160013:.*", + ".*Error verifying revision 16.", + "svnadmin: E145001:.*", + "svnadmin: E145001:.*", + ".*Error verifying revision 18.", + "svnadmin: E160013:.*", + "svnadmin: E160013:.*", + "svnadmin: E205012:.*"], False) if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin verify'.", @@ -2284,9 +2322,10 @@ def verify_invalid_path_changes(sbox): sbox.repo_dir) exp_out = svntest.verify.RegexListOutput([".*Verified revision 0.", - ".*Verified revision 1.", - ".*Error verifying revision 2."]) - exp_err = svntest.verify.RegexListOutput(["svnadmin: E160020:.*"], False) + ".*Verified revision 1."]) + exp_err = svntest.verify.RegexListOutput([".*Error verifying revision 2.", + "svnadmin: E160020:.*", + "svnadmin: E160020:.*"], False) if (svntest.main.fs_has_rep_sharing()): exp_out.insert(0, ".*Verifying.*metadata.*") @@ -2301,8 +2340,13 @@ def verify_invalid_path_changes(sbox): "--quiet", sbox.repo_dir) + exp_out = [] + exp_err = svntest.verify.RegexListOutput([".*Error verifying revision 2.", + "svnadmin: E160020:.*", + "svnadmin: E160020:.*"], False) + if svntest.verify.verify_outputs("Output of 'svnadmin verify' is unexpected.", - None, errput, None, "svnadmin: E160020:.*"): + output, errput, exp_out, exp_err): raise svntest.Failure # Don't leave a corrupt repository Index: subversion/tests/libsvn_fs_fs/fs-fs-fuzzy-test.c =================================================================== --- subversion/tests/libsvn_fs_fs/fs-fs-fuzzy-test.c (revision 1687456) +++ subversion/tests/libsvn_fs_fs/fs-fs-fuzzy-test.c (working copy) @@ -117,8 +117,9 @@ fuzzing_1_byte_1_rev(const char *repo_name, svn_fs_set_warning_func(svn_repos_fs(repos), dont_filter_warnings, NULL); /* This shall detect the corruption and return an error. */ - err = svn_repos_verify_fs3(repos, revision, revision, TRUE, FALSE, FALSE, - NULL, NULL, NULL, NULL, iterpool); + err = svn_repos_verify_fs3(repos, revision, revision, FALSE, FALSE, + NULL, NULL, NULL, NULL, NULL, NULL, + iterpool); /* Case-only changes in checksum digests are not an error. * We allow upper case chars to be used in MD5 checksums in all other Index: subversion/tests/libsvn_fs_fs/fs-fs-private-test.c =================================================================== --- subversion/tests/libsvn_fs_fs/fs-fs-private-test.c (revision 1687456) +++ subversion/tests/libsvn_fs_fs/fs-fs-private-test.c (working copy) @@ -361,9 +361,10 @@ receive_index(const svn_fs_fs__p2l_entry_t *entry, return SVN_NO_ERROR; } +#define REPO_NAME "test-repo-load-index-test" + static svn_error_t * -load_index_test(const svn_test_opts_t *opts, apr_pool_t *pool, - const char *repo_name, svn_boolean_t keep_going) +load_index(const svn_test_opts_t *opts, apr_pool_t *pool) { svn_repos_t *repos; svn_revnum_t rev; @@ -381,7 +382,7 @@ static svn_error_t * "pre-1.9 SVN doesn't have FSFS indexes"); /* Create a filesystem */ - SVN_ERR(create_greek_repo(&repos, &rev, opts, repo_name, pool, pool)); + SVN_ERR(create_greek_repo(&repos, &rev, opts, REPO_NAME, pool, pool)); /* Read the original index contents for REV in ENTRIES. */ SVN_ERR(svn_fs_fs__dump_index(svn_repos_fs(repos), rev, receive_index, @@ -397,34 +398,21 @@ static svn_error_t * APR_ARRAY_PUSH(alt_entries, svn_fs_fs__p2l_entry_t *) = &entry; SVN_ERR(svn_fs_fs__load_index(svn_repos_fs(repos), rev, alt_entries, pool)); - SVN_TEST_ASSERT_ERROR(svn_repos_verify_fs3(repos, rev, rev, - keep_going, FALSE, FALSE, - NULL, NULL, NULL, NULL, pool), - (keep_going - ? SVN_ERR_REPOS_VERIFY_FAILED - : SVN_ERR_FS_INDEX_CORRUPTION)); + SVN_TEST_ASSERT_ERROR(svn_repos_verify_fs3(repos, rev, rev, FALSE, FALSE, + NULL, NULL, NULL, NULL, NULL, + NULL, pool), + SVN_ERR_FS_INDEX_CORRUPTION); /* Restore the original index. */ SVN_ERR(svn_fs_fs__load_index(svn_repos_fs(repos), rev, entries, pool)); - SVN_ERR(svn_repos_verify_fs3(repos, rev, rev, keep_going, FALSE, FALSE, + SVN_ERR(svn_repos_verify_fs3(repos, rev, rev, FALSE, FALSE, NULL, NULL, NULL, NULL, NULL, NULL, pool)); return SVN_NO_ERROR; } -static svn_error_t * -load_index(const svn_test_opts_t *opts, - apr_pool_t *pool) -{ - return load_index_test(opts, pool, "test-repo-load-index-test", FALSE); -} +#undef REPO_NAME -static svn_error_t * -load_index_keep_going(const svn_test_opts_t *opts, - apr_pool_t *pool) -{ - return load_index_test(opts, pool, "test-repo-load-index-full-test", TRUE); -} /* The test table. */ @@ -440,8 +428,6 @@ static struct svn_test_descriptor_t test_funcs[] = "dump the P2L index"), SVN_TEST_OPTS_PASS(load_index, "load the P2L index"), - SVN_TEST_OPTS_PASS(load_index_keep_going, - "load the P2L index (full verification)"), SVN_TEST_NULL };