You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/11/27 17:30:13 UTC

svn commit: r1546092 - in /subversion/branches/verify-keep-going/subversion: include/svn_repos.h libsvn_repos/dump.c svnadmin/svnadmin.c

Author: stsp
Date: Wed Nov 27 16:30:13 2013
New Revision: 1546092

URL: http://svn.apache.org/r1546092
Log:
On the verify-keep-going branch, move the entire error summary logic
into svnadmin, rather than splitting it between svnadmin and libsvn_repos.

This simplifies the code on this branch and reduces the diff to trunk.

Suggested by: julianfoad

* subversion/include/svn_repos.h
  (svn_repos_notify_failure_summary, svn_repos_notify_summary): Remove.
   Not needed anymore since svnadmin now deals with error summaries internally.

* subversion/libsvn_repos/dump.c
  (notify_verification_summary, notify_verification_error_summary): Remove.
  (error_list, err_cleanup, populate_summary): Moved to svnadmin.c.
  (svn_repos_verify_fs3): Remove code dealing with error summary. This
   function is now identical to its current implementation on trunk.

* subversion/svnadmin/svnadmin.c
  (error_list, err_cleanup, populate_summary): Moved here from dump.c.
  (repos_notify_handler_baton): New baton type which can store error
   summary information.
  (repos_notify_handler): Gather error summary information. Remove code
   to handle now obsolete notifications.
  (subcommand_dump, subcommand_load, subcommand_recover, subcommand_pack,
   subcommand_upgrade): Handle the new baton type.
  (subcommand_verify): Handle new baton type and display an error summary
   if verify --keep-going encounters errors.

Modified:
    subversion/branches/verify-keep-going/subversion/include/svn_repos.h
    subversion/branches/verify-keep-going/subversion/libsvn_repos/dump.c
    subversion/branches/verify-keep-going/subversion/svnadmin/svnadmin.c

Modified: subversion/branches/verify-keep-going/subversion/include/svn_repos.h
URL: http://svn.apache.org/viewvc/subversion/branches/verify-keep-going/subversion/include/svn_repos.h?rev=1546092&r1=1546091&r2=1546092&view=diff
==============================================================================
--- subversion/branches/verify-keep-going/subversion/include/svn_repos.h (original)
+++ subversion/branches/verify-keep-going/subversion/include/svn_repos.h Wed Nov 27 16:30:13 2013
@@ -263,13 +263,7 @@ typedef enum svn_repos_notify_action_t
   svn_repos_notify_cleanup_revprops,
 
   /** The repository format got bumped. @since New in 1.9. */
