subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From julianf...@apache.org
Subject svn commit: r1717778 - in /subversion/trunk: subversion/tests/cmdline/svnmover_tests.py tools/dev/svnmover/merge3.c tools/dev/svnmover/svnmover.c tools/dev/svnmover/svnmover.h
Date Thu, 03 Dec 2015 14:59:32 GMT
Author: julianfoad
Date: Thu Dec  3 14:59:31 2015
New Revision: 1717778

URL: http://svn.apache.org/viewvc?rev=1717778&view=rev
Log:
In 'svnmover', improve how we diff and merge subtrees.

When merging a subtree, notice all changes to each element that is in any of
the three subtrees. In other words, for an element that is present in only
the source or only the target subtree, don't ignore any changes that may
have happened to that element on the other side of the merge, even though it
is outside the given subtrees on that side. Add a regression test for this.

When merging an element that is the branch root in one or two of the three
(YCA, source, target) inputs, but is a non-root element in others, ignore
that element's parentage. Otherwise, if we try to merge that difference in
parentage, we would mess up the tree structure of the target branch.
svnmover_tests.py 16 "merge from subbranch to subtree" covers this case, and
would fail if only the "notice all changes" change above were committed.

* tools/dev/svnmover/merge3.c
  (is_branch_root_element): New, copied from svnmover.c.
  (branch_merge_subtree_r): Diff the elements in the union of all three
    subtrees, not just those in each pair of subtrees.

* tools/dev/svnmover/svnmover.h
  (svnmover_element_differences): Take the set of elements to diff as an
    input. Allow other inputs to be null.

* tools/dev/svnmover/svnmover.c
  (svnmover_element_differences): Take the set of elements to diff as an
    input. Allow other inputs to be null.
  (txn_is_changed): Update caller, passing null.
  (branch_elements_replay): Rename from 'subtree_replay'. Take the set of
    elements to diff as an input. Simplify.
  (svn_branch__replay,
   update_wc_base_r,
   subtree_diff): Update callers, passing null.
  (show_subtree_diff): Update the doc string.

* subversion/tests/cmdline/svnmover_tests.py
  (merge_move_into_subtree): New test.
  (test_list): Run it.

Modified:
    subversion/trunk/subversion/tests/cmdline/svnmover_tests.py
    subversion/trunk/tools/dev/svnmover/merge3.c
    subversion/trunk/tools/dev/svnmover/svnmover.c
    subversion/trunk/tools/dev/svnmover/svnmover.h

Modified: subversion/trunk/subversion/tests/cmdline/svnmover_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnmover_tests.py?rev=1717778&r1=1717777&r2=1717778&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnmover_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnmover_tests.py Thu Dec  3 14:59:31 2015
@@ -1625,6 +1625,55 @@ def simple_branch(sbox):
                            ['A /top0/Y (from /top0/X:1)',
                             'A /top0/Y/A (from /top0/X/A:2)'])
 
