Fix exception on conflicts with recursive merge
When there are conflicts with a recursive merge, the conflicting paths are stored in unmergedPaths (field in ResolveMerger). Later, when the MergeResult is constructed in MergeCommand, getBaseCommit is called, which computes the merge base a second time. In case of RecursiveMerger, getBaseCommit merges the multiple merge bases into one. It does this not by creating a new ResolveMerger but instead calling mergeTrees. The problem with mergeTrees is that at the end, it checks if unmergedPaths is non-empty and returns false in that case. Because unmergedPaths was already non-empty because of the real merge, it thinks that there were conflicts when computing the merge base again, when there really were none. This can be fixed by storing the base commit when computing it and then returning that instead of computing it a second time. Note that another possible fix would be to just use a new ResolveMerger for merging the merge bases instead. This would also remove the need to remember the old value of dircache, inCore and workingTreeIterator (see RecursiveMerger#getBaseCommit). Bug: 419641 Change-Id: Ib2ebf4e177498c22a9098aa225e3cfcf16bbd958 Signed-off-by: Robin Stocker <robin@nibor.org>
This commit is contained in:
parent
1128326add
commit
7dc8a4f089
|
@ -56,8 +56,11 @@
|
||||||
import org.eclipse.jgit.api.MergeResult.MergeStatus;
|
import org.eclipse.jgit.api.MergeResult.MergeStatus;
|
||||||
import org.eclipse.jgit.api.errors.InvalidMergeHeadsException;
|
import org.eclipse.jgit.api.errors.InvalidMergeHeadsException;
|
||||||
import org.eclipse.jgit.junit.RepositoryTestCase;
|
import org.eclipse.jgit.junit.RepositoryTestCase;
|
||||||
|
import org.eclipse.jgit.junit.TestRepository;
|
||||||
|
import org.eclipse.jgit.junit.TestRepository.BranchBuilder;
|
||||||
import org.eclipse.jgit.lib.Constants;
|
import org.eclipse.jgit.lib.Constants;
|
||||||
import org.eclipse.jgit.lib.Ref;
|
import org.eclipse.jgit.lib.Ref;
|
||||||
|
import org.eclipse.jgit.lib.Repository;
|
||||||
import org.eclipse.jgit.lib.RepositoryState;
|
import org.eclipse.jgit.lib.RepositoryState;
|
||||||
import org.eclipse.jgit.merge.MergeStrategy;
|
import org.eclipse.jgit.merge.MergeStrategy;
|
||||||
import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
|
import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
|
||||||
|
@ -1532,6 +1535,37 @@ public void testFastForwardOnlyNotPossible() throws Exception {
|
||||||
|
|
||||||
assertEquals(MergeStatus.ABORTED, result.getMergeStatus());
|
assertEquals(MergeStatus.ABORTED, result.getMergeStatus());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testRecursiveMergeWithConflict() throws Exception {
|
||||||
|
TestRepository<Repository> db_t = new TestRepository<Repository>(db);
|
||||||
|
BranchBuilder master = db_t.branch("master");
|
||||||
|
RevCommit m0 = master.commit().add("f", "1\n2\n3\n4\n5\n6\n7\n8\n9\n")
|
||||||
|
.message("m0").create();
|
||||||
|
RevCommit m1 = master.commit()
|
||||||
|
.add("f", "1-master\n2\n3\n4\n5\n6\n7\n8\n9\n").message("m1")
|
||||||
|
.create();
|
||||||
|
db_t.getRevWalk().parseCommit(m1);
|
||||||
|
|
||||||
|
BranchBuilder side = db_t.branch("side");
|
||||||
|
RevCommit s1 = side.commit().parent(m0)
|
||||||
|
.add("f", "1\n2\n3\n4\n5\n6\n7\n8\n9-side\n").message("s1")
|
||||||
|
.create();
|
||||||
|
RevCommit s2 = side.commit().parent(m1)
|
||||||
|
.add("f", "1-master\n2\n3\n4\n5\n6\n7-res(side)\n8\n9-side\n")
|
||||||
|
.message("s2(merge)").create();
|
||||||
|
master.commit().parent(s1)
|
||||||
|
.add("f", "1-master\n2\n3\n4\n5\n6\n7-conflict\n8\n9-side\n")
|
||||||
|
.message("m2(merge)").create();
|
||||||
|
|
||||||
|
Git git = Git.wrap(db);
|
||||||
|
git.checkout().setName("master").call();
|
||||||
|
|
||||||
|
MergeResult result = git.merge().setStrategy(MergeStrategy.RECURSIVE)
|
||||||
|
.include("side", s2).call();
|
||||||
|
assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus());
|
||||||
|
}
|
||||||
|
|
||||||
private static void setExecutable(Git git, String path, boolean executable) {
|
private static void setExecutable(Git git, String path, boolean executable) {
|
||||||
FS.DETECTED.setExecute(
|
FS.DETECTED.setExecute(
|
||||||
new File(git.getRepository().getWorkTree(), path), executable);
|
new File(git.getRepository().getWorkTree(), path), executable);
|
||||||
|
|
|
@ -379,8 +379,7 @@ public MergeResult call() throws GitAPIException, NoHeadException,
|
||||||
if (failingPaths != null) {
|
if (failingPaths != null) {
|
||||||
repo.writeMergeCommitMsg(null);
|
repo.writeMergeCommitMsg(null);
|
||||||
repo.writeMergeHeads(null);
|
repo.writeMergeHeads(null);
|
||||||
return new MergeResult(null,
|
return new MergeResult(null, merger.getBaseCommitId(),
|
||||||
merger.getBaseCommit(0, 1),
|
|
||||||
new ObjectId[] {
|
new ObjectId[] {
|
||||||
headCommit.getId(), srcCommit.getId() },
|
headCommit.getId(), srcCommit.getId() },
|
||||||
MergeStatus.FAILED, mergeStrategy,
|
MergeStatus.FAILED, mergeStrategy,
|
||||||
|
@ -390,8 +389,7 @@ public MergeResult call() throws GitAPIException, NoHeadException,
|
||||||
.formatWithConflicts(mergeMessage,
|
.formatWithConflicts(mergeMessage,
|
||||||
unmergedPaths);
|
unmergedPaths);
|
||||||
repo.writeMergeCommitMsg(mergeMessageWithConflicts);
|
repo.writeMergeCommitMsg(mergeMessageWithConflicts);
|
||||||
return new MergeResult(null,
|
return new MergeResult(null, merger.getBaseCommitId(),
|
||||||
merger.getBaseCommit(0, 1),
|
|
||||||
new ObjectId[] { headCommit.getId(),
|
new ObjectId[] { headCommit.getId(),
|
||||||
srcCommit.getId() },
|
srcCommit.getId() },
|
||||||
MergeStatus.CONFLICTING, mergeStrategy,
|
MergeStatus.CONFLICTING, mergeStrategy,
|
||||||
|
|
|
@ -191,14 +191,14 @@ public RevCommit call() throws NoMessageException, UnmergedPathsException,
|
||||||
.getFailingPaths();
|
.getFailingPaths();
|
||||||
if (failingPaths != null)
|
if (failingPaths != null)
|
||||||
failingResult = new MergeResult(null,
|
failingResult = new MergeResult(null,
|
||||||
merger.getBaseCommit(0, 1),
|
merger.getBaseCommitId(),
|
||||||
new ObjectId[] { headCommit.getId(),
|
new ObjectId[] { headCommit.getId(),
|
||||||
srcParent.getId() },
|
srcParent.getId() },
|
||||||
MergeStatus.FAILED, MergeStrategy.RECURSIVE,
|
MergeStatus.FAILED, MergeStrategy.RECURSIVE,
|
||||||
merger.getMergeResults(), failingPaths, null);
|
merger.getMergeResults(), failingPaths, null);
|
||||||
else
|
else
|
||||||
failingResult = new MergeResult(null,
|
failingResult = new MergeResult(null,
|
||||||
merger.getBaseCommit(0, 1),
|
merger.getBaseCommitId(),
|
||||||
new ObjectId[] { headCommit.getId(),
|
new ObjectId[] { headCommit.getId(),
|
||||||
srcParent.getId() },
|
srcParent.getId() },
|
||||||
MergeStatus.CONFLICTING,
|
MergeStatus.CONFLICTING,
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright (C) 2008-2009, Google Inc.
|
* Copyright (C) 2008-2013, Google Inc.
|
||||||
* and other copyright owners as documented in the project's IP log.
|
* and other copyright owners as documented in the project's IP log.
|
||||||
*
|
*
|
||||||
* This program and the accompanying materials are made available
|
* This program and the accompanying materials are made available
|
||||||
|
@ -54,8 +54,8 @@
|
||||||
import org.eclipse.jgit.lib.Constants;
|
import org.eclipse.jgit.lib.Constants;
|
||||||
import org.eclipse.jgit.lib.ObjectId;
|
import org.eclipse.jgit.lib.ObjectId;
|
||||||
import org.eclipse.jgit.lib.ObjectInserter;
|
import org.eclipse.jgit.lib.ObjectInserter;
|
||||||
import org.eclipse.jgit.lib.Repository;
|
|
||||||
import org.eclipse.jgit.lib.ObjectReader;
|
import org.eclipse.jgit.lib.ObjectReader;
|
||||||
|
import org.eclipse.jgit.lib.Repository;
|
||||||
import org.eclipse.jgit.revwalk.RevCommit;
|
import org.eclipse.jgit.revwalk.RevCommit;
|
||||||
import org.eclipse.jgit.revwalk.RevObject;
|
import org.eclipse.jgit.revwalk.RevObject;
|
||||||
import org.eclipse.jgit.revwalk.RevTree;
|
import org.eclipse.jgit.revwalk.RevTree;
|
||||||
|
@ -63,7 +63,6 @@
|
||||||
import org.eclipse.jgit.revwalk.filter.RevFilter;
|
import org.eclipse.jgit.revwalk.filter.RevFilter;
|
||||||
import org.eclipse.jgit.treewalk.AbstractTreeIterator;
|
import org.eclipse.jgit.treewalk.AbstractTreeIterator;
|
||||||
import org.eclipse.jgit.treewalk.CanonicalTreeParser;
|
import org.eclipse.jgit.treewalk.CanonicalTreeParser;
|
||||||
import org.eclipse.jgit.treewalk.EmptyTreeIterator;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Instance of a specific {@link MergeStrategy} for a single {@link Repository}.
|
* Instance of a specific {@link MergeStrategy} for a single {@link Repository}.
|
||||||
|
@ -186,24 +185,11 @@ public boolean merge(final AnyObjectId... tips) throws IOException {
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create an iterator to walk the merge base of two commits.
|
* @return the ID of the commit that was used as merge base for merging, or
|
||||||
*
|
* null if no merge base was used or it was set manually
|
||||||
* @param a
|
* @since 3.2
|
||||||
* the first commit in {@link #sourceObjects}.
|
|
||||||
* @param b
|
|
||||||
* the second commit in {@link #sourceObjects}.
|
|
||||||
* @return the new iterator
|
|
||||||
* @throws IncorrectObjectTypeException
|
|
||||||
* one of the input objects is not a commit.
|
|
||||||
* @throws IOException
|
|
||||||
* objects are missing or multiple merge bases were found.
|
|
||||||
* @since 3.0
|
|
||||||
*/
|
*/
|
||||||
protected AbstractTreeIterator mergeBase(RevCommit a, RevCommit b)
|
public abstract ObjectId getBaseCommitId();
|
||||||
throws IOException {
|
|
||||||
RevCommit base = getBaseCommit(a, b);
|
|
||||||
return (base == null) ? new EmptyTreeIterator() : openTree(base.getTree());
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Return the merge base of two commits.
|
* Return the merge base of two commits.
|
||||||
|
@ -217,7 +203,10 @@ protected AbstractTreeIterator mergeBase(RevCommit a, RevCommit b)
|
||||||
* one of the input objects is not a commit.
|
* one of the input objects is not a commit.
|
||||||
* @throws IOException
|
* @throws IOException
|
||||||
* objects are missing or multiple merge bases were found.
|
* objects are missing or multiple merge bases were found.
|
||||||
|
* @deprecated use {@link #getBaseCommitId()} instead, as that does not
|
||||||
|
* require walking the commits again
|
||||||
*/
|
*/
|
||||||
|
@Deprecated
|
||||||
public RevCommit getBaseCommit(final int aIdx, final int bIdx)
|
public RevCommit getBaseCommit(final int aIdx, final int bIdx)
|
||||||
throws IncorrectObjectTypeException,
|
throws IncorrectObjectTypeException,
|
||||||
IOException {
|
IOException {
|
||||||
|
|
|
@ -106,5 +106,10 @@ protected boolean mergeImpl() throws IOException {
|
||||||
public ObjectId getResultTreeId() {
|
public ObjectId getResultTreeId() {
|
||||||
return sourceTrees[treeIndex];
|
return sourceTrees[treeIndex];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public ObjectId getBaseCommitId() {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -49,14 +49,19 @@
|
||||||
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
||||||
import org.eclipse.jgit.errors.MissingObjectException;
|
import org.eclipse.jgit.errors.MissingObjectException;
|
||||||
import org.eclipse.jgit.lib.AnyObjectId;
|
import org.eclipse.jgit.lib.AnyObjectId;
|
||||||
|
import org.eclipse.jgit.lib.ObjectId;
|
||||||
import org.eclipse.jgit.lib.Repository;
|
import org.eclipse.jgit.lib.Repository;
|
||||||
|
import org.eclipse.jgit.revwalk.RevCommit;
|
||||||
import org.eclipse.jgit.revwalk.RevTree;
|
import org.eclipse.jgit.revwalk.RevTree;
|
||||||
import org.eclipse.jgit.treewalk.AbstractTreeIterator;
|
import org.eclipse.jgit.treewalk.AbstractTreeIterator;
|
||||||
|
import org.eclipse.jgit.treewalk.EmptyTreeIterator;
|
||||||
|
|
||||||
/** A merge of 2 trees, using a common base ancestor tree. */
|
/** A merge of 2 trees, using a common base ancestor tree. */
|
||||||
public abstract class ThreeWayMerger extends Merger {
|
public abstract class ThreeWayMerger extends Merger {
|
||||||
private RevTree baseTree;
|
private RevTree baseTree;
|
||||||
|
|
||||||
|
private ObjectId baseCommitId;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create a new merge instance for a repository.
|
* Create a new merge instance for a repository.
|
||||||
*
|
*
|
||||||
|
@ -109,6 +114,11 @@ public boolean merge(final AnyObjectId... tips) throws IOException {
|
||||||
return super.merge(tips);
|
return super.merge(tips);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public ObjectId getBaseCommitId() {
|
||||||
|
return baseCommitId;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create an iterator to walk the merge base.
|
* Create an iterator to walk the merge base.
|
||||||
*
|
*
|
||||||
|
@ -119,6 +129,15 @@ public boolean merge(final AnyObjectId... tips) throws IOException {
|
||||||
protected AbstractTreeIterator mergeBase() throws IOException {
|
protected AbstractTreeIterator mergeBase() throws IOException {
|
||||||
if (baseTree != null)
|
if (baseTree != null)
|
||||||
return openTree(baseTree);
|
return openTree(baseTree);
|
||||||
return mergeBase(sourceCommits[0], sourceCommits[1]);
|
RevCommit baseCommit = (baseCommitId != null) ? walk
|
||||||
|
.parseCommit(baseCommitId) : getBaseCommit(sourceCommits[0],
|
||||||
|
sourceCommits[1]);
|
||||||
|
if (baseCommit == null) {
|
||||||
|
baseCommitId = null;
|
||||||
|
return new EmptyTreeIterator();
|
||||||
|
} else {
|
||||||
|
baseCommitId = baseCommit.toObjectId();
|
||||||
|
return openTree(baseCommit.getTree());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue