Author: stsp
Date: Fri Sep 16 13:16:37 2016
New Revision: 1761029

URL: http://svn.apache.org/viewvc?rev=1761029&view=rev
Log:
Change the way the conflict resolver merges incoming moves.

When ^/trunk/A is moved to ^/trunk/B and this move is merged to a working
copy of ^/branch, we'd like to create a corresponding move from ^/branch/A
to ^/branch/B. However, the merge adds the copy-half of the incoming move
as a copy from ^/trunk/B.

The resolver used to merge changes from ^/branch into B (copied from ^/trunk/B),
and then performed a meta-data-only move from A to B. This approach has some
problems, such as unwanted 'replacements' of children.
For example:
If the branch added file A/new, trunk moved A to B, and we're merging trunk
into a wc of branch, then the 'merge incoming move' option would produce a
merge commit where B/new was replaced:
  D /branch/A
  A /branch/B (from /branch/A:4)
  R /branch/B/new (from /branch/A/new:4)
This replacment happened because the meta-data-only move of A to B created
a new undesired op-depth layer in wc.db.

So we'll try another approach: Revert the copy of B added by the merge,
move A to B locally, and then run a merge from ^/trunk into B.
This is not perfect either but the end result is closer to what we would like.
Reverting local changes might not always be a good idea but the resolver
already assumes that users do not make manual changes to the working copy
before resolving conflicts. And reverting an otherwise unmodified copy of
an existing repository subtree cannot lead to data loss.

* subversion/libsvn_client/conflicts.c
  (resolve_incoming_move_dir_merge): Use a "revert, then move, then merge"
   resolution strategy as described above.

* subversion/tests/libsvn_client/conflicts-test.c
  (test_merge_incoming_move_dir,
   test_merge_incoming_move_dir2): Fix test expectations.

