Index: subversion/include/svn_repos.h =================================================================== --- subversion/include/svn_repos.h (revision 1685592) +++ subversion/include/svn_repos.h (working copy) @@ -2848,10 +2848,11 @@ 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 notify_func is not null, then call it with @a notify_baton and + * The @a notify_func function will be called 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.) + * revision, the revision number is #SVN_INVALID_REVNUM.) @a notify_func + * is mandatory and cannot be @c NULL. * * For each FS-specific structure warning: * @c action = svn_repos_notify_verify_rev_structure @@ -2888,10 +2889,11 @@ 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. + * Returned error indicates an error associated with the verification + * process itself (typically #SVN_ERR_CANCELLED). These errors are always + * returned immediately. Return value of #SVN_NO_ERROR indicates that + * the verification process has completed and that the corresponding errors, + * if any, were reported via the @a notify_func callback. * * @since New in 1.9. */ Index: subversion/libsvn_repos/deprecated.c =================================================================== --- subversion/libsvn_repos/deprecated.c (revision 1685592) +++ subversion/libsvn_repos/deprecated.c (working copy) @@ -759,6 +759,31 @@ svn_repos_dump_fs2(svn_repos_t *repos, pool)); } +typedef struct verify_compat_notify_baton_t +{ + svn_error_t *notify_err; + svn_repos_notify_func_t notify_func; + void *notify_baton; +} verify_compat_notify_baton_t; + +static void +verify_compat_notify_func(void *baton, + const svn_repos_notify_t *notify, + apr_pool_t *scratch_pool) +{ + verify_compat_notify_baton_t *b = baton; + + if (notify->action == svn_repos_notify_failure) + { + b->notify_err = svn_error_compose_create(b->notify_err, + svn_error_dup(notify->err)); + } + else if (b->notify_func) + { + b->notify_func(b->notify_baton, notify, scratch_pool); + } +} + svn_error_t * svn_repos_verify_fs2(svn_repos_t *repos, svn_revnum_t start_rev, @@ -769,17 +794,18 @@ svn_repos_verify_fs2(svn_repos_t *repos, void *cancel_baton, apr_pool_t *pool) { - return svn_error_trace(svn_repos_verify_fs3(repos, - start_rev, - end_rev, - FALSE, - FALSE, - FALSE, - notify_func, - notify_baton, - cancel_func, - cancel_baton, - pool)); + verify_compat_notify_baton_t baton; + svn_error_t *err; + + baton.notify_err = SVN_NO_ERROR; + baton.notify_func = notify_func; + baton.notify_baton = notify_baton; + + err = svn_repos_verify_fs3(repos, start_rev, end_rev, + FALSE, FALSE, FALSE, verify_compat_notify_func, + &baton, cancel_func, cancel_baton, pool); + + return svn_error_compose_create(err, baton.notify_err); } svn_error_t * Index: subversion/libsvn_repos/dump.c =================================================================== --- subversion/libsvn_repos/dump.c (revision 1685592) +++ subversion/libsvn_repos/dump.c (working copy) @@ -2274,9 +2274,6 @@ notify_verification_error(svn_revnum_t rev, { 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; @@ -2375,14 +2372,13 @@ svn_repos_verify_fs3(svn_repos_t *repos, svn_fs_t *fs = svn_repos_fs(repos); svn_revnum_t youngest; svn_revnum_t rev; - apr_pool_t *iterpool = svn_pool_create(pool); svn_repos_notify_t *notify; 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; + SVN_ERR_ASSERT(notify_func); + /* Determine the current youngest revision of the filesystem. */ SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool)); @@ -2406,18 +2402,14 @@ svn_repos_verify_fs3(svn_repos_t *repos, /* Create a notify object that we can reuse within the loop and a forwarding structure for notifications from inside svn_fs_verify(). */ - if (notify_func) - { - notify = svn_repos_notify_create(svn_repos_notify_verify_rev_end, pool); + notify = svn_repos_notify_create(svn_repos_notify_verify_rev_end, pool); + verify_notify = verify_fs_notify_func; + verify_notify_baton = apr_palloc(pool, sizeof(*verify_notify_baton)); + verify_notify_baton->notify_func = notify_func; + verify_notify_baton->notify_baton = notify_baton; + verify_notify_baton->notify + = svn_repos_notify_create(svn_repos_notify_verify_rev_structure, pool); - verify_notify = verify_fs_notify_func; - verify_notify_baton = apr_palloc(pool, sizeof(*verify_notify_baton)); - verify_notify_baton->notify_func = notify_func; - verify_notify_baton->notify_baton = notify_baton; - verify_notify_baton->notify - = svn_repos_notify_create(svn_repos_notify_verify_rev_structure, pool); - } - /* Verify global metadata and backend-specific data first. */ err = svn_fs_verify(svn_fs_path(fs, pool), svn_fs_config(fs, pool), start_rev, end_rev, @@ -2431,104 +2423,60 @@ svn_repos_verify_fs3(svn_repos_t *repos, else if (err) { notify_verification_error(SVN_INVALID_REVNUM, err, notify_func, - notify_baton, iterpool); + notify_baton, pool); + svn_error_clear(err); if (!keep_going) { - /* Return the error, the caller doesn't want us to continue. */ - return svn_error_trace(err); + /* The caller doesn't want us to continue. */ + return SVN_NO_ERROR; } - else - { - /* Clear the error and keep going. */ - failed_metadata = TRUE; - svn_error_clear(err); - } } if (!metadata_only) - for (rev = start_rev; rev <= end_rev; rev++) - { - svn_pool_clear(iterpool); - - /* Wrapper function to catch the possible errors. */ - err = verify_one_revision(fs, rev, notify_func, notify_baton, - start_rev, check_normalization, - cancel_func, cancel_baton, - iterpool); - - if (err && err->apr_err == SVN_ERR_CANCELLED) - { - return svn_error_trace(err); - } - 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); - } - } - else if (notify_func) - { - /* Tell the caller that we're done with this revision. */ - notify->revision = rev; - notify_func(notify_baton, notify, iterpool); - } - } - - /* We're done. */ - if (notify_func) { - notify = svn_repos_notify_create(svn_repos_notify_verify_end, iterpool); - notify_func(notify_baton, notify, iterpool); - } + apr_pool_t *iterpool = svn_pool_create(pool); - svn_pool_destroy(iterpool); + for (rev = start_rev; rev <= end_rev; rev++) + { + svn_pool_clear(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); + /* Wrapper function to catch the possible errors. */ + err = verify_one_revision(fs, rev, notify_func, notify_baton, + start_rev, check_normalization, + cancel_func, cancel_baton, + iterpool); - 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 (err && err->apr_err == SVN_ERR_CANCELLED) + { + return svn_error_trace(err); + } + else if (err) + { + notify_verification_error(rev, err, notify_func, notify_baton, + iterpool); + svn_error_clear(err); - 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); + if (!keep_going) + { + /* The caller doesn't want us to continue. */ + svn_pool_destroy(iterpool); + return SVN_NO_ERROR; + } + } 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); + { + /* Tell the caller that we're done with this revision. */ + notify->revision = rev; + notify_func(notify_baton, notify, iterpool); + } } + svn_pool_destroy(iterpool); } + /* We're done. */ + notify = svn_repos_notify_create(svn_repos_notify_verify_end, pool); + notify_func(notify_baton, notify, pool); + return SVN_NO_ERROR; } Index: subversion/svnadmin/svnadmin.c =================================================================== --- subversion/svnadmin/svnadmin.c (revision 1685592) +++ subversion/svnadmin/svnadmin.c (working copy) @@ -846,46 +846,14 @@ subcommand_deltify(apr_getopt_t *os, void *baton, return SVN_NO_ERROR; } -/* Structure for errors encountered during 'svnadmin verify --keep-going'. */ -struct verification_error -{ - svn_revnum_t rev; - svn_error_t *err; -}; - -/* Pool cleanup function to clear an svn_error_t *. */ -static apr_status_t -err_cleanup(void *data) -{ - svn_error_t *err = data; - - svn_error_clear(err); - - return APR_SUCCESS; -} - struct repos_notify_handler_baton { /* Stream to write progress and other non-error output to. */ svn_stream_t *feedback_stream; - - /* 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. */ - apr_pool_t *result_pool; }; /* 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. */ + response stream for svn_repos_dump_fs2(), svn_repos_hotcopy3() and + others. */ static void repos_notify_handler(void *baton, const svn_repos_notify_t *notify, @@ -894,14 +862,6 @@ repos_notify_handler(void *baton, struct repos_notify_handler_baton *b = baton; svn_stream_t *feedback_stream = b->feedback_stream; - /* 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 +870,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"), @@ -942,22 +876,6 @@ repos_notify_handler(void *baton, notify->revision)); return; - case svn_repos_notify_verify_rev_end: - svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool, - _("* Verified revision %ld.\n"), - notify->revision)); - return; - - case svn_repos_notify_verify_rev_structure: - if (notify->revision == SVN_INVALID_REVNUM) - svn_error_clear(svn_stream_puts(feedback_stream, - _("* Verifying repository metadata ...\n"))); - else - svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool, - _("* Verifying metadata at revision %ld ...\n"), - notify->revision)); - return; - case svn_repos_notify_pack_shard_start: { const char *shardstr = apr_psprintf(scratch_pool, @@ -1132,7 +1050,108 @@ repos_notify_handler(void *baton, } } +struct verify_notify_handler_baton +{ + /* Stream to write progress and other non-error output to. */ + svn_stream_t *feedback_stream; + /* List of encountered errors. */ + apr_array_header_t *errors; + + /* Pool for data collected during notifications. */ + apr_pool_t *result_pool; +}; + +/* Structure for errors encountered during 'svnadmin verify'. */ +struct verification_error +{ + svn_revnum_t rev; + svn_error_t *err; +}; + +/* Pool cleanup function to clear an svn_error_t *. */ +static apr_status_t err_cleanup(void *data) +{ + svn_error_t *err = data; + + svn_error_clear(err); + + return APR_SUCCESS; +} + +/* Implementation of svn_repos_notify_func_t to handle notifications + originating from svn_repos_verify_fs(). */ +static void +verify_notify_handler(void *baton, + const svn_repos_notify_t *notify, + apr_pool_t *scratch_pool) +{ + struct verify_notify_handler_baton *b = baton; + + switch (notify->action) + { + case svn_repos_notify_warning: + svn_error_clear(svn_cmdline_fprintf(stderr, scratch_pool, + "WARNING 0x%04x: %s\n", + notify->warning, + notify->warning_str)); + return; + + case svn_repos_notify_failure: + { + struct verification_error *verr; + + if (notify->revision == SVN_INVALID_REVNUM) + { + svn_error_clear(svn_cmdline_fputs( + _("* Error verifying repository metadata.\n"), + stderr, scratch_pool)); + } + else + { + svn_error_clear(svn_cmdline_fprintf(stderr, scratch_pool, + _("* Error verifying revision %ld.\n"), + notify->revision)); + } + svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */, + "svnadmin: "); + + /* Remember the error in B->ERRORS. */ + verr = apr_pcalloc(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->errors, struct verification_error *) = verr; + + return; + } + + case svn_repos_notify_verify_rev_end: + svn_error_clear(svn_stream_printf(b->feedback_stream, scratch_pool, + _("* Verified revision %ld.\n"), + notify->revision)); + return; + + case svn_repos_notify_verify_rev_structure: + if (notify->revision == SVN_INVALID_REVNUM) + { + svn_error_clear(svn_stream_puts(b->feedback_stream, + _("* Verifying repository metadata ...\n"))); + } + else + { + svn_error_clear(svn_stream_printf(b->feedback_stream, scratch_pool, + _("* Verifying metadata at revision %ld ...\n"), + notify->revision)); + } + return; + + default: + return; + } +} + /* Baton for recode_write(). */ struct recode_write_baton { @@ -1795,7 +1814,63 @@ subcommand_pack(apr_getopt_t *os, void *baton, apr ¬ify_baton, check_cancel, NULL, pool)); } +static void +print_error_summary(const apr_array_header_t *errors, + FILE *stream, apr_pool_t *pool) +{ + int rev_maxlength; + svn_revnum_t end_revnum; + apr_pool_t *iterpool; + int i; + svn_error_clear( + svn_cmdline_fputs(_("\n-----Summary of corrupt revisions-----\n"), + stream, pool)); + + /* The standard column width for the revision number is 6 characters. + 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(errors, errors->nelts - 1, + struct verification_error *)->rev; + while (end_revnum >= 1000000) + { + rev_maxlength++; + end_revnum = end_revnum / 10; + } + + iterpool = svn_pool_create(pool); + for (i = 0; i < errors->nelts; i++) + { + struct verification_error *verr; + svn_error_t *err; + const char *rev_str; + + svn_pool_clear(iterpool); + + verr = APR_ARRAY_IDX(errors, i, struct verification_error *); + + if (verr->rev != SVN_INVALID_REVNUM) + { + 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_cmdline_fprintf(stream, iterpool, + "%s: E%06d: %s\n", rev_str, + err->apr_err, message)); + } + } + } + svn_pool_destroy(iterpool); +} + /* This implements `svn_opt_subcommand_t'. */ static svn_error_t * subcommand_verify(apr_getopt_t *os, void *baton, apr_pool_t *pool) @@ -1804,10 +1879,7 @@ 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; + struct verify_notify_handler_baton notify_baton = { 0 }; /* Expect no more arguments. */ SVN_ERR(parse_args(NULL, os, 0, 0, pool)); @@ -1851,97 +1923,38 @@ 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) { - 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->keep_going) - notify_baton.error_summary = - apr_array_make(pool, 0, sizeof(struct verification_error *)); - else - notify_baton.silent_errors = TRUE; - - notify_baton.result_pool = pool; + notify_baton.feedback_stream = svn_stream_empty(pool); } else { - notify_func = NULL; - notify_baton_p = NULL; + notify_baton.feedback_stream = recode_stream_create(stdout, 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); + notify_baton.errors = apr_array_make(pool, 0, + sizeof(struct verification_error *)); + notify_baton.result_pool = pool; - /* Show the --keep-going error summary. */ - if (!opt_state->quiet - && opt_state->keep_going - && notify_baton.error_summary->nelts > 0) + SVN_ERR(svn_repos_verify_fs3(repos, lower, upper, + opt_state->keep_going, + opt_state->check_normalization, + opt_state->metadata_only, + verify_notify_handler, ¬ify_baton, + check_cancel, NULL, pool)); + + if (notify_baton.errors->nelts > 0) { - int rev_maxlength; - svn_revnum_t end_revnum; - apr_pool_t *iterpool; - int i; + if (opt_state->keep_going && !opt_state->quiet) + print_error_summary(notify_baton.errors, stdout, pool); - svn_error_clear( - svn_stream_puts(notify_baton.feedback_stream, - _("\n-----Summary of corrupt revisions-----\n"))); - - /* The standard column width for the revision number is 6 characters. - 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, - struct verification_error *)->rev; - while (end_revnum >= 1000000) - { - rev_maxlength++; - end_revnum = end_revnum / 10; - } - - iterpool = svn_pool_create(pool); - for (i = 0; i < notify_baton.error_summary->nelts; i++) - { - struct verification_error *verr; - svn_error_t *err; - const char *rev_str; - - svn_pool_clear(iterpool); - - verr = APR_ARRAY_IDX(notify_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) - { - 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)); - } - } - - svn_pool_destroy(iterpool); + return svn_error_createf(SVN_ERR_REPOS_VERIFY_FAILED, NULL, + _("Repository '%s' failed to verify"), + svn_dirent_local_style( + opt_state->repository_path, pool)); } - return svn_error_trace(verify_err); + return SVN_NO_ERROR; } /* This implements `svn_opt_subcommand_t'. */ Index: subversion/tests/cmdline/svnadmin_tests.py =================================================================== --- subversion/tests/cmdline/svnadmin_tests.py (revision 1685592) +++ 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: E165011:.*"]) + + 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,22 @@ 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:.*", + "svnadmin: E165011:.*"]) + else: + exp_err = \ + svntest.verify.RegexListOutput([".*Error verifying revision 2.", + "svnadmin: E160004:.*", + "svnadmin: E160004:.*", + "svnadmin: E165011:.*"]) + if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin verify'.", output, errput, exp_out, exp_err): raise svntest.Failure @@ -2110,8 +2128,20 @@ 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:.*", + "svnadmin: E165011:.*"]) + else: + exp_err = \ + svntest.verify.RegexListOutput([".*Error verifying revision 2.", + "svnadmin: E160004:.*", + "svnadmin: E160004:.*", + "svnadmin: E165011:.*"]) + 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 +2182,12 @@ def verify_keep_going_quiet(sbox): ".*Error verifying revision 3.", "svnadmin: E160004:.*", "svnadmin: E160004:.*", - "svnadmin: E165011:.*"], False) + "svnadmin: E165011:.*"]) # 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 +2262,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,6 +2294,9 @@ def verify_invalid_path_changes(sbox): if svntest.main.is_fs_log_addressing(): exp_out.insert(1, ".*Verifying.*metadata.*") + # ### I think that this test and verify_denormalized_names() both would + # benefit from explicit STDERR expectations, i.e., with match_all=True, + # but I skipped this part, as the patch is mostly an illustration. exp_err = svntest.verify.RegexListOutput(["svnadmin: E160020:.*", "svnadmin: E145001:.*", "svnadmin: E160013:.*"], False) @@ -2284,8 +2310,7 @@ def verify_invalid_path_changes(sbox): sbox.repo_dir) exp_out = svntest.verify.RegexListOutput([".*Verified revision 0.", - ".*Verified revision 1.", - ".*Error verifying revision 2."]) + ".*Verified revision 1."]) exp_err = svntest.verify.RegexListOutput(["svnadmin: E160020:.*"], False) if (svntest.main.fs_has_rep_sharing()): @@ -2327,14 +2352,8 @@ def verify_denormalized_names(sbox): ".*Verified revision 1.", ".*Verified revision 2.", ".*Verified revision 3.", - # A/{Eacute}/{aring}lpha - "WARNING 0x0003: Duplicate representation of path 'A/.*/.*lpha'", ".*Verified revision 4.", ".*Verified revision 5.", - # Q/{aring}lpha - "WARNING 0x0004: Duplicate representation of path '/Q/.*lpha'" - # A/{Eacute} - " in svn:mergeinfo property of 'A/.*'", ".*Verified revision 6.", ".*Verified revision 7."] @@ -2346,7 +2365,13 @@ def verify_denormalized_names(sbox): expected_output_regex_list.insert(0, ".* Verifying metadata at revision 0.*") exp_out = svntest.verify.RegexListOutput(expected_output_regex_list) - exp_err = svntest.verify.ExpectedOutput([]) + exp_err = svntest.verify.RegexListOutput( + # A/{Eacute}/{aring}lpha + ["WARNING 0x0003: Duplicate representation of path 'A/.*/.*lpha'", + # Q/{aring}lpha + "WARNING 0x0004: Duplicate representation of path '/Q/.*lpha'" + # A/{Eacute} + " in svn:mergeinfo property of 'A/.*'"]) svntest.verify.verify_outputs( "Unexpected error while running 'svnadmin verify'.", Index: subversion/tests/libsvn_fs_fs/fs-fs-fuzzy-test.c =================================================================== --- subversion/tests/libsvn_fs_fs/fs-fs-fuzzy-test.c (revision 1685592) +++ subversion/tests/libsvn_fs_fs/fs-fs-fuzzy-test.c (working copy) @@ -49,6 +49,20 @@ dont_filter_warnings(void *baton, svn_error_t *err return; } +static void +verify_notify_func(void *baton, + const svn_repos_notify_t *notify, + apr_pool_t *scratch_pool) +{ + svn_error_t **verify_err_p = baton; + + if (notify->action == svn_repos_notify_failure) + { + *verify_err_p = svn_error_compose_create(*verify_err_p, + svn_error_dup(notify->err)); + } +} + /*** Test core code ***/ @@ -116,9 +130,10 @@ fuzzing_1_byte_1_rev(const char *repo_name, SVN_ERR(svn_repos_open3(&repos, repo_name, fs_config, iterpool, iterpool)); 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); + /* This shall detect the corruption. */ + SVN_ERR(svn_repos_verify_fs3(repos, revision, revision, TRUE, FALSE, + FALSE, verify_notify_func, &err, + 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 1685592) +++ subversion/tests/libsvn_fs_fs/fs-fs-private-test.c (working copy) @@ -361,6 +361,20 @@ receive_index(const svn_fs_fs__p2l_entry_t *entry, return SVN_NO_ERROR; } +static void +verify_notify_func(void *baton, + const svn_repos_notify_t *notify, + apr_pool_t *scratch_pool) +{ + svn_error_t **verify_err_p = baton; + + if (notify->action == svn_repos_notify_failure) + { + *verify_err_p = svn_error_compose_create(*verify_err_p, + svn_error_dup(notify->err)); + } +} + 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) @@ -370,6 +384,7 @@ load_index_test(const svn_test_opts_t *opts, apr_p apr_array_header_t *entries = apr_array_make(pool, 41, sizeof(void *)); apr_array_header_t *alt_entries = apr_array_make(pool, 1, sizeof(void *)); svn_fs_fs__p2l_entry_t entry; + svn_error_t *err; /* Bail (with success) on known-untestable scenarios */ if (strcmp(opts->fs_type, "fsfs") != 0) @@ -396,18 +411,18 @@ load_index_test(const svn_test_opts_t *opts, apr_p entry.item.revision = SVN_INVALID_REVNUM; APR_ARRAY_PUSH(alt_entries, svn_fs_fs__p2l_entry_t *) = &entry; + err = SVN_NO_ERROR; 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_ERR(svn_repos_verify_fs3(repos, rev, rev, keep_going, FALSE, FALSE, + verify_notify_func, &err, NULL, NULL, pool)); + SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_INDEX_CORRUPTION); /* Restore the original index. */ + err = SVN_NO_ERROR; 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, - NULL, NULL, NULL, NULL, pool)); + verify_notify_func, &err, NULL, NULL, pool)); + SVN_ERR(err); return SVN_NO_ERROR; }