subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rhuij...@apache.org
Subject svn commit: r1443451 - in /subversion/trunk/subversion: libsvn_client/merge.c tests/cmdline/merge_tree_conflict_tests.py
Date Thu, 07 Feb 2013 13:18:25 GMT
Author: rhuijben
Date: Thu Feb  7 13:18:24 2013
New Revision: 1443451

URL: http://svn.apache.org/viewvc?rev=1443451&view=rev
Log:
Resolve issue #3806, "incoming replacements on local deletes produce
inconsistent tree conflicts".

* subversion/libsvn_client/merge.c
  (check_moved_away): Remove function.
  (record_tree_conflict): Specialize delete and add tree conflicts for moves.
    Store installed conflict in parent directory baton to allow specializing
    later. Make the notify optional.

  (mark_dir_edited,
   mark_file_edited): Remove tree conflict specializing here. Update caller.

  (merge_file_opened): Handle incoming replacement conflicts. Update the
    existing delete conflicts if necessary. Don't notify when overwriting the
    conflict.

  (merge_file_deleted): Update caller.

  (merge_dir_opened): Handle incoming replacement conflicts. Update the
    existing delete conflicts if necessary. Don't notify when overwriting the
    conflict.

  (merge_dir_deleted): Update caller.

* subversion/tests/cmdline/merge_tree_conflict_tests.py
  (merge_replace_causes_tree_conflict2): Remove XFail and update expected
    results for slight output changes.

Modified:
    subversion/trunk/subversion/libsvn_client/merge.c
    subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py

Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1443451&r1=1443450&r2=1443451&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Thu Feb  7 13:18:24 2013
@@ -1124,28 +1124,6 @@ prepare_merge_props_changed(const apr_ar
   return SVN_NO_ERROR;
 }
 
-/* Set *OP_ROOT_ABSPATH if the node at LOCAL_ABSPATH was moved away
- * locally, or set *OP_ROOT_ABSPATH to NULL otherwise. Do not raise an
- * error if the node at LOCAL_ABSPATH does not exist.
- *
- * ### Currently this looks at the highest moved-to but should merge
- * ### be looking at the lowest moved-to?  Or perhaps only the base
- * ### moved-to? */
-static svn_error_t *
-check_moved_away(const char **op_root_abspath,
-                 svn_wc_context_t *wc_ctx,
-                 const char *local_abspath,
-                 apr_pool_t *scratch_pool)
-{
-  const char *moved_to_abspath;
-
-  SVN_ERR(svn_wc__node_was_moved_away(&moved_to_abspath, op_root_abspath,
-                                      wc_ctx, local_abspath,
-                                      scratch_pool, scratch_pool));
-  
-  return SVN_NO_ERROR;
-}
-
 #define CONFLICT_REASON_NONE       ((svn_wc_conflict_reason_t)-1)
 #define CONFLICT_REASON_SKIP       ((svn_wc_conflict_reason_t)-2)
 #define CONFLICT_REASON_SKIP_WC    ((svn_wc_conflict_reason_t)-3)
