summaryrefslogtreecommitdiffstatsabout
diff options
context:
space:
mode:
authorMathias Kinzler2011-01-28 09:03:02 (EST)
committer Mathias Kinzler2011-01-28 09:03:02 (EST)
commite8a1328d05aa55b7ace0d101e34b87422219c959 (patch)
treea1c475e37f8456cc95752a1144b3589a824370e9
parentc544e96a4cd027a127bc6e6ad5277091b3c0da73 (diff)
downloadjgit-e8a1328d05aa55b7ace0d101e34b87422219c959.zip
jgit-e8a1328d05aa55b7ace0d101e34b87422219c959.tar.gz
jgit-e8a1328d05aa55b7ace0d101e34b87422219c959.tar.bz2
RebaseCommand: detect and handle fast-forward properlyrefs/changes/64/2364/3
This bug was hidden by an incomplete test: the current Rebase implementation using the "git rebase -i" pattern does not work correctly if fast-forwarding is involved. The reason for this is that the log command does not return any commits in this case. In addition, a check for already merged commits was introduced to avoid spurious conflicts. Change-Id: Ib9898fe0f982fa08e41f1dca9452c43de715fdb6 Signed-off-by: Mathias Kinzler <mathias.kinzler@sap.com>
-rw-r--r--org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java35
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java105
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java6
3 files changed, 139 insertions, 7 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
index af3bb1c..f1a3f78 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
@@ -132,7 +132,7 @@ public class RebaseCommandTest extends RepositoryTestCase {
// create a topic branch
createBranch(first, "refs/heads/topic");
// create file2 on master
- writeTrashFile("file2", "file2");
+ File file2 = writeTrashFile("file2", "file2");
git.add().addFilepattern("file2").call();
git.commit().setMessage("Add file2").call();
assertTrue(new File(db.getWorkTree(), "file2").exists());
@@ -141,7 +141,38 @@ public class RebaseCommandTest extends RepositoryTestCase {
assertFalse(new File(db.getWorkTree(), "file2").exists());
RebaseResult res = git.rebase().setUpstream("refs/heads/master").call();
- assertEquals(Status.UP_TO_DATE, res.getStatus());
+ assertTrue(new File(db.getWorkTree(), "file2").exists());
+ checkFile(file2, "file2");
+ assertEquals(Status.FAST_FORWARD, res.getStatus());
+ }
+
+ @Test
+ public void testFastForwardWithMultipleCommits() throws Exception {
+ // create file1 on master
+ writeTrashFile(FILE1, FILE1);
+ git.add().addFilepattern(FILE1).call();
+ RevCommit first = git.commit().setMessage("Add file1").call();
+
+ assertTrue(new File(db.getWorkTree(), FILE1).exists());
+ // create a topic branch
+ createBranch(first, "refs/heads/topic");
+ // create file2 on master
+ File file2 = writeTrashFile("file2", "file2");
+ git.add().addFilepattern("file2").call();
+ git.commit().setMessage("Add file2").call();
+ assertTrue(new File(db.getWorkTree(), "file2").exists());
+ // write a second commit
+ writeTrashFile("file2", "file2 new content");
+ git.add().addFilepattern("file2").call();
+ git.commit().setMessage("Change content of file2").call();
+
+ checkoutBranch("refs/heads/topic");
+ assertFalse(new File(db.getWorkTree(), "file2").exists());
+
+ RebaseResult res = git.rebase().setUpstream("refs/heads/master").call();
+ assertTrue(new File(db.getWorkTree(), "file2").exists());
+ checkFile(file2, "file2 new content");
+ assertEquals(Status.FAST_FORWARD, res.getStatus());
}
@Test
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 e5e6045..7241042 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
@@ -63,8 +63,10 @@ import java.util.Map;
import org.eclipse.jgit.JGitText;
import org.eclipse.jgit.api.RebaseResult.Status;
import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.api.errors.InvalidRefNameException;
import org.eclipse.jgit.api.errors.JGitInternalException;
import org.eclipse.jgit.api.errors.NoHeadException;
+import org.eclipse.jgit.api.errors.RefAlreadyExistsException;
import org.eclipse.jgit.api.errors.RefNotFoundException;
import org.eclipse.jgit.api.errors.UnmergedPathsException;
import org.eclipse.jgit.api.errors.WrongRepositoryStateException;
@@ -189,6 +191,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
public RebaseResult call() throws NoHeadException, RefNotFoundException,
JGitInternalException, GitAPIException {
RevCommit newHead = null;
+ boolean lastStepWasForward = false;
checkCallable();
checkParameters();
try {
@@ -237,11 +240,17 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
monitor.beginTask(MessageFormat.format(
JGitText.get().applyingCommit, commitToPick
.getShortMessage()), ProgressMonitor.UNKNOWN);
- // TODO if the first parent of commitToPick is the current HEAD,
- // we should fast-forward instead of cherry-pick to avoid
+ // if the first parent of commitToPick is the current HEAD,
+ // we do a fast-forward instead of cherry-pick to avoid
// unnecessary object rewriting
- newHead = new Git(repo).cherryPick().include(commitToPick)
- .call();
+ newHead = tryFastForward(commitToPick);
+ lastStepWasForward = newHead != null;
+ if (!lastStepWasForward)
+ // TODO if the content of this commit is already merged here
+ // we should skip this step in order to avoid confusing
+ // pseudo-changed
+ newHead = new Git(repo).cherryPick().include(commitToPick)
+ .call();
monitor.endTask();
if (newHead == null) {
return stop(commitToPick);
@@ -274,6 +283,8 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
}
}
FileUtils.delete(rebaseDir, FileUtils.RECURSIVE);
+ if (lastStepWasForward)
+ return new RebaseResult(Status.FAST_FORWARD);
return new RebaseResult(Status.OK);
}
return new RebaseResult(Status.UP_TO_DATE);
@@ -496,6 +507,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
RevCommit headCommit = walk.lookupCommit(headId);
monitor.beginTask(JGitText.get().obtainingCommitsForCherryPick,
ProgressMonitor.UNKNOWN);
+
LogCommand cmd = new Git(repo).log().addRange(upstreamCommit,
headCommit);
Iterable<RevCommit> commitsToUse = cmd.call();
@@ -503,6 +515,22 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
cherryPickList.add(commit);
}
+ // if the upstream commit is in a direct line to the current head,
+ // the log command will not report any commits; in this case,
+ // we create the cherry-pick list ourselves
+ if (cherryPickList.isEmpty()) {
+ Iterable<RevCommit> parents = new Git(repo).log().add(
+ upstreamCommit).call();
+ for (RevCommit parent : parents) {
+ if (parent.equals(headCommit))
+ break;
+ if (parent.getParentCount() != 1)
+ throw new JGitInternalException(
+ JGitText.get().canOnlyCherryPickCommitsWithOneParent);
+ cherryPickList.add(parent);
+ }
+ }
+
// nothing to do: return with UP_TO_DATE_RESULT
if (cherryPickList.isEmpty())
return RebaseResult.UP_TO_DATE_RESULT;
@@ -548,6 +576,75 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
return null;
}
+ /**
+ * checks if we can fast-forward and returns the new head if it is possible
+ *
+ * @param newCommit
+ * @return the new head, or null
+ * @throws RefNotFoundException
+ * @throws IOException
+ */
+ public RevCommit tryFastForward(RevCommit newCommit)
+ throws RefNotFoundException, IOException {
+ Ref head = repo.getRef(Constants.HEAD);
+ if (head == null || head.getObjectId() == null)
+ throw new RefNotFoundException(MessageFormat.format(
+ JGitText.get().refNotResolved, Constants.HEAD));
+
+ ObjectId headId = head.getObjectId();
+ if (headId == null)
+ throw new RefNotFoundException(MessageFormat.format(
+ JGitText.get().refNotResolved, Constants.HEAD));
+ RevCommit headCommit = walk.lookupCommit(headId);
+ if (walk.isMergedInto(newCommit, headCommit))
+ return newCommit;
+
+ String headName;
+ if (head.isSymbolic())
+ headName = head.getTarget().getName();
+ else
+ headName = "detached HEAD";
+ return tryFastForward(headName, headCommit, newCommit);
+ }
+
+ private RevCommit tryFastForward(String headName, RevCommit oldCommit,
+ RevCommit newCommit) throws IOException, JGitInternalException {
+ boolean tryRebase = false;
+ for (RevCommit parentCommit : newCommit.getParents())
+ if (parentCommit.equals(oldCommit))
+ tryRebase = true;
+ if (!tryRebase)
+ return null;
+
+ CheckoutCommand co = new CheckoutCommand(repo);
+ try {
+ co.setName(newCommit.name()).call();
+ if (headName.startsWith(Constants.R_HEADS)) {
+ RefUpdate rup = repo.updateRef(headName);
+ rup.setExpectedOldObjectId(oldCommit);
+ rup.setNewObjectId(newCommit);
+ rup.setRefLogMessage("Fast-foward from " + oldCommit.name()
+ + " to " + newCommit.name(), false);
+ Result res = rup.update(walk);
+ switch (res) {
+ case FAST_FORWARD:
+ case NO_CHANGE:
+ case FORCED:
+ break;
+ default:
+ throw new IOException("Could not fast-forward");
+ }
+ }
+ return newCommit;
+ } catch (RefAlreadyExistsException e) {
+ throw new JGitInternalException(e.getMessage(), e);
+ } catch (RefNotFoundException e) {
+ throw new JGitInternalException(e.getMessage(), e);
+ } catch (InvalidRefNameException e) {
+ throw new JGitInternalException(e.getMessage(), e);
+ }
+ }
+
private void checkParameters() throws WrongRepositoryStateException {
if (this.operation != Operation.BEGIN) {
// these operations are only possible while in a rebasing state
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java
index bdbddda..b60336c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java
@@ -67,7 +67,11 @@ public class RebaseResult {
/**
* Already up-to-date
*/
- UP_TO_DATE;
+ UP_TO_DATE,
+ /**
+ * Fast-forward, HEAD points to the new commit
+ */
+ FAST_FORWARD;
}
static final RebaseResult UP_TO_DATE_RESULT = new RebaseResult(