You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2015/01/16 18:26:47 UTC

svn commit: r1652465 - in /subversion/trunk/subversion: include/private/svn_repos_private.h libsvn_repos/load-fs-vtable.c svnrdump/load_editor.c

Author: julianfoad
Date: Fri Jan 16 17:26:46 2015
New Revision: 1652465

URL: http://svn.apache.org/r1652465
Log:
Factor out mergeinfo adjustment from 'svnadmin load' and 'svnrdump load'.

Part of issue #4483 "De-duplicate mergeinfo processing code in svnadmin load,
svnrdump load, svndumpfilter".

No functional change.

* subversion/include/private/svn_repos_private.h,
  (svn_repos__adjust_mergeinfo_property): New.

* subversion/libsvn_repos/load-fs-vtable.c
  (prefix_mergeinfo_paths): Remove note about duplication.
  (renumber_mergeinfo_revs): Take separate arguments instead of batons.
    Remove note about duplication.
  (svn_repos__adjust_mergeinfo_property): Make semi-public, renaming from
    'adjust_mergeinfo_property'. Take separate arguments instead of batons.
  (set_node_property): Adjust the caller accordingly.

* subversion/svnrdump/load_editor.c
  (prefix_mergeinfo_paths,
   renumber_mergeinfo_revs,
   adjust_mergeinfo_property): Delete.
  (set_node_property): Call svn_repos__adjust_mergeinfo_property() instead.
    Wrap any error with a simple message.

Modified:
    subversion/trunk/subversion/include/private/svn_repos_private.h
    subversion/trunk/subversion/libsvn_repos/load-fs-vtable.c
    subversion/trunk/subversion/svnrdump/load_editor.c

Modified: subversion/trunk/subversion/include/private/svn_repos_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_repos_private.h?rev=1652465&r1=1652464&r2=1652465&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_repos_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_repos_private.h Fri Jan 16 17:26:46 2015
@@ -258,6 +258,39 @@ svn_repos__authz_pool_get(svn_authz_t **
                           svn_repos_t *preferred_repos,
                           apr_pool_t *pool);
 
+/* Adjust mergeinfo paths and revisions in ways that are useful when loading
+ * a dump stream.
+ *
+ * Set *NEW_VALUE_P to an adjusted version of the mergeinfo property value
+ * supplied in OLD_VALUE, with the following adjustments.
+ *
+ *   - Normalize line endings: if all CRLF, change to LF; but error if
+ *     mixed. If this normalization is performed, send a notification type
+ *     svn_repos_notify_load_normalized_mergeinfo to NOTIFY_FUNC/NOTIFY_BATON.
+ *
+ *   - Prefix all the merge source paths with PARENT_DIR, if not null.
+ *
+ *   - Adjust any mergeinfo revisions not older than OLDEST_DUMPSTREAM_REV
+ *     by using REV_MAP which maps (svn_revnum_t) old rev to (svn_revnum_t)
+ *     new rev.
+ *
+ *   - Adjust any mergeinfo revisions older than OLDEST_DUMPSTREAM_REV by
+ *     (-OLDER_REVS_OFFSET), dropping any revisions that become <= 0.
+ *
+ * Allocate *NEW_VALUE_P in RESULT_POOL.
+ */
+svn_error_t *
+svn_repos__adjust_mergeinfo_property(svn_string_t **new_value_p,
+                                     const svn_string_t *old_value,
+                                     const char *parent_dir,
+                                     apr_hash_t *rev_map,
+                                     svn_revnum_t oldest_dumpstream_rev,
+                                     apr_int32_t older_revs_offset,
+                                     svn_repos_notify_func_t notify_func,
+                                     void *notify_baton,
+                                     apr_pool_t *result_pool,
+                                     apr_pool_t *scratch_pool);
+
 /** @} */
 
 #ifdef __cplusplus

Modified: subversion/trunk/subversion/libsvn_repos/load-fs-vtable.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/load-fs-vtable.c?rev=1652465&r1=1652464&r2=1652465&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/load-fs-vtable.c (original)
+++ subversion/trunk/subversion/libsvn_repos/load-fs-vtable.c Fri Jan 16 17:26:46 2015
@@ -41,6 +41,7 @@
 #include "private/svn_fspath.h"
 #include "private/svn_dep_compat.h"
 #include "private/svn_mergeinfo_private.h"
