You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by sv...@apache.org on 2014/12/01 05:00:41 UTC

svn commit: r1642629 - in /subversion/branches/1.7.x: ./ STATUS subversion/libsvn_repos/dump.c subversion/tests/libsvn_repos/repos-test.c

Author: svn-role
Date: Mon Dec  1 04:00:40 2014
New Revision: 1642629

URL: http://svn.apache.org/r1642629
Log:
Merge the 1.7.x-r1574868 branch:

 * r1574868
   Don't let invalid mergeinfo stop 'svnadmin dump' from producing a dump.
     Part of issue #4476 "Mergeinfo containing r0 makes svnsync and svnadmin
     dump fail".
   Justification:
     A formatting error in what is essentially client-side metadata, albeit
     with repository-side interpretation as well, should never prevent
     dumping the repository. This issue only occurs when svnadmin is trying
     to warn us of mergeinfo revisions that might be unexpected, so it is
     not essential.
   Notes:
     There is a work-around, at least in theory: a dump starting from
     revision 0 or 1 does not suffer from this problem.
   Branch:
     ^/subversion/branches/1.7.x-r1574868
   Votes:
     +1: julianfoad, rhuijben, stefan2

Modified:
    subversion/branches/1.7.x/   (props changed)
    subversion/branches/1.7.x/STATUS
    subversion/branches/1.7.x/subversion/libsvn_repos/dump.c
    subversion/branches/1.7.x/subversion/tests/libsvn_repos/repos-test.c

Propchange: subversion/branches/1.7.x/
------------------------------------------------------------------------------
  Merged /subversion/branches/1.7.x-r1574868:r1575332-1642628
  Merged /subversion/trunk:r1574868

Modified: subversion/branches/1.7.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/STATUS?rev=1642629&r1=1642628&r2=1642629&view=diff
==============================================================================
--- subversion/branches/1.7.x/STATUS (original)
+++ subversion/branches/1.7.x/STATUS Mon Dec  1 04:00:40 2014
@@ -120,24 +120,6 @@ Veto-blocked changes:
 Approved changes:
 =================
 
- * r1574868
-   Don't let invalid mergeinfo stop 'svnadmin dump' from producing a dump.
-     Part of issue #4476 "Mergeinfo containing r0 makes svnsync and svnadmin
-     dump fail".
-   Justification:
-     A formatting error in what is essentially client-side metadata, albeit
-     with repository-side interpretation as well, should never prevent
-     dumping the repository. This issue only occurs when svnadmin is trying
-     to warn us of mergeinfo revisions that might be unexpected, so it is
-     not essential.
-   Notes:
-     There is a work-around, at least in theory: a dump starting from
-     revision 0 or 1 does not suffer from this problem.
-   Branch:
-     ^/subversion/branches/1.7.x-r1574868
-   Votes:
-     +1: julianfoad, rhuijben, stefan2
-
  * r1641564
    Fix issue 4185: file external not following history.
    Justification:

Modified: subversion/branches/1.7.x/subversion/libsvn_repos/dump.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_repos/dump.c?rev=1642629&r1=1642628&r2=1642629&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/libsvn_repos/dump.c (original)
+++ subversion/branches/1.7.x/subversion/libsvn_repos/dump.c Mon Dec  1 04:00:40 2014
@@ -212,6 +212,48 @@ make_dir_baton(const char *path,
   return new_db;
 }
 
+/* If the mergeinfo in MERGEINFO_STR refers to any revisions older than
+ * OLDEST_DUMPED_REV, issue a warning and set *FOUND_OLD_MERGEINFO to TRUE,
+ * otherwise leave *FOUND_OLD_MERGEINFO unchanged.
+ */
+static svn_error_t *
+verify_mergeinfo_revisions(svn_boolean_t *found_old_mergeinfo,
+                           const char *mergeinfo_str,
+                           svn_revnum_t oldest_dumped_rev,
+                           svn_repos_notify_func_t notify_func,
+                           void *notify_baton,
+                           apr_pool_t *pool)
+{
+  svn_mergeinfo_t mergeinfo, old_mergeinfo;
+
+  SVN_ERR(svn_mergeinfo_parse(&mergeinfo, mergeinfo_str, pool));
+  SVN_ERR(svn_mergeinfo__filter_mergeinfo_by_ranges(
+            &old_mergeinfo, mergeinfo,
+            oldest_dumped_rev - 1, 0,
+            TRUE, pool, pool));
+
+  if (apr_hash_count(old_mergeinfo))
+    {
+      svn_repos_notify_t *notify =
+        svn_repos_notify_create(svn_repos_notify_warning, pool);
+
+      notify->warning = svn_repos_notify_warning_found_old_mergeinfo;
+      notify->warning_str = apr_psprintf(
+        pool,
+        _("Mergeinfo referencing revision(s) prior "
+          "to the oldest dumped revision (r%ld). "
+          "Loading this dump may result in invalid "
+          "mergeinfo."),
+        oldest_dumped_rev);
+
+      if (found_old_mergeinfo)
+        *found_old_mergeinfo = TRUE;
+      notify_func(notify_baton, notify, pool);
+    }
+
+  return SVN_NO_ERROR;
+}
+
 
 /* This helper is the main "meat" of the editor -- it does all the
    work of writing a node record.
@@ -473,31 +515,13 @@ dump_node(struct edit_baton *eb,
                                                      APR_HASH_KEY_STRING);
           if (mergeinfo_str)
             {
-              svn_mergeinfo_t mergeinfo, old_mergeinfo;
-
-              SVN_ERR(svn_mergeinfo_parse(&mergeinfo, mergeinfo_str->data,
-                                          pool));
-              SVN_ERR(svn_mergeinfo__filter_mergeinfo_by_ranges(
-                &old_mergeinfo, mergeinfo,
-                eb->oldest_dumped_rev - 1, 0,
-                TRUE, pool, pool));
-              if (apr_hash_count(old_mergeinfo))
-                {
-                  svn_repos_notify_t *notify =
-                    svn_repos_notify_create(svn_repos_notify_warning, pool);
-
-                  notify->warning = svn_repos_notify_warning_found_old_mergeinfo;
-                  notify->warning_str = apr_psprintf(
-                    pool,
-                    _("Mergeinfo referencing revision(s) prior "
-                      "to the oldest dumped revision (r%ld). "
-                      "Loading this dump may result in invalid "
-                      "mergeinfo."),
-                    eb->oldest_dumped_rev);
-
-                  eb->found_old_mergeinfo = TRUE;
-                  eb->notify_func(eb->notify_baton, notify, pool);
-                }
+              /* An error in verifying the mergeinfo must not prevent dumping
+                 the data. Ignore any such error. */
+              svn_error_clear(verify_mergeinfo_revisions(
+                                eb->found_old_mergeinfo,
+                                mergeinfo_str->data, eb->oldest_dumped_rev,
+                                eb->notify_func, eb->notify_baton,
+                                pool));
             }
         }
 

