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 2014/09/26 16:36:46 UTC

svn commit: r1627797 - /subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c

Author: julianfoad
Date: Fri Sep 26 14:36:46 2014
New Revision: 1627797

URL: http://svn.apache.org/r1627797
Log:
On the 'move-tracking-2' branch: In the 'svnmover' program make all
operations excepty 'branchify' properly recurse into sub-branches.

* subversion/svnmover/svnmover.c
  Lots of changes.

Modified:
    subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c

Modified: subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c
URL: http://svn.apache.org/viewvc/subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c?rev=1627797&r1=1627796&r2=1627797&view=diff
==============================================================================
--- subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c (original)
+++ subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c Fri Sep 26 14:36:46 2014
@@ -60,6 +60,7 @@
 #include "private/svn_editor3.h"
 #include "private/svn_ra_private.h"
 #include "private/svn_string_private.h"
+#include "private/svn_sorts_private.h"
 
 /* Version compatibility check */
 static svn_error_t *
@@ -297,6 +298,17 @@ struct action {
 
 /* ====================================================================== */
 
+/* WITH_OVERLAPPING_EIDS means an outer branch maintains EIDs for all the
+ * elements of the nested branches in it, rather than just for the root
+ * element of each immediate child branch.
+ *
+ * The idea is that this may facilitate some sort of tracking of moves
+ * into and out of subbranches, but the idea is not fully developed.
+ *
+ * This is only partially implemented, and may not be a useful idea.
+ */
+/* #define WITH_OVERLAPPING_EIDS */
+
 /* ### */
 #define SVN_ERR_BRANCHING 123456
 
@@ -361,6 +373,32 @@ typedef struct svn_branch_family_t
  * A branch definition object describes the characteristics common to all
  * instances of a branch (with a given BID) in its family. (There is one
  * instance of this branch within each branch of its outer families.)
+ *
+ * Often, all branches in a family have the same root element. For example,
+ * branching /trunk to /branches/br1 results in:
+ *
+ *      family 1, branch 1, root-EID 100
+ *          EID 100 => /trunk
+ *          ...
+ *      family 1, branch 2, root-EID 100
+ *          EID 100 => /branches/br1
+ *          ...
+ *
+ * However, the root element of one branch may correspond to a non-root
+ * element of another branch; such a branch could be called a "subtree
+ * branch". Using the same example, branching from the trunk subtree
+ * /trunk/D (which is not itself a branch root) results in:
+ *
+ *      family 1, branch 3: root-EID = 104
+ *          EID 100 => (nil)
+ *          ...
+ *          EID 104 => /branches/branch-of-trunk-subtree-D
+ *          ...
+ *
+ * If family 1 were nested inside an outer family, then there could be
+ * multiple branch-instances for each branch-definition. In the above
+ * example, all instances of (family 1, branch 1) will have root-EID 100,
+ * and all instances of (family 1, branch 3) will have root-EID 104.
  */
 typedef struct svn_branch_definition_t
 {
@@ -377,10 +415,7 @@ typedef struct svn_branch_definition_t
 
   /* --- Contents of this object --- */
 
-  /* The EID within its family of its pathwise root element. Typically,
-     all branches in a family have the same root element. However, the
-     root element of one branch may correspond to a non-root element of
-     another branch; such a branch could be called a "subtree branch". */
+  /* The EID within its family of its pathwise root element. */
   int root_eid;
 } svn_branch_definition_t;
 
@@ -490,44 +525,118 @@ svn_branch_instance_create(svn_branch_de
 static const char *
 branch_get_root_path(const svn_branch_instance_t *branch);
 
-/*  */
+/* In BRANCH, set or change the path mapping for EID to point to PATH.
+ *
+ * Do not check for or remove any previous mappings; just overwrite
+ * the mapping for EID to this new PATH and for PATH to this new EID.
+ * EID MUST be a valid EID belonging to BRANCH's family.
+ *
+ * Duplicate EID and PATH into the mapping's pool.
+ */
 static void
-branch_set_eid_to_path(svn_branch_instance_t *branch,
-                       int eid,
-                       const char *path)
+branch_mapping_set(svn_branch_instance_t *branch,
+                   int eid,
+                   const char *path)
 {
   apr_pool_t *pool = apr_hash_pool_get(branch->eid_to_path);
   int *eid_stored = apr_pmemdup(pool, &eid, sizeof(eid));
 
   SVN_ERR_ASSERT_NO_RETURN(eid >= branch->definition->family->first_eid
                            && eid < branch->definition->family->next_eid);
-  SVN_ERR_ASSERT_NO_RETURN(eid == branch->definition->root_eid
-                           || svn_relpath_skip_ancestor(
-                                branch_get_root_path(branch), path));
 
+  /* overwrite the EID-to-new-PATH and part of the mapping */
   path = apr_pstrdup(pool, path);
   apr_hash_set(branch->eid_to_path, eid_stored, sizeof(eid), path);
   svn_hash_sets(branch->path_to_eid, path, eid_stored);
 }
 
-/*  */
+/* In BRANCH, remove the path mapping for EID if there is one.
+ *
+ * Do not check for any previous mappings, or for validity of EID or any
+ * mappings.
+ */
 static void
-branch_mapping_remove(svn_branch_instance_t *branch,
+branch_mapping_unset_by_eid(svn_branch_instance_t *branch,
+                            int eid)
+{
+  const char *path = apr_hash_get(branch->eid_to_path, &eid, sizeof(eid));
+
+  if (path)
+    {
+      apr_hash_set(branch->eid_to_path, &eid, sizeof(eid), NULL);
+      svn_hash_sets(branch->path_to_eid, path, NULL);
+    }
+}
+
+/* In BRANCH, set or change the path mapping for EID to point to PATH.
+ *
+ * If a mapping from EID to a different path already exists, update it
+ * to map to PATH. PATH MUST NOT already be mapped to a different EID.
+ * PATH MUST be pathwise within the root path of BRANCH.
+ */
+static void
+branch_mapping_update(svn_branch_instance_t *branch,
                       int eid,
                       const char *path)
 {
-  SVN_ERR_ASSERT_NO_RETURN(eid >= branch->definition->family->first_eid
-                           && eid < branch->definition->family->next_eid);
+  int *old_eid_p = svn_hash_gets(branch->path_to_eid, path);
+
   SVN_ERR_ASSERT_NO_RETURN(eid == branch->definition->root_eid
                            || svn_relpath_skip_ancestor(
                                 branch_get_root_path(branch), path));
+  /* we don't allow mapping a new EID to an existing path */
+  SVN_ERR_ASSERT_NO_RETURN(! old_eid_p || *old_eid_p == eid);
+
+  /* Remove any mapping to an old path, as the path-to-EID part of such a
+     mapping would not be overwritten simply by setting the new mapping. */
+  branch_mapping_unset_by_eid(branch, eid);
+
+  branch_mapping_set(branch, eid, path);
+}
+
+/* In BRANCH, remove the path mapping for EID, so that the element EID
+ * is considered as no longer existing in BRANCH.
+ *
+ * A mapping for EID MUST already exist in BRANCH.
+ */
+static void
+branch_mapping_remove_by_eid(svn_branch_instance_t *branch,
+                             int eid)
+{
+  const char *path = apr_hash_get(branch->eid_to_path, &eid, sizeof(eid));
+
+  SVN_ERR_ASSERT_NO_RETURN(eid >= branch->definition->family->first_eid
+                           && eid < branch->definition->family->next_eid);
+  SVN_ERR_ASSERT_NO_RETURN(path);
 
   apr_hash_set(branch->eid_to_path, &eid, sizeof(eid), NULL);
   svn_hash_sets(branch->path_to_eid, path, NULL);
 }
 
+/* In BRANCH, remove the path mapping for PATH, so that the element
+ * at PATH is considered as no longer existing in BRANCH.
+ *
+ * A mapping for PATH MUST already exist in BRANCH.
+ */
+static void
+branch_mapping_remove_by_path(svn_branch_instance_t *branch,
+                              const char *path)
+{
+  int *eid_p = svn_hash_gets(branch->path_to_eid, path);
+
+  SVN_ERR_ASSERT_NO_RETURN(svn_relpath_skip_ancestor(
+                             branch_get_root_path(branch), path));
+  SVN_ERR_ASSERT_NO_RETURN(eid_p);
+
+  apr_hash_set(branch->eid_to_path, eid_p, sizeof(*eid_p), NULL);
+  svn_hash_sets(branch->path_to_eid, path, NULL);
+}
+
 /* Return the EID in BRANCH of the element at repos-relpath PATH,
- * or -1 if PATH is not currently present in BRANCH. */
+ * or -1 if PATH is not currently present in BRANCH.
+ *
+ * PATH MUST be pathwise within BRANCH.
+ */
 static int
 branch_get_eid_by_path(const svn_branch_instance_t *branch,
                        const char *path)
@@ -543,7 +652,11 @@ branch_get_eid_by_path(const svn_branch_
 }
 
 /* Return the repos-relpath for element EID of BRANCH, or NULL if EID
- * is not currently present in BRANCH. */
+ * is not currently present in BRANCH.
+ *
+ * EID MUST be a valid EID belonging to BRANCH's family, but the element
+ * need not be present in any branch.
+ */
 static const char *
 branch_get_path_by_eid(const svn_branch_instance_t *branch,
                        int eid)
@@ -563,6 +676,167 @@ branch_get_root_path(const svn_branch_in
   return branch_get_path_by_eid(branch, branch->definition->root_eid);
 }
 
+/*  */
+static svn_boolean_t
+branch_is_root_path(const svn_branch_instance_t *branch,
+                    const char *rrpath)
+{
+  return branch_get_eid_by_path(branch, rrpath) == branch->definition->root_eid;
+}
+
+/* In BRANCH, update the path mappings to reflect a delete of a subtree
+ * at FROM_PATH.
+ *
+ * FROM_PATH MUST be an existing path in BRANCH, and may be the root path
+ * of BRANCH.
+ *
+ * If INCLUDE_SELF is true, include the element at FROM_PATH, otherwise
+ * only act on children (recursively) of FROM_PATH.
+ */
+static svn_error_t *
+branch_mappings_delete(svn_branch_instance_t *branch,
+                       const char *from_path,
+                       svn_boolean_t *include_self,
+                       apr_pool_t *scratch_pool)
+{
+  apr_hash_index_t *hi;
+
+  for (hi = apr_hash_first(scratch_pool, branch->eid_to_path);
+       hi; hi = apr_hash_next(hi))
+    {
+      const int *eid_p = apr_hash_this_key(hi);
+      const char *this_path = apr_hash_this_val(hi);
+      const char *remainder = svn_relpath_skip_ancestor(from_path, this_path);
+
+      if (remainder && (include_self || remainder[0]))
+        {
+          branch_mapping_unset_by_eid(branch, *eid_p);
+        }
+    }
+
+  return SVN_NO_ERROR;
+}
+
+/* In BRANCH, update the path mappings to reflect a move of a subtree
+ * from FROM_PATH to TO_PATH.
+ *
+ * FROM_PATH MUST be an existing path in BRANCH, and may be the root path
+ * of BRANCH.
+ */
+static svn_error_t *
+branch_mappings_move(svn_branch_instance_t *branch,
+                     const char *from_path,
+                     const char *to_path,
+                     apr_pool_t *scratch_pool)
+{
+  apr_hash_index_t *hi;
+
+  for (hi = apr_hash_first(scratch_pool, branch->eid_to_path);
+       hi; hi = apr_hash_next(hi))
+    {
+      const int *eid_p = apr_hash_this_key(hi);
+      const char *this_from_path = apr_hash_this_val(hi);
+      const char *remainder = svn_relpath_skip_ancestor(from_path, this_from_path);
+
+      if (remainder)
+        {
+          const char *this_to_path = svn_relpath_join(to_path, remainder,
+                                                      scratch_pool);
+
+          branch_mapping_unset_by_eid(branch, *eid_p);
+          branch_mapping_set(branch, *eid_p, this_to_path);
+        }
+    }
+
+  return SVN_NO_ERROR;
+}
+
+static int
+family_add_new_element(svn_branch_family_t *family);
+
+/* In TO_BRANCH, assign new EIDs and path mappings to reflect the copying
+ * of a subtree from FROM_BRANCH:FROM_PATH to TO_BRANCH:TO_PATH. Assign
+ * a new EID in TO_BRANCH's family for each copied element.
+ *
+ * FROM_BRANCH and TO_BRANCH may be the same or different branch instances
+ * in the same or different branch families.
+ *
+ * FROM_PATH MUST be an existing path in FROM_BRANCH, and may be the
+ * root path of FROM_BRANCH.
+ *
+ * If INCLUDE_SELF is true, include the element at FROM_PATH, otherwise
+ * only act on children (recursively) of FROM_PATH.
+ */
+static svn_error_t *
+branch_mappings_copy(svn_branch_instance_t *from_branch,
+                     const char *from_path,
+                     svn_branch_instance_t *to_branch,
+                     const char *to_path,
+                     svn_boolean_t *include_self,
+                     apr_pool_t *scratch_pool)
+{
+  apr_hash_index_t *hi;
+
+  for (hi = apr_hash_first(scratch_pool, from_branch->eid_to_path);
+       hi; hi = apr_hash_next(hi))
+    {
+      const char *this_from_path = apr_hash_this_val(hi);
+      const char *remainder = svn_relpath_skip_ancestor(from_path, this_from_path);
+
+      if (remainder && (include_self || remainder[0]))
+        {
+          const char *this_to_path = svn_relpath_join(to_path, remainder,
+                                                      scratch_pool);
+          int new_eid = family_add_new_element(to_branch->definition->family);
+
+          branch_mapping_set(to_branch, new_eid, this_to_path);
+        }
+    }
+
+  return SVN_NO_ERROR;
+}
+
+/* In TO_BRANCH, update the path mappings to reflect a branching of a
+ * subtree from FROM_BRANCH:FROM_PATH to TO_BRANCH:TO_PATH.
+ *
+ * FROM_BRANCH and TO_BRANCH must be different branch instances in the
+ * same branch family.
+ *
+ * FROM_PATH MUST be an existing path in FROM_BRANCH, and may be the
+ * root path of FROM_BRANCH.
+ *
+ * TO_PATH MUST be a path in TO_BRANCH
+ */
+static svn_error_t *
+branch_mappings_branch(svn_branch_instance_t *from_branch,
+                       const char *from_path,
+                       svn_branch_instance_t *to_branch,
+                       const char *to_path,
+                       apr_pool_t *scratch_pool)
+{
+  apr_hash_index_t *hi;
+
+  SVN_ERR_ASSERT(from_branch->definition->family == to_branch->definition->family);
+
+  for (hi = apr_hash_first(scratch_pool, from_branch->eid_to_path);
+       hi; hi = apr_hash_next(hi))
+    {
+      const int *eid_p = apr_hash_this_key(hi);
+      const char *this_from_path = apr_hash_this_val(hi);
+      const char *remainder = svn_relpath_skip_ancestor(from_path, this_from_path);
+
+      if (remainder)
+        {
+          const char *this_to_path = svn_relpath_join(to_path, remainder,
+                                                      scratch_pool);
+
+          branch_mapping_set(to_branch, *eid_p, this_to_path);
+        }
+    }
+
+  return SVN_NO_ERROR;
+}
+
 /* Return an array of pointers to the branch instances that are sub-branches
  * of BRANCH. */
 static apr_array_header_t *
@@ -595,6 +869,75 @@ branch_get_sub_branches(const svn_branch
   return sub_branches;
 }
 
+/* Delete the branch instance object BRANCH and any nested branch instances.
+ */
+static svn_error_t *
+branch_instance_delete_r(svn_branch_instance_t *branch,
+                         apr_pool_t *scratch_pool)
+{
+  apr_array_header_t *subbranches
+    = branch_get_sub_branches(branch, scratch_pool, scratch_pool);
+  int i;
+
+  /* Delete nested branch instances, recursively */
+  for (i = 0; i < subbranches->nelts; i++)
+    {
+      svn_branch_instance_t *b = APR_ARRAY_IDX(subbranches, i, void *);
+
+      branch_instance_delete_r(b, scratch_pool);
+    }
+
+  /* Remove the record of this branch instance */
+  for (i = 0; i < branch->definition->family->branch_instances->nelts; i++)
+    {
+      svn_branch_instance_t *b = APR_ARRAY_IDX(subbranches, i, void *);
+
+      if (b == branch)
+        {
+          svn_sort__array_delete(branch->definition->family->branch_instances,
+                                 i, 1);
+        }
+    }
+
+  return SVN_NO_ERROR;
+}
+
+/* Copy the branch instance object BRANCH and any nested branch instances.
+ *
+ * ### Currently, the mapping will be empty.
+ *     TODO: Duplicate the mapping, based on a new path.
+ */
+static svn_error_t *
+branch_instance_copy_r(svn_branch_instance_t **new_branch_p,
+                       svn_branch_instance_t *branch,
+                       apr_pool_t *result_pool,
+                       apr_pool_t *scratch_pool)
+{
+  apr_array_header_t *subbranches
+    = branch_get_sub_branches(branch, scratch_pool, scratch_pool);
+  int i;
+  svn_branch_instance_t *new_branch;
+
+  /* Duplicate this branch instance */
+  new_branch = svn_branch_instance_create(branch->definition, result_pool);
+  /* ### branch_mapping_dup(new_branch, branch) */
+
+  /* Record this new branch instance in its family */
+  APR_ARRAY_PUSH(branch->definition->family->branch_instances, void *)
+    = new_branch;
+
+  /* Copy nested branch instances, recursively */
+  for (i = 0; i < subbranches->nelts; i++)
+    {
+      svn_branch_instance_t *b = APR_ARRAY_IDX(subbranches, i, void *);
+
+      branch_instance_copy_r(&b, b, result_pool, scratch_pool);
+    }
+
+  *new_branch_p = new_branch;
+  return SVN_NO_ERROR;
+}
+
 /* Create a new, empty family in OUTER_FAMILY.
  */
 static svn_branch_family_t *
@@ -639,7 +982,7 @@ family_add_new_branch(svn_branch_family_
   APR_ARRAY_PUSH(family->branch_instances, void *) = branch_instance;
 
   /* Initialize the root element */
-  branch_set_eid_to_path(branch_instance, root_eid, root_rrpath);
+  branch_mapping_update(branch_instance, root_eid, root_rrpath);
   return branch_instance;
 }
 
@@ -755,7 +1098,7 @@ parse_branch_info(svn_branch_instance_t 
       this_path = line->data + this_path_start_pos;
       if (strcmp(this_path, "(null)") != 0)
         {
-          branch_set_eid_to_path(branch_instance, this_eid, this_path);
+          branch_mapping_update(branch_instance, this_eid, this_path);
         }
     }
 
@@ -1168,6 +1511,28 @@ same_branch(const svn_branch_instance_t 
  * throw an error (branch nesting violation).
  */
 static svn_error_t *
+verify_path_in_branch(const svn_branch_instance_t *branch,
+                      const char *rrpath,
+                      apr_pool_t *scratch_pool)
+{
+  svn_branch_instance_t *target_branch
+    = repos_get_branch_by_path(NULL, branch->definition->family->repos, rrpath,
+                               scratch_pool);
+
+  if (! same_branch(target_branch, branch))
+    return svn_error_createf(SVN_ERR_BRANCHING, NULL,
+                             _("path '%s' is in branch '%s', "
+                               "not in this branch '%s'"),
+                             rrpath,
+                             branch_get_root_path(target_branch),
+                             branch_get_root_path(branch));
+  return SVN_NO_ERROR;
+}
+
+/* If the location LOC is not in the branch BRANCH,
+ * throw an error (branch nesting violation).
+ */
+static svn_error_t *
 verify_source_in_branch(const svn_branch_instance_t *branch,
                         svn_editor3_txn_path_t loc,
                         apr_pool_t *scratch_pool)
@@ -1210,17 +1575,20 @@ verify_target_in_branch(const svn_branch
   return SVN_NO_ERROR;
 }
 
-/* Set *TXN_PATHS_P to an array of (const char *) repos-relative paths
- * of all the children (recursively) of BRANCH:LOC, not including itself.
+/* Set *PATHS_P to an array of (const char *) repos-relative paths of
+ * all the child elements (recursively) of BRANCH:LOC_RRPATH, excluding
+ * itself.
+ *
+ * LOC_RRPATH must be a path inside BRANCH. If no element of BRANCH
+ * exists at LOC_RRPATH, return an empty list.
  */
 static svn_error_t *
-svn_branch_walk(apr_array_header_t **paths_p,
-                svn_branch_instance_t *branch,
-                svn_editor3_txn_path_t loc,
-                apr_pool_t *result_pool,
-                apr_pool_t *scratch_pool)
+branch_get_subtree_paths(apr_array_header_t **paths_p,
+                         svn_branch_instance_t *branch,
+                         const char *loc_rrpath,
+                         apr_pool_t *result_pool,
+                         apr_pool_t *scratch_pool)
 {
-  const char *loc_rrpath = txn_path_to_relpath(loc, scratch_pool);
   apr_hash_index_t *hi;
 
   SVN_ERR_ASSERT(svn_relpath_skip_ancestor(branch_get_root_path(branch),
@@ -1308,13 +1676,331 @@ svn_branch_mkdir(svn_branch_instance_t *
   eid = family_add_new_element(branch->definition->family);
 
   SVN_ERR(svn_editor3_mk(editor, svn_node_dir, parent_loc, new_name));
-  branch_set_eid_to_path(branch, eid, loc_rrpath);
+  branch_mapping_update(branch, eid, loc_rrpath);
+  return SVN_NO_ERROR;
+}
+
+/* Adjust BRANCH and its subbranches (recursively),
+ * to reflect deletion of a subtree from FROM_PATH.
+ *
+ * FROM_PATH MUST be the location of a non-root element of BRANCH.
+ * If FROM_PATH is the root of a subbranch and/or contains nested
+ * subbranches, also delete them.
+ *
+ * <ifdef WITH_OVERLAPPING_EIDS> Also delete from each nested subbranch
+ *   that contains FROM_PATH.</>
+ */
+static svn_error_t *
+branch_delete_subtree_r(svn_branch_instance_t *branch,
+                        const char *from_path,
+                        apr_pool_t *scratch_pool)
+{
+  int eid;
+  apr_array_header_t *subbranches;
+  int i;
+
+  /* FROM_PATH MUST be the location of a non-root element of BRANCH. */
+  eid = branch_get_eid_by_path(branch, from_path);
+  if (eid < 0)
+    return svn_error_createf(SVN_ERR_BRANCHING, NULL,
+                             _("in branch %d, can't delete '%s': not found"),
+                             branch->definition->bid, from_path);
+  if (eid == branch->definition->root_eid)
+    return svn_error_createf(SVN_ERR_BRANCHING, NULL,
+                             _("in branch %d, can't delete '%s': is root of this branch"),
+                             branch->definition->bid, from_path);
+
+  /* Delete any nested subbranches at or inside FROM_PATH.
+     (If overlapping EIDs supported: also delete overlapping parts.) */
+  subbranches = branch_get_sub_branches(branch, scratch_pool, scratch_pool);
+  for (i = 0; i < subbranches->nelts; i++)
+    {
+      svn_branch_instance_t *subbranch
+        = APR_ARRAY_IDX(subbranches, i, void *);
+      const char *subbranch_root_path = branch_get_root_path(subbranch);
+      const char *subbranch_within_from_path
+        = svn_relpath_skip_ancestor(from_path, subbranch_root_path);
+
+      if (subbranch_within_from_path)
+        {
+          /* Delete the whole subbranch (recursively) */
+          SVN_ERR(branch_instance_delete_r(subbranch, scratch_pool));
+        }
+
+#ifdef WITH_OVERLAPPING_EIDS
+      /* If FROM_PATH is inside (but not the root of) this subbranch,
+         delete it from this subbranch.
+         (It's not the root -- the first 'if' clause caught that.)
+       */
+      else if (svn_relpath_skip_ancestor(subbranch_root_path, from_path))
+        {
+          SVN_ERR(branch_delete_subtree_r(subbranch, from_path,
+                                          scratch_pool));
+        }
+#endif
+    }
+
+  /* update the path mappings in this branch */
+  SVN_ERR(branch_mappings_delete(branch, from_path, TRUE /*include_self*/,
+                                 scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
+/* Adjust BRANCH and its subbranches (recursively),
+ * to reflect a move of a subtree from FROM_PATH to TO_PATH.
+ *
+ * FROM_PATH must be an existing element of BRANCH. (It may be the root.)
+ * If FROM_PATH is the root of a subbranch and/or contains nested
+ * subbranches, also move them.
+ *
+ * TO_PATH must be a non-existing path in an existing parent directory in
+ * BRANCH.
+ *
+ * <ifdef WITH_OVERLAPPING_EIDS> Also delete from / add to / move within
+ *   each nested subbranch that contains FROM_PATH / TO_PATH / both
+ *   (respectively). </>
+ */
+static svn_error_t *
+branch_move_subtree_r(svn_branch_instance_t *branch,
+                      const char *from_path,
+                      const char *to_path,
+                      apr_pool_t *scratch_pool)
+{
+  apr_array_header_t *subbranches;
+  int i;
+
+  /* update the path mappings in this branch */
+  SVN_ERR(branch_mappings_move(branch, from_path, to_path, scratch_pool));
+
+  /* handle subbranches */
+  subbranches = branch_get_sub_branches(branch, scratch_pool, scratch_pool);
+  for (i = 0; i < subbranches->nelts; i++)
+    {
+      svn_branch_instance_t *subbranch
+        = APR_ARRAY_IDX(subbranches, i, void *);
+      const char *subbranch_root_path = branch_get_root_path(subbranch);
+      const char *subbranch_within_from_path
+        = svn_relpath_skip_ancestor(from_path, subbranch_root_path);
+#ifdef WITH_OVERLAPPING_EIDS
+      const char *from_path_within_subbranch
+        = svn_relpath_skip_ancestor(subbranch_root_path, from_path);
+      const char *to_path_within_subbranch
+        = svn_relpath_skip_ancestor(subbranch_root_path, to_path);
+#endif
+
+      if (subbranch_within_from_path)
+        {
+          const char *subbranch_root_to_path
+            = svn_relpath_join(to_path, subbranch_within_from_path,
+                               scratch_pool);
+
+          /* Move the whole subbranch (recursively) */
+          SVN_ERR(branch_move_subtree_r(subbranch,
+                                        subbranch_root_path,
+                                        subbranch_root_to_path,
+                                        scratch_pool));
+        }
+
+#ifdef WITH_OVERLAPPING_EIDS
+      /* If FROM_PATH or TO_PATH is inside (but not the root of) this
+         subbranch, move within or delete from or add to this subbranch.
+         (FROM_PATH is not the root -- the first 'if' clause caught that.)
+         (TO_PATH is not the root -- the root exists and TO_PATH doesn't.)
+       */
+      else if (from_path_within_subbranch && to_path_within_subbranch)
+        {
+          SVN_ERR(branch_move_subtree_r(subbranch, from_path, to_path,
+                                        scratch_pool));
+        }
+      else if (from_path_within_subbranch)
+        {
+          SVN_ERR(branch_delete_subtree_r(subbranch, from_path,
+                                          scratch_pool));
+        }
+      else if (to_path_within_subbranch)
+        {
+          /* ### Copy the external subtree at FROM_PATH (including nested
+                 branches? -- but they're overlapping anyway so no need)
+                 to SUBBRANCH:TO_PATH, as added or "copied"? elements. */
+        }
+#endif
+    }
+  return SVN_NO_ERROR;
+}
+
+/* Adjust OUTER_BRANCH and its subbranches (recursively),
+ * to reflect branching a subtree from FROM_BRANCH:FROM_PATH to
+ * create a new subbranch of OUTER_BRANCH at TO_PATH.
+ *
+ * FROM_BRANCH must be an immediate child branch of OUTER_BRANCH.
+ *
+ * FROM_PATH must be an existing element of FROM_BRANCH. It may be the
+ * root of FROM_BRANCH. It must not be the root of a subbranch of
+ * FROM_BRANCH.
+ *
+ * TO_PATH must be a non-existing path in an existing parent directory in
+ * OUTER_BRANCH.
+ *
+ * <ifdef WITH_OVERLAPPING_EIDS> ### ? </>
+ */
+static svn_error_t *
+branch_branch_subtree_r(svn_branch_instance_t **new_branch_p,
+                        svn_branch_instance_t *outer_branch,
+                        svn_branch_instance_t *from_branch,
+                        const char *from_path,
+                        const char *to_path,
+                        apr_pool_t *scratch_pool)
+{
+  int inner_eid = branch_get_eid_by_path(from_branch, from_path);
+  int to_outer_eid;
+  svn_branch_instance_t *new_branch;
+  apr_array_header_t *subbranches;
+  int i;
+
+  /* FROM_BRANCH must be an immediate child branch of OUTER_BRANCH. */
+  /* SVN_ERR_ASSERT(...); */
+
+  /* SVN_ERR_ASSERT(...); */
+
+  /* assign new eid to root node (outer branch) */
+  to_outer_eid = family_add_new_element(outer_branch->definition->family);
+  branch_mapping_update(outer_branch, to_outer_eid, to_path);
+
+  /* create new inner branch definition & instance */
+  new_branch = family_add_new_branch(from_branch->definition->family,
+                                     inner_eid, to_path);
+
+  /* populate new branch instance with path mappings */
+  SVN_ERR(branch_mappings_branch(from_branch, from_path,
+                                 new_branch, to_path, scratch_pool));
+
+  /* branch any subbranches of FROM_BRANCH */
+  subbranches = branch_get_sub_branches(from_branch, scratch_pool, scratch_pool);
+  for (i = 0; i < subbranches->nelts; i++)
+    {
+      svn_branch_instance_t *subbranch
+        = APR_ARRAY_IDX(subbranches, i, void *);
+      const char *subbranch_root_path = branch_get_root_path(subbranch);
+      const char *subbranch_within_from_path
+        = svn_relpath_skip_ancestor(from_path, subbranch_root_path);
+
+      if (subbranch_within_from_path)
+        {
+          const char *subbranch_root_to_path
+            = svn_relpath_join(to_path, subbranch_within_from_path,
+                               scratch_pool);
+
+          /* branch this subbranch into NEW_BRANCH (recursing) */
+          SVN_ERR(branch_branch_subtree_r(NULL,
+                                          new_branch,
+                                          subbranch, subbranch_root_path,
+                                          subbranch_root_to_path,
+                                          scratch_pool));
+        }
+    }
+
+  if (new_branch_p)
+    *new_branch_p = new_branch;
+  return SVN_NO_ERROR;
+}
+
+/* Adjust BRANCH and its subbranches (recursively),
+ * to reflect a copy of a subtree from FROM_PATH to TO_PATH.
+ *
+ * FROM_PATH must be an existing element of BRANCH. (It may be the root.)
+ * If FROM_PATH is the root of a subbranch and/or contains nested
+ * subbranches, also copy them (by branching).
+ *
+ * TO_PATH must be a non-existing path in an existing parent directory in
+ * BRANCH.
+ *
+ * <ifdef WITH_OVERLAPPING_EIDS> Also delete from / add to / copy within
+ *   each nested subbranch that contains FROM_PATH / TO_PATH / both
+ *   (respectively). </>
+ */
+static svn_error_t *
+branch_copy_subtree_r(svn_branch_instance_t *branch,
+                      const char *from_path,
+                      const char *to_path,
+                      apr_pool_t *scratch_pool)
+{
+  apr_array_header_t *subbranches;
+  int i;
+
+  /* assign new EIDs and update the path mappings in this branch */
+  SVN_ERR(branch_mappings_copy(branch, from_path,
+                               branch, to_path,
+                               TRUE /*include_self*/, scratch_pool));
+
+  /* handle subbranches */
+  subbranches = branch_get_sub_branches(branch, scratch_pool, scratch_pool);
+  for (i = 0; i < subbranches->nelts; i++)
+    {
+      svn_branch_instance_t *subbranch
+        = APR_ARRAY_IDX(subbranches, i, void *);
+      const char *subbranch_root_path = branch_get_root_path(subbranch);
+      const char *subbranch_within_from_path
+        = svn_relpath_skip_ancestor(from_path, subbranch_root_path);
+#ifdef WITH_OVERLAPPING_EIDS
+      const char *from_path_within_subbranch
+        = svn_relpath_skip_ancestor(subbranch_root_path, from_path);
+      const char *to_path_within_subbranch
+        = svn_relpath_skip_ancestor(subbranch_root_path, to_path);
+#endif
+
+      if (subbranch_within_from_path)
+        {
+          const char *subbranch_root_to_path
+            = svn_relpath_join(to_path, subbranch_within_from_path,
+                               scratch_pool);
+
+          /* branch the whole subbranch (recursively) */
+          SVN_ERR(branch_branch_subtree_r(NULL,
+                                          branch,
+                                          subbranch, subbranch_root_path,
+                                          subbranch_root_to_path,
+                                          scratch_pool));
+        }
+
+#ifdef WITH_OVERLAPPING_EIDS
+      /* If FROM_PATH or TO_PATH is inside (but not the root of) this
+         subbranch, copy within or branch from or add to this subbranch.
+         (FROM_PATH is not the root -- the first 'if' clause caught that.)
+         (TO_PATH is not the root -- the root exists and TO_PATH doesn't.)
+       */
+      else if (from_path_within_subbranch && to_path_within_subbranch)
+        {
+          SVN_ERR(branch_copy_subtree_r(subbranch, from_path, to_path,
+                                        scratch_pool));
+        }
+      else if (from_path_within_subbranch)
+        {
+          SVN_ERR(branch_branch_subtree_r(branch,
+                                          subbranch, from_path,
+                                          to_path,
+                                          scratch_pool));
+        }
+      else if (to_path_within_subbranch)
+        {
+          /* ### Copy the external subtree at FROM_PATH (including nested
+                 branches? -- but they're overlapping anyway so no need)
+                 to SUBBRANCH:TO_PATH, as added or "copied"? elements. */
+        }
+#endif
+    }
   return SVN_NO_ERROR;
 }
 
 /* In BRANCH, move the subtree at FROM_LOC to PARENT_LOC:NEW_NAME.
  *
- * ### Needs to adjust any sub-branches within FROM_LOC.
+ * FROM_LOC must be an existing non-root element of BRANCH. It may also
+ * be the root of a subbranch and/or contain nested subbranches; these
+ * will also be moved.
+ *
+ * PARENT_LOC must be an existing directory element in BRANCH.
+ * <ifdef WITH_OVERLAPPING_EIDS> PARENT_LOC may be a subbranch root or
+ * inside a subbranch. <else> PARENT_LOC is not a subbranch root. </>.
  */
 static svn_error_t *
 svn_branch_mv(svn_branch_instance_t *branch,
@@ -1327,18 +2013,25 @@ svn_branch_mv(svn_branch_instance_t *bra
   svn_editor3_txn_path_t to_loc = txn_path_join(parent_loc, new_name, scratch_pool);
   const char *from_path = txn_path_to_relpath(from_loc, scratch_pool);
   const char *to_path = txn_path_to_relpath(to_loc, scratch_pool);
-  int eid;
 
   SVN_ERR(verify_source_in_branch(branch, from_loc, scratch_pool));
   SVN_ERR(verify_target_in_branch(branch, parent_loc, scratch_pool));
 
-  eid = branch_get_eid_by_path(branch, from_path);
   SVN_ERR(svn_editor3_mv(editor, from_loc.peg, parent_loc, new_name));
-  branch_set_eid_to_path(branch, eid, to_path);
+
+  SVN_ERR(branch_move_subtree_r(branch, from_path, to_path, scratch_pool));
   return SVN_NO_ERROR;
 }
 
 /* In BRANCH, copy the subtree at FROM_LOC to PARENT_LOC:NEW_NAME.
+ * Any nested subbranches inside FROM_LOC will be copied by branching.
+ *
+ * FROM_LOC must be an existing (root or non-root) element of BRANCH,
+ * and must not be the root of a subbranch of BRANCH.
+ *
+ * PARENT_LOC must be an existing directory element in BRANCH.
+ * <ifdef WITH_OVERLAPPING_EIDS> PARENT_LOC may be a subbranch root or
+ * inside a subbranch. <else> PARENT_LOC is not a subbranch root. </>.
  */
 static svn_error_t *
 svn_branch_cp(svn_branch_instance_t *branch,
@@ -1348,53 +2041,24 @@ svn_branch_cp(svn_branch_instance_t *bra
               apr_pool_t *scratch_pool)
 {
   svn_editor3_t *editor = branch->definition->family->repos->editor;
-  const char *from_rrpath = txn_path_to_relpath(from_loc, scratch_pool);
-  svn_editor3_txn_path_t to_loc;
-  const char *to_rrpath;
-  apr_array_header_t *paths;
-  int i;
+  svn_editor3_txn_path_t to_loc = txn_path_join(parent_loc, new_name, scratch_pool);
+  const char *from_path = txn_path_to_relpath(from_loc, scratch_pool);
+  const char *to_path = txn_path_to_relpath(to_loc, scratch_pool);
 
   SVN_ERR(verify_source_in_branch(branch, from_loc, scratch_pool));
   SVN_ERR(verify_target_in_branch(branch, parent_loc, scratch_pool));
 
   SVN_ERR(svn_editor3_cp(editor, from_loc.peg, parent_loc, new_name));
 
-  /* assign new eids to all elements in the copy */
-  to_loc = txn_path_join(parent_loc, new_name, scratch_pool);
-  to_rrpath = txn_path_to_relpath(to_loc, scratch_pool);
-  branch_set_eid_to_path(branch,
-                         family_add_new_element(branch->definition->family),
-                         to_rrpath);
-  /* Ideally we'd now walk the target loc, but as svn_editor3_t doesn't
-     support walking a created location, we walk the source and convert
-     source paths to target paths. */
-  SVN_ERR(svn_branch_walk(&paths, branch, from_loc,
-                          scratch_pool, scratch_pool));
-  for (i = 0; i < paths->nelts; i++)
-    {
-      const char *this_from_path = APR_ARRAY_IDX(paths, i, const char *);
-      const char *this_relpath = svn_relpath_skip_ancestor(from_rrpath,
-                                                           this_from_path);
-      const char *this_to_path = svn_relpath_join(to_rrpath, this_relpath,
-                                                  scratch_pool);
-      int eid = family_add_new_element(branch->definition->family);
-
-      branch_set_eid_to_path(branch, eid, this_to_path);
-    }
-
+  SVN_ERR(branch_copy_subtree_r(branch, from_path, to_path, scratch_pool));
   return SVN_NO_ERROR;
 }
 
 /* In OUTER_BRANCH, branch the existing sub-branch at FROM_LOC to create
  * a new branch at PARENT_LOC:NEW_NAME.
  *
- * FROM_LOC must be either the root path of an immediate sub-branch of
- * OUTER_BRANCH, or a non-root path in such a sub-branch.
- *
- *   copy
- *   assign new eid (in outer branch) to root node
- *   assign new bid to new (inner) branch
- *   leave eids of inner br unchanged
+ * FROM_LOC must be (root or non-root) path of an immediate sub-branch of
+ * OUTER_BRANCH.
  */
 static svn_error_t *
 svn_branch_branch(svn_branch_instance_t *outer_branch,
@@ -1410,10 +2074,6 @@ svn_branch_branch(svn_branch_instance_t 
   const char *to_rrpath = txn_path_to_relpath(to_loc, scratch_pool);
   svn_branch_instance_t *from_inner_branch;
   int inner_eid;
-  int to_outer_eid;
-  svn_branch_instance_t *to_inner_branch;
-  apr_array_header_t *paths;
-  int i;
 
   SVN_ERR(branch_find_subbranch_element_by_location(
             &from_inner_branch, &inner_eid,
@@ -1439,35 +2099,11 @@ svn_branch_branch(svn_branch_instance_t 
   /* copy */
   SVN_ERR(svn_editor3_cp(editor, from_loc.peg, parent_loc, new_name));
 
-  /* assign new eid to root node (outer branch) */
-  to_outer_eid = family_add_new_element(outer_branch->definition->family);
-  branch_set_eid_to_path(outer_branch, to_outer_eid, to_rrpath);
-
-  /* create new inner branch definition & instance */
-  to_inner_branch = family_add_new_branch(from_inner_branch->definition->family,
-                                          inner_eid, to_rrpath);
-
-  /* Populate new branch instance with eid-path mappings */
-  SVN_ERR(svn_branch_walk(&paths, from_inner_branch, from_loc,
-                          scratch_pool, scratch_pool));
-  for (i = 0; i < paths->nelts; i++)
-    {
-      const char *this_from_path = APR_ARRAY_IDX(paths, i, const char *);
-      const char *this_rrpath = svn_relpath_skip_ancestor(from_rrpath,
-                                                          this_from_path);
-      const char *this_to_path = svn_relpath_join(to_rrpath, this_rrpath,
-                                                  scratch_pool);
-      int eid;
-
-      eid = branch_get_eid_by_path(from_inner_branch, this_from_path);
-      branch_set_eid_to_path(to_inner_branch, eid, this_to_path);
-
-#ifdef WITH_OVERLAPPING_EIDS
-      /* Also assign new EIDs in outer branch */
-      eid = family_add_new_element(outer_branch->definition->family);
-      branch_set_eid_to_path(outer_branch, eid, this_to_path);
-#endif
-    }
+  SVN_ERR(branch_branch_subtree_r(NULL,
+                                  outer_branch,
+                                  from_inner_branch, from_rrpath,
+                                  to_rrpath,
+                                  scratch_pool));
 
   return SVN_NO_ERROR;
 }
@@ -1494,40 +2130,36 @@ svn_branch_branchify(svn_branch_instance
   /* ### TODO: First check ROOT_LOC is not already a branch root
          and the subtree at ROOT_LOC does not contain any branch roots. */
 
-  svn_branch_family_t *family
+  svn_branch_family_t *new_family
     = family_add_new_subfamily(outer_branch->definition->family);
-  int new_root_eid = family_add_new_element(family);
+  int new_root_eid = family_add_new_element(new_family);
   const char *new_root_rrpath = txn_path_to_relpath(new_root_loc, scratch_pool);
   svn_branch_instance_t *new_branch
-    = family_add_new_branch(family, new_root_eid, new_root_rrpath);
-  apr_array_header_t *paths;
-  int i;
+    = family_add_new_branch(new_family, new_root_eid, new_root_rrpath);
 
   SVN_DBG(("branchify(%s): new fid=%d, bid=%d",
-           txn_path_str(new_root_loc, scratch_pool), family->fid, new_branch->definition->bid));
+           txn_path_str(new_root_loc, scratch_pool), new_family->fid, new_branch->definition->bid));
 
-  /* Walk the subtree paths in the outer branch, (re-)assigning them to
-     the new branch. */
-  SVN_ERR(svn_branch_walk(&paths, outer_branch, new_root_loc,
-                          scratch_pool, scratch_pool));
-  for (i = 0; i < paths->nelts; i++)
-    {
-      const char *this_path = APR_ARRAY_IDX(paths, i, const char *);
-      int eid = family_add_new_element(new_branch->definition->family);
-
-      branch_set_eid_to_path(new_branch, eid, this_path);
+  /* assign new EIDs and update the path mappings in this branch */
+  SVN_ERR(branch_mappings_copy(outer_branch, new_root_rrpath,
+                               new_branch, new_root_rrpath,
+                               FALSE /*include_self*/, scratch_pool));
 
 #ifndef WITH_OVERLAPPING_EIDS
-      /* Remove old EID in outer branch */
-      eid = branch_get_eid_by_path(outer_branch, this_path);
-      branch_mapping_remove(outer_branch, eid, this_path);
+  /* remove old EIDs in outer branch */
+  SVN_ERR(branch_mappings_delete(outer_branch, new_root_rrpath,
+                                 FALSE /*include_self*/, scratch_pool));
 #endif
-    }
 
   return SVN_NO_ERROR;
 }
 
-/*  */
+/* In BRANCH, remove the subtree at LOC, including any nested branches.
+ *
+ * LOC MUST be the location of a non-root element of BRANCH.
+ * <ifdef WITH_OVERLAPPING_EIDS>
+ *   LOC may be both in BRANCH and in one or more nested subbranches.</>
+ */
 static svn_error_t *
 svn_branch_rm(svn_branch_instance_t *branch,
               svn_editor3_txn_path_t loc,
@@ -1535,25 +2167,10 @@ svn_branch_rm(svn_branch_instance_t *bra
 {
   svn_editor3_t *editor = branch->definition->family->repos->editor;
   const char *rrpath = txn_path_to_relpath(loc, scratch_pool);
-  int eid = branch_get_eid_by_path(branch, rrpath);
-  apr_array_header_t *paths;
-  int i;
-
-  branch_mapping_remove(branch, eid, rrpath);
-
-  SVN_ERR(svn_branch_walk(&paths, branch, loc,
-                          scratch_pool, scratch_pool));
-  for (i = 0; i < paths->nelts; i++)
-    {
-      const char *this_path = APR_ARRAY_IDX(paths, i, const char *);
-      int this_eid;
-
-      /* Remove old EID */
-      this_eid = branch_get_eid_by_path(branch, this_path);
-      branch_mapping_remove(branch, this_eid, this_path);
-    }
 
   SVN_ERR(svn_editor3_rm(editor, loc));
+
+  SVN_ERR(branch_delete_subtree_r(branch, rrpath, scratch_pool));
   return SVN_NO_ERROR;
 }