+#include "private/svn_repos_private.h"
 
 /*----------------------------------------------------------------------*/
 
@@ -192,8 +193,6 @@ change_node_prop(svn_fs_root_t *txn_root
 
 /* Prepend the mergeinfo source paths in MERGEINFO_ORIG with PARENT_DIR, and
    return it in *MERGEINFO_VAL. */
-/* ### FIXME:  Consider somehow sharing code with
-   ### svnrdump/load_editor.c:prefix_mergeinfo_paths() */
 static svn_error_t *
 prefix_mergeinfo_paths(svn_string_t **mergeinfo_val,
                        const svn_string_t *mergeinfo_orig,
@@ -225,13 +224,20 @@ prefix_mergeinfo_paths(svn_string_t **me
 
 /* Examine the mergeinfo in INITIAL_VAL, renumber revisions in rangelists
    as appropriate, and return the (possibly new) mergeinfo in *FINAL_VAL
-   (allocated from POOL). */
-/* ### FIXME:  Consider somehow sharing code with
-   ### svnrdump/load_editor.c:renumber_mergeinfo_revs() */
+   (allocated from POOL).
+
+   Adjust any mergeinfo revisions not older than OLDEST_DUMPSTREAM_REV by
+   using REV_MAP which maps (svn_revnum_t) old rev to (svn_revnum_t) new rev.
+
+   Adjust any mergeinfo revisions older than OLDEST_DUMPSTREAM_REV by
+   (-OLDER_REVS_OFFSET), dropping any that become <= 0.
+ */
 static svn_error_t *
 renumber_mergeinfo_revs(svn_string_t **final_val,
                         const svn_string_t *initial_val,
-                        struct revision_baton *rb,
+                        apr_hash_t *rev_map,
+                        svn_revnum_t oldest_dumpstream_rev,
+                        apr_int32_t older_revs_offset,
                         apr_pool_t *pool)
 {
   apr_pool_t *subpool = svn_pool_create(pool);
@@ -246,19 +252,22 @@ renumber_mergeinfo_revs(svn_string_t **f
      Remove mergeinfo older than the oldest revision in the dump stream
      and adjust its revisions by the difference between the head rev of
      the target repository and the current dump stream rev. */
-  if (rb->pb->oldest_dumpstream_rev > 1)
+  if (oldest_dumpstream_rev > 1)
     {
+      /* predates_stream_mergeinfo := mergeinfo that refers to revs before
+         oldest_dumpstream_rev */
       SVN_ERR(svn_mergeinfo__filter_mergeinfo_by_ranges(
         &predates_stream_mergeinfo, mergeinfo,
-        rb->pb->oldest_dumpstream_rev - 1, 0,
+        oldest_dumpstream_rev - 1, 0,
         TRUE, subpool, subpool));
+      /* mergeinfo := mergeinfo that refers to revs >= oldest_dumpstream_rev */
       SVN_ERR(svn_mergeinfo__filter_mergeinfo_by_ranges(
         &mergeinfo, mergeinfo,
-        rb->pb->oldest_dumpstream_rev - 1, 0,
+        oldest_dumpstream_rev - 1, 0,
         FALSE, subpool, subpool));
       SVN_ERR(svn_mergeinfo__adjust_mergeinfo_rangelists(
         &predates_stream_mergeinfo, predates_stream_mergeinfo,
-        -rb->rev_offset, subpool, subpool));
+        -older_revs_offset, subpool, subpool));
     }
   else
     {
@@ -269,7 +278,6 @@ renumber_mergeinfo_revs(svn_string_t **f
     {
       const char *merge_source = apr_hash_this_key(hi);
       svn_rangelist_t *rangelist = apr_hash_this_val(hi);
-      struct parse_baton *pb = rb->pb;
       int i;
 
       /* Possibly renumber revisions in merge source's rangelist. */
@@ -278,27 +286,27 @@ renumber_mergeinfo_revs(svn_string_t **f
           svn_revnum_t rev_from_map;
           svn_merge_range_t *range = APR_ARRAY_IDX(rangelist, i,
                                                    svn_merge_range_t *);
-          rev_from_map = get_revision_mapping(pb->rev_map, range->start);
+          rev_from_map = get_revision_mapping(rev_map, range->start);
           if (SVN_IS_VALID_REVNUM(rev_from_map))
             {
               range->start = rev_from_map;
             }
-          else if (range->start == pb->oldest_dumpstream_rev - 1)
+          else if (range->start == oldest_dumpstream_rev - 1)
             {
               /* Since the start revision of svn_merge_range_t are not
                  inclusive there is one possible valid start revision that
-                 won't be found in the PB->REV_MAP mapping of load stream
+                 won't be found in the REV_MAP mapping of load stream
                  revsions to loaded revisions: The revision immediately
                  preceding the oldest revision from the load stream.
                  This is a valid revision for mergeinfo, but not a valid
-                 copy from revision (which PB->REV_MAP also maps for) so it
+                 copy from revision (which REV_MAP also maps for) so it
                  will never be in the mapping.
 
                  If that is what we have here, then find the mapping for the
                  oldest rev from the load stream and subtract 1 to get the
                  renumbered, non-inclusive, start revision. */
-              rev_from_map = get_revision_mapping(pb->rev_map,
-                                                  pb->oldest_dumpstream_rev);
+              rev_from_map = get_revision_mapping(rev_map,
+                                                  oldest_dumpstream_rev);
               if (SVN_IS_VALID_REVNUM(rev_from_map))
                 range->start = rev_from_map - 1;
             }
@@ -315,7 +323,7 @@ renumber_mergeinfo_revs(svn_string_t **f
               continue;
             }
 
-          rev_from_map = get_revision_mapping(pb->rev_map, range->end);
+          rev_from_map = get_revision_mapping(rev_map, range->end);
           if (SVN_IS_VALID_REVNUM(rev_from_map))
             range->end = rev_from_map;
         }
@@ -758,18 +766,18 @@ set_revision_property(void *baton,
 }
 
 
-/* Adjust mergeinfo:
- *   - normalize line endings (if all CRLF, change to LF; but error if mixed);
- *   - adjust revision numbers (see renumber_mergeinfo_revs());
- *   - adjust paths (see prefix_mergeinfo_paths()).
- */
-static svn_error_t *
-adjust_mergeinfo_property(struct revision_baton *rb,
-                          svn_string_t **new_value_p,
-                          const svn_string_t *old_value,
-                          apr_pool_t *result_pool)
+svn_error_t *
+svn_repos__adjust_mergeinfo_property(svn_string_t **new_value_p,
+                                     const svn_string_t *old_value,
+                                     const char *parent_dir,
+                                     apr_hash_t *rev_map,
+                                     svn_revnum_t oldest_dumpstream_rev,
+                                     apr_int32_t older_revs_offset,
+                                     svn_repos_notify_func_t notify_func,
+                                     void *notify_baton,
+                                     apr_pool_t *result_pool,
+                                     apr_pool_t *scratch_pool)
 {
-  struct parse_baton *pb = rb->pb;
   svn_string_t prop_val = *old_value;
 
   /* Tolerate mergeinfo with "\r\n" line endings because some
@@ -790,29 +798,29 @@ adjust_mergeinfo_property(struct revisio
       prop_val.data = prop_eol_normalized;
       prop_val.len = strlen(prop_eol_normalized);
 
-      if (pb->notify_func)
+      if (notify_func)
         {
-          /* ### TODO: Use proper scratch pool instead of pb->notify_pool */
           svn_repos_notify_t *notify
                   = svn_repos_notify_create(
                                 svn_repos_notify_load_normalized_mergeinfo,
-                                pb->notify_pool);
+                                scratch_pool);
 
-          pb->notify_func(pb->notify_baton, notify, pb->notify_pool);
-          svn_pool_clear(pb->notify_pool);
+          notify_func(notify_baton, notify, scratch_pool);
         }
     }
 
   /* Renumber mergeinfo as appropriate. */
-  SVN_ERR(renumber_mergeinfo_revs(new_value_p, &prop_val, rb,
+  SVN_ERR(renumber_mergeinfo_revs(new_value_p, &prop_val,
+                                  rev_map, oldest_dumpstream_rev,
+                                  older_revs_offset,
                                   result_pool));
 
-  if (pb->parent_dir)
+  if (parent_dir)
     {
-      /* Prefix the merge source paths with PB->parent_dir. */
+      /* Prefix the merge source paths with PARENT_DIR. */
       /* ASSUMPTION: All source paths are included in the dump stream. */
       SVN_ERR(prefix_mergeinfo_paths(new_value_p, *new_value_p,
-                                     pb->parent_dir, result_pool));
+                                     parent_dir, result_pool));
     }
 
   return SVN_NO_ERROR;
@@ -842,7 +850,14 @@ set_node_property(void *baton,
       svn_string_t *new_value;
       svn_error_t *err;
 
-      err = adjust_mergeinfo_property(rb, &new_value, value, nb->pool);
+      err = svn_repos__adjust_mergeinfo_property(&new_value, value,
+                                                 pb->parent_dir,
+                                                 pb->rev_map,
+                                                 pb->oldest_dumpstream_rev,
+                                                 rb->rev_offset,
+                                                 pb->notify_func, pb->notify_baton,
+                                                 nb->pool, pb->notify_pool);
+      svn_pool_clear(pb->notify_pool);
       if (err)
         {
           if (pb->validate_props)

Modified: subversion/trunk/subversion/svnrdump/load_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/load_editor.c?rev=1652465&r1=1652464&r2=1652465&view=diff
==============================================================================
--- subversion/trunk/subversion/svnrdump/load_editor.c (original)
+++ subversion/trunk/subversion/svnrdump/load_editor.c Fri Jan 16 17:26:46 2015
@@ -171,160 +171,6 @@ get_revision_mapping(apr_hash_t *rev_map
 }
 
 
-/* Prepend the mergeinfo source paths in MERGEINFO_ORIG with
-   PARENT_DIR, and return it in *MERGEINFO_VAL. */
-/* ### FIXME:  Consider somehow sharing code with
-   ### libsvn_repos/load-fs-vtable.c:prefix_mergeinfo_paths() */
-static svn_error_t *
-prefix_mergeinfo_paths(svn_string_t **mergeinfo_val,
-                       const svn_string_t *mergeinfo_orig,
-                       const char *parent_dir,
-                       apr_pool_t *pool)
-{
-  apr_hash_t *prefixed_mergeinfo, *mergeinfo;
-  apr_hash_index_t *hi;
-  void *rangelist;
-
-  SVN_ERR(svn_mergeinfo_parse(&mergeinfo, mergeinfo_orig->data, pool));
-  prefixed_mergeinfo = apr_hash_make(pool);
-  for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
-    {
-      const void *key;
-      const char *path, *merge_source;
-
-      apr_hash_this(hi, &key, NULL, &rangelist);
-      merge_source = svn_relpath_canonicalize(key, pool);
-
-      /* The svn:mergeinfo property syntax demands a repos abspath */
-      path = svn_fspath__canonicalize(svn_relpath_join(parent_dir,
-                                                       merge_source, pool),
-                                      pool);
-      svn_hash_sets(prefixed_mergeinfo, path, rangelist);
-    }
-  return svn_mergeinfo_to_string(mergeinfo_val, prefixed_mergeinfo, pool);
-}
-
-
-/* Examine the mergeinfo in INITIAL_VAL, renumber revisions in rangelists
-   as appropriate, and return the (possibly new) mergeinfo in *FINAL_VAL
-   (allocated from POOL). */
-/* ### FIXME:  Consider somehow sharing code with
-   ### libsvn_repos/load-fs-vtable.c:renumber_mergeinfo_revs() */
-static svn_error_t *
-renumber_mergeinfo_revs(svn_string_t **final_val,
-                        const svn_string_t *initial_val,
-                        struct revision_baton *rb,
-                        apr_pool_t *pool)
-{
-  apr_pool_t *subpool = svn_pool_create(pool);
-  svn_mergeinfo_t mergeinfo, predates_stream_mergeinfo;
-  svn_mergeinfo_t final_mergeinfo = apr_hash_make(subpool);
-  apr_hash_index_t *hi;
-
-  SVN_ERR(svn_mergeinfo_parse(&mergeinfo, initial_val->data, subpool));
-
-  /* Issue #3020
-     http://subversion.tigris.org/issues/show_bug.cgi?id=3020#desc16
-     Remove mergeinfo older than the oldest revision in the dump stream
-     and adjust its revisions by the difference between the head rev of
-     the target repository and the current dump stream rev. */
-  if (rb->pb->oldest_dumpstream_rev > 1)
-    {
-      SVN_ERR(svn_mergeinfo__filter_mergeinfo_by_ranges(
-                  &predates_stream_mergeinfo, mergeinfo,
-                  rb->pb->oldest_dumpstream_rev - 1, 0,
-                  TRUE, subpool, subpool));
-      SVN_ERR(svn_mergeinfo__filter_mergeinfo_by_ranges(
-                  &mergeinfo, mergeinfo,
-                  rb->pb->oldest_dumpstream_rev - 1, 0,
-                  FALSE, subpool, subpool));
-      SVN_ERR(svn_mergeinfo__adjust_mergeinfo_rangelists(
-                  &predates_stream_mergeinfo,
-                  predates_stream_mergeinfo,
-                  -rb->rev_offset, subpool, subpool));
-    }
-  else
-    {
-      predates_stream_mergeinfo = NULL;
-    }
-
-  for (hi = apr_hash_first(subpool, mergeinfo); hi; hi = apr_hash_next(hi))
-    {
-      svn_rangelist_t *rangelist;
-      struct parse_baton *pb = rb->pb;
-      int i;
-      const void *path;
-      apr_ssize_t pathlen;
-      void *val;
-
-      apr_hash_this(hi, &path, &pathlen, &val);
-      rangelist = val;
-
-      /* Possibly renumber revisions in merge source's rangelist. */
-      for (i = 0; i < rangelist->nelts; i++)
-        {
-          svn_revnum_t rev_from_map;
-          svn_merge_range_t *range = APR_ARRAY_IDX(rangelist, i,
-                                                   svn_merge_range_t *);
-          rev_from_map = get_revision_mapping(pb->rev_map, range->start);
-          if (SVN_IS_VALID_REVNUM(rev_from_map))
-            {
-              range->start = rev_from_map;
-            }
-          else if (range->start == pb->oldest_dumpstream_rev - 1)
-            {
-              /* Since the start revision of svn_merge_range_t are not
-                 inclusive there is one possible valid start revision that
-                 won't be found in the PB->REV_MAP mapping of load stream
-                 revsions to loaded revisions: The revision immediately
-                 preceding the oldest revision from the load stream.
-                 This is a valid revision for mergeinfo, but not a valid
-                 copy from revision (which PB->REV_MAP also maps for) so it
-                 will never be in the mapping.
-
-                 If that is what we have here, then find the mapping for the
-                 oldest rev from the load stream and subtract 1 to get the
-                 renumbered, non-inclusive, start revision. */
-              rev_from_map = get_revision_mapping(pb->rev_map,
-                                                  pb->oldest_dumpstream_rev);
-              if (SVN_IS_VALID_REVNUM(rev_from_map))
-                range->start = rev_from_map - 1;
-            }
-          else
-            {
-              /* If we can't remap the start revision then don't even bother
-                 trying to remap the end revision.  It's possible we might
-                 actually succeed at the latter, which can result in invalid
-                 mergeinfo with a start rev > end rev.  If that gets into the
-                 repository then a world of bustage breaks loose anytime that
-                 bogus mergeinfo is parsed.  See
-                 http://subversion.tigris.org/issues/show_bug.cgi?id=3020#desc16.
-                 */
-              continue;
-            }
-
-          rev_from_map = get_revision_mapping(pb->rev_map, range->end);
-          if (SVN_IS_VALID_REVNUM(rev_from_map))
-            range->end = rev_from_map;
-        }
-      apr_hash_set(final_mergeinfo, path, pathlen, rangelist);
-    }
-
-  if (predates_stream_mergeinfo)
-    {
-      SVN_ERR(svn_mergeinfo_merge2(final_mergeinfo, predates_stream_mergeinfo,
-                                   subpool, subpool));
-    }
-
-  SVN_ERR(svn_mergeinfo__canonicalize_ranges(final_mergeinfo, subpool));
-
-  SVN_ERR(svn_mergeinfo_to_string(final_val, final_mergeinfo, pool));
-  svn_pool_destroy(subpool);
-
-  return SVN_NO_ERROR;
-}
-
-
 static svn_error_t *
 commit_callback(const svn_commit_info_t *commit_info,
                 void *baton,
@@ -867,70 +713,35 @@ set_revision_property(void *baton,
   return SVN_NO_ERROR;
 }
 
-/* Adjust mergeinfo:
- *   - normalize line endings (if all CRLF, change to LF; but error if mixed);
- *   - adjust revision numbers (see renumber_mergeinfo_revs());
- *   - adjust paths (see prefix_mergeinfo_paths()).
- */
-static svn_error_t *
-adjust_mergeinfo_property(struct revision_baton *rb,
-                          svn_string_t **new_value_p,
-                          const svn_string_t *old_value,
-                          apr_pool_t *result_pool)
-{
-  struct parse_baton *pb = rb->pb;
-  svn_string_t prop_val = *old_value;
-
-  /* Tolerate mergeinfo with "\r\n" line endings because some
-     dumpstream sources might contain as much.  If so normalize
-     the line endings to '\n' and notify that we have made this
-     correction. */
-  if (strstr(prop_val.data, "\r"))
-    {
-      const char *prop_eol_normalized;
-
-      SVN_ERR(svn_subst_translate_cstring2(prop_val.data,
-                                           &prop_eol_normalized,
-                                           "\n",  /* translate to LF */
-                                           FALSE, /* no repair */
-                                           NULL,  /* no keywords */
-                                           FALSE, /* no expansion */
-                                           result_pool));
-      prop_val.data = prop_eol_normalized;
-      prop_val.len = strlen(prop_eol_normalized);
-
-      /* ### TODO: notify? */
-    }
-
-  /* Renumber mergeinfo as appropriate. */
-  SVN_ERR(renumber_mergeinfo_revs(new_value_p, &prop_val, rb,
-                                  result_pool));
-
-  if (pb->parent_dir)
-    {
-      /* Prefix the merge source paths with PB->parent_dir. */
-      /* ASSUMPTION: All source paths are included in the dump stream. */
-      SVN_ERR(prefix_mergeinfo_paths(new_value_p, *new_value_p,
-                                     pb->parent_dir, result_pool));
-    }
-
-  return SVN_NO_ERROR;
-}
-
 static svn_error_t *
 set_node_property(void *baton,
                   const char *name,
                   const svn_string_t *value)
 {
   struct node_baton *nb = baton;
+  struct revision_baton *rb = nb->rb;
+  struct parse_baton *pb = rb->pb;
   const struct svn_delta_editor_t *commit_editor = nb->rb->pb->commit_editor;
   apr_pool_t *pool = nb->rb->pool;
 
   if (value && strcmp(name, SVN_PROP_MERGEINFO) == 0)
     {
       svn_string_t *new_value;
+      svn_error_t *err;
+
+      err = svn_repos__adjust_mergeinfo_property(&new_value, value,
+                                                 pb->parent_dir,
+                                                 pb->rev_map,
+                                                 pb->oldest_dumpstream_rev,
+                                                 rb->rev_offset,
+                                                 NULL, NULL, /*notify*/
+                                                 pool, pool);
+      if (err)
+        {
+          return svn_error_quick_wrap(err,
+                                      _("Invalid svn:mergeinfo value"));
+        }
 
-      SVN_ERR(adjust_mergeinfo_property(nb->rb, &new_value, value, pool));
       value = new_value;
     }