+def merge_move_into_subtree(sbox):
+  "merge move into subtree"
+  sbox_build_svnmover(sbox, content=initial_content_ttb)
+  repo_url = sbox.repo_url
+
+  # This tests the behaviour of merging a subtree. In this case, we expect
+  # each element in the union of (YCA subtree, source subtree, target subtree)
+  # to be merged. (Other behaviours -- such as merging only the elements in
+  # the intersection of those three subtrees -- could be provided in future.)
+  #
+  # This test tests a merge with no conflicts.
+
+  # create initial state in trunk
+  # (r2)
+  test_svnmover2(sbox, '/trunk',
+                 reported_br_diff('trunk') +
+                 reported_add('A') +
+                 reported_add('B2') +
+                 reported_add('B2/C2'),
+                'mkdir A',
+                'mkdir B2',
+                'mkdir B2/C2')
+
+  # branch (r3)
+  test_svnmover2(sbox, '',
+                 reported_br_diff('') +
+                 reported_br_add('branches/br1'),
+                'branch trunk branches/br1')
+
+  # on trunk: move B2 into subtree A (r4)
+  test_svnmover2(sbox, 'trunk',
+                 reported_br_diff('trunk') +
+                 reported_move('B2', 'A/B2'),
+                'mv B2 A/B2')
+
+  # on branch: make a non-conflicting change to 'B2' (r5)
+  test_svnmover2(sbox, 'branches/br1',
+                 reported_br_diff('branches/br1') +
+                 reported_move('B2', 'B3'),
+                'mv B2 B3')
+
+  # merge subtree 'A' from trunk to branch (r6)
+  # expect the move-into-subtree to be merged with the rename-outside-subtree
+  test_svnmover2(sbox, '',
+                 reported_mg_diff() +
+                 reported_br_diff('branches/br1') +
+                 reported_move('B3', 'A/B3'),
+                'merge trunk/A@4 branches/br1/A trunk/A@2')
+
 ######################################################################
 
 test_list = [ None,
@@ -1655,6 +1704,7 @@ test_list = [ None,
               replace_via_rm_cp,
               see_the_revision_just_committed,
               simple_branch,
+              merge_move_into_subtree,
             ]
 
 if __name__ == '__main__':

Modified: subversion/trunk/tools/dev/svnmover/merge3.c
URL: http://svn.apache.org/viewvc/subversion/trunk/tools/dev/svnmover/merge3.c?rev=1717778&r1=1717777&r2=1717778&view=diff
==============================================================================
--- subversion/trunk/tools/dev/svnmover/merge3.c (original)
+++ subversion/trunk/tools/dev/svnmover/merge3.c Thu Dec  3 14:59:31 2015
@@ -50,6 +50,9 @@
 
 /* ====================================================================== */
 
+#define is_branch_root_element(branch, eid) \
+  (svn_branch__root_eid(branch) == (eid))
+
 /* Return a string suitable for appending to a displayed element name or
  * element id to indicate that it is a subbranch root element for SUBBRANCH.
  * Return "" if SUBBRANCH is null.
@@ -1124,6 +1127,9 @@ detect_orphans(apr_hash_t **orphans_p,
 
 /* Merge ...
  *
+ * The elements to merge are the union of the elements in the three input
+ * subtrees (SRC, TGT, YCA).
+ *
  * Merge any sub-branches in the same way, recursively.
  *
  * ### TODO: Store the merge result separately, without overwriting the
@@ -1173,14 +1179,12 @@ branch_merge_subtree_r(svn_branch__txn_t
   SVN_ERR(svn_branch__get_subtree(src->branch, &s_src, src->eid, scratch_pool));
   SVN_ERR(svn_branch__get_subtree(tgt->branch, &s_tgt, tgt->eid, scratch_pool));
   SVN_ERR(svn_branch__get_subtree(yca->branch, &s_yca, yca->eid, scratch_pool));
-  SVN_ERR(svnmover_element_differences(&diff_yca_src,
-                                       s_yca->tree, s_src->tree,
-                                       scratch_pool, scratch_pool));
-  /* ### We only need to query for YCA:TO differences in elements that are
-         different in YCA:FROM, but right now we ask for all differences. */
-  SVN_ERR(svnmover_element_differences(&diff_yca_tgt,
-                                       s_yca->tree, s_tgt->tree,
-                                       scratch_pool, scratch_pool));
+
+  /* ALL_ELEMENTS enumerates the elements in union of subtrees YCA,SRC,TGT. */
+  all_elements = hash_overlay(s_src->tree->e_map,
+                              s_tgt->tree->e_map);
+  all_elements = hash_overlay(s_yca->tree->e_map,
+                              all_elements);
 
   SVN_ERR(svn_branch__state_get_elements(src->branch, &src_elements,
                                          scratch_pool));
@@ -1188,10 +1192,21 @@ branch_merge_subtree_r(svn_branch__txn_t
                                          scratch_pool));
   SVN_ERR(svn_branch__state_get_elements(yca->branch, &yca_elements,
                                          scratch_pool));