-  svn_repos_notify_format_bumped,
-
-  /** Summary of corrupt revisions. @since New in 1.9. */
-  svn_repos_notify_failure_summary,
-
-  /** Summary title string. @since New in 1.9. */
-  svn_repos_notify_summary
+  svn_repos_notify_format_bumped
 } svn_repos_notify_action_t;
 
 /** The type of error occurring.

Modified: subversion/branches/verify-keep-going/subversion/libsvn_repos/dump.c
URL: http://svn.apache.org/viewvc/subversion/branches/verify-keep-going/subversion/libsvn_repos/dump.c?rev=1546092&r1=1546091&r2=1546092&view=diff
==============================================================================
--- subversion/branches/verify-keep-going/subversion/libsvn_repos/dump.c (original)
+++ subversion/branches/verify-keep-going/subversion/libsvn_repos/dump.c Wed Nov 27 16:30:13 2013
@@ -2040,36 +2040,6 @@ notify_verification_error(svn_revnum_t r
   notify_func(notify_baton, notify_failure, pool);
 }
 
-static void
-notify_verification_summary(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_summary, pool);
-  notify_failure->err = err;
-  notify_failure->revision = SVN_INVALID_REVNUM;
-  notify_func(notify_baton, notify_failure, pool);
-}
-
-static void
-notify_verification_error_summary(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_summary,
-                                           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,
@@ -2148,38 +2118,6 @@ verify_fs2_notify_func(svn_revnum_t revi
                             notify_baton->notify, pool);
 }
 
-/* Structure for populating the errors. */
-struct error_list
-{
-  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_clear(data);
-
-  return APR_SUCCESS;
-}
-
-/* Populate the error summary. */
-static void
-populate_summary(apr_array_header_t **error_summary,
-                 svn_revnum_t rev,
-                 svn_error_t *err,
-                 apr_pool_t *result_pool)
-{
-  struct error_list *el = apr_palloc(result_pool, sizeof(*el));
-
-  el->rev = rev;
-  el->err = svn_error_dup(err);
-
-  apr_pool_cleanup_register(result_pool, el->err, err_cleanup,
-                            apr_pool_cleanup_null);
-  APR_ARRAY_PUSH(*error_summary, struct error_list *) = el;
-}
-
 /* cache entry (de-)serialization support for svn_node_kind_t. */
 static svn_error_t *
 serialize_node_kind(void **data,
@@ -2224,7 +2162,6 @@ svn_repos_verify_fs3(svn_repos_t *repos,
   svn_fs_progress_notify_func_t verify_notify = NULL;
   struct verify_fs2_notify_func_baton_t *verify_notify_baton = NULL;
   svn_error_t *err;
-  apr_array_header_t *error_summary;
   svn_boolean_t found_corruption = FALSE;
   svn_cache__t *verified_dirents_cache = NULL;
 
@@ -2300,8 +2237,6 @@ svn_repos_verify_fs3(svn_repos_t *repos,
                                   FALSE,
                                   pool));
 
-  error_summary = apr_array_make(pool, 0, sizeof(struct error_list *));
-          
   for (rev = start_rev; rev <= end_rev; rev++)
     {
       svn_pool_clear(iterpool);
@@ -2317,18 +2252,12 @@ svn_repos_verify_fs3(svn_repos_t *repos,
           found_corruption = TRUE;
           notify_verification_error(rev, err, notify_func, notify_baton,
                                     iterpool);
+          svn_error_clear(err);
 
           if (keep_going)
-            {
-              populate_summary(&error_summary, rev, err, pool);
-              svn_error_clear(err);
-              continue;
-            }
+            continue;
           else
-            {
-              svn_error_clear(err);
-              break;
-            }
+            break;
         }
 
       if (notify_func)
@@ -2338,22 +2267,6 @@ svn_repos_verify_fs3(svn_repos_t *repos,
         }
     }
 
-  /* Show the summary. */
-  if (notify_func && keep_going && found_corruption)
-    {
-      int i;
-
-      notify_verification_summary(err, notify_func, notify_baton, iterpool);
-      for (i = 0; i < error_summary->nelts; i++)
-        {
-          struct error_list *err_list = APR_ARRAY_IDX(error_summary, i,
-                                                      struct error_list *);
-          notify_verification_error_summary(err_list->rev, err_list->err,
-                                            notify_func, notify_baton,
-                                            iterpool);
-        }
-    }
-
   /* We're done. */
   if (notify_func)
     {

Modified: subversion/branches/verify-keep-going/subversion/svnadmin/svnadmin.c
URL: http://svn.apache.org/viewvc/subversion/branches/verify-keep-going/subversion/svnadmin/svnadmin.c?rev=1546092&r1=1546091&r2=1546092&view=diff
==============================================================================
--- subversion/branches/verify-keep-going/subversion/svnadmin/svnadmin.c (original)
+++ subversion/branches/verify-keep-going/subversion/svnadmin/svnadmin.c Wed Nov 27 16:30:13 2013
@@ -805,6 +805,44 @@ subcommand_deltify(apr_getopt_t *os, voi
   return SVN_NO_ERROR;
 }
 
+/* Structure for errors shown in the --keep-going summary. */
+struct error_list
+{
+  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_clear(data);
+
+  return APR_SUCCESS;
+}
+
+/* Add an error to the --keep-going error summary. */
+static void
+populate_summary(apr_array_header_t *error_summary,
+                 svn_revnum_t rev,
+                 svn_error_t *err,
+                 apr_pool_t *result_pool)
+{
+  struct error_list *el = apr_palloc(result_pool, sizeof(*el));
+
+  el->rev = rev;
+  el->err = svn_error_dup(err);
+
+  apr_pool_cleanup_register(result_pool, el->err, err_cleanup,
+                            apr_pool_cleanup_null);
+  APR_ARRAY_PUSH(error_summary, struct error_list *) = el;
+}
+
+struct repos_notify_handler_baton {
+  svn_stream_t *feedback_stream;
+  apr_array_header_t *error_summary;
+  apr_pool_t *result_pool;
+} repos_notify_handler_baton;
 
 /* Implementation of svn_repos_notify_func_t to wrap the output to a
    response stream for svn_repos_dump_fs2() and svn_repos_verify_fs() */
@@ -813,7 +851,8 @@ repos_notify_handler(void *baton,
                      const svn_repos_notify_t *notify,
                      apr_pool_t *scratch_pool)
 {
-  svn_stream_t *feedback_stream = baton;
+  struct repos_notify_handler_baton *b = baton;
+  svn_stream_t *feedback_stream = b->feedback_stream;
 
   switch (notify->action)
   {
@@ -829,39 +868,13 @@ repos_notify_handler(void *baton,
                                     _("* Error verifying revision %ld.\n"),
                                     notify->revision));
       if (notify->err)
-        svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */,
-                          "svnadmin: ");
-      return;
-
-    case svn_repos_notify_failure_summary:
-      if (notify->revision != SVN_INVALID_REVNUM)
         {
-          svn_error_t *err;
-          const char *rev_str;
-          
-          rev_str = apr_psprintf(scratch_pool, "r%ld", notify->revision);
-          for (err = svn_error_purge_tracing(notify->err);
-               err != SVN_NO_ERROR; err = err->child)
-            {
-              char buf[512];
-              const char *message;
-              
-              if (err != SVN_NO_ERROR)
-                {
-                  message = svn_err_best_message(err, buf, sizeof(buf));
-                  svn_error_clear(svn_stream_printf(feedback_stream,
-                                                    scratch_pool,
-                                                    "%6s: E%06d: %s\n",
-                                                    rev_str, err->apr_err,
-                                                    message));
-                }
-            }
-      }
-      return;
-
-    case svn_repos_notify_summary:
-      svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
-                            _("\n-----Summary of corrupt revisions-----\n")));
+          svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */,
+                            "svnadmin: ");
+          if (b->error_summary && notify->revision != SVN_INVALID_REVNUM)
+            populate_summary(b->error_summary, notify->revision,
+                             notify->err, b->result_pool);
+        }
       return;
 
     case svn_repos_notify_dump_rev_end:
