Fix DateRevQueue tie breaks with more than 2 elements

DateRevQueue is expected to give out the commits that have higher
commit time. But in case of tie(same commit time), it should give
the commit that is inserted first. This is inferred from the
testInsertTie test case written for DateRevQueue. Also that test
case, right now uses just two commits which caused it not to fail
with the current implementation, so added another commit to make
the test more robust.

By fixing the DateRevQueue, we would also match the behaviour of
LogCommand.addRange(c1,c2) with git log c1..c2. A test case for
the same is added to show that current behaviour is not the
expected one.

By fixing addRange(), the order in which commits are applied during
a rebase is altered. Rebase logic should have never depended upon
LogCommand.addRange() since the intended order of addRange() is not
the order a rebase should use. So, modify the RebaseCommand to use
RevWalk directly with TopoNonIntermixSortGenerator.

Add a new LogCommandTest.addRangeWithMerge() test case which creates
commits in the following order:

         A - B - C - M
              \     /
                -D-

Using git 2.30.0, git log B..M outputs:  M C D
LogCommand.addRange(B, M) without this fix outputs: M D C
LogCommand.addRange(B, M) with this fix outputs: M C D

Change-Id: I30cc3ba6c97f0960f64e9e021df96ff276f63db7
Signed-off-by: Adithya Chakilam <achakila@codeaurora.org>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LogCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LogCommandTest.java
index 6460c7988ad96ccf74ca730bf583901c6b642fc9..c563d5a47fde322594c80f827aaf54137722b2b7 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LogCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LogCommandTest.java
@@ -232,6 +232,53 @@ public class LogCommandTest extends RepositoryTestCase {
                assertFalse(i.hasNext());
        }
 
+       /**
+        * <pre>
+        * A - B - C - M
+        *      \     /
+        *        -D(side)
+        * </pre>
+        */
+       @Test
+       public void addRangeWithMerge() throws Exception{
+               String fileA = "fileA";
+               String fileB = "fileB";
+               Git git = Git.wrap(db);
+
+               writeTrashFile(fileA, fileA);
+               git.add().addFilepattern(fileA).call();
+               git.commit().setMessage("commit a").call();
+
+               writeTrashFile(fileA, fileA);
+               git.add().addFilepattern(fileA).call();
+               RevCommit b = git.commit().setMessage("commit b").call();
+
+               writeTrashFile(fileA, fileA);
+               git.add().addFilepattern(fileA).call();
+               RevCommit c = git.commit().setMessage("commit c").call();
+
+               createBranch(b, "refs/heads/side");
+               checkoutBranch("refs/heads/side");
+
+               writeTrashFile(fileB, fileB);
+               git.add().addFilepattern(fileB).call();
+               RevCommit d = git.commit().setMessage("commit d").call();
+
+               checkoutBranch("refs/heads/master");
+               MergeResult m = git.merge().include(d.getId()).call();
+               assertEquals(MergeResult.MergeStatus.MERGED, m.getMergeStatus());
+
+               Iterator<RevCommit> rangeLog = git.log().addRange(b.getId(), m.getNewHead()).call().iterator();
+
+               RevCommit commit = rangeLog.next();
+               assertEquals(m.getNewHead(), commit.getId());
+               commit = rangeLog.next();
+               assertEquals(c.getId(), commit.getId());
+               commit = rangeLog.next();
+               assertEquals(d.getId(), commit.getId());
+               assertFalse(rangeLog.hasNext());
+       }
+
        private void setCommitsAndMerge() throws Exception {
                Git git = Git.wrap(db);
                writeTrashFile("file1", "1\n2\n3\n4\n");

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/DateRevQueueTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/DateRevQueueTest.java
index a9dfe15c97e82d58c7b24408fa2d538f6988d510..852d18c351cae8d40c404dacc2bc17c472cdd5f8 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/DateRevQueueTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/DateRevQueueTest.java
@@ -59,20 +59,26 @@ public class DateRevQueueTest extends RevQueueTestCase<DateRevQueue> {
        public void testInsertTie() throws Exception {
                final RevCommit a = parseBody(commit());
                final RevCommit b = parseBody(commit(0, a));
+               final RevCommit c = parseBody(commit(0, b));
+
                {
                        q = create();
                        q.add(a);
                        q.add(b);
+                       q.add(c);
 
                        assertCommit(a, q.next());
                        assertCommit(b, q.next());
+                       assertCommit(c, q.next());
                        assertNull(q.next());
                }
                {
                        q = create();
+                       q.add(c);
                        q.add(b);
                        q.add(a);
 
+                       assertCommit(c, q.next());
                        assertCommit(b, q.next());
                        assertCommit(a, q.next());
                        assertNull(q.next());

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
index 6678af163a85327f0951d3c10f51693845f7f4e5..836175dceaa07d81ab0a380eb7de7fecaf5554ed 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
@@ -67,6 +67,7 @@ import org.eclipse.jgit.lib.RefUpdate.Result;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.merge.MergeStrategy;
 import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevSort;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.revwalk.filter.RevFilter;
 import org.eclipse.jgit.submodule.SubmoduleWalk.IgnoreSubmoduleMode;
@@ -1137,15 +1138,19 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 
        private List<RevCommit> calculatePickList(RevCommit headCommit)
                        throws GitAPIException, NoHeadException, IOException {
-               Iterable<RevCommit> commitsToUse;
-               try (Git git = new Git(repo)) {
-                       LogCommand cmd = git.log().addRange(upstreamCommit, headCommit);
-                       commitsToUse = cmd.call();
-               }
                List<RevCommit> cherryPickList = new ArrayList<>();
-               for (RevCommit commit : commitsToUse) {
-                       if (preserveMerges || commit.getParentCount() == 1)
-                               cherryPickList.add(commit);
+               try (RevWalk r = new RevWalk(repo)) {
+                       r.sort(RevSort.TOPO_KEEP_BRANCH_TOGETHER, true);
+                       r.sort(RevSort.COMMIT_TIME_DESC, true);
+                       r.markUninteresting(r.lookupCommit(upstreamCommit));
+                       r.markStart(r.lookupCommit(headCommit));
+                       Iterator<RevCommit> commitsToUse = r.iterator();
+                       while (commitsToUse.hasNext()) {
+                               RevCommit commit = commitsToUse.next();
+                               if (preserveMerges || commit.getParentCount() == 1) {
+                                       cherryPickList.add(commit);
+                               }
+                       }
                }
                Collections.reverse(cherryPickList);
 

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DateRevQueue.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DateRevQueue.java
index b875be9270c7fd6c44d567131e11f0b84a66f527..0cabf07057a2c5f018c823f01717a2b39beffa9c 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DateRevQueue.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DateRevQueue.java
@@ -93,7 +93,7 @@ public class DateRevQueue extends AbstractRevQueue {
                        head = n;
                } else {
                        Entry p = q.next;
-                       while (p != null && p.commit.commitTime > when) {
+                       while (p != null && p.commit.commitTime >= when) {
                                q = p;
                                p = q.next;
                        }