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>
This commit is contained in:
Christian Halstrick 2010-11-26 00:30:08 +01:00
parent 7e298c9ed5
commit 049827d708
7 changed files with 81 additions and 24 deletions

View File

@ -54,10 +54,9 @@
import java.util.List; import java.util.List;
import org.eclipse.jgit.diff.DiffAlgorithm; import org.eclipse.jgit.diff.DiffAlgorithm;
import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm;
import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.diff.DiffEntry;
import org.eclipse.jgit.diff.DiffFormatter; 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.RawTextComparator;
import org.eclipse.jgit.diff.RenameDetector; import org.eclipse.jgit.diff.RenameDetector;
import org.eclipse.jgit.dircache.DirCacheIterator; import org.eclipse.jgit.dircache.DirCacheIterator;
@ -101,19 +100,9 @@ void noRenames(@SuppressWarnings("unused") boolean on) {
detectRenames = Boolean.FALSE; 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") @Option(name = "--algorithm", metaVar = "metaVar_diffAlg", usage = "usage_diffAlgorithm")
void setAlgorithm(SupportedAlgorithm s) { void setAlgorithm(SupportedAlgorithm s) {
diffFmt.setDiffAlgorithm(s.algorithm); diffFmt.setDiffAlgorithm(DiffAlgorithm.getAlgorithm(s));
} }
@Option(name = "-l", usage = "usage_renameLimit") @Option(name = "-l", usage = "usage_renameLimit")

View File

@ -166,7 +166,7 @@ public void testSeperateModifications() throws IOException {
} }
private String merge(String commonBase, String ours, String theirs) throws IOException { 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)); T(commonBase), T(ours), T(theirs));
ByteArrayOutputStream bo=new ByteArrayOutputStream(50); ByteArrayOutputStream bo=new ByteArrayOutputStream(50);
fmt.formatMerge(bo, r, "B", "O", "T", Constants.CHARACTER_ENCODING); fmt.formatMerge(bo, r, "B", "O", "T", Constants.CHARACTER_ENCODING);

View File

@ -53,6 +53,38 @@
* a unique instance per thread. * a unique instance per thread.
*/ */
public abstract class DiffAlgorithm { 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. * Compare two sequences and identify a list of edits between them.
* *

View File

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

View File

@ -57,6 +57,12 @@ public class ConfigConstants {
/** The "remote" section */ /** The "remote" section */
public static final String CONFIG_REMOTE_SECTION = "remote"; 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 */ /** The "autocrlf" key */
public static final String CONFIG_KEY_AUTOCRLF = "autocrlf"; public static final String CONFIG_KEY_AUTOCRLF = "autocrlf";

View File

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

View File

@ -58,6 +58,8 @@
import java.util.Map; import java.util.Map;
import org.eclipse.jgit.JGitText; 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.RawText;
import org.eclipse.jgit.diff.RawTextComparator; import org.eclipse.jgit.diff.RawTextComparator;
import org.eclipse.jgit.diff.Sequence; import org.eclipse.jgit.diff.Sequence;
@ -71,6 +73,7 @@
import org.eclipse.jgit.errors.IndexWriteException; import org.eclipse.jgit.errors.IndexWriteException;
import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.NoWorkTreeException; import org.eclipse.jgit.errors.NoWorkTreeException;
import org.eclipse.jgit.lib.ConfigConstants;
import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
@ -136,6 +139,7 @@ public enum MergeFailureReason {
private WorkingTreeIterator workingTreeIterator; private WorkingTreeIterator workingTreeIterator;
private MergeAlgorithm mergeAlgorithm;
/** /**
* @param local * @param local
@ -143,6 +147,11 @@ public enum MergeFailureReason {
*/ */
protected ResolveMerger(Repository local, boolean inCore) { protected ResolveMerger(Repository local, boolean inCore) {
super(local); 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" }; commitNames = new String[] { "BASE", "OURS", "THEIRS" };
oi = getObjectInserter(); oi = getObjectInserter();
this.inCore = inCore; this.inCore = inCore;
@ -459,7 +468,7 @@ private boolean contentMerge(CanonicalTreeParser base,
MergeFormatter fmt = new MergeFormatter(); MergeFormatter fmt = new MergeFormatter();
// do the merge // do the merge
MergeResult<RawText> result = MergeAlgorithm.merge( MergeResult<RawText> result = mergeAlgorithm.merge(
RawTextComparator.DEFAULT, RawTextComparator.DEFAULT,
getRawText(base.getEntryObjectId(), db), getRawText(base.getEntryObjectId(), db),
getRawText(ours.getEntryObjectId(), db), getRawText(ours.getEntryObjectId(), db),