From 049827d7080201fe24f2728b26d681434327f72a Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Fri, 26 Nov 2010 00:30:08 +0100 Subject: [PATCH] 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 Signed-off-by: Philipp Thun --- .../src/org/eclipse/jgit/pgm/Diff.java | 15 ++------- .../jgit/merge/MergeAlgorithmTest.java | 2 +- .../org/eclipse/jgit/diff/DiffAlgorithm.java | 32 +++++++++++++++++++ .../org/eclipse/jgit/diff/DiffFormatter.java | 10 +++++- .../org/eclipse/jgit/lib/ConfigConstants.java | 6 ++++ .../eclipse/jgit/merge/MergeAlgorithm.java | 29 ++++++++++++----- .../org/eclipse/jgit/merge/ResolveMerger.java | 11 ++++++- 7 files changed, 81 insertions(+), 24 deletions(-) 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 6062f35827..506031713a 100644 --- 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 9b4b7146f6..b17c5278e1 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 @@ -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 f6859a29c3..b20e3258b6 100644 --- 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 be9a86eda9..fa0cb9c473 100644 --- 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 4ca11d0faf..9f742062b3 100644 --- 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 aee0ba940a..62febd62c2 100644 --- 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 MergeResult merge( + public MergeResult merge( SequenceComparator cmp, S base, S ours, S theirs) { List sequences = new ArrayList(3); sequences.add(base); sequences.add(ours); sequences.add(theirs); - MergeResult result = new MergeResult(sequences); - EditList oursEdits = MyersDiff.INSTANCE.diff(cmp, base, ours); + MergeResult result = new MergeResult(sequences); + EditList oursEdits = diffAlg.diff(cmp, base, ours); Iterator baseToOurs = oursEdits.iterator(); - EditList theirsEdits = MyersDiff.INSTANCE.diff(cmp, base, theirs); + EditList theirsEdits = diffAlg.diff(cmp, base, theirs); Iterator 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 a6119edfae..a1bda41284 100644 --- 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 result = MergeAlgorithm.merge( + MergeResult result = mergeAlgorithm.merge( RawTextComparator.DEFAULT, getRawText(base.getEntryObjectId(), db), getRawText(ours.getEntryObjectId(), db), -- 2.39.5