Hi,

This patch is a follow up of the long discussion we had in thread [1]. This patch implements a new switch "--keep-going" to svnadmin verify.

If "--keep-going" is set(True), svnadmin verify would continue to run till verifying all the revisions i.e, it would not stop at the very first occurance of a corruption/error found in the repo and would report corruptions wherever found.


I have worked on your suggestions and inputs for this patch. Kindly share your thoughts. Attaching the patch and the log message with this mail.


Thanks and regards
Prabhu

[1] http://svn.haxx.se/dev/archive-2012-10/0271.shtml

Implement svnadmin verify --keep-going, which would continue the verification
even if there is some corruption, after printing the errors to stderr.

* subversion/svnadmin/svnadmin.c
  (svnadmin__cmdline_options_t): Add keep-going option.
  (svnadmin_opt_state): Add keep-going option.
  (subcommand_verify): Switch to the new svn_repos_verify_fs3 function instead
   of svn_repos_verify_fs2, and pass the keep-going option.
  (repos_notify_handler): Handle svn_repos_notify_failure notification by
   printing warnings to stderr with the respective revision number.

* subversion/include/svn_repos.h
  (svn_repos_notify_action_t): Add svn_repos_notify_failure to notify failure.
  (svn_repos_verify_fs3): Newly added to handle "keep-going" option.
  (svn_repos_notify_t): Add "err", the error chain which indicates what went
   wrong during verification.

* subversion/libsvn_repos/dump.c
  (svn_repos_verify_fs3): Handle "keep-going". If "keep-going" is TRUE, the
   error message is notified and verification process continues.
  When a repository fails to verify, return an error SVN_ERR_REPOS_CORRUPTED.
  (notify_verification_error): New function to notify the verification
  failure error message.

* subversion/libsvn_repos/deprecated.c
  (svn_repos_verify_fs2): Deprecate. Call svn_repos_verify_fs3 with
   keep_going as FALSE by default to keep the old default implementation.

* subversion/libsvn_repos/notify.c
  (svn_repos_notify_create): Initialise the error chain "err" to SVN_NO_ERROR.

* subversion/tests/cmdline/svnadmin_tests.py
  (verify_keep_going): New test case to test svnadmin verify and the new
  switch --keep-going.

Patch by: Prabhu Gnana Sundar <prabhugs{_AT_}collab.net>
Index: subversion/tests/cmdline/svnadmin_tests.py
===================================================================
--- subversion/tests/cmdline/svnadmin_tests.py	(revision 1411074)
+++ subversion/tests/cmdline/svnadmin_tests.py	(working copy)
@@ -1835,6 +1835,114 @@
   svntest.main.run_svnadmin("recover", sbox.repo_dir)
 
 
+def verify_keep_going(sbox):
+  "svnadmin verify --keep-going test"
+  test_create(sbox)
+  dumpfile_skeleton = open(os.path.join(os.path.dirname(sys.argv[0]),
+                                                        'svnadmin_tests_data',
+                                                        'skeleton_repos.dump')).read()
+  load_dumpstream(sbox, dumpfile_skeleton, '--ignore-uuid')
+
+  r2 = fsfs_file(sbox.repo_dir, 'revs', '6')
+  fp = open(r2, 'wb')
+  fp.write("""id: 3-6.0.r6/0
+type: file
+count: 0
+text: 2 0 60 48 02d086e41b03058c5f1af6282c1f483f cc67e4dd7cd8ca83095c8b95f65b6698b39cb263 5-5/_5
+props: 2 73 34 0 25e6c2f7558b7484000d4d090dea5b92
+cpath: /Projects/docs/README
+copyroot: 0 /
+
+PLAIN
+K 6
+README
+V 15
+file 3-6.0.r6/0
+END
+ENDREP
+id: 1-6.0.r6/275
+type: dir
+count: 0
+text: 6 226 36 0 2c3f6410944c6ff8667fa1b3e78f45a2
+cpath: /Projects/docs
+copyroot: 0 /
+
+PLAIN
+K 9
+Project-X
+V 14
+dir 1-3.0.r3/0
+K 9
+Project-Y
+V 14
+dir 1-4.0.r4/0
+K 9
+Project-Z
+V 14
+dir 1-5.0.r5/0
+K 4
+docs
+V 16
+dir 1-6.0.r6/275
+END
+ENDREP
+id: 0-1.0.r6/548
+type: dir
+pred: 0-1.0.r5/195
+count: 4
+text: 6 398 137 0 c758f548da93cc57d68af0610766b549
+cpath: /Projects
+copyroot: 0 /
+
+PLAIN
+K 8
+Projects
+V 16
+dir 0-1.0.r6/548
+K 6
+README
+V 17
+file 0-2.0.r2/120
+END
+ENDREP
+id: 0.0.r6/772
+type: dir
+pred: 0.0.r5/418
+count: 6
+text: 6 686 73 0 353c6bbf43b0f2ae474d85e206337bbd
+cpath: /
+copyroot: 0 /
+
+_1.0.t5-5 add-dir false false /Projects/docs
+
+_3.0.t5-5 add-file true true /Projects/docs/README
+
+
+""")
+  fp.close()
+  exit_code, output, errput = svntest.main.run_svnadmin("verify",
+                                                        sbox.repo_dir)
+  exit_code, output, errput2 = svntest.main.run_svnadmin("verify",
+                                                         "--keep-going",
+                                                         sbox.repo_dir)
+
+  if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin verify'.",
+                                 [], errput, None, ".*svnadmin: E165005: .*"):
+    raise svntest.Failure
+
+  if svntest.verify.verify_outputs("Unexpected error while running 'svnadmin verify'.",
+                                   [], errput2, None,
+                                   ["* Verified revision 0.\n",
+                                   "* Verified revision 1.\n",
+                                   "* Verified revision 2.\n",
+                                   "* Verified revision 3.\n",
+                                   "* Verified revision 4.\n",
+                                   "* Verified revision 5.\n",
+                                   "* Error verifying revision 6.\n",
+                                   "svnadmin: E200004: Could not convert '' into a number\n",
+                                   "svnadmin: E165005: Repository 'svn-test-work/repositories/svnadmin_tests-31' failed to verify\n"]):
+      raise svntest.Failure
+
 ########################################################################
 # Run the tests
 