@@ -1096,7 +1109,7 @@ subcommand_dump(apr_getopt_t *os, void *
   svn_stream_t *stdout_stream;
   svn_revnum_t lower = SVN_INVALID_REVNUM, upper = SVN_INVALID_REVNUM;
   svn_revnum_t youngest;
-  svn_stream_t *progress_stream = NULL;
+  struct repos_notify_handler_baton notify_baton = { 0 };
 
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
@@ -1130,12 +1143,12 @@ subcommand_dump(apr_getopt_t *os, void *
 
   /* Progress feedback goes to STDERR, unless they asked to suppress it. */
   if (! opt_state->quiet)
-    progress_stream = recode_stream_create(stderr, pool);
+    notify_baton.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,
-                             progress_stream, check_cancel, NULL, pool));
+                             &notify_baton, check_cancel, NULL, pool));
 
   return SVN_NO_ERROR;
 }
@@ -1282,7 +1295,8 @@ subcommand_load(apr_getopt_t *os, void *
   struct svnadmin_opt_state *opt_state = baton;
   svn_repos_t *repos;
   svn_revnum_t lower = SVN_INVALID_REVNUM, upper = SVN_INVALID_REVNUM;
-  svn_stream_t *stdin_stream, *stdout_stream = NULL;
+  svn_stream_t *stdin_stream;
+  struct repos_notify_handler_baton notify_baton = { 0 };
 
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
@@ -1316,7 +1330,7 @@ subcommand_load(apr_getopt_t *os, void *
 
   /* Progress feedback goes to STDOUT, unless they asked to suppress it. */
   if (! opt_state->quiet)
-    stdout_stream = recode_stream_create(stdout, pool);
+    notify_baton.feedback_stream = recode_stream_create(stdout, pool);
 
   err = svn_repos_load_fs4(repos, stdin_stream, lower, upper,
                            opt_state->uuid_action, opt_state->parent_dir,
@@ -1324,7 +1338,7 @@ subcommand_load(apr_getopt_t *os, void *
                            opt_state->use_post_commit_hook,
                            !opt_state->bypass_prop_validation,
                            opt_state->quiet ? NULL : repos_notify_handler,
-                           stdout_stream, check_cancel, NULL, pool);
+                           &notify_baton, 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 "
@@ -1371,12 +1385,12 @@ subcommand_recover(apr_getopt_t *os, voi
   svn_repos_t *repos;
   svn_error_t *err;
   struct svnadmin_opt_state *opt_state = baton;
-  svn_stream_t *stdout_stream;
+  struct repos_notify_handler_baton notify_baton = { 0 };
 
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
 
-  SVN_ERR(svn_stream_for_stdout(&stdout_stream, pool));
+  SVN_ERR(svn_stream_for_stdout(&notify_baton.feedback_stream, pool));
 
   /* Restore default signal handlers until after we have acquired the
    * exclusive lock so that the user interrupt before we actually
@@ -1384,7 +1398,7 @@ subcommand_recover(apr_getopt_t *os, voi
   setup_cancellation_signals(SIG_DFL);
 
   err = svn_repos_recover4(opt_state->repository_path, TRUE,
-                           repos_notify_handler, stdout_stream,
+                           repos_notify_handler, &notify_baton,
                            check_cancel, NULL, pool);
   if (err)
     {
@@ -1402,7 +1416,7 @@ subcommand_recover(apr_getopt_t *os, voi
                                    " 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, stdout_stream,
+                                 repos_notify_handler, &notify_baton,
                                  check_cancel, NULL, pool));
     }
 
@@ -1657,7 +1671,7 @@ subcommand_pack(apr_getopt_t *os, void *
 {
   struct svnadmin_opt_state *opt_state = baton;
   svn_repos_t *repos;
-  svn_stream_t *progress_stream = NULL;
+  struct repos_notify_handler_baton notify_baton = { 0 };
 
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
@@ -1666,11 +1680,11 @@ subcommand_pack(apr_getopt_t *os, void *
 
   /* Progress feedback goes to STDOUT, unless they asked to suppress it. */
   if (! opt_state->quiet)
-    progress_stream = recode_stream_create(stdout, pool);
+    notify_baton.feedback_stream = recode_stream_create(stdout, pool);
 
   return svn_error_trace(
     svn_repos_fs_pack2(repos, !opt_state->quiet ? repos_notify_handler : NULL,
-                       progress_stream, check_cancel, NULL, pool));
+                       &notify_baton, check_cancel, NULL, pool));
 }
 
 
@@ -1682,7 +1696,8 @@ subcommand_verify(apr_getopt_t *os, void
   svn_repos_t *repos;
   svn_fs_t *fs;
   svn_revnum_t youngest, lower, upper;
-  svn_stream_t *progress_stream = NULL;
+  struct repos_notify_handler_baton notify_baton = { 0 };
+  svn_error_t *verify_err;
 
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
@@ -1727,15 +1742,63 @@ subcommand_verify(apr_getopt_t *os, void
     }
 
   if (! opt_state->quiet)
-    progress_stream = recode_stream_create(stdout, pool);
+    {
+      notify_baton.feedback_stream = recode_stream_create(stdout, pool);
+      notify_baton.error_summary = apr_array_make(pool, 0,
+                                                  sizeof(struct error_list *));
+      notify_baton.result_pool = pool;
+    }
+
+  verify_err = svn_repos_verify_fs3(repos, lower, upper,
+                                    opt_state->keep_going,
+                                    opt_state->check_ucs_norm,
+                                    !opt_state->quiet
+                                    ? repos_notify_handler : NULL,
+                                    &notify_baton, check_cancel,
+                                    NULL, pool);
+
+  /* Show the error summary. */
+  if (!opt_state->quiet && opt_state->keep_going &&
+      notify_baton.error_summary->nelts > 0)
+    {
+      apr_pool_t *iterpool;
+      int i;
+
+      svn_error_clear(
+        svn_stream_printf(notify_baton.feedback_stream, pool,
+                          _("\n-----Summary of corrupt revisions-----\n")));
 
-  return svn_error_trace(svn_repos_verify_fs3(repos, lower, upper,
-                                              opt_state->keep_going,
-                                              opt_state->check_ucs_norm,
-                                              !opt_state->quiet
-                                              ? repos_notify_handler : NULL,
-                                              progress_stream, check_cancel,
-                                              NULL, pool));
+      iterpool = svn_pool_create(pool);
+      for (i = 0; i < notify_baton.error_summary->nelts; i++)
+        {
+          struct error_list *err_list;
+          svn_error_t *err;
+          const char *rev_str;
+          
+          svn_pool_clear(iterpool);
+
+          err_list = APR_ARRAY_IDX(notify_baton.error_summary, i,
+                                   struct error_list *);
+          rev_str = apr_psprintf(iterpool, "r%ld", err_list->rev);
+          for (err = svn_error_purge_tracing(err_list->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,
+                                                "%6s: E%06d: %s\n",
+                                                rev_str, err->apr_err,
+                                                message));
+            }
+        }
+
+       svn_pool_destroy(iterpool);
+    }
+
+  return svn_error_trace(verify_err);
 }
 
 /* This implements `svn_opt_subcommand_t'. */
