From 979e3467112618cc787e161097986212eaaa4533 Mon Sep 17 00:00:00 2001 From: Stefan Lay Date: Wed, 6 Nov 2013 11:16:34 +0100 Subject: [PATCH] Interactive Rebase: Do actions if there were conflicts If a commit was marked for edit, reword, squash or fixup, but the interactive rebase stopped because of a conflict, the step was not done after conflict resolution. This is done now. Change-Id: If8e7ccc50469165744f2b8a53d180f9ba0f72330 Signed-off-by: Stefan Lay Signed-off-by: Matthias Sohn --- .../eclipse/jgit/api/RebaseCommandTest.java | 207 ++++++++++++++++++ .../org/eclipse/jgit/api/RebaseCommand.java | 197 ++++++++++------- 2 files changed, 324 insertions(+), 80 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 531e1b0e2..241d099d1 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 @@ -2327,6 +2327,213 @@ public String modifyCommitMessage(String commit) { } + @Test + public void testRebaseShouldStopForEditInCaseOfConflict() + throws Exception { + // create file1 on master + writeTrashFile(FILE1, FILE1); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Add file1\nnew line").call(); + assertTrue(new File(db.getWorkTree(), FILE1).exists()); + + //change file1 + writeTrashFile(FILE1, FILE1 + "a"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Change file1").call(); + + //change file1 + writeTrashFile(FILE1, FILE1 + "b"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Change file1").call(); + + RebaseResult result = git.rebase().setUpstream("HEAD~2") + .runInteractively(new InteractiveHandler() { + + public void prepareSteps(List steps) { + steps.remove(0); + steps.get(0).setAction(Action.EDIT); + } + + public String modifyCommitMessage(String commit) { + return commit; + } + }).call(); + assertEquals(Status.STOPPED, result.getStatus()); + git.add().addFilepattern(FILE1).call(); + result = git.rebase().setOperation(Operation.CONTINUE).call(); + assertEquals(Status.EDIT, result.getStatus()); + + } + + @Test + public void testRebaseShouldStopForRewordInCaseOfConflict() + throws Exception { + // create file1 on master + writeTrashFile(FILE1, FILE1); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Add file1\nnew line").call(); + assertTrue(new File(db.getWorkTree(), FILE1).exists()); + + // change file1 + writeTrashFile(FILE1, FILE1 + "a"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Change file1").call(); + + // change file1 + writeTrashFile(FILE1, FILE1 + "b"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Change file1").call(); + + RebaseResult result = git.rebase().setUpstream("HEAD~2") + .runInteractively(new InteractiveHandler() { + + public void prepareSteps(List steps) { + steps.remove(0); + steps.get(0).setAction(Action.REWORD); + } + + public String modifyCommitMessage(String commit) { + return "rewritten commit message"; + } + }).call(); + assertEquals(Status.STOPPED, result.getStatus()); + git.add().addFilepattern(FILE1).call(); + result = git.rebase().runInteractively(new InteractiveHandler() { + + public void prepareSteps(List steps) { + steps.remove(0); + steps.get(0).setAction(Action.REWORD); + } + + public String modifyCommitMessage(String commit) { + return "rewritten commit message"; + } + }).setOperation(Operation.CONTINUE).call(); + assertEquals(Status.OK, result.getStatus()); + Iterator logIterator = git.log().all().call().iterator(); + String actualCommitMag = logIterator.next().getShortMessage(); + assertEquals("rewritten commit message", actualCommitMag); + + } + + @Test + public void testRebaseShouldSquashInCaseOfConflict() throws Exception { + // create file1 on master + writeTrashFile(FILE1, FILE1); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Add file1\nnew line").call(); + assertTrue(new File(db.getWorkTree(), FILE1).exists()); + + // change file2 + writeTrashFile("file2", "file2"); + git.add().addFilepattern("file2").call(); + git.commit().setMessage("Change file2").call(); + + // change file1 + writeTrashFile(FILE1, FILE1 + "a"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Change file1").call(); + + // change file1 + writeTrashFile(FILE1, FILE1 + "b"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Change file1").call(); + + RebaseResult result = git.rebase().setUpstream("HEAD~3") + .runInteractively(new InteractiveHandler() { + + public void prepareSteps(List steps) { + steps.get(0).setAction(Action.PICK); + steps.remove(1); + steps.get(1).setAction(Action.SQUASH); + } + + public String modifyCommitMessage(String commit) { + return "squashed message"; + } + }).call(); + assertEquals(Status.STOPPED, result.getStatus()); + git.add().addFilepattern(FILE1).call(); + result = git.rebase().runInteractively(new InteractiveHandler() { + + public void prepareSteps(List steps) { + steps.get(0).setAction(Action.PICK); + steps.remove(1); + steps.get(1).setAction(Action.SQUASH); + } + + public String modifyCommitMessage(String commit) { + return "squashed message"; + } + }).setOperation(Operation.CONTINUE).call(); + assertEquals(Status.OK, result.getStatus()); + Iterator logIterator = git.log().all().call().iterator(); + String actualCommitMag = logIterator.next().getShortMessage(); + assertEquals("squashed message", actualCommitMag); + } + + @Test + public void testRebaseShouldFixupInCaseOfConflict() throws Exception { + // create file1 on master + writeTrashFile(FILE1, FILE1); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Add file1").call(); + assertTrue(new File(db.getWorkTree(), FILE1).exists()); + + // change file2 + writeTrashFile("file2", "file2"); + git.add().addFilepattern("file2").call(); + git.commit().setMessage("Change file2").call(); + + // change file1 + writeTrashFile(FILE1, FILE1 + "a"); + git.add().addFilepattern(FILE1).call(); + git.commit().setMessage("Change file1").call(); + + // change file1, add file3 + writeTrashFile(FILE1, FILE1 + "b"); + writeTrashFile("file3", "file3"); + git.add().addFilepattern(FILE1).call(); + git.add().addFilepattern("file3").call(); + git.commit().setMessage("Change file1, add file3").call(); + + RebaseResult result = git.rebase().setUpstream("HEAD~3") + .runInteractively(new InteractiveHandler() { + + public void prepareSteps(List steps) { + steps.get(0).setAction(Action.PICK); + steps.remove(1); + steps.get(1).setAction(Action.FIXUP); + } + + public String modifyCommitMessage(String commit) { + return commit; + } + }).call(); + assertEquals(Status.STOPPED, result.getStatus()); + git.add().addFilepattern(FILE1).call(); + result = git.rebase().runInteractively(new InteractiveHandler() { + + public void prepareSteps(List steps) { + steps.get(0).setAction(Action.PICK); + steps.remove(1); + steps.get(1).setAction(Action.FIXUP); + } + + public String modifyCommitMessage(String commit) { + return "commit"; + } + }).setOperation(Operation.CONTINUE).call(); + assertEquals(Status.OK, result.getStatus()); + Iterator logIterator = git.log().all().call().iterator(); + String actualCommitMsg = logIterator.next().getShortMessage(); + assertEquals("Change file2", actualCommitMsg); + actualCommitMsg = logIterator.next().getShortMessage(); + assertEquals("Add file1", actualCommitMsg); + assertTrue(new File(db.getWorkTree(), "file3").exists()); + + } + private File getTodoFile() { File todoFile = new File(db.getDirectory(), GIT_REBASE_TODO); return todoFile; 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 ea48688ff..80f46ccf5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java @@ -61,11 +61,13 @@ import org.eclipse.jgit.api.RebaseResult.Status; import org.eclipse.jgit.api.ResetCommand.ResetType; import org.eclipse.jgit.api.errors.CheckoutConflictException; +import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.InvalidRebaseStepException; 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.NoMessageException; import org.eclipse.jgit.api.errors.RefAlreadyExistsException; import org.eclipse.jgit.api.errors.RefNotFoundException; import org.eclipse.jgit.api.errors.UnmergedPathsException; @@ -75,6 +77,7 @@ import org.eclipse.jgit.dircache.DirCacheCheckout; import org.eclipse.jgit.dircache.DirCacheIterator; import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.AbbreviatedObjectId; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.NullProgressMonitor; @@ -197,6 +200,10 @@ public enum Operation { private boolean stopAfterInitialization = false; + private RevCommit newHead; + + private boolean lastStepWasForward; + /** * @param repo */ @@ -220,8 +227,8 @@ protected RebaseCommand(Repository repo) { */ public RebaseResult call() throws GitAPIException, NoHeadException, RefNotFoundException, WrongRepositoryStateException { - RevCommit newHead = null; - boolean lastStepWasForward = false; + newHead = null; + lastStepWasForward = false; checkCallable(); checkParameters(); try { @@ -261,7 +268,19 @@ public RebaseResult call() throws GitAPIException, NoHeadException, if (operation == Operation.CONTINUE) { newHead = continueRebase(); - + List doneLines = repo.readRebaseTodo( + rebaseState.getPath(DONE), true); + RebaseTodoLine step = doneLines.get(doneLines.size() - 1); + if (newHead != null + && step.getAction() != Action.PICK) { + RebaseTodoLine newStep = new RebaseTodoLine( + step.getAction(), + AbbreviatedObjectId.fromObjectId(newHead), + step.getShortMessage()); + RebaseResult result = processStep(newStep, false); + if (result != null) + return result; + } File amendFile = rebaseState.getFile(AMEND); boolean amendExists = amendFile.exists(); if (amendExists) { @@ -280,8 +299,6 @@ public RebaseResult call() throws GitAPIException, NoHeadException, if (operation == Operation.SKIP) newHead = checkoutCurrentHead(); - ObjectReader or = repo.newObjectReader(); - List steps = repo.readRebaseTodo( rebaseState.getPath(GIT_REBASE_TODO), false); if (steps.size() == 0) { @@ -296,81 +313,9 @@ public RebaseResult call() throws GitAPIException, NoHeadException, for (int i = 0; i < steps.size(); i++) { RebaseTodoLine step = steps.get(i); popSteps(1); - if (Action.COMMENT.equals(step.getAction())) - continue; - Collection ids = or.resolve(step.getCommit()); - if (ids.size() != 1) - throw new JGitInternalException( - "Could not resolve uniquely the abbreviated object ID"); - RevCommit commitToPick = walk - .parseCommit(ids.iterator().next()); - if (monitor.isCancelled()) - return new RebaseResult(commitToPick, Status.STOPPED); - try { - monitor.beginTask(MessageFormat.format( - JGitText.get().applyingCommit, - commitToPick.getShortMessage()), - ProgressMonitor.UNKNOWN); - // 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 = 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 - String ourCommitName = getOurCommitName(); - CherryPickResult cherryPickResult = new Git(repo) - .cherryPick().include(commitToPick) - .setOurCommitName(ourCommitName) - .setReflogPrefix("rebase:").call(); //$NON-NLS-1$ - switch (cherryPickResult.getStatus()) { - case FAILED: - if (operation == Operation.BEGIN) - return abort(new RebaseResult( - cherryPickResult.getFailingPaths())); - else - return stop(commitToPick, Status.STOPPED); - case CONFLICTING: - return stop(commitToPick, Status.STOPPED); - case OK: - newHead = cherryPickResult.getNewHead(); - } - } - boolean isSquash = false; - switch (step.getAction()) { - case PICK: - continue; // continue rebase process on pick command - case REWORD: - String oldMessage = commitToPick.getFullMessage(); - String newMessage = interactiveHandler - .modifyCommitMessage(oldMessage); - newHead = new Git(repo).commit().setMessage(newMessage) - .setAmend(true).call(); - continue; - case EDIT: - rebaseState.createFile(AMEND, commitToPick.name()); - return stop(commitToPick, Status.EDIT); - case COMMENT: - break; - case SQUASH: - isSquash = true; - //$FALL-THROUGH$ - case FIXUP: - resetSoftToParent(); - RebaseTodoLine nextStep = (i >= steps.size() - 1 ? null - : steps.get(i + 1)); - File messageFixupFile = rebaseState.getFile(MESSAGE_FIXUP); - File messageSquashFile = rebaseState - .getFile(MESSAGE_SQUASH); - if (isSquash && messageFixupFile.exists()) - messageFixupFile.delete(); - newHead = doSquashFixup(isSquash, commitToPick, - nextStep, messageFixupFile, messageSquashFile); - } - } finally { - monitor.endTask(); + RebaseResult result = processStep(step, true); + if (result != null) { + return result; } } return finishRebase(newHead, lastStepWasForward); @@ -381,6 +326,98 @@ public RebaseResult call() throws GitAPIException, NoHeadException, } } + private RebaseResult processStep(RebaseTodoLine step, boolean shouldPick) + throws IOException, GitAPIException { + if (Action.COMMENT.equals(step.getAction())) + return null; + ObjectReader or = repo.newObjectReader(); + + Collection ids = or.resolve(step.getCommit()); + if (ids.size() != 1) + throw new JGitInternalException( + "Could not resolve uniquely the abbreviated object ID"); + RevCommit commitToPick = walk.parseCommit(ids.iterator().next()); + if (shouldPick) { + if (monitor.isCancelled()) + return new RebaseResult(commitToPick, Status.STOPPED); + RebaseResult result = cherryPickCommit(commitToPick); + if (result != null) + return result; + } + boolean isSquash = false; + switch (step.getAction()) { + case PICK: + return null; // continue rebase process on pick command + case REWORD: + String oldMessage = commitToPick.getFullMessage(); + String newMessage = interactiveHandler + .modifyCommitMessage(oldMessage); + newHead = new Git(repo).commit().setMessage(newMessage) + .setAmend(true).call(); + return null; + case EDIT: + rebaseState.createFile(AMEND, commitToPick.name()); + return stop(commitToPick, Status.EDIT); + case COMMENT: + break; + case SQUASH: + isSquash = true; + //$FALL-THROUGH$ + case FIXUP: + resetSoftToParent(); + List steps = repo.readRebaseTodo( + rebaseState.getPath(GIT_REBASE_TODO), false); + RebaseTodoLine nextStep = steps.size() > 0 ? steps.get(0) : null; + File messageFixupFile = rebaseState.getFile(MESSAGE_FIXUP); + File messageSquashFile = rebaseState.getFile(MESSAGE_SQUASH); + if (isSquash && messageFixupFile.exists()) + messageFixupFile.delete(); + newHead = doSquashFixup(isSquash, commitToPick, nextStep, + messageFixupFile, messageSquashFile); + } + return null; + } + + private RebaseResult cherryPickCommit(RevCommit commitToPick) + throws IOException, GitAPIException, NoMessageException, + UnmergedPathsException, ConcurrentRefUpdateException, + WrongRepositoryStateException, NoHeadException { + try { + monitor.beginTask(MessageFormat.format( + JGitText.get().applyingCommit, + commitToPick.getShortMessage()), ProgressMonitor.UNKNOWN); + // 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 = 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 + String ourCommitName = getOurCommitName(); + CherryPickResult cherryPickResult = new Git(repo).cherryPick() + .include(commitToPick).setOurCommitName(ourCommitName) + .setReflogPrefix("rebase:").call(); //$NON-NLS-1$ + switch (cherryPickResult.getStatus()) { + case FAILED: + if (operation == Operation.BEGIN) + return abort(new RebaseResult( + cherryPickResult.getFailingPaths())); + else + return stop(commitToPick, Status.STOPPED); + case CONFLICTING: + return stop(commitToPick, Status.STOPPED); + case OK: + newHead = cherryPickResult.getNewHead(); + } + } + return null; + } finally { + monitor.endTask(); + } + } + private RebaseResult finishRebase(RevCommit newHead, boolean lastStepWasForward) throws IOException { String headName = rebaseState.readFile(HEAD_NAME);