Make diff algorithm configurable

The diff algorithm which is used by Merge, Cherry-Pick, Rebase
should be configurable. A new configuration parameter "diff.algorithm"
is introduced which currently accepts the values "myers" or
"histogram". Based on this parameter for example the ResolveMerger
will choose a diff algorithm. The reason for this is bug 331078.
This bug shows that JGit is more compatible with C Git when
histogram diff is in place. But since histogram diff is quite new we
need an easy way to fall back to Myers diff.

Bug: 331078
Change-Id: I2549c992e478d991c61c9508ad826d1a9e539ae3
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Philipp Thun <philipp.thun@sap.com>

diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Diff.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Diff.java
index 6062f35827c7d9d235f55302ce10340fc4f97c75..506031713af520ea452c019e6cd73453dfd97da5 100644 (file)
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Diff.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Diff.java
@@ -54,10 +54,9 @@ import java.text.MessageFormat;
 import java.util.List;
 
 import org.eclipse.jgit.diff.DiffAlgorithm;
+import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm;
 import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.diff.DiffFormatter;
-import org.eclipse.jgit.diff.HistogramDiff;
-import org.eclipse.jgit.diff.MyersDiff;
 import org.eclipse.jgit.diff.RawTextComparator;
 import org.eclipse.jgit.diff.RenameDetector;
 import org.eclipse.jgit.dircache.DirCacheIterator;
@@ -101,19 +100,9 @@ class Diff extends TextBuiltin {
                detectRenames = Boolean.FALSE;
        }
 
