You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Michal Matyl <Mi...@zf.com> on 2016/02/11 11:19:02 UTC
(Unknown)
Hello,
Recently I have encountered highly unexpected SVN behavior that resulted in code duplication after merging a branch into trunk. I'm not sure whether this behavior comes from a bug or my wrong understanding of merge process. After some time of investigation I come to a conclusion that this is most likely a bug, so I'd like to ask you to verify it. I have prepared and attached a repo that consists of minimal reproducible example. I will refer to the example to describe my expectation vs actual SVN behavior.
The example is about a simple branch merging with default settings and a classic conflict situation, no fancy features or complex trunk-to-branch back merging, so I belive simple prose description of the problem is enough.
First developer creates branch-01 and inserts few lines into existing file. Another developer creates branch-02 and makes exactly the same changes as first developer, i.e. inserts the same lines with the same content. The only difference in the second's developer branch is whitespace change (space/tab doesn't matter) in a line preceding inserted lines.
branch-01 is merged into trunk and then branch-02 is merged into trunk. At this point I expect a conflict to occur, possibly with whitespace change to be merged w/o conflict. However SVN doesn't recognize the conflict and BOTH developers' changes are properly merged into trunk resulting with duplicated content.
One need to merge branch-02 into trunk's WC rev.5.
If you remove whitespace change in branch-02, the conflict is properly detected. Several different cases tested and defined the problem as "a whitespace change in the line preceding conflicted block"
This situation happened on 64bit Win7 using TortoiseSVN with internal diff/merge tools setting - TortoiseUDiff and TortoiseMerge. Version infromation is as follows:
TortoiseSVN 1.8.11, Build 26392 - 64 Bit , 2015/03/19 18:50:20
Subversion 1.8.13, -release
apr 1.5.1
apr-util 1.5.4
serf 1.3.8
OpenSSL 1.0.2a 19 Mar 2015
zlib 1.2.8
WC is trunk rev.5, then after selecting "Merge" following options are selected: "Merge a range of revisions" -> branch-02 is selected with "all revisions" -> default settings for "Merge options", i.e. "Compare whitespaces".
When I change "Merge options" to "Ignore all whitespaces" then conflict is properly detected (and that's what I expect for "Compare whitespaces" too).
To exclude Tortoise impact on this behavior I repeated the process with svn command line under Cygwin:
with default merge settings the result is same as for Tortoise: no conflict and content duplication
svn merge http://remote_test_repo/branches/branch-02 test_wc/
with "ignore--all-space" the conflict is detected as expected
svn merge -x --ignore-all-space http://remote_test_repo/branches/branch-02 test_wc/
SVN version information:
$ svn --version
svn, version 1.9.3 (r1718519)
compiled Dec 15 2015, 09:46:48 on i686-pc-cygwin
The repo attached was created using mentioned Tortoise, so it's SVN 1.8. Let me know if you need more information.
I haven't found any SVN's JIRA ticket related to such behavior, hopefully it's not reported all over again. Thanks for attention.
Kind Regards
Michał Matyl
RE: merging adjacent changes
Posted by Michal Matyl <Mi...@zf.com>.
Hello,
Thanks for paying attention to this. Unfortunately I can't submit new item in SVN's JIRA. Apart from discussion raised by my example can someone please fill an issue for this ? It's already identified by Daniel as a bug. Per his suggestion I have also rewritten the example into Win batch script.
Regards
Michal
-----Original Message-----
From: Philip Martin [mailto:philip.martin@wandisco.com]
Sent: Monday, February 22, 2016 6:24 PM
To: Bert Huijben
Cc: 'Daniel Shahaf'; 'Michal Matyl'; dev@subversion.apache.org; jcorvel@apache.org; philip@apache.org
Subject: merging adjacent changes
"Bert Huijben" <be...@qqmail.nl> writes:
> Personally I agree that I would like to see a text-conflict raised in
> this specific case where the change regions touch each other. But then
> there is that old discussion, ....
I added the test because we had already, perhaps inadvertently, implemented the behaviour.
Back in 2003 we didn't have 'svn resolve' so resolving a text conflict required the user to edit the file. Now we have 'svn resolved' which may make resolving text conflicts easier. Perhaps that allows us to change Subversion to create more conflicts?
We know merge is essentially imperfect or fuzzy, the question is where to draw the line. Which, if any, of these cases should merge without conflict and what should the merged result be?
1. common first second
ancestor branch branch
A A A
B Bbr1 B
C C Cbr2
D D D
2. common first second
ancestor branch branch
A A A
B Bbr1 B
C new new
D C C
D D
3. common first second
ancestor branch branch
A A A
B Bbr1 B
C new new
D C Cbr2
D D
4. common first second
ancestor branch branch
A A A
B Bbr1 B
C newbr1 newbr2
D C Cbr2
D D
There are probably more cases.
--
Philip Martin
WANdisco
merging adjacent changes
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:
> Personally I agree that I would like to see a text-conflict raised in
> this specific case where the change regions touch each other. But then
> there is that old discussion, ....
I added the test because we had already, perhaps inadvertently,
implemented the behaviour.
Back in 2003 we didn't have 'svn resolve' so resolving a text conflict
required the user to edit the file. Now we have 'svn resolved' which
may make resolving text conflicts easier. Perhaps that allows us to
change Subversion to create more conflicts?
We know merge is essentially imperfect or fuzzy, the question is where
to draw the line. Which, if any, of these cases should merge without
conflict and what should the merged result be?
1. common first second
ancestor branch branch
A A A
B Bbr1 B
C C Cbr2
D D D
2. common first second
ancestor branch branch
A A A
B Bbr1 B
C new new
D C C
D D
3. common first second
ancestor branch branch
A A A
B Bbr1 B
C new new
D C Cbr2
D D
4. common first second
ancestor branch branch
A A A
B Bbr1 B
C newbr1 newbr2
D C Cbr2
D D
There are probably more cases.
--
Philip Martin
WANdisco
RE: (unknown)
Posted by Bert Huijben <be...@qqmail.nl>.
[Moving thread to dev@s.a.o from users@]
> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: maandag 22 februari 2016 13:21
> To: 'Daniel Shahaf' <d....@daniel.shahaf.name>; 'Michal Matyl'
> <Mi...@zf.com>
> Cc: users@subversion.apache.org
> Subject: RE: (unknown)
>
<snip>
> I just reviewed most of the code that is responsible for this behavior... and I
> have to conclude it works 'as designed'.
>
> 1) We determine the changes on both sides.
> * On one side we see a replacement of line 'C', by a different set of tokens.
> * And on the other side we see an insertion of a set of tokens after 'C'.
> (This code is in subversion/diff/lcs.c)
>
> 2) Then we combine the changes of both sides.
> * We can apply the first change as there are no overlapping regions
> * We can apply the second change as there no overlapping regions
> (This code is in subversion/diff/diff3.c)
>
> In our very abstract implementation things work as intended... so now we
> have to determine how we want to fix things. -> back to design phase.
>
>
> I have a working patch that determines that these changes touch each other
> and then marks them as conflict, but that still doesn't produce the kind of
> conflict that you would really want here. And I'm not sure
>
> In the optimal case we would flag one conflict containing both changes *as
> one*, but that will take more work.
>
>
>
> Note that the 'whitespace' (noted in original report) is completely unrelated
> to this issue. Our diff code works with tokens, while the whitespace is
> handled in the tokenizer. I can easily reproduce this issue without any
> whitespace changes.
I completed the patch to a form, where I like the result.
[[
Index: subversion/libsvn_diff/diff3.c
===================================================================
--- subversion/libsvn_diff/diff3.c (revision 1731660)
+++ subversion/libsvn_diff/diff3.c (working copy)
@@ -320,16 +320,17 @@ svn_diff_diff3_2(svn_diff_t **diff,
suffix_lines, subpool);
lcs_ol = svn_diff__lcs(position_list[0], position_list[2], token_counts[0],
token_counts[2], num_tokens, prefix_lines,
suffix_lines, subpool);
/* 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;
apr_off_t latest_start = 1;
apr_off_t original_sync;
apr_off_t modified_sync;
apr_off_t latest_sync;
apr_off_t common_length;
@@ -429,16 +430,17 @@ svn_diff_diff3_2(svn_diff_t **diff,
is_modified = lcs_om->position[0]->offset - original_start > 0
|| lcs_om->position[1]->offset - modified_start > 0;
is_latest = lcs_ol->position[0]->offset - original_start > 0
|| lcs_ol->position[1]->offset - latest_start > 0;
if (is_modified || is_latest)
{
+ svn_boolean_t add_diff = TRUE;
modified_length = modified_sync - modified_start;
latest_length = latest_sync - latest_start;
(*diff_ref) = apr_palloc(pool, sizeof(**diff_ref));
(*diff_ref)->original_start = original_start - 1;
(*diff_ref)->original_length = original_sync - original_start;
(*diff_ref)->modified_start = modified_start - 1;
@@ -450,26 +452,47 @@ svn_diff_diff3_2(svn_diff_t **diff,
if (is_modified && is_latest)
{
svn_diff__resolve_conflict(*diff_ref,
&position_list[1],
&position_list[2],
num_tokens,
pool);
}
- 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;
}
- else
+ else if (is_latest
+ && (!diff_last
+ || diff_last->type != svn_diff__type_diff_modified))
{
(*diff_ref)->type = svn_diff__type_diff_latest;
}
+ 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 */
if (lcs_om->length == 0 || lcs_ol->length == 0)
break;
modified_length = lcs_om->length
- (original_sync - lcs_om->position[0]->offset);
@@ -485,16 +508,17 @@ svn_diff_diff3_2(svn_diff_t **diff,
(*diff_ref)->original_start = original_sync - 1;
(*diff_ref)->original_length = common_length;
(*diff_ref)->modified_start = modified_sync - 1;
(*diff_ref)->modified_length = common_length;
(*diff_ref)->latest_start = latest_sync - 1;
(*diff_ref)->latest_length = common_length;
(*diff_ref)->resolved_diff = NULL;
+ diff_last = *diff_ref;
diff_ref = &(*diff_ref)->next;
}
/* Set the new offsets */
original_start = original_sync + common_length;
modified_start = modified_sync + common_length;
latest_start = latest_sync + common_length;
Index: subversion/tests/libsvn_diff/diff-diff3-test.c
===================================================================
--- subversion/tests/libsvn_diff/diff-diff3-test.c (revision 1731660)
+++ subversion/tests/libsvn_diff/diff-diff3-test.c (working copy)
@@ -2991,30 +2991,32 @@ three_way_double_add(apr_pool_t *pool)
will be a PASS. */
"A\n"
"B\n"
"<<<<<<< doubleadd2\n"
"C\n"
"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 */
"Q\n" /* New line 2b */
"R\n" /* New line 3b */
">>>>>>> doubleadd3\n"
"J\n"
"K\n"
"L",
NULL,
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"
"J\n"
"K\n"
"L",
"A\n"
@@ -3039,28 +3041,31 @@ three_way_double_add(apr_pool_t *pool)
/* With s/C/O/ we expect something like this,
but the current (1.9/trunk) result is a
succeeded merge to a combined result.
### I'm guessing this result needs tweaks before it
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,
svn_diff_conflict_display_modified_original_latest,
pool));
return SVN_NO_ERROR;
}
@@ -3102,14 +3107,14 @@ static struct svn_test_descriptor_t test_funcs[] =
SVN_TEST_PASS2(test_identical_suffix,
"identical suffix starts at the boundary of a chunk"),
SVN_TEST_PASS2(test_token_compare,
"compare tokens at the chunk boundary"),
SVN_TEST_PASS2(two_way_issue_3362_v1,
"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
};
SVN_TEST_MAIN
]]
But the result triggered an interesting regression test failure, with an even more interesting comment
/* 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
*/
This comment was added in 2003 after the discussion on that url.
I really don't know what we should do here now.
Personally I agree that I would like to see a text-conflict raised in this specific case where the change regions touch each other. But then there is that old discussion, ....
Bert
RE: (unknown)
Posted by Bert Huijben <be...@qqmail.nl>.
[Moving thread to dev@s.a.o from users@]
> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: maandag 22 februari 2016 13:21
> To: 'Daniel Shahaf' <d....@daniel.shahaf.name>; 'Michal Matyl'
> <Mi...@zf.com>
> Cc: users@subversion.apache.org
> Subject: RE: (unknown)
>
<snip>
> I just reviewed most of the code that is responsible for this behavior... and I
> have to conclude it works 'as designed'.
>
> 1) We determine the changes on both sides.
> * On one side we see a replacement of line 'C', by a different set of tokens.
> * And on the other side we see an insertion of a set of tokens after 'C'.
> (This code is in subversion/diff/lcs.c)
>
> 2) Then we combine the changes of both sides.
> * We can apply the first change as there are no overlapping regions
> * We can apply the second change as there no overlapping regions
> (This code is in subversion/diff/diff3.c)
>
> In our very abstract implementation things work as intended... so now we
> have to determine how we want to fix things. -> back to design phase.
>
>
> I have a working patch that determines that these changes touch each other
> and then marks them as conflict, but that still doesn't produce the kind of
> conflict that you would really want here. And I'm not sure
>
> In the optimal case we would flag one conflict containing both changes *as
> one*, but that will take more work.
>
>
>
> Note that the 'whitespace' (noted in original report) is completely unrelated
> to this issue. Our diff code works with tokens, while the whitespace is
> handled in the tokenizer. I can easily reproduce this issue without any
> whitespace changes.
I completed the patch to a form, where I like the result.
[[
Index: subversion/libsvn_diff/diff3.c
===================================================================
--- subversion/libsvn_diff/diff3.c (revision 1731660)
+++ subversion/libsvn_diff/diff3.c (working copy)
@@ -320,16 +320,17 @@ svn_diff_diff3_2(svn_diff_t **diff,
suffix_lines, subpool);
lcs_ol = svn_diff__lcs(position_list[0], position_list[2], token_counts[0],
token_counts[2], num_tokens, prefix_lines,
suffix_lines, subpool);
/* 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;
apr_off_t latest_start = 1;
apr_off_t original_sync;
apr_off_t modified_sync;
apr_off_t latest_sync;
apr_off_t common_length;
@@ -429,16 +430,17 @@ svn_diff_diff3_2(svn_diff_t **diff,
is_modified = lcs_om->position[0]->offset - original_start > 0
|| lcs_om->position[1]->offset - modified_start > 0;
is_latest = lcs_ol->position[0]->offset - original_start > 0
|| lcs_ol->position[1]->offset - latest_start > 0;
if (is_modified || is_latest)
{
+ svn_boolean_t add_diff = TRUE;
modified_length = modified_sync - modified_start;
latest_length = latest_sync - latest_start;
(*diff_ref) = apr_palloc(pool, sizeof(**diff_ref));
(*diff_ref)->original_start = original_start - 1;
(*diff_ref)->original_length = original_sync - original_start;
(*diff_ref)->modified_start = modified_start - 1;
@@ -450,26 +452,47 @@ svn_diff_diff3_2(svn_diff_t **diff,
if (is_modified && is_latest)
{
svn_diff__resolve_conflict(*diff_ref,
&position_list[1],
&position_list[2],
num_tokens,
pool);
}
- 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;
}
- else
+ else if (is_latest
+ && (!diff_last
+ || diff_last->type != svn_diff__type_diff_modified))
{
(*diff_ref)->type = svn_diff__type_diff_latest;
}
+ 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 */
if (lcs_om->length == 0 || lcs_ol->length == 0)
break;
modified_length = lcs_om->length
- (original_sync - lcs_om->position[0]->offset);
@@ -485,16 +508,17 @@ svn_diff_diff3_2(svn_diff_t **diff,
(*diff_ref)->original_start = original_sync - 1;
(*diff_ref)->original_length = common_length;
(*diff_ref)->modified_start = modified_sync - 1;
(*diff_ref)->modified_length = common_length;
(*diff_ref)->latest_start = latest_sync - 1;
(*diff_ref)->latest_length = common_length;
(*diff_ref)->resolved_diff = NULL;
+ diff_last = *diff_ref;
diff_ref = &(*diff_ref)->next;
}
/* Set the new offsets */
original_start = original_sync + common_length;
modified_start = modified_sync + common_length;
latest_start = latest_sync + common_length;
Index: subversion/tests/libsvn_diff/diff-diff3-test.c
===================================================================
--- subversion/tests/libsvn_diff/diff-diff3-test.c (revision 1731660)
+++ subversion/tests/libsvn_diff/diff-diff3-test.c (working copy)
@@ -2991,30 +2991,32 @@ three_way_double_add(apr_pool_t *pool)
will be a PASS. */
"A\n"
"B\n"
"<<<<<<< doubleadd2\n"
"C\n"
"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 */
"Q\n" /* New line 2b */
"R\n" /* New line 3b */
">>>>>>> doubleadd3\n"
"J\n"
"K\n"
"L",
NULL,
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"
"J\n"
"K\n"
"L",
"A\n"
@@ -3039,28 +3041,31 @@ three_way_double_add(apr_pool_t *pool)
/* With s/C/O/ we expect something like this,
but the current (1.9/trunk) result is a
succeeded merge to a combined result.
### I'm guessing this result needs tweaks before it
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,
svn_diff_conflict_display_modified_original_latest,
pool));
return SVN_NO_ERROR;
}
@@ -3102,14 +3107,14 @@ static struct svn_test_descriptor_t test_funcs[] =
SVN_TEST_PASS2(test_identical_suffix,
"identical suffix starts at the boundary of a chunk"),
SVN_TEST_PASS2(test_token_compare,
"compare tokens at the chunk boundary"),
SVN_TEST_PASS2(two_way_issue_3362_v1,
"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
};
SVN_TEST_MAIN
]]
But the result triggered an interesting regression test failure, with an even more interesting comment
/* 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
*/
This comment was added in 2003 after the discussion on that url.
I really don't know what we should do here now.
Personally I agree that I would like to see a text-conflict raised in this specific case where the change regions touch each other. But then there is that old discussion, ....
Bert
RE: (unknown)
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> Sent: zondag 14 februari 2016 15:35
> To: Michal Matyl <Mi...@zf.com>
> Cc: users@subversion.apache.org
> Subject: Re: (unknown)
>
> Michal Matyl wrote on Thu, Feb 11, 2016 at 10:19:02 +0000:
> > The example is about a simple branch merging with default settings and
> > a classic conflict situation, no fancy features or complex
> > trunk-to-branch back merging, so I belive simple prose description of
> > the problem is enough.
>
> The prose did not suffice: I would not have understood the problem
> without the attachment. The preferred way to describe a bug is by
> a script that reproduces it.
>
> For future reference, we provide template scripts at:
> https://subversion.apache.org/docs/community-
> guide/issues.html#reporting-bugs
> (fourth paragraph)
>
> > First developer creates branch-01 and inserts few lines into existing
> > file. Another developer creates branch-02 and makes exactly the same
> > changes as first developer, i.e. inserts the same lines with the same
> > content. The only difference in the second's developer branch is
> > whitespace change (space/tab doesn't matter) in a line preceding
> > inserted lines.
> >
>
> You are describing a merge where
>
> * The difference between the OLD and MINE is:
>
> Index: branches/branch-01/test_file.txt
>
> ==========================================================
> =========
> --- branches/branch-01/test_file.txt (revision 3)
> +++ branches/branch-01/test_file.txt (revision 4)
> @@ -1,6 +1,9 @@
> A
> B
> C
> +D
> +E
> +F
> J
> K
> L
> \ No newline at end of file
>
> * The difference between OLD and THEIRS is:
>
> Index: branches/branch-02/test_file.txt
>
> ==========================================================
> =========
> --- branches/branch-02/test_file.txt (revision 6)
> +++ branches/branch-02/test_file.txt (revision 7)
> @@ -1,6 +1,9 @@
> A
> B
> -C
> + C
> +D
> +E
> +F
> J
> K
> L
> \ No newline at end of file
>
> * There are no local mods.
>
> That merge results in:
>
> Index: trunk/test_file.txt
>
> ==========================================================
> =========
> --- trunk/test_file.txt (revision 7)
> +++ trunk/test_file.txt (revision 8)
> @@ -1,9 +1,12 @@
> A
> B
> -C
> + C
> D
> E
> F
> +D
> +E
> +F
> J
> K
> L
> \ No newline at end of file
>
> Yes, I also think that is a bug: a text conflict should have been
> flagged.
I just reviewed most of the code that is responsible for this behavior... and I have to conclude it works 'as designed'.
1) We determine the changes on both sides.
* On one side we see a replacement of line 'C', by a different set of tokens.
* And on the other side we see an insertion of a set of tokens after 'C'.
(This code is in subversion/diff/lcs.c)
2) Then we combine the changes of both sides.
* We can apply the first change as there are no overlapping regions
* We can apply the second change as there no overlapping regions
(This code is in subversion/diff/diff3.c)
In our very abstract implementation things work as intended... so now we have to determine how we want to fix things. -> back to design phase.
I have a working patch that determines that these changes touch each other and then marks them as conflict, but that still doesn't produce the kind of conflict that you would really want here. And I'm not sure
In the optimal case we would flag one conflict containing both changes *as one*, but that will take more work.
Note that the 'whitespace' (noted in original report) is completely unrelated to this issue. Our diff code works with tokens, while the whitespace is handled in the tokenizer. I can easily reproduce this issue without any whitespace changes.
Bert
Re: (unknown)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Michal Matyl wrote on Thu, Feb 11, 2016 at 10:19:02 +0000:
> The example is about a simple branch merging with default settings and
> a classic conflict situation, no fancy features or complex
> trunk-to-branch back merging, so I belive simple prose description of
> the problem is enough.
The prose did not suffice: I would not have understood the problem
without the attachment. The preferred way to describe a bug is by
a script that reproduces it.
For future reference, we provide template scripts at:
https://subversion.apache.org/docs/community-guide/issues.html#reporting-bugs
(fourth paragraph)
> First developer creates branch-01 and inserts few lines into existing
> file. Another developer creates branch-02 and makes exactly the same
> changes as first developer, i.e. inserts the same lines with the same
> content. The only difference in the second's developer branch is
> whitespace change (space/tab doesn't matter) in a line preceding
> inserted lines.
>
You are describing a merge where
* The difference between the OLD and MINE is:
Index: branches/branch-01/test_file.txt
===================================================================
--- branches/branch-01/test_file.txt (revision 3)
+++ branches/branch-01/test_file.txt (revision 4)
@@ -1,6 +1,9 @@
A
B
C
+D
+E
+F
J
K
L
\ No newline at end of file
* The difference between OLD and THEIRS is:
Index: branches/branch-02/test_file.txt
===================================================================
--- branches/branch-02/test_file.txt (revision 6)
+++ branches/branch-02/test_file.txt (revision 7)
@@ -1,6 +1,9 @@
A
B
-C
+ C
+D
+E
+F
J
K
L
\ No newline at end of file
* There are no local mods.
That merge results in:
Index: trunk/test_file.txt
===================================================================
--- trunk/test_file.txt (revision 7)
+++ trunk/test_file.txt (revision 8)
@@ -1,9 +1,12 @@
A
B
-C
+ C
D
E
F
+D
+E
+F
J
K
L
\ No newline at end of file
Yes, I also think that is a bug: a text conflict should have been
flagged.
Could you please file an issue for this?
If you'd like to help fixing the bug, submitting a regression test (as
a patch against subversion/tests/cmdline/) would be a great first step.
I tested with 1.9.3.
Thanks,
Daniel