diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BlameCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BlameCommandTest.java index 20e93f88c..743e16de0 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BlameCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BlameCommandTest.java @@ -386,4 +386,74 @@ public class BlameCommandTest extends RepositoryTestCase { assertEquals(commit, lines.getSourceCommit(1)); assertEquals(commit, lines.getSourceCommit(2)); } + + @Test + public void testConflictingMerge1() throws Exception { + Git git = new Git(db); + + RevCommit base = commitFile("file.txt", join("0", "1", "2", "3", "4"), + "master"); + + git.checkout().setName("side").setCreateBranch(true) + .setStartPoint(base).call(); + RevCommit side = commitFile("file.txt", + join("0", "1 side", "2", "3 on side", "4"), "side"); + + commitFile("file.txt", join("0", "1", "2"), "master"); + + checkoutBranch("refs/heads/master"); + git.merge().include(side).call(); + + // The merge results in a conflict, which we resolve using mostly the + // side branch contents. Especially the "4" survives. + RevCommit merge = commitFile("file.txt", + join("0", "1 side", "2", "3 resolved", "4"), "master"); + + BlameCommand command = new BlameCommand(db); + command.setFilePath("file.txt"); + BlameResult lines = command.call(); + + assertEquals(5, lines.getResultContents().size()); + assertEquals(base, lines.getSourceCommit(0)); + assertEquals(side, lines.getSourceCommit(1)); + assertEquals(base, lines.getSourceCommit(2)); + assertEquals(merge, lines.getSourceCommit(3)); + assertEquals(base, lines.getSourceCommit(4)); + } + + // this test inverts the order of the master and side commit and is + // otherwise identical to testConflictingMerge1 + @Test + public void testConflictingMerge2() throws Exception { + Git git = new Git(db); + + RevCommit base = commitFile("file.txt", join("0", "1", "2", "3", "4"), + "master"); + + commitFile("file.txt", join("0", "1", "2"), "master"); + + git.checkout().setName("side").setCreateBranch(true) + .setStartPoint(base).call(); + RevCommit side = commitFile("file.txt", + join("0", "1 side", "2", "3 on side", "4"), "side"); + + checkoutBranch("refs/heads/master"); + git.merge().include(side).call(); + + // The merge results in a conflict, which we resolve using mostly the + // side branch contents. Especially the "4" survives. + RevCommit merge = commitFile("file.txt", + join("0", "1 side", "2", "3 resolved", "4"), "master"); + + BlameCommand command = new BlameCommand(db); + command.setFilePath("file.txt"); + BlameResult lines = command.call(); + + assertEquals(5, lines.getResultContents().size()); + assertEquals(base, lines.getSourceCommit(0)); + assertEquals(side, lines.getSourceCommit(1)); + assertEquals(base, lines.getSourceCommit(2)); + assertEquals(merge, lines.getSourceCommit(3)); + assertEquals(base, lines.getSourceCommit(4)); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java index b7ad9d1a1..c198385e8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java @@ -121,7 +121,7 @@ public class BlameGenerator { /** Revision pool used to acquire commits from. */ private RevWalk revPool; - /** Indicates the commit has already been processed. */ + /** Indicates the commit was put into the queue at least once. */ private RevFlag SEEN; private ObjectReader reader; @@ -146,7 +146,7 @@ public class BlameGenerator { /** * Create a blame generator for the repository and path (relative to * repository) - * + * * @param repository * repository to access revision data from. * @param path @@ -532,6 +532,7 @@ public class BlameGenerator { private void push(BlobCandidate toInsert) { Candidate c = queue; if (c != null) { + c.remove(SEEN); // will be pushed by toInsert c.regionList = null; toInsert.parent = c; } @@ -539,8 +540,24 @@ public class BlameGenerator { } private void push(Candidate toInsert) { - // Mark sources to ensure they get discarded (above) if - // another path to the same commit. + if (toInsert.has(SEEN)) { + // We have already added a Candidate for this commit to the queue, + // this can happen if the commit is a merge base for two or more + // parallel branches that were merged together. + // + // It is likely the candidate was not yet processed. The queue + // sorts descending by commit time and usually descendant commits + // have higher timestamps than the ancestors. + // + // Find the existing candidate and merge the new candidate's + // region list into it. + for (Candidate p = queue; p != null; p = p.queueNext) { + if (p.canMergeRegions(toInsert)) { + p.mergeRegions(toInsert); + return; + } + } + } toInsert.add(SEEN); // Insert into the queue using descending commit time, so @@ -567,8 +584,6 @@ public class BlameGenerator { RevCommit parent = n.getParent(0); if (parent == null) return split(n.getNextCandidate(0), n); - if (parent.has(SEEN)) - return false; revPool.parseHeaders(parent); if (find(parent, n.sourcePath)) { @@ -636,20 +651,12 @@ public class BlameGenerator { private boolean processMerge(Candidate n) throws IOException { int pCnt = n.getParentCount(); - for (int pIdx = 0; pIdx < pCnt; pIdx++) { - RevCommit parent = n.getParent(pIdx); - if (parent.has(SEEN)) - continue; - revPool.parseHeaders(parent); - } - // If any single parent exactly matches the merge, follow only // that one parent through history. ObjectId[] ids = null; for (int pIdx = 0; pIdx < pCnt; pIdx++) { RevCommit parent = n.getParent(pIdx); - if (parent.has(SEEN)) - continue; + revPool.parseHeaders(parent); if (!find(parent, n.sourcePath)) continue; if (!(n instanceof ReverseCandidate) && idBuf.equals(n.sourceBlob)) { @@ -668,8 +675,6 @@ public class BlameGenerator { renames = new DiffEntry[pCnt]; for (int pIdx = 0; pIdx < pCnt; pIdx++) { RevCommit parent = n.getParent(pIdx); - if (parent.has(SEEN)) - continue; if (ids != null && ids[pIdx] != null) continue; @@ -702,8 +707,6 @@ public class BlameGenerator { Candidate[] parents = new Candidate[pCnt]; for (int pIdx = 0; pIdx < pCnt; pIdx++) { RevCommit parent = n.getParent(pIdx); - if (parent.has(SEEN)) - continue; Candidate p; if (renames != null && renames[pIdx] != null) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java index 95b4cec02..cbc5b43a6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/Candidate.java @@ -124,10 +124,18 @@ class Candidate { return null; } + boolean has(RevFlag flag) { + return sourceCommit.has(flag); + } + void add(RevFlag flag) { sourceCommit.add(flag); } + void remove(RevFlag flag) { + sourceCommit.remove(flag); + } + int getTime() { return sourceCommit.getCommitTime(); } @@ -275,6 +283,42 @@ class Candidate { return r; } + boolean canMergeRegions(Candidate other) { + return sourceCommit == other.sourceCommit + && sourcePath.getPath().equals(other.sourcePath.getPath()); + } + + void mergeRegions(Candidate other) { + // regionList is always sorted by resultStart. Merge join two + // linked lists, preserving the ordering. Combine neighboring + // regions to reduce the number of results seen by callers. + Region a = clearRegionList(); + Region b = other.clearRegionList(); + Region t = null; + + while (a != null && b != null) { + if (a.resultStart < b.resultStart) { + Region n = a.next; + t = add(t, this, a); + a = n; + } else { + Region n = b.next; + t = add(t, this, b); + b = n; + } + } + + if (a != null) { + Region n = a.next; + t = add(t, this, a); + t.next = n; + } else /* b != null */{ + Region n = b.next; + t = add(t, this, b); + t.next = n; + } + } + @SuppressWarnings("nls") @Override public String toString() { @@ -369,11 +413,22 @@ class Candidate { return parent; } + @Override + boolean has(RevFlag flag) { + return true; // Pretend flag was added; sourceCommit is null. + } + @Override void add(RevFlag flag) { // Do nothing, sourceCommit is null. } + @Override + + void remove(RevFlag flag) { + // Do nothing, sourceCommit is null. + } + @Override int getTime() { return Integer.MAX_VALUE;