-       enum SupportedAlgorithm {
-               myers(MyersDiff.INSTANCE), histogram(new HistogramDiff());
-
-               public DiffAlgorithm algorithm;
-
-               SupportedAlgorithm(DiffAlgorithm a) {
-                       algorithm = a;
-               }
-       };
-
        @Option(name = "--algorithm", metaVar = "metaVar_diffAlg", usage = "usage_diffAlgorithm")
        void setAlgorithm(SupportedAlgorithm s) {
-               diffFmt.setDiffAlgorithm(s.algorithm);
+               diffFmt.setDiffAlgorithm(DiffAlgorithm.getAlgorithm(s));
        }
 
        @Option(name = "-l", usage = "usage_renameLimit")

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 9b4b7146f6bf93576b69e7fafb99db65a928a87a..b17c5278e1bd4d27cf467ea64ae4a0b935bc6719 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmTest.java
@@ -166,7 +166,7 @@ public class MergeAlgorithmTest extends TestCase {
        }
 
        private String merge(String commonBase, String ours, String theirs) throws IOException {
-               MergeResult r = MergeAlgorithm.merge(RawTextComparator.DEFAULT,
+               MergeResult r = new MergeAlgorithm().merge(RawTextComparator.DEFAULT,
                                T(commonBase), T(ours), T(theirs));
                ByteArrayOutputStream bo=new ByteArrayOutputStream(50);
                fmt.formatMerge(bo, r, "B", "O", "T", Constants.CHARACTER_ENCODING);

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffAlgorithm.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffAlgorithm.java
index f6859a29c3a7318b37cee798dcb7adc5375cebab..b20e3258b6ce2a20daec20e6ae2d8a5a41c15d99 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffAlgorithm.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffAlgorithm.java
@@ -53,6 +53,38 @@ package org.eclipse.jgit.diff;
  * a unique instance per thread.
  */
 public abstract class DiffAlgorithm {
+       /**
+        * Supported diff algorithm
+        */
+       public enum SupportedAlgorithm {
+               /**
+                * Myers diff algorithm
+                */
+               MYERS,
+
+               /**
+                * Histogram diff algorithm
+                */
+               HISTOGRAM
+       }
+
+       /**
+        * @param alg
+        *            the diff algorithm for which an implementation should be
+        *            returned
+        * @return an implementation of the specified diff algorithm
+        */
+       public static DiffAlgorithm getAlgorithm(SupportedAlgorithm alg) {
+               switch (alg) {
+               case MYERS:
+                       return MyersDiff.INSTANCE;
+               case HISTOGRAM:
+                       return new HistogramDiff();
+               default:
+                       throw new IllegalArgumentException();
+               }
+       }
+
        /**
         * Compare two sequences and identify a list of edits between them.
         *

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
index be9a86eda9fbeabc468a8ee485877320a36574af..fa0cb9c4734df470241e678de6b462c23404e7e9 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
@@ -63,6 +63,7 @@ import java.util.Collections;
 import java.util.List;
 
 import org.eclipse.jgit.JGitText;
+import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm;
 import org.eclipse.jgit.diff.DiffEntry.ChangeType;
 import org.eclipse.jgit.errors.AmbiguousObjectException;
 import org.eclipse.jgit.errors.CorruptObjectException;
@@ -70,6 +71,7 @@ import org.eclipse.jgit.errors.LargeObjectException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.lib.AbbreviatedObjectId;
 import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
@@ -118,7 +120,7 @@ public class DiffFormatter {
 
        private int abbreviationLength = 7;
 
-       private DiffAlgorithm diffAlgorithm = new HistogramDiff();
+       private DiffAlgorithm diffAlgorithm;
 
        private RawTextComparator comparator = RawTextComparator.DEFAULT;
 
@@ -178,6 +180,12 @@ public class DiffFormatter {
                        setNewPrefix("");
                }
                setDetectRenames(dc.isRenameDetectionEnabled());
+
+               diffAlgorithm = DiffAlgorithm.getAlgorithm(db.getConfig().getEnum(
+                               ConfigConstants.CONFIG_DIFF_SECTION, null,
+                               ConfigConstants.CONFIG_KEY_ALGORITHM,
+                               SupportedAlgorithm.HISTOGRAM));
+
        }
 
        /**

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
index 4ca11d0faf330ae3b7ad0b32cd65c5fdd29cf9fa..9f742062b3301fc3050b01b89215bd76d33d9883 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
@@ -57,6 +57,12 @@ public class ConfigConstants {
        /** The "remote" section */
        public static final String CONFIG_REMOTE_SECTION = "remote";
 
+       /** The "diff" section */
+       public static final String CONFIG_DIFF_SECTION = "diff";
+
+       /** The "algorithm" key */
+       public static final String CONFIG_KEY_ALGORITHM = "algorithm";
+
        /** The "autocrlf" key */
        public static final String CONFIG_KEY_AUTOCRLF = "autocrlf";
 

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 aee0ba940a12ccdfc0d4c2052adec5a5b381da3d..62febd62c2df224bd7d563a599d670672bab4786 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java
@@ -47,6 +47,7 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+import org.eclipse.jgit.diff.DiffAlgorithm;
 import org.eclipse.jgit.diff.Edit;
 import org.eclipse.jgit.diff.EditList;
 import org.eclipse.jgit.diff.MyersDiff;
@@ -56,15 +57,27 @@ import org.eclipse.jgit.merge.MergeChunk.ConflictState;
 
 /**
  * Provides the merge algorithm which does a three-way merge on content provided
- * as RawText. Makes use of {@link MyersDiff} to compute the diffs.
+ * as RawText. By default {@link MyersDiff} is used as diff algorithm.
  */
 public final class MergeAlgorithm {
+       private final DiffAlgorithm diffAlg;
 
        /**
-        * Since this class provides only static methods I add a private default
-        * constructor to prevent instantiation.
+        * Creates a new MergeAlgorithm which uses {@link MyersDiff} as diff
+        * algorithm
         */
-       private MergeAlgorithm() {
+       public MergeAlgorithm() {
+               this(MyersDiff.INSTANCE);
+       }
+
+       /**
+        * Creates a new MergeAlgorithm
+        *
+        * @param diff
+        *            the diff algorithm used by this merge
+        */
+       public MergeAlgorithm(DiffAlgorithm diff) {
+               this.diffAlg = diff;
        }
 
        // An special edit which acts as a sentinel value by marking the end the
@@ -83,16 +96,16 @@ public final class MergeAlgorithm {
         * @param theirs the second sequence to be merged
         * @return the resulting content
         */
-       public static <S extends Sequence> MergeResult<S> merge(
+       public <S extends Sequence> MergeResult<S> merge(
                        SequenceComparator<S> cmp, S base, S ours, S theirs) {
                List<S> sequences = new ArrayList<S>(3);
                sequences.add(base);
                sequences.add(ours);
                sequences.add(theirs);
-               MergeResult result = new MergeResult<S>(sequences);
-               EditList oursEdits = MyersDiff.INSTANCE.diff(cmp, base, ours);
+               MergeResult<S> result = new MergeResult<S>(sequences);
+               EditList oursEdits = diffAlg.diff(cmp, base, ours);
                Iterator<Edit> baseToOurs = oursEdits.iterator();
-               EditList theirsEdits = MyersDiff.INSTANCE.diff(cmp, base, theirs);
+               EditList theirsEdits = diffAlg.diff(cmp, base, theirs);
                Iterator<Edit> baseToTheirs = theirsEdits.iterator();
                int current = 0; // points to the next line (first line is 0) of base
                                 // which was not handled yet

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
index a6119edfae85dc36e8907d0c7542d7d6d4a76d81..a1bda4128490309f1ff3c8cdea09202862475b75 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
@@ -58,6 +58,8 @@ import java.util.List;
 import java.util.Map;
 
 import org.eclipse.jgit.JGitText;
+import org.eclipse.jgit.diff.DiffAlgorithm;
+import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm;
 import org.eclipse.jgit.diff.RawText;
 import org.eclipse.jgit.diff.RawTextComparator;
 import org.eclipse.jgit.diff.Sequence;
@@ -71,6 +73,7 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.IndexWriteException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.NoWorkTreeException;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
@@ -136,6 +139,7 @@ public class ResolveMerger extends ThreeWayMerger {
 
        private WorkingTreeIterator workingTreeIterator;
 
+       private MergeAlgorithm mergeAlgorithm;
 
        /**
         * @param local
@@ -143,6 +147,11 @@ public class ResolveMerger extends ThreeWayMerger {
         */
        protected ResolveMerger(Repository local, boolean inCore) {
                super(local);
+               SupportedAlgorithm diffAlg = local.getConfig().getEnum(
+                               ConfigConstants.CONFIG_DIFF_SECTION, null,
+                               ConfigConstants.CONFIG_KEY_ALGORITHM,
+                               SupportedAlgorithm.HISTOGRAM);
+               mergeAlgorithm = new MergeAlgorithm(DiffAlgorithm.getAlgorithm(diffAlg));
                commitNames = new String[] { "BASE", "OURS", "THEIRS" };
                oi = getObjectInserter();
                this.inCore = inCore;
@@ -459,7 +468,7 @@ public class ResolveMerger extends ThreeWayMerger {
                MergeFormatter fmt = new MergeFormatter();
 
                // do the merge
-               MergeResult<RawText> result = MergeAlgorithm.merge(
+               MergeResult<RawText> result = mergeAlgorithm.merge(
                                RawTextComparator.DEFAULT,
                                getRawText(base.getEntryObjectId(), db),
                                getRawText(ours.getEntryObjectId(), db),