-  all_elements = hash_overlay(src_elements->e_map,
-                              tgt_elements->e_map);
-  all_elements = hash_overlay(yca_elements->e_map,
-                              all_elements);
+
+  /* Find the two changes for each element that is in any of the subtrees,
+     even for an element that is (for example) not in YCA or SRC but has
+     been moved into TGT. */
+  SVN_ERR(svnmover_element_differences(&diff_yca_src,
+                                       yca_elements, src_elements,
+                                       all_elements,
+                                       scratch_pool, scratch_pool));
+  /* ### We only need to know about YCA:TGT differences for elements that
+         differ in YCA:SRC, but right now we ask for all differences. */
+  SVN_ERR(svnmover_element_differences(&diff_yca_tgt,
+                                       yca_elements, tgt_elements,
+                                       all_elements,
+                                       scratch_pool, scratch_pool));
+
   for (SVN_EID__HASH_ITER_SORTED_BY_EID(ei, all_elements, scratch_pool))
     {
       int eid = ei->eid;
@@ -1225,6 +1240,24 @@ branch_merge_subtree_r(svn_branch__txn_t
       e_src = e_yca_src[1];
       e_tgt = e_yca_tgt ? e_yca_tgt[1] : e_yca_src[0];
 
+      /* If some but not all of the three subtree-root elements are branch
+         roots, then we will see the parentage of this element changing to
+         or from 'no parent' in one or both sides of the merge. We want to
+         ignore this part of the difference, as parentage of a subtree root
+         element is by definition not part of a 'subtree', so blank it out.
+         (If we merged it, it could break the single-rooted-tree invariant
+         of the target branch.)
+       */
+      if (is_branch_root_element(src->branch, eid)
+          || is_branch_root_element(tgt->branch, eid)
+          || is_branch_root_element(yca->branch, eid))
+        {
+          e_src = svn_element__content_create(
+                    e_tgt->parent_eid, e_tgt->name, e_src->payload, iterpool);
+          e_yca = svn_element__content_create(
+                    e_tgt->parent_eid, e_tgt->name, e_yca->payload, iterpool);
+        }
+
       element_merge(&result, &conflict,
                     eid, e_src, e_tgt, e_yca,
                     &policy,

Modified: subversion/trunk/tools/dev/svnmover/svnmover.c
URL: http://svn.apache.org/viewvc/subversion/trunk/tools/dev/svnmover/svnmover.c?rev=1717778&r1=1717777&r2=1717778&view=diff
==============================================================================
--- subversion/trunk/tools/dev/svnmover/svnmover.c (original)
+++ subversion/trunk/tools/dev/svnmover/svnmover.c Thu Dec  3 14:59:31 2015
@@ -484,19 +484,27 @@ svn_error_t *
 svnmover_element_differences(apr_hash_t **diff_p,
                              const svn_element__tree_t *left,
                              const svn_element__tree_t *right,
+                             apr_hash_t *elements,
                              apr_pool_t *result_pool,
                              apr_pool_t *scratch_pool)
 {
   apr_hash_t *diff = apr_hash_make(result_pool);
   apr_hash_index_t *hi;
 
+  if (! left)
+    left = svn_element__tree_create(NULL, 0 /*root_eid*/, scratch_pool);
+  if (! right)
+    right = svn_element__tree_create(NULL, 0 /*root_eid*/, scratch_pool);
+
   /*SVN_DBG(("element_differences(b%s r%ld, b%s r%ld, e%d)",
            svn_branch__get_id(left->branch, scratch_pool), left->rev,
            svn_branch__get_id(right->branch, scratch_pool), right->rev,
            right->eid));*/
 
-  for (hi = apr_hash_first(scratch_pool,
-                           hash_overlay(left->e_map, right->e_map));
+  if (!elements)
+    elements = hash_overlay(left->e_map, right->e_map);
+
+  for (hi = apr_hash_first(scratch_pool, elements);
        hi; hi = apr_hash_next(hi))
     {
       int e = svn_eid__hash_this_key(hi);
@@ -629,6 +637,7 @@ txn_is_changed(svn_branch__txn_t *edit_t
       SVN_ERR(svnmover_element_differences(&diff,
                                            edit_branch_elements,
                                            base_branch_elements,
+                                           NULL /*all elements*/,
                                            scratch_pool, scratch_pool));
       if (apr_hash_count(diff))
         {
@@ -640,28 +649,42 @@ txn_is_changed(svn_branch__txn_t *edit_t
   return SVN_NO_ERROR;
 }
 
-/* Replay differences between S_LEFT and S_RIGHT into EDITOR:EDIT_BRANCH.
+/* Replay the whole-element changes between LEFT_BRANCH and RIGHT_BRANCH
+ * into EDIT_BRANCH.
+ *
+ * Replaying means, for each element E that is changed (added, modified
+ * or deleted) between left and right branches, we set element E in
+ * EDIT_BRANCH to whole value of E in RIGHT_BRANCH. This is not like
+ * merging: each change resets an element's whole value.
  *
- * S_LEFT and/or S_RIGHT may be null meaning an empty set.
+ * ELEMENTS_TO_DIFF (eid -> [anything]) says which elements to diff; if
+ * null, diff all elements in the union of left & right branches.
+ *
+ * LEFT_BRANCH and/or RIGHT_BRANCH may be null which means the equivalent
+ * of an empty branch.
  *
  * Non-recursive: single branch only.
  */
 static svn_error_t *
-subtree_replay(svn_branch__state_t *edit_branch,
-               const svn_element__tree_t *s_left,
-               const svn_element__tree_t *s_right,
-               apr_pool_t *scratch_pool)
+branch_elements_replay(svn_branch__state_t *edit_branch,
+                       const svn_branch__state_t *left_branch,
+                       const svn_branch__state_t *right_branch,
+                       apr_hash_t *elements_to_diff,
+                       apr_pool_t *scratch_pool)
 {
+  svn_element__tree_t *s_left = NULL, *s_right = NULL;
   apr_hash_t *diff_left_right;
   apr_hash_index_t *hi;
 
-  if (! s_left)
-    s_left = svn_element__tree_create(NULL, 0 /*root_eid*/, scratch_pool);
-  if (! s_right)
-    s_right = svn_element__tree_create(NULL, 0 /*root_eid*/, scratch_pool);
-
+  if (left_branch)
+    SVN_ERR(svn_branch__state_get_elements(left_branch, &s_left,
+                                           scratch_pool));
+  if (right_branch)
+    SVN_ERR(svn_branch__state_get_elements(right_branch, &s_right,
+                                           scratch_pool));
   SVN_ERR(svnmover_element_differences(&diff_left_right,
                                        s_left, s_right,
+                                       elements_to_diff,
                                        scratch_pool, scratch_pool));
 
   /* Go through the per-element differences. */
@@ -676,25 +699,16 @@ subtree_replay(svn_branch__state_t *edit
                      || svn_element__payload_invariants(e0->payload));
       SVN_ERR_ASSERT(!e1
                      || svn_element__payload_invariants(e1->payload));
-      if (e0 || e1)
+      if (e1)
         {
-          if (e0 && e1)
-            {
-              SVN_ERR(svn_branch__state_alter_one(edit_branch, eid,
-                                                  e1->parent_eid, e1->name,
-                                                  e1->payload, scratch_pool));
-            }
-          else if (e0)
-            {
-              SVN_ERR(svn_branch__state_delete_one(edit_branch, eid,
-                                                   scratch_pool));
-            }
-          else
-            {
-              SVN_ERR(svn_branch__state_alter_one(edit_branch, eid,
-                                                  e1->parent_eid, e1->name,
-                                                  e1->payload, scratch_pool));
-            }
+          SVN_ERR(svn_branch__state_alter_one(edit_branch, eid,
+                                              e1->parent_eid, e1->name,
+                                              e1->payload, scratch_pool));
+        }
+      else
+        {
+          SVN_ERR(svn_branch__state_delete_one(edit_branch, eid,
+                                               scratch_pool));
         }
     }
 
@@ -748,17 +762,10 @@ svn_branch__replay(svn_branch__txn_t *ed
   if (right_branch)
     {
       /* Replay this branch */
-      svn_element__tree_t *s_left = NULL;
-      svn_element__tree_t *s_right = NULL;
+      apr_hash_t *elements_to_diff = NULL;  /*means the union of left & right*/
 
-      if (left_branch)
-        SVN_ERR(svn_branch__state_get_elements(left_branch, &s_left,
-                                               scratch_pool));
-      if (right_branch)
-        SVN_ERR(svn_branch__state_get_elements(right_branch, &s_right,
-                                               scratch_pool));
-      SVN_ERR(subtree_replay(edit_branch, s_left, s_right,
-                             scratch_pool));
+      SVN_ERR(branch_elements_replay(edit_branch, left_branch, right_branch,
+                                     elements_to_diff, scratch_pool));
     }
   else
     {
@@ -941,22 +948,19 @@ update_wc_base_r(svnmover_wc_t *wc,
                  svn_revnum_t new_rev,
                  apr_pool_t *scratch_pool)
 {
-  svn_element__tree_t *base_elements, *working_elements;
+  svn_element__tree_t *base_elements = NULL, *working_elements = NULL;
   apr_hash_t *committed_elements;
   apr_hash_index_t *hi;
 
   if (base_branch)
     SVN_ERR(svn_branch__state_get_elements(base_branch, &base_elements,
                                            scratch_pool));
-  else
-    base_elements = svn_element__tree_create(NULL, 0, scratch_pool);
   if (work_branch)
     SVN_ERR(svn_branch__state_get_elements(work_branch, &working_elements,
                                            scratch_pool));
-  else
-    working_elements = svn_element__tree_create(NULL, 0, scratch_pool);
   SVN_ERR(svnmover_element_differences(&committed_elements,
                                        base_elements, working_elements,
+                                       NULL /*all elements*/,
                                        scratch_pool, scratch_pool));
 
   for (hi = apr_hash_first(scratch_pool, committed_elements);
@@ -1872,6 +1876,7 @@ typedef struct diff_item_t
 } diff_item_t;
 
 /* Return differences between branch subtrees S_LEFT and S_RIGHT.
+ * Diff the union of S_LEFT's and S_RIGHT's elements.
  *
  * Set *DIFF_CHANGES to a hash of (eid -> diff_item_t).
  *
@@ -1893,6 +1898,7 @@ subtree_diff(apr_hash_t **diff_changes,
 
   SVN_ERR(svnmover_element_differences(&diff_left_right,
                                        s_left->tree, s_right->tree,
+                                       NULL /*union of s_left & s_right*/,
                                        result_pool, scratch_pool));
 
   for (hi = apr_hash_first(scratch_pool, diff_left_right);
@@ -1950,6 +1956,8 @@ diff_ordering_major_paths(const struct s
 /* Display differences between subtrees LEFT and RIGHT, which are subtrees
  * of branches LEFT_BID and RIGHT_BID respectively.
  *
+ * Diff the union of LEFT's and RIGHT's elements.
+ *
  * Use EDITOR to fetch content when needed.
  *
  * Write a line containing HEADER before any other output, if it is not

Modified: subversion/trunk/tools/dev/svnmover/svnmover.h
URL: http://svn.apache.org/viewvc/subversion/trunk/tools/dev/svnmover/svnmover.h?rev=1717778&r1=1717777&r2=1717778&view=diff
==============================================================================
--- subversion/trunk/tools/dev/svnmover/svnmover.h (original)
+++ subversion/trunk/tools/dev/svnmover/svnmover.h Thu Dec  3 14:59:31 2015
@@ -94,7 +94,12 @@ typedef struct svnmover_wc_version_t
 } svnmover_wc_version_t;
 
 /* Return (left, right) pairs of element content that differ between
- * subtrees LEFT and RIGHT.
+ * LEFT and RIGHT.
+ *
+ * Examine only the elements listed in ELEMENTS, a hash of (eid ->
+ * [anything]). If ELEMENTS is NULL, use the union of LEFT and RIGHT.
+ *
+ * LEFT and/or RIGHT may be null, meaning an empty set of elements.
  *
  * Set *DIFF_P to a hash of (eid -> (svn_element__content_t *)[2]).
  */
@@ -102,6 +107,7 @@ svn_error_t *
 svnmover_element_differences(apr_hash_t **diff_p,
                              const svn_element__tree_t *left,
                              const svn_element__tree_t *right,
+                             apr_hash_t *elements,
                              apr_pool_t *result_pool,
                              apr_pool_t *scratch_pool);
 
@@ -157,8 +163,10 @@ struct conflict_storage_t
 
 /* Merge SRC into TGT, using the common ancestor YCA.
  *
- * Merge the two sets of changes: YCA -> SRC and YCA -> TGT, applying
- * the result to the transaction at TGT.
+ * The elements to merge are the union of the elements in the three input
+ * subtrees (SRC, TGT, YCA). For each such element, merge the two changes:
+ * YCA -> SRC and YCA -> TGT, applying the result to TGT which is assumed
+ * to be a branch in EDIT_TXN.
  *
  * If conflicts arise, return them in *CONFLICT_STORAGE_P; otherwise set
  * that to null.



Mime
View raw message