From beeb1f6d08a907493b3660837fd4059322ca969a Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Wed, 27 Oct 2010 22:12:46 +0200 Subject: [PATCH] Fix Severe Bug in Merge Algorithm As described in Bug 328551 there was a bug that the merge algorithm was not always reporting conflicts when the same line was deleted and modified. This problem was introduced during commit 0c017188b4d41cc80c297e35097095026734b3d4 when reported conflicts have been checked for common pre- and suffixes. This was fixed here by better determining whether after stripping off common prefixes and suffixes from a conflicting region there is still some conflicting part left. I also added a unit test to test this situation. Bug: 328551 Change-Id: Iec6c9055d00e5049938484a27ab98dda2577afc4 Signed-off-by: Christian Halstrick --- .../jgit/merge/MergeAlgorithmTest.java | 11 ++++++++ .../eclipse/jgit/merge/MergeAlgorithm.java | 27 +++++++++++++------ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java index 47f329251..b51329e29 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java @@ -98,6 +98,7 @@ public class MergeAlgorithmTest extends TestCase { String replace_BCDEGI_by_ZZZZZZ=A+Z+Z+Z+Z+F+Z+H+Z+J; String replace_CEFGHJ_by_YYYYYY=A+B+Y+D+Y+Y+Y+Y+I+Y; String replace_BDE_by_ZZY=A+Z+C+Z+Y+F+G+H+I+J; + String delete_C=A+B+D+E+F+G+H+I+J; /** * Check for a conflict where the second text was changed similar to the @@ -190,6 +191,16 @@ public class MergeAlgorithmTest extends TestCase { merge(base, replace_C_by_Z, replace_C_by_Z)); } + /** + * Check that a deleted vs. a modified line shows up as conflict (see Bug + * 328551) + * + * @throws IOException + */ + public void testDeleteVsModify() throws IOException { + assertEquals(A+B+XXX_0+XXX_1+Z+XXX_2+D+E+F+G+H+I+J, merge(base, delete_C, replace_C_by_Z)); + } + private String merge(String commonBase, String ours, String theirs) throws IOException { MergeResult r=MergeAlgorithm.merge(RawTextComparator.DEFAULT, new RawText(Constants.encode(commonBase)), new RawText(Constants.encode(ours)), new RawText(Constants.encode(theirs))); ByteArrayOutputStream bo=new ByteArrayOutputStream(50); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java index d945f1965..868248713 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java @@ -201,28 +201,39 @@ public final class MergeAlgorithm { // A conflicting region is found. Strip off common lines in // in the beginning and the end of the conflicting region - int conflictLen = Math.min(oursEndB - oursBeginB, theirsEndB - - theirsBeginB); + + // Determine the minimum length of the conflicting areas in OURS + // and THEIRS. Also determine how much bigger the conflicting + // area in THEIRS is compared to OURS. All that is needed to + // limit the search for common areas at the beginning or end + // (the common areas cannot be bigger then the smaller + // conflicting area. The delta is needed to know whether the + // complete conflicting area is common in OURS and THEIRS. + int minBSize = oursEndB - oursBeginB; + int BSizeDelta = minBSize - (theirsEndB - theirsBeginB); + if (BSizeDelta > 0) + minBSize -= BSizeDelta; + int commonPrefix = 0; - while (commonPrefix < conflictLen + while (commonPrefix < minBSize && cmp.equals(ours, oursBeginB + commonPrefix, theirs, theirsBeginB + commonPrefix)) commonPrefix++; - conflictLen -= commonPrefix; + minBSize -= commonPrefix; int commonSuffix = 0; - while (commonSuffix < conflictLen + while (commonSuffix < minBSize && cmp.equals(ours, oursEndB - commonSuffix - 1, theirs, theirsEndB - commonSuffix - 1)) commonSuffix++; - conflictLen -= commonSuffix; + minBSize -= commonSuffix; // Add the common lines at start of conflict if (commonPrefix > 0) result.add(1, oursBeginB, oursBeginB + commonPrefix, ConflictState.NO_CONFLICT); - // Add the conflict - if (conflictLen > 0) { + // Add the conflict (Only if there is a conflict left to report) + if (minBSize > 0 || BSizeDelta != 0) { result.add(1, oursBeginB + commonPrefix, oursEndB - commonSuffix, ConflictState.FIRST_CONFLICTING_RANGE);