Modified:
    subversion/trunk/subversion/libsvn_client/conflicts.c
    subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c

Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/conflicts.c?rev=1761029&r1=1761028&r2=1761029&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
+++ subversion/trunk/subversion/libsvn_client/conflicts.c Fri Sep 16 13:16:37 
2016
@@ -6548,12 +6548,39 @@ resolve_incoming_move_dir_merge(svn_clie
 
   if (operation == svn_wc_operation_merge)
     {
-      /* Merge YCA_URL->VICTIM_URL into the incoming move target directory. */
+      const char *move_target_url;
+      svn_opt_revision_t incoming_new_opt_rev;
+
+      /* Revert the incoming move target directory. */
+      SVN_ERR(svn_wc_revert5(ctx->wc_ctx, moved_to_abspath, svn_depth_infinity,
+                             FALSE, NULL, TRUE, FALSE,
+                             NULL, NULL, /* no cancellation */
+                             ctx->notify_func2, ctx->notify_baton2,
+                             scratch_pool));
+
+      /* The move operation is not part of natural history. We must replicate
+       * this move in our history. Record a move in the working copy. */
+      err = svn_wc__move2(ctx->wc_ctx, local_abspath, moved_to_abspath,
+                          FALSE, /* this is not a meta-data only move */
+                          TRUE, /* allow mixed-revisions just in case */
+                          NULL, NULL, /* don't allow user to cancel here */
+                          ctx->notify_func2, ctx->notify_baton2,
+                          scratch_pool);
+      if (err)
+        goto unlock_wc;
+
+      /* Merge YCA_URL@YCA_REV->MOVE_TARGET_URL@MERGE_RIGHT into move target. 
*/
+      move_target_url = apr_pstrcat(scratch_pool, repos_root_url, "/",
+                                    details->move_target_repos_relpath,
+                                    SVN_VA_NULL);
+      incoming_new_opt_rev.kind = svn_opt_revision_number;
+      incoming_new_opt_rev.value.number = incoming_new_pegrev;
       err = svn_client__merge_locked(&conflict_report,
                                      yca_loc->url, &yca_opt_rev,
-                                     victim_url, &victim_opt_rev,
+                                     move_target_url, &incoming_new_opt_rev,
                                      moved_to_abspath, svn_depth_infinity,
-                                     FALSE, FALSE, FALSE, FALSE, FALSE,
+                                     TRUE, TRUE, /* do a no-ancestry merge */
+                                     FALSE, FALSE, FALSE,
                                      TRUE, /* Allow mixed-rev just in case,
                                             * since conflict victims can't be
                                             * updated to straighten out
@@ -6562,31 +6589,32 @@ resolve_incoming_move_dir_merge(svn_clie
       if (err)
         goto unlock_wc;
     }
-
-  /* Merge local modifications into the incoming move target dir. */
-  err = svn_wc__has_local_mods(&is_modified, ctx->wc_ctx, local_abspath,
-                               TRUE, ctx->cancel_func, ctx->cancel_baton,
-                               scratch_pool);
-  if (err)
-    goto unlock_wc;
-
-  if (is_modified)
+  else
     {
-      err = svn_wc__conflict_tree_merge_local_changes(ctx->wc_ctx,
-                                                      local_abspath,
-                                                      moved_to_abspath,
-                                                      ctx->cancel_func,
-                                                      ctx->cancel_baton,
-                                                      ctx->notify_func2,
-                                                      ctx->notify_baton2,
-                                                      scratch_pool);
+      SVN_ERR_ASSERT(operation == svn_wc_operation_update ||
+                     operation == svn_wc_operation_switch);
+
+      /* Merge local modifications into the incoming move target dir. */
+      err = svn_wc__has_local_mods(&is_modified, ctx->wc_ctx, local_abspath,
+                                   TRUE, ctx->cancel_func, ctx->cancel_baton,
+                                   scratch_pool);
       if (err)
         goto unlock_wc;
-    }
 
-  if (operation == svn_wc_operation_update ||
-      operation == svn_wc_operation_switch)
-    {
+      if (is_modified)
+        {
+          err = svn_wc__conflict_tree_merge_local_changes(ctx->wc_ctx,
+                                                          local_abspath,
+                                                          moved_to_abspath,
+                                                          ctx->cancel_func,
+                                                          ctx->cancel_baton,
+                                                          ctx->notify_func2,
+                                                          ctx->notify_baton2,
+                                                          scratch_pool);
+          if (err)
+            goto unlock_wc;
+        }
+
       /* The move operation is part of our natural history.
        * Delete the tree conflict victim (clears the tree conflict marker). */
       err = svn_wc_delete4(ctx->wc_ctx, local_abspath, FALSE, FALSE,
@@ -6596,25 +6624,6 @@ resolve_incoming_move_dir_merge(svn_clie
       if (err)
         goto unlock_wc;
     }
-  else if (operation == svn_wc_operation_merge)
-    {
-      /* The move operation is not part of natural history. We must replicate
-       * this move in our history. Record a move in the working copy. */
-      err = svn_wc__move2(ctx->wc_ctx, local_abspath, moved_to_abspath,
-                          TRUE, /* this is a meta-data only move */
-                          FALSE, /* mixed-revisions don't apply to files */
-                          NULL, NULL, /* don't allow user to cancel here */
-                          NULL, NULL, /* no extra notification */
-                          scratch_pool);
-      if (err)
-        goto unlock_wc;
-    }
-  else
-    return svn_error_createf(SVN_ERR_WC_CORRUPT, NULL,
-                             _("Invalid operation code '%d' recorded for "
-                               "conflict at '%s'"), operation,
-                             svn_dirent_local_style(local_abspath,
-                                                    scratch_pool));
 
   if (ctx->notify_func2)
     {

Modified: subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c?rev=1761029&r1=1761028&r2=1761029&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c Fri Sep 16 
13:16:37 2016
@@ -2204,7 +2204,7 @@ test_merge_incoming_move_dir(const svn_t
   SVN_TEST_ASSERT(!status->conflicted);
   SVN_TEST_ASSERT(status->node_status == svn_wc_status_added);
   SVN_TEST_ASSERT(status->text_status == svn_wc_status_normal);
-  SVN_TEST_ASSERT(status->prop_status == svn_wc_status_modified);
+  SVN_TEST_ASSERT(status->prop_status == svn_wc_status_none);
   SVN_TEST_ASSERT(status->copied);
   SVN_TEST_ASSERT(!status->switched);
   SVN_TEST_ASSERT(!status->file_external);
@@ -2213,10 +2213,7 @@ test_merge_incoming_move_dir(const svn_t
   SVN_TEST_ASSERT(status->moved_to_abspath == NULL);
 
   /* Ensure that the edited file has the expected content. */
-  child_path = svn_relpath_join(branch_path,
-                                svn_relpath_join(deleted_dir_name,
-                                                 deleted_dir_child,
-                                                 b->pool),
+  child_path = svn_relpath_join(moved_to_path, deleted_dir_child,
                                 b->pool);
   SVN_ERR(svn_stream_open_readonly(&stream, sbox_wc_path(b, child_path),
                                    b->pool, b->pool));
@@ -2294,7 +2291,7 @@ test_merge_incoming_move_dir2(const svn_
   SVN_TEST_ASSERT(!status->conflicted);
   SVN_TEST_ASSERT(status->node_status == svn_wc_status_added);
   SVN_TEST_ASSERT(status->text_status == svn_wc_status_normal);
-  SVN_TEST_ASSERT(status->prop_status == svn_wc_status_modified);
+  SVN_TEST_ASSERT(status->prop_status == svn_wc_status_none);
   SVN_TEST_ASSERT(status->copied);
   SVN_TEST_ASSERT(!status->switched);
   SVN_TEST_ASSERT(!status->file_external);
@@ -2303,10 +2300,7 @@ test_merge_incoming_move_dir2(const svn_
   SVN_TEST_ASSERT(status->moved_to_abspath == NULL);
 
   /* Ensure that the edited file has the expected content. */
-  child_path = svn_relpath_join(branch_path,
-                                svn_relpath_join(deleted_dir_name,
-                                                 deleted_dir_child,
-                                                 b->pool),
+  child_path = svn_relpath_join(moved_to_path, deleted_dir_child,
                                 b->pool);
   SVN_ERR(svn_stream_open_readonly(&stream, sbox_wc_path(b, child_path),
                                    b->pool, b->pool));


Reply via email to