@@ -2122,18 +2185,18 @@ subcommand_upgrade(apr_getopt_t *os, voi
 {
   svn_error_t *err;
   struct svnadmin_opt_state *opt_state = baton;
-  svn_stream_t *stdout_stream;
+  struct repos_notify_handler_baton notify_baton = { 0 };
 
   /* Expect no more arguments. */
   SVN_ERR(parse_args(NULL, os, 0, 0, pool));
 
-  SVN_ERR(svn_stream_for_stdout(&stdout_stream, pool));
+  SVN_ERR(svn_stream_for_stdout(&notify_baton.feedback_stream, pool));
 
   /* Restore default signal handlers. */
   setup_cancellation_signals(SIG_DFL);
 
   err = svn_repos_upgrade2(opt_state->repository_path, TRUE,
-                           repos_notify_handler, stdout_stream, pool);
+                           repos_notify_handler, &notify_baton, pool);
   if (err)
     {
       if (APR_STATUS_IS_EAGAIN(err->apr_err))
@@ -2151,7 +2214,7 @@ subcommand_upgrade(apr_getopt_t *os, voi
                                        " 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, stdout_stream,
+                                     repos_notify_handler, &notify_baton,
                                      pool));
         }
       else if (err->apr_err == SVN_ERR_FS_UNSUPPORTED_UPGRADE)