You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2016/03/08 17:21:06 UTC

svn commit: r1734106 - in /subversion/trunk/subversion: libsvn_client/merge.c tests/cmdline/merge_tree_conflict_tests.py

Author: stsp
Date: Tue Mar  8 16:21:06 2016
New Revision: 1734106

URL: http://svn.apache.org/viewvc?rev=1734106&view=rev
Log:
Fix a bug where the wrong source left revision was recorded in a tree
conflict for items added within the merge source left/right revision range.

See the regression test added in this commit for a bug reproduction recipe.

* subversion/libsvn_client/merge.c
  (find_nearest_ancestor_with_intersecting_ranges): Add forward declaration.
  (record_tree_conflict): Use find_nearest_ancestor_with_intersecting_ranges()
   to figure out the actual revision range merged for a node, and record this
   revision range in the tree conflict.
   
* subversion/tests/cmdline/merge_tree_conflict_tests.py
  (added_revision_recording_in_tree_conflict, test_list): New regression test. 

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=1734106&r1=1734105&r2=1734106&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Tue Mar  8 16:21:06 2016
@@ -1269,6 +1269,15 @@ record_skip(merge_cmd_baton_t *merge_b,
   return SVN_NO_ERROR;
 }
 
+/* Forward declaration */
+static svn_client__merge_path_t *
+find_nearest_ancestor_with_intersecting_ranges(
+  svn_revnum_t *start,
+  svn_revnum_t *end,
+  const apr_array_header_t *children_with_mergeinfo,
+  svn_boolean_t path_is_own_ancestor,
+  const char *local_abspath);
+
 /* Record a tree conflict in the WC, unless this is a dry run or a record-
  * only merge, or if a tree conflict is already flagged for the VICTIM_PATH.
  * (The latter can happen if a merge-tracking-aware merge is doing multiple
@@ -1340,11 +1349,45 @@ record_tree_conflict(merge_cmd_baton_t *
             reason = svn_wc_conflict_reason_moved_here;
         }
 
-      SVN_ERR(make_conflict_versions(&left, &right, local_abspath,
-                                     merge_left_node_kind,
-                                     merge_right_node_kind,
-                                     &merge_b->merge_source, merge_b->target,
-                                     result_pool, scratch_pool));
+      if (HONOR_MERGEINFO(merge_b) && merge_b->merge_source.ancestral)
+        {
+          struct merge_source_t *source;
+          svn_client__pathrev_t *loc1;
+          svn_client__pathrev_t *loc2;
+          svn_merge_range_t range =
+            {SVN_INVALID_REVNUM, SVN_INVALID_REVNUM, TRUE};
+
+          /* We are honoring mergeinfo so do not blindly record
+           * a conflict describing the merge of
+           * SOURCE->LOC1->URL@SOURCE->LOC1->REV through
+           * SOURCE->LOC2->URL@SOURCE->LOC2->REV
+           * but figure out the actual revision range merged. */
+          (void)find_nearest_ancestor_with_intersecting_ranges(
+            &(range.start), &(range.end),
+            merge_b->notify_begin.nodes_with_mergeinfo,
+            action != svn_wc_conflict_action_delete,
+            local_abspath);
+          loc1 = svn_client__pathrev_dup(merge_b->merge_source.loc1,
+                                         scratch_pool);
+          loc2 = svn_client__pathrev_dup(merge_b->merge_source.loc2,
+                                         scratch_pool);
+          loc1->rev = range.start;
+          loc2->rev = range.end;
+          source = merge_source_create(loc1, loc2,
+                                       merge_b->merge_source.ancestral,
+                                       scratch_pool);
+          SVN_ERR(make_conflict_versions(&left, &right, local_abspath,
+                                         merge_left_node_kind,
+                                         merge_right_node_kind,
+                                         source, merge_b->target,
+                                         result_pool, scratch_pool));
+        }
+      else
+        SVN_ERR(make_conflict_versions(&left, &right, local_abspath,
+                                       merge_left_node_kind,
+                                       merge_right_node_kind,
+                                       &merge_b->merge_source, merge_b->target,
+                                       result_pool, scratch_pool));
 
       /* Fix up delete of file, add of dir replacement (or other way around) */
       if (existing_conflict != NULL && existing_conflict->src_left_version)

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=1734106&r1=1734105&r2=1734106&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py Tue Mar  8 16:21:06 2016
@@ -2192,6 +2192,112 @@ def merge_obstruction_recording(sbox):
 
   # A resolver action could be smarter though...
 
+def added_revision_recording_in_tree_conflict(sbox):
+  "tree conflict stores added revision for victim"
+
+  sbox.build(empty=True)
+  wc_dir = sbox.wc_dir
+
+  sbox.simple_mkdir('trunk')
+  sbox.simple_commit() #r1
+
+  # Create a branch
+  svntest.actions.run_and_verify_svn(None, [],
+                                     'copy', sbox.repo_url + '/trunk',
+                                     sbox.repo_url + '/branch',
+                                     '-mcopy') # r2
+
+  sbox.simple_add_text('The file on trunk\n', 'trunk/foo')
+  sbox.simple_commit() #r3
+
+  sbox.simple_update()
+
+  # Merge ^/trunk into ^/branch
+  expected_output = svntest.wc.State(sbox.ospath('branch'), {
+    'foo'          : Item(status='A '),
+  })
+  expected_mergeinfo_output = wc.State(sbox.ospath('branch'), {
+    ''                  : Item(status=' U')
+  })
+  expected_elision_output = wc.State(wc_dir, {
+  })
+  expected_disk = wc.State('', {
+    'foo'              : Item(contents="The file on trunk\n"),
+    '.'                 : Item(props={u'svn:mergeinfo': u'/trunk:2-3'}),
+  })
+  expected_status = wc.State(sbox.ospath('branch'), {
+    ''                  : Item(status=' M', wc_rev='3'),
+    'foo'              : Item(status='A ', copied='+', wc_rev='-'),
+  })
+  expected_skip = wc.State('', {
+  })
+  svntest.actions.run_and_verify_merge(sbox.ospath('branch'), None, None,
+                                       sbox.repo_url + '/trunk',
+                                       None,
+                                       expected_output,
+                                       expected_mergeinfo_output,
+                                       expected_elision_output,
+                                       expected_disk,
+                                       expected_status,
+                                       expected_skip,
+                                       check_props=True)
+
+  sbox.simple_commit() #r4
+
+  # Edit the file on the branch
+  sbox.simple_append('branch/foo', 'The file on the branch\n')
+  sbox.simple_commit() #r5
+
+  # Replace file with a directory on trunk
+  sbox.simple_rm('trunk/foo')
+  sbox.simple_mkdir('trunk/foo')
+  sbox.simple_commit() #r6
+
+  sbox.simple_update()
+
+  # Merge ^/trunk into ^/branch
+  expected_output = svntest.wc.State(sbox.ospath('branch'), {
+    'foo'               : Item(status='  ', treeconflict='C')
+  })
+  expected_mergeinfo_output = wc.State(sbox.ospath('branch'), {
+    ''                  : Item(status=' U'),
+  })
+  expected_elision_output = wc.State(wc_dir, {
+  })
+  expected_disk = wc.State('', {
+    'foo'     : Item(contents="The file on trunk\nThe file on the branch\n"),
+      '.'     : Item(props={u'svn:mergeinfo': u'/trunk:2-6'}),
+  })
+  expected_status = wc.State(sbox.ospath('branch'), {
+    ''                  : Item(status=' M', wc_rev='6'),
+    'foo'               : Item(status='  ', treeconflict='C', wc_rev='6'),
+  })
+  expected_skip = wc.State('', {
+  })
+  svntest.actions.run_and_verify_merge(sbox.ospath('branch'), None, None,
+                                       sbox.repo_url + '/trunk',
+                                       None,
+                                       expected_output,
+                                       expected_mergeinfo_output,
+                                       expected_elision_output,
+                                       expected_disk,
+                                       expected_status,
+                                       expected_skip,
+                                       check_props=True)
+
+  # Ensure that revisions in tree conflict info match what we expect.
+  # We used to record source left as ^/trunk/foo@1 instead of ^/trunk/foo@3.
+  # Note that foo was first added in r3.
+  expected_info = [
+    {
+      "Path" : re.escape(sbox.ospath('branch/foo')),
+      "Tree conflict": re.escape(
+        'local file edit, incoming replace with dir upon merge' +
+        ' Source  left: (file) ^/trunk/foo@3' +
+        ' Source right: (dir) ^/trunk/foo@6'),
+    },
+  ]
+  svntest.actions.run_and_verify_info(expected_info, sbox.ospath('branch/foo'))
 
 ########################################################################
 # Run the tests
@@ -2225,6 +2331,7 @@ test_list = [ None,
               merge_replace_on_del_fails,
               merge_conflict_details,
               merge_obstruction_recording,
+              added_revision_recording_in_tree_conflict,
              ]
 
 if __name__ == '__main__':