You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2016/02/22 18:42:58 UTC
svn commit: r1731699 - in /subversion/trunk/subversion: libsvn_diff/diff3.c
tests/cmdline/patch_tests.py tests/libsvn_diff/diff-diff3-test.c
Author: rhuijben
Date: Mon Feb 22 17:42:58 2016
New Revision: 1731699
URL: http://svn.apache.org/viewvc?rev=1731699&view=rev
Log:
Make our 3-way-diff/merge handling raise a conflict in a very specific case
where changes in different ranges that touched each other were applied as
adding both ranges in Subversion while other merge tools create a conflict.
With our current diff output on these conflicts the conflict is much easier
to diagnose and resolve than it was in 2003 when this behavior was first
documented by adding a regression test.
This resolves the problem
Found by: Michal Matyl <Michal.Matyl{_AT_}zf.com>
which is essentially about the controversial behavior discussed in 2003 in
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=35014
The patch_tests.py change is a good example of the change.
* subversion/libsvn_diff/diff3.c
(svn_diff_diff3_2): Merge adjacent svn_diff__type_diff_modified and
svn_diff__type_diff_latest ranges to a single conflict range.
* subversion/tests/cmdline/patch_tests.py
(patch_merge): Remove XFail marker. Update expected result.
* subversion/tests/libsvn_diff/diff-diff3-test.c
(merge_adjacent_changes): Update expected result.
(three_way_double_add): Tweak expected result.
(test_funcs): Mark three_way_double_add as PASS.
Modified:
subversion/trunk/subversion/libsvn_diff/diff3.c
subversion/trunk/subversion/tests/cmdline/patch_tests.py
subversion/trunk/subversion/tests/libsvn_diff/diff-diff3-test.c
Modified: subversion/trunk/subversion/libsvn_diff/diff3.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/diff3.c?rev=1731699&r1=1731698&r2=1731699&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_diff/diff3.c (original)
+++ subversion/trunk/subversion/libsvn_diff/diff3.c Mon Feb 22 17:42:58 2016
@@ -325,6 +325,7 @@ svn_diff_diff3_2(svn_diff_t **diff,
/* Produce a merged diff */
{
svn_diff_t **diff_ref = diff;
+ svn_diff_t *diff_last = NULL;
apr_off_t original_start = 1;
apr_off_t modified_start = 1;
@@ -434,6 +435,7 @@ svn_diff_diff3_2(svn_diff_t **diff,
if (is_modified || is_latest)
{
+ svn_boolean_t add_diff = TRUE;
modified_length = modified_sync - modified_start;
latest_length = latest_sync - latest_start;
@@ -454,17 +456,41 @@ svn_diff_diff3_2(svn_diff_t **diff,
&position_list[2],
num_tokens,
pool);
+ /* add_diff = TRUE */
}
- else if (is_modified)
+ else if (is_modified
+ && (!diff_last
+ || diff_last->type != svn_diff__type_diff_latest))
{
(*diff_ref)->type = svn_diff__type_diff_modified;
+ /* add_diff = TRUE */
}
- else
+ else if (is_latest
+ && (!diff_last
+ || diff_last->type != svn_diff__type_diff_modified))
{
(*diff_ref)->type = svn_diff__type_diff_latest;
+ /* add_diff = TRUE */
}
+ else
+ {
+ /* We have a latest and a modified region that touch each other,
+ but not directly change the same location. Create a single
+ conflict region to properly mark a conflict, and to ease
+ resolving. */
+ diff_last->type = svn_diff__type_conflict;
+ diff_last->original_length += (*diff_ref)->original_length;
+ diff_last->modified_length += (*diff_ref)->modified_length;
+ diff_last->latest_length += (*diff_ref)->latest_length;
- diff_ref = &(*diff_ref)->next;
+ add_diff = FALSE;
+ }
+
+ if (add_diff)
+ {
+ diff_last = *diff_ref;
+ diff_ref = &(*diff_ref)->next;
+ }
}
/* Detect EOF */
@@ -490,6 +516,7 @@ svn_diff_diff3_2(svn_diff_t **diff,
(*diff_ref)->latest_length = common_length;
(*diff_ref)->resolved_diff = NULL;
+ diff_last = *diff_ref;
diff_ref = &(*diff_ref)->next;
}
Modified: subversion/trunk/subversion/tests/cmdline/patch_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/patch_tests.py?rev=1731699&r1=1731698&r2=1731699&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/patch_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/patch_tests.py Mon Feb 22 17:42:58 2016
@@ -7679,7 +7679,6 @@ def patch_missed_trail(sbox):
expected_output, expected_disk,
expected_status, expected_skip)
-@XFail()
def patch_merge(sbox):
"patching a specific merge"
@@ -7742,24 +7741,31 @@ def patch_merge(sbox):
expected_output, None,
None, expected_skip)
- # Currently we see D E F doubled, that is certainly bad behavior.
- # I could imaging that just applying the 'C' line change would be ok,
- # but most likely a text conflict is the proper thing to do here.
expected_disk = svntest.main.greek_state.copy()
expected_disk.add({
'new.txt' : Item(contents='A\n'
'B\n'
- ' C\n'
+ '<<<<<<< .mine\n'
+ 'C\n'
'D\n'
'E\n'
'F\n'
- #'D\n' # Doubled???
- #'E\n' # Doubled???
- #'F\n' # Doubled???
- 'J\n'
- 'K\n'
- 'L')})
- expected_output.tweak('new.txt', status='G ')
+ '||||||| .r2\n'
+ 'C\n'
+ '=======\n'
+ ' C\n'
+ 'D\n'
+ 'E\n'
+ 'F\n'
+ '>>>>>>> .r3\n'
+ 'J\n'
+ 'K\n'
+ 'L'),
+ 'new.txt.mine' : Item(contents="A\nB\nC\nD\nE\nF\nJ\nK\nL"),
+ 'new.txt.r2' : Item(contents="A\nB\nC\nJ\nK\nL"),
+ 'new.txt.r3' : Item(contents="A\nB\n C\nD\nE\nF\nJ\nK\nL"),
+ })
+ expected_output.tweak('new.txt', status='C ')
svntest.actions.run_and_verify_update(wc_dir, expected_output, expected_disk,
None, [])
@@ -7773,7 +7779,32 @@ def patch_merge(sbox):
expected_output, None,
None, expected_skip)
- expected_output.tweak('new.txt', status='G ')
+ expected_output.tweak('new.txt', status='C ')
+ expected_disk = svntest.main.greek_state.copy()
+ expected_disk.add({
+ 'new.txt' : Item(contents='A\n'
+ 'B\n'
+ '<<<<<<< .working\n'
+ 'C\n'
+ 'D\n'
+ 'E\n'
+ 'F\n'
+ '||||||| .merge-left.r2\n'
+ 'C\n'
+ '=======\n'
+ ' C\n'
+ 'D\n'
+ 'E\n'
+ 'F\n'
+ '>>>>>>> .merge-right.r3\n'
+ 'J\n'
+ 'K\n'
+ 'L'),
+ 'new.txt.working' : Item(contents="A\nB\nC\nD\nE\nF\nJ\nK\nL"),
+ 'new.txt.merge-left.r2' : Item(contents="A\nB\nC\nJ\nK\nL"),
+ 'new.txt.merge-right.r3' : Item(contents="A\nB\n C\nD\nE\nF\nJ\nK\nL"),
+ })
+
svntest.actions.run_and_verify_merge(wc_dir, 2, 3, repo_url, repo_url,
expected_output, None, None,
expected_disk, None,
Modified: subversion/trunk/subversion/tests/libsvn_diff/diff-diff3-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_diff/diff-diff3-test.c?rev=1731699&r1=1731698&r2=1731699&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_diff/diff-diff3-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_diff/diff-diff3-test.c Mon Feb 22 17:42:58 2016
@@ -2407,6 +2407,8 @@ merge_with_part_already_present(apr_pool
/* Merge is more "aggressive" about resolving conflicts than traditional
* patch or diff3. Some people consider this behaviour to be a bug, see
* http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=35014
+ *
+ * The original behavior of this test (added in 2003) was tweaked in 2016.
*/
static svn_error_t *
merge_adjacent_changes(apr_pool_t *pool)
@@ -2428,8 +2430,13 @@ merge_adjacent_changes(apr_pool_t *pool)
"zig\n"
"foo\n"
+ "<<<<<<< adj2\n"
"new_bar\n"
- "new_baz\n",
+ "baz\n"
+ "=======\n"
+ "bar\n"
+ "new_baz\n"
+ ">>>>>>> adj3\n",
NULL,
svn_diff_conflict_display_modified_latest,
@@ -2996,6 +3003,8 @@ three_way_double_add(apr_pool_t *pool)
"D\n" /* New line 1a */
"E\n" /* New line 2a */
"F\n" /* New line 3a*/
+ "||||||| doubleadd1\n"
+ "C\n"
"=======\n"
"O\n"
"P\n" /* New line 1b */
@@ -3009,7 +3018,7 @@ three_way_double_add(apr_pool_t *pool)
svn_diff_conflict_display_modified_original_latest,
pool));
- SVN_ERR(three_way_merge("doubleadd1", "doubleadd2", "doubleadd3",
+ SVN_ERR(three_way_merge("doubleadd4", "doubleadd5", "doubleadd6",
"A\n"
"B\n"
"C\n"
@@ -3044,18 +3053,21 @@ three_way_double_add(apr_pool_t *pool)
will be a PASS. */
"A\n"
"B\n"
- "<<<<<<< doubleadd2\n"
+ "<<<<<<< doubleadd5\n"
"C\n"
"D\n" /* New line 1a */
"E\n" /* New line 2a */
"F\n" /* New line 3a*/
+ "||||||| doubleadd4\n"
+ "C\n"
+ "J\n"
"=======\n"
"O\n"
"P\n" /* New line 1b */
"Q\n" /* New line 2b */
"R\n" /* New line 3b */
"J\n"
- ">>>>>>> doubleadd3\n"
+ ">>>>>>> doubleadd6\n"
"K\n"
"L",
NULL,
@@ -3107,7 +3119,7 @@ static struct svn_test_descriptor_t test
"2-way issue #3362 test v1"),
SVN_TEST_PASS2(two_way_issue_3362_v2,
"2-way issue #3362 test v2"),
- SVN_TEST_XFAIL2(three_way_double_add,
+ SVN_TEST_PASS2(three_way_double_add,
"3-way merge, double add"),
SVN_TEST_NULL
};