Modified: subversion/branches/1.7.x/subversion/tests/libsvn_repos/repos-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/tests/libsvn_repos/repos-test.c?rev=1642629&r1=1642628&r2=1642629&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/tests/libsvn_repos/repos-test.c (original)
+++ subversion/branches/1.7.x/subversion/tests/libsvn_repos/repos-test.c Mon Dec  1 04:00:40 2014
@@ -2527,6 +2527,65 @@ issue_4060(const svn_test_opts_t *opts,
 }
 
 
+/* Notification receiver for test_dump_bad_mergeinfo(). This does not
+   need to do anything, it just needs to exist.
+ */
+static void
+dump_r0_mergeinfo_notifier(void *baton,
+                           const svn_repos_notify_t *notify,
+                           apr_pool_t *scratch_pool)
+{
+}
+
+/* Regression test for part the 'dump' part of issue #4476 "Mergeinfo
+   containing r0 makes svnsync and svnadmin dump fail". */
+static svn_error_t *
+test_dump_r0_mergeinfo(const svn_test_opts_t *opts,
+                       apr_pool_t *pool)
+{
+  svn_repos_t *repos;
+  svn_fs_t *fs;
+  svn_fs_txn_t *txn;
+  svn_fs_root_t *txn_root;
+  svn_revnum_t youngest_rev = 0;
+  const svn_string_t *bad_mergeinfo = svn_string_create("/foo:0", pool);
+
+  SVN_ERR(svn_test__create_repos(&repos, "test-repo-dump-r0-mergeinfo",
+                                 opts, pool));
+  fs = svn_repos_fs(repos);
+
+  /* Revision 1:  Any commit will do, here  */
+  SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, pool));
+  SVN_ERR(svn_fs_txn_root(&txn_root, txn, pool));
+  SVN_ERR(svn_fs_make_dir(txn_root, "/bar", pool));
+  SVN_ERR(svn_repos_fs_commit_txn(NULL, repos, &youngest_rev, txn, pool));
+  SVN_TEST_ASSERT(SVN_IS_VALID_REVNUM(youngest_rev));
+
+  /* Revision 2:  Add bad mergeinfo */
+  SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, pool));
+  SVN_ERR(svn_fs_txn_root(&txn_root, txn, pool));
+  SVN_ERR(svn_fs_change_node_prop(txn_root, "/bar", "svn:mergeinfo", bad_mergeinfo, pool));
+  SVN_ERR(svn_repos_fs_commit_txn(NULL, repos, &youngest_rev, txn, pool));
+  SVN_TEST_ASSERT(SVN_IS_VALID_REVNUM(youngest_rev));
+
+  /* Test that a dump completes without error. In order to exercise the
+     functionality under test -- that is, in order for the dump to try to
+     parse the mergeinfo it is dumping -- the dump must start from a
+     revision greater than 1 and must take a notification callback. */
+  {
+    svn_stringbuf_t *stringbuf = svn_stringbuf_create("", pool);
+    svn_stream_t *stream = svn_stream_from_stringbuf(stringbuf, pool);
+
+    SVN_ERR(svn_repos_dump_fs3(repos, stream, 2, SVN_INVALID_REVNUM,
+                               FALSE, FALSE,
+                               dump_r0_mergeinfo_notifier, NULL,
+                               NULL, NULL,
+                               pool));
+  }
+
+  return SVN_NO_ERROR;
+}
+
 /* The test table.  */
 
 struct svn_test_descriptor_t test_funcs[] =
@@ -2562,5 +2621,7 @@ struct svn_test_descriptor_t test_funcs[
                        "test svn_repos_get_file_revsN"),
     SVN_TEST_OPTS_PASS(issue_4060,
                        "test issue 4060"),
+    SVN_TEST_OPTS_PASS(test_dump_r0_mergeinfo,
+                       "test dumping with r0 mergeinfo"),
     SVN_TEST_NULL
   };