@@ -1323,12 +1301,11 @@ record_tree_conflict(merge_cmd_baton_t *
                      svn_node_kind_t node_kind,
                      svn_wc_conflict_action_t action,
                      svn_wc_conflict_reason_t reason,
-                     const char *moved_away_op_root_abspath,
+                     const svn_wc_conflict_description2_t *existing_conflict,
+                     svn_boolean_t notify_tc,
                      apr_pool_t *scratch_pool)
 {
-  svn_wc_conflict_description2_t *cf;
-  const svn_wc_conflict_version_t *left;
-  const svn_wc_conflict_version_t *right;
+  svn_wc_context_t *wc_ctx = merge_b->ctx->wc_ctx;
 
   if (merge_b->merge_source.ancestral
       || merge_b->reintegrate_merge)
@@ -1342,30 +1319,80 @@ record_tree_conflict(merge_cmd_baton_t *
 
   if (!merge_b->record_only && !merge_b->dry_run)
     {
+       svn_wc_conflict_description2_t *conflict;
+       const svn_wc_conflict_version_t *left;
+       const svn_wc_conflict_version_t *right;
+       const char *moved_away_op_root_abspath = NULL;
        apr_pool_t *result_pool = parent_baton ? parent_baton->pool
                                               : scratch_pool;
 
+      if (reason == svn_wc_conflict_reason_deleted)
+        {
+          const char *op_root_abspath;
+
+          SVN_ERR(svn_wc__node_was_moved_away(NULL, &op_root_abspath,
+                                              wc_ctx, local_abspath,
+                                              scratch_pool, scratch_pool));
+
+          if (op_root_abspath)
+            {
+              /* Local abspath has been moved away itself, as we only create
+                 tree conflict on the obstructing op-root.
+
+                 If only a descendant is moved away, we call the node itself
+                 deleted. */
+              reason = svn_wc_conflict_reason_moved_away;
+              moved_away_op_root_abspath = "I have absolute no idea what to put in this path,
nor why i should add a path, because I don't change the move";
+              /* local_abspath would be nonsense as that is what the conflict is
+                 on, and the move destination should be obtained by another api
+                 as that might just change while the conflict exists */
+            }
+        }
+      else if (reason == svn_wc_conflict_reason_added)
+        {
+          const char *moved_from_abspath;
+          SVN_ERR(svn_wc__node_was_moved_here(&moved_from_abspath, NULL,
+                                              wc_ctx, local_abspath,
+                                              scratch_pool, scratch_pool));
+          if (moved_from_abspath)
+            reason = svn_wc_conflict_reason_moved_here;
+        }
+
       SVN_ERR(make_conflict_versions(&left, &right, local_abspath, node_kind,
                                      &merge_b->merge_source, merge_b->target,
                                      result_pool, scratch_pool));
 
-      cf = svn_wc_conflict_description_create_tree2(
+      conflict = svn_wc_conflict_description_create_tree2(
                         local_abspath, node_kind, svn_wc_operation_merge,
                         left, right, result_pool);
 
-      cf->action = action;
-      cf->reason = reason;
-      
+      conflict->action = action;
+      conflict->reason = reason;
+
       /* May return SVN_ERR_WC_PATH_UNEXPECTED_STATUS */
-      SVN_ERR(svn_wc__add_tree_conflict(merge_b->ctx->wc_ctx, cf,
+      if (existing_conflict)
+        SVN_ERR(svn_wc__del_tree_conflict(wc_ctx, local_abspath,
+                                          scratch_pool));
+
+      SVN_ERR(svn_wc__add_tree_conflict(merge_b->ctx->wc_ctx, conflict,
                                         moved_away_op_root_abspath,
                                         scratch_pool));
 
+      if (parent_baton)
+        {
+          if (! parent_baton->new_tree_conflicts)
+            parent_baton->new_tree_conflicts = apr_hash_make(result_pool);
+
+          svn_hash_sets(parent_baton->new_tree_conflicts,
+                        apr_pstrdup(result_pool, local_abspath),
+                        conflict);
+        }
+
       /* ### TODO: Store in parent baton */
     }
 
   /* On a replacement we currently get two tree conflicts */
-  if (merge_b->ctx->notify_func2)
+  if (merge_b->ctx->notify_func2 && notify_tc)
     {
       svn_wc_notify_t *notify;
 
@@ -1595,30 +1622,11 @@ mark_dir_edited(merge_cmd_baton_t *merge
   else if (db->tree_conflict_reason != CONFLICT_REASON_NONE)
     {
       /* open_directory() decided that a tree conflict should be raised */
-      const char *moved_away_abspath = NULL;
-
-      if (db->tree_conflict_reason == svn_wc_conflict_reason_deleted)
-        {
-          const char *ignored_path;
-          SVN_ERR(check_moved_away(&ignored_path, merge_b->ctx->wc_ctx,
-                                   local_abspath, scratch_pool));
-
-          if (ignored_path)
-            {
-              /* Local abspath has been moved away itself, as we only create
-                 tree conflict on the obstructing op-root.
-
-                 If only a descendant is moved away, we call the node itself
-                 deleted. */
-              db->tree_conflict_reason = svn_wc_conflict_reason_moved_away;
-              moved_away_abspath = local_abspath;
-            }
-        }
 
       SVN_ERR(record_tree_conflict(merge_b, local_abspath, db->parent_baton,
                                    svn_node_dir, db->tree_conflict_action,
                                    db->tree_conflict_reason,
-                                   moved_away_abspath,
+                                   NULL, TRUE,
                                    scratch_pool));
     }
 
@@ -1692,31 +1700,12 @@ mark_file_edited(merge_cmd_baton_t *merg
     }
   else if (fb->tree_conflict_reason != CONFLICT_REASON_NONE)
     {
-      /* open_directory() decided that a tree conflict should be raised */
-      const char *moved_away_abspath = NULL;
-
-      if (fb->tree_conflict_reason == svn_wc_conflict_reason_deleted)
-        {
-          const char *ignored_path;
-          SVN_ERR(check_moved_away(&ignored_path, merge_b->ctx->wc_ctx,
-                                   local_abspath, scratch_pool));
-
-          if (ignored_path)
-            {
-              /* Local abspath has been moved away itself, as we only create
-                 tree conflict on the obstructing op-root.
-
-                 If only a descendant is moved away, we call the node itself
-                 deleted. */
-              fb->tree_conflict_reason = svn_wc_conflict_reason_moved_away;
-              moved_away_abspath = local_abspath;
-            }
-        }
+      /* open_file() decided that a tree conflict should be raised */
 
       SVN_ERR(record_tree_conflict(merge_b, local_abspath, fb->parent_baton,
                                    svn_node_file, fb->tree_conflict_action,
                                    fb->tree_conflict_reason,
-                                   moved_away_abspath,
+                                   NULL, TRUE,
                                    scratch_pool));
     }
 
@@ -1867,6 +1856,8 @@ merge_file_opened(void **new_file_baton,
     }
   else
     {
+      const svn_wc_conflict_description2_t *old_tc = NULL;
+
       /* The node doesn't exist pre-merge: We have an addition */
       fb->added = TRUE;
       fb->tree_conflict_action = svn_wc_conflict_action_add;
@@ -1880,13 +1871,36 @@ merge_file_opened(void **new_file_baton,
           svn_hash_sets(pdb->pending_deletes, local_abspath, NULL);
         }
 
-      if (svn_hash_gets(merge_b->tree_conflicted_abspaths, local_abspath))
+      if (pdb
+          && pdb->new_tree_conflicts
+          && (old_tc = svn_hash_gets(pdb->new_tree_conflicts, local_abspath)))
         {
-          *skip = TRUE;
+          fb->tree_conflict_action = svn_wc_conflict_action_replace;
+          fb->tree_conflict_reason = old_tc->reason;
 
-          /* ### TODO: Update the tree conflict to store that this is a replace */
+          if (old_tc->reason == svn_wc_conflict_reason_deleted
+              || old_tc->reason == svn_wc_conflict_reason_moved_away)
+            {
+              /* Issue #3806: Incoming replacements on local deletes produce
+                 inconsistent result.
 
-          return SVN_NO_ERROR;
+                 In this specific case we can continue applying the add part
+                 of the replacement. */
+            }
+          else
+            {
+              *skip = TRUE;
+
+              /* Update the tree conflict to store that this is a replace */
+              SVN_ERR(record_tree_conflict(merge_b, local_abspath, pdb,
+                                           svn_node_file,
+                                           fb->tree_conflict_action,
+                                           fb->tree_conflict_reason,
+                                           old_tc, FALSE,
+                                           scratch_pool));
+
+              return SVN_NO_ERROR;
+            }
         }
       else if (! (merge_b->dry_run
                   && ((pdb && pdb->added) || fb->add_is_replace)))
@@ -2388,7 +2402,8 @@ merge_file_deleted(const char *relpath,
       SVN_ERR(record_tree_conflict(merge_b, local_abspath, fb->parent_baton,
                                    svn_node_file,
                                    svn_wc_conflict_action_delete,
-                                   svn_wc_conflict_reason_edited, NULL,
+                                   svn_wc_conflict_reason_edited,
+                                   NULL, TRUE,
                                    scratch_pool));
     }
   
@@ -2593,6 +2608,8 @@ merge_dir_opened(void **new_dir_baton,
     }
   else
     {
+      const svn_wc_conflict_description2_t *old_tc = NULL;
+
       /* The node doesn't exist pre-merge: We have an addition */
       db->added = TRUE;
       db->tree_conflict_action = svn_wc_conflict_action_add;
@@ -2606,17 +2623,41 @@ merge_dir_opened(void **new_dir_baton,
           svn_hash_sets(pdb->pending_deletes, local_abspath, NULL);
         }
 
-      if (svn_hash_gets(merge_b->tree_conflicted_abspaths, local_abspath))
+      if (pdb
+          && pdb->new_tree_conflicts
+          && (old_tc = svn_hash_gets(pdb->new_tree_conflicts, local_abspath)))
         {
-          *skip = TRUE;
-          *skip_children = TRUE;
+          db->tree_conflict_action = svn_wc_conflict_action_replace;
+          db->tree_conflict_reason = old_tc->reason;
 
-          /* ### TODO: Update the tree conflict to store that this is a replace */
+          if (old_tc->reason == svn_wc_conflict_reason_deleted
+             || old_tc->reason == svn_wc_conflict_reason_moved_away)
+            {
+              /* Issue #3806: Incoming replacements on local deletes produce
+                 inconsistent result.
 
-          return SVN_NO_ERROR;
+                 In this specific case we can continue applying the add part
+                 of the replacement. */
+            }
+          else
+            {
+              *skip = TRUE;
+              *skip_children = TRUE;
+
+              /* Update the tree conflict to store that this is a replace */
+              SVN_ERR(record_tree_conflict(merge_b, local_abspath, pdb,
+                                           svn_node_dir,
+                                           db->tree_conflict_action,
+                                           db->tree_conflict_reason,
+                                           old_tc, FALSE,
+                                           scratch_pool));
+
+              return SVN_NO_ERROR;
+            }
         }
-      else if (! (merge_b->dry_run
-                  && ((pdb && pdb->added) || db->add_is_replace)))
+      
+      if (! (merge_b->dry_run
+             && ((pdb && pdb->added) || db->add_is_replace)))
         {
           svn_wc_notify_state_t obstr_state;
           svn_node_kind_t kind;
@@ -2683,6 +2724,19 @@ merge_dir_opened(void **new_dir_baton,
             SVN_ERR(svn_io_dir_make(local_abspath, APR_OS_DEFAULT,
                                     scratch_pool));
 
+          if (old_tc)
+            {
+              /* svn_wc_add4 and svn_wc_add_from_disk2 can't add a node
+                 over an existing tree conflict */
+
+              /* ### These functions should take some tree conflict argument
+                     and allow overwriting the tc when one is passed */
+
+              SVN_ERR(svn_wc__del_tree_conflict(merge_b->ctx->wc_ctx,
+                                                local_abspath,
+                                                scratch_pool));
+            }
+
           if (merge_b->same_repos)
             {
               const char *original_url;
@@ -2715,6 +2769,17 @@ merge_dir_opened(void **new_dir_baton,
                                             NULL, NULL /* no notify! */,
                                             scratch_pool));
             }
+
+          if (old_tc != NULL)
+            {
+              /* ### Should be atomic with svn_wc_add(4|_from_disk2)() */
+              SVN_ERR(record_tree_conflict(merge_b, local_abspath, pdb,
+                                           svn_node_dir,
+                                           db->tree_conflict_action,
+                                           db->tree_conflict_reason,
+                                           old_tc, FALSE,
+                                           scratch_pool));
+            }
         }
 
       if (! db->shadowed && !merge_b->record_only)
@@ -3120,7 +3185,8 @@ merge_dir_deleted(const char *relpath,
       SVN_ERR(record_tree_conflict(merge_b, local_abspath, db->parent_baton,
                                    svn_node_dir,
                                    svn_wc_conflict_action_delete,
-                                   svn_wc_conflict_reason_edited, NULL,
+                                   svn_wc_conflict_reason_edited,
+                                   NULL, TRUE,
                                    scratch_pool));
     }
   else

Modified: subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py?rev=1443451&r1=1443450&r2=1443451&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py Thu Feb  7 13:18:24
2013
@@ -1721,7 +1721,6 @@ def merge_replace_causes_tree_conflict(s
   actions.run_and_verify_status(wc_dir, expected_status)
 
 #----------------------------------------------------------------------
-@XFail()
 @Issue(3806)
 def merge_replace_causes_tree_conflict2(sbox):
   "replace vs. delete tree-conflicts"
@@ -1782,13 +1781,18 @@ def merge_replace_causes_tree_conflict2(
                         'A/D/H', 'A/D/H/chi', 'A/D/H/omega', 'A/D/H/psi',
                         status='D ')
 
+  # H is now a file. This hides the status of the descendants.
+  expected_status.remove('A/D/H/chi', 'A/D/H/psi', 'A/D/H/omega')
+
   # Merge them one by one to see all the errors.
 
   ### A file-with-file replacement onto a deleted file.
   # svn merge $URL/A/mu $URL/branch/mu A/mu
   expected_stdout = expected_merge_output(None, [
     '   C ' + A_mu + '\n',  # merge
+    'A    ' + A_mu + '\n',  # merge
     " U   " + A + "\n",     # mergeinfo
+    " U   " + A_mu + "\n",  # mergeinfo -> 'RM' status
   ], target=A, two_url=True, tree_conflicts=1)
 
   actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'merge',
@@ -1800,13 +1804,14 @@ def merge_replace_causes_tree_conflict2(
   #
   #  D     C merge_tree_conflict_tests-23\A\mu
   #      >   local delete, incoming replace upon merge
-  expected_status.tweak('A/mu', status='R ', wc_rev='-', copied='+',
+  expected_status.tweak('A/mu', status='RM', wc_rev='-', copied='+',
                         treeconflict='C')
 
   ### A dir-with-dir replacement onto a deleted directory.
   # svn merge $URL/A/B $URL/branch/B A/B
   expected_stdout = expected_merge_output(None, [
     '   C ' + A_B_E + '\n',   # merge
+    'A    ' + A_B_E + '\n',   # merge
     " U   " + A_B + "\n",     # mergeinfo
   ], target=A_B, two_url=True, tree_conflicts=1)
 
@@ -1826,8 +1831,8 @@ def merge_replace_causes_tree_conflict2(
   # svn merge --depth=immediates $URL/A/D $URL/branch/D A/D
   expected_stdout = expected_merge_output(None, [
     '   C ' + A_D_H + '\n',   # merge
+    'A    ' + A_D_H + '\n',   # merge
     " U   " + A_D + "\n",     # mergeinfo
-    " U   " + A_D_G + "\n",
   ], target=A_D, two_url=True, tree_conflicts=1)
 
   actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'merge',
@@ -1846,7 +1851,9 @@ def merge_replace_causes_tree_conflict2(
   # svn merge $URL/A/D/G $URL/branch/D/G A/D/G
   expected_stdout = expected_merge_output(None, [
     '   C ' + A_D_G_pi + '\n',  # merge
-  ], target=A_D_G, elides=[A_D_G_pi, A_D_G], two_url=True, tree_conflicts=1)
+    'A    ' + A_D_G_pi + '\n',  # merge
+    " U   " + A_D_G + "\n",     # mergeinfo
+  ], target=A_D_G, two_url=True, tree_conflicts=1)
 
   actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'merge',
     url_A_D_G, url_branch_D_G, A_D_G)
@@ -1865,7 +1872,8 @@ def merge_replace_causes_tree_conflict2(
 
   # Check the tree conflict types:
   expected_stdout = '(R.*)|(Summary of conflicts.*)|(  Tree conflicts.*)' \
-                    '|(.*local delete, incoming replace upon merge.*)'
+                    '|(.*local delete, incoming replace upon merge.*)' \
+                    '|(      \>.*)'
   tree_conflicted_path = [A_B_E, A_mu, A_D_G_pi, A_D_H]
   for path in tree_conflicted_path:
     actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'st',



Mime
View raw message