@@ -1871,6 +1979,7 @@
               locking,
               mergeinfo_race,
               recover_old,
+              verify_keep_going,
              ]
 
 if __name__ == '__main__':
Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c	(revision 1411074)
+++ subversion/svnadmin/main.c	(working copy)
@@ -175,6 +175,7 @@
   {
     svnadmin__version = SVN_OPT_FIRST_LONGOPT_ID,
     svnadmin__incremental,
+    svnadmin__keep_going,
     svnadmin__deltas,
     svnadmin__ignore_uuid,
     svnadmin__force_uuid,
@@ -280,6 +281,9 @@
     {"pre-1.6-compatible",     svnadmin__pre_1_6_compatible, 0,
      N_("deprecated; see --compatible-version")},
 
+    {"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"
@@ -471,7 +475,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} }
 };
@@ -501,6 +505,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 */
@@ -744,6 +749,21 @@
                                         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));
+/*        svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
+                                          _("svnadmin: E%d: Error verifying revision %ld\n"),
+                                          notify->err->apr_err,
+                                          notify->revision));
+*/      
+      if (notify->err)
+        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"),
@@ -1533,10 +1553,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'. */
@@ -2032,6 +2054,9 @@
           opt_state.compatible_version = compatible_version;
         }
         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 1411074)
+++ 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 1411074)
+++ 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,
Index: subversion/libsvn_repos/deprecated.c
===================================================================
--- subversion/libsvn_repos/deprecated.c	(revision 1411074)
+++ 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 1411074)
+++ 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,25 @@
                              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)
+    {
+      rev = SVN_INVALID_REVNUM;
+      if (!keep_going)
+        notify_verification_error(rev, err, notify_func, notify_baton,
+                                  iterpool);
+      found_corruption = TRUE;
+      svn_error_clear(err);
+      if (!keep_going)
+        return svn_error_createf(SVN_ERR_REPOS_CORRUPTED, NULL,
+                                 _("Repository '%s' failed to verify"),
+                                 svn_dirent_local_style(svn_repos_path(repos,
+                                                                       pool),
+                                                        pool));
+    }
+  else
+    SVN_ERR(err);
 
   /* Create a notify object that we can reuse within the loop. */
   if (notify_func)
@@ -1413,19 +1450,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 +1482,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 +1519,11 @@
   /* Per-backend verification. */
   svn_pool_destroy(iterpool);
 
+  if (found_corruption)
+    return svn_error_createf(SVN_ERR_REPOS_CORRUPTED, NULL,
+                             _("Repository '%s' failed to verify"),
+                             svn_dirent_local_style(svn_repos_path(repos,
+                                                                   pool),
+                                                    pool));
   return SVN_NO_ERROR;
 }
Index: subversion/libsvn_repos/notify.c
===================================================================
--- subversion/libsvn_repos/notify.c	(revision 1411074)
+++ 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;
 }

Reply via email to