Update the revert command and things relating to revert

Cherry-pick has been fixed, but even though revert does
basically the same thing, the fixes were not carried over here.

- Recognize the revert-states, analogous to the cherry picking states
- Make reset handle a revert-in-progress
- Update REVERT_HEAD and MERGE_MSG when revert fails due to conflicts
- Clear revert state on commit and reset
- Format the message similarily to how cherry-pick does. This is
  not exactly how C Git does it.

The interface is still not the same as for cherry-picking.

Change-Id: I8ea956fcbc9526d62a2365360feea23a9280eba3
Signed-off-by: Chris Aniszczyk <zx@twitter.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RevertCommandTest.java

@@ -48,10 +48,17 @@ import java.io.File;
 import java.io.IOException;
 import java.util.Iterator;
 
+import org.eclipse.jgit.api.MergeResult.MergeStatus;
+import org.eclipse.jgit.api.ResetCommand.ResetType;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.api.errors.JGitInternalException;
+import org.eclipse.jgit.dircache.DirCache;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.FileMode;
+import org.eclipse.jgit.lib.RepositoryState;
 import org.eclipse.jgit.lib.RepositoryTestCase;
+import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.storage.file.ReflogReader;
 import org.junit.Test;
@@ -111,4 +118,205 @@ public class RevertCommandTest extends RepositoryTestCase {
 				.startsWith("revert: Revert \""));
 
 	}
+
+	@Test
+	public void testRevertDirtyIndex() throws Exception {
+		Git git = new Git(db);
+		RevCommit sideCommit = prepareRevert(git);
+
+		// modify and add file a
+		writeTrashFile("a", "a(modified)");
+		git.add().addFilepattern("a").call();
+		// do not commit
+
+		doRevertAndCheckResult(git, sideCommit,
+				MergeFailureReason.DIRTY_INDEX);
+	}
+
+	@Test
+	public void testRevertDirtyWorktree() throws Exception {
+		Git git = new Git(db);
+		RevCommit sideCommit = prepareRevert(git);
+
+		// modify file a
+		writeTrashFile("a", "a(modified)");
+		// do not add and commit
+
+		doRevertAndCheckResult(git, sideCommit,
+				MergeFailureReason.DIRTY_WORKTREE);
+	}
+
+	@Test
+	public void testRevertConflictResolution() throws Exception {
+		Git git = new Git(db);
+		RevCommit sideCommit = prepareRevert(git);
+
+		RevertCommand revert = git.revert();
+		RevCommit newHead = revert.include(sideCommit.getId()).call();
+		assertNull(newHead);
+		MergeResult result = revert.getFailingResult();
+		assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus());
+		assertTrue(new File(db.getDirectory(), Constants.MERGE_MSG).exists());
+		assertEquals("Revert \"" + sideCommit.getShortMessage()
+				+ "\"\n\nThis reverts commit " + sideCommit.getId().getName()
+				+ ".\n\nConflicts:\n\ta\n",
+				db.readMergeCommitMsg());
+		assertTrue(new File(db.getDirectory(), Constants.REVERT_HEAD)
+				.exists());
+		assertEquals(sideCommit.getId(), db.readRevertHead());
+		assertEquals(RepositoryState.REVERTING, db.getRepositoryState());
+
+		// Resolve
+		writeTrashFile("a", "a");
+		git.add().addFilepattern("a").call();
+
+		assertEquals(RepositoryState.REVERTING_RESOLVED,
+				db.getRepositoryState());
+
+		git.commit().setOnly("a").setMessage("resolve").call();
+
+		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
+	}
+
+	@Test
+	public void testRevertkConflictReset() throws Exception {
+		Git git = new Git(db);
+
+		RevCommit sideCommit = prepareRevert(git);
+
+		RevertCommand revert = git.revert();
+		RevCommit newHead = revert.include(sideCommit.getId()).call();
+		assertNull(newHead);
+		MergeResult result = revert.getFailingResult();
+
+		assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus());
+		assertEquals(RepositoryState.REVERTING, db.getRepositoryState());
+		assertTrue(new File(db.getDirectory(), Constants.REVERT_HEAD)
+				.exists());
+
+		git.reset().setMode(ResetType.MIXED).setRef("HEAD").call();
+
+		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
+		assertFalse(new File(db.getDirectory(), Constants.REVERT_HEAD)
+				.exists());
+	}
+
+	@Test
+	public void testRevertOverExecutableChangeOnNonExectuableFileSystem()
+			throws Exception {
+		Git git = new Git(db);
+		File file = writeTrashFile("test.txt", "a");
+		assertNotNull(git.add().addFilepattern("test.txt").call());
+		assertNotNull(git.commit().setMessage("commit1").call());
+
+		assertNotNull(git.checkout().setCreateBranch(true).setName("a").call());
+
+		writeTrashFile("test.txt", "b");
+		assertNotNull(git.add().addFilepattern("test.txt").call());
+		RevCommit commit2 = git.commit().setMessage("commit2").call();
+		assertNotNull(commit2);
+
+		assertNotNull(git.checkout().setName(Constants.MASTER).call());
+
+		DirCache cache = db.lockDirCache();
+		cache.getEntry("test.txt").setFileMode(FileMode.EXECUTABLE_FILE);
+		cache.write();
+		assertTrue(cache.commit());
+		cache.unlock();
+
+		assertNotNull(git.commit().setMessage("commit3").call());
+
+		db.getFS().setExecute(file, false);
+		git.getRepository()
+				.getConfig()
+				.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_FILEMODE, false);
+
+		RevertCommand revert = git.revert();
+		RevCommit newHead = revert.include(commit2).call();
+		assertNotNull(newHead);
+	}
+
+	@Test
+	public void testRevertConflictMarkers() throws Exception {
+		Git git = new Git(db);
+		RevCommit sideCommit = prepareRevert(git);
+
+		RevertCommand revert = git.revert();
+		RevCommit newHead = revert.include(sideCommit.getId())
+				.call();
+		assertNull(newHead);
+		MergeResult result = revert.getFailingResult();
+		assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus());
+
+		String expected = "<<<<<<< master\na(latest)\n=======\na\n>>>>>>> ca96c31 second master\n";
+		checkFile(new File(db.getWorkTree(), "a"), expected);
+	}
+
+	@Test
+	public void testRevertOurCommitName() throws Exception {
+		Git git = new Git(db);
+		RevCommit sideCommit = prepareRevert(git);
+
+		RevertCommand revert = git.revert();
+		RevCommit newHead = revert.include(sideCommit.getId())
+				.setOurCommitName("custom name").call();
+		assertNull(newHead);
+		MergeResult result = revert.getFailingResult();
+		assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus());
+
+		String expected = "<<<<<<< custom name\na(latest)\n=======\na\n>>>>>>> ca96c31 second master\n";
+		checkFile(new File(db.getWorkTree(), "a"), expected);
+	}
+
+	private RevCommit prepareRevert(final Git git) throws Exception {
+		// create, add and commit file a
+		writeTrashFile("a", "a");
+		git.add().addFilepattern("a").call();
+		git.commit().setMessage("first master").call();
+
+		// First commit
+		checkoutBranch("refs/heads/master");
+		// modify, add and commit file a
+		writeTrashFile("a", "a(previous)");
+		git.add().addFilepattern("a").call();
+		RevCommit oldCommit = git.commit().setMessage("second master").call();
+
+		// modify, add and commit file a
+		writeTrashFile("a", "a(latest)");
+		git.add().addFilepattern("a").call();
+		git.commit().setMessage("side").call();
+
+		return oldCommit;
+	}
+
+	private void doRevertAndCheckResult(final Git git,
+			final RevCommit sideCommit, final MergeFailureReason reason)
+			throws Exception {
+		// get current index state
+		String indexState = indexState(CONTENT);
+
+		// revert
+		RevertCommand revert = git.revert();
+		RevCommit resultCommit = revert.include(sideCommit.getId()).call();
+		assertNull(resultCommit);
+		MergeResult result = revert.getFailingResult();
+		assertEquals(MergeStatus.FAILED, result.getMergeStatus());
+		// staged file a causes DIRTY_INDEX
+		assertEquals(1, result.getFailingPaths().size());
+		assertEquals(reason, result.getFailingPaths().get("a"));
+		assertEquals("a(modified)", read(new File(db.getWorkTree(), "a")));
+		// index shall be unchanged
+		assertEquals(indexState, indexState(CONTENT));
+		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
+
+		if (reason == null) {
+			ReflogReader reader = db.getReflogReader(Constants.HEAD);
+			assertTrue(reader.getLastEntry().getComment()
+					.startsWith("revert: "));
+			reader = db.getReflogReader(db.getBranch());
+			assertTrue(reader.getLastEntry().getComment()
+					.startsWith("revert: "));
+		}
+	}
 }

org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@@ -73,7 +73,7 @@ cannotStoreObjects=cannot store objects
 cannotUnloadAModifiedTree=Cannot unload a modified tree.
 cannotWorkWithOtherStagesThanZeroRightNow=Cannot work with other stages than zero right now. Won't write corrupt index.
 canOnlyCherryPickCommitsWithOneParent=Cannot cherry-pick commit ''{0}'' because it has {1} parents, only commits with exactly one parent are supported.
-canOnlyRevertCommitsWithOneParent=Can only revert commits which have exactly one parent
+canOnlyRevertCommitsWithOneParent=Cannot revert commit ''{0}'' because it has {1} parents, only commits with exactly one parent are supported
 cantFindObjectInReversePackIndexForTheSpecifiedOffset=Can't find object in (reverse) pack index for the specified offset {0}
 cantPassMeATree=Can't pass me a tree!
 channelMustBeInRange0_255=channel {0} must be in range [0, 255]

org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java

@@ -249,6 +249,9 @@ public class CommitCommand extends GitCommand<RevCommit> {
 							} else if (state == RepositoryState.CHERRY_PICKING_RESOLVED) {
 								repo.writeMergeCommitMsg(null);
 								repo.writeCherryPickHead(null);
+							} else if (state == RepositoryState.REVERTING_RESOLVED) {
+								repo.writeMergeCommitMsg(null);
+								repo.writeRevertHead(null);
 							}
 							return revCommit;
 						}

org.eclipse.jgit/src/org/eclipse/jgit/api/ResetCommand.java

@@ -149,6 +149,8 @@ public class ResetCommand extends GitCommand<Ref> {
 			final boolean cherryPicking = state
 					.equals(RepositoryState.CHERRY_PICKING)
 					|| state.equals(RepositoryState.CHERRY_PICKING_RESOLVED);
+			final boolean reverting = state.equals(RepositoryState.REVERTING)
+					|| state.equals(RepositoryState.REVERTING_RESOLVED);
 
 			// resolve the ref to a commit
 			final ObjectId commitId;
@@ -219,6 +221,8 @@ public class ResetCommand extends GitCommand<Ref> {
 					resetMerge();
 				else if (cherryPicking)
 					resetCherryPick();
+				else if (reverting)
+					resetRevert();
 				else if (repo.readSquashCommitMsg() != null)
 					repo.writeSquashCommitMsg(null /* delete */);
 			}
@@ -376,4 +380,9 @@ public class ResetCommand extends GitCommand<Ref> {
 		repo.writeMergeCommitMsg(null);
 	}
 
+	private void resetRevert() throws IOException {
+		repo.writeRevertHead(null);
+		repo.writeMergeCommitMsg(null);
+	}
+
 }

org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java

@@ -66,6 +66,7 @@ import org.eclipse.jgit.lib.ObjectIdRef;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Ref.Storage;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.merge.MergeMessageFormatter;
 import org.eclipse.jgit.merge.MergeStrategy;
 import org.eclipse.jgit.merge.ResolveMerger;
 import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
@@ -86,6 +87,8 @@ import org.eclipse.jgit.treewalk.FileTreeIterator;
 public class RevertCommand extends GitCommand<RevCommit> {
 	private List<Ref> commits = new LinkedList<Ref>();
 
+	private String ourCommitName = null;
+
 	private List<Ref> revertedRefs = new LinkedList<Ref>();
 
 	private MergeResult failingResult;
@@ -143,18 +146,31 @@ public class RevertCommand extends GitCommand<RevCommit> {
 				RevCommit srcCommit = revWalk.parseCommit(srcObjectId);
 
 				// get the parent of the commit to revert
-				if (srcCommit.getParentCount() != 1) {
+				if (srcCommit.getParentCount() != 1)
 					throw new MultipleParentsNotAllowedException(
-							JGitText.get().canOnlyRevertCommitsWithOneParent);
+							MessageFormat.format(
-				}
+									JGitText.get().canOnlyRevertCommitsWithOneParent,
+									srcCommit.name(),
+									Integer.valueOf(srcCommit.getParentCount())));
+
 				RevCommit srcParent = srcCommit.getParent(0);
 				revWalk.parseHeaders(srcParent);
 
+				String ourName = calculateOurName(headRef);
+				String revertName = srcCommit.getId().abbreviate(7).name()
+						+ " " + srcCommit.getShortMessage(); //$NON-NLS-1$
+
 				ResolveMerger merger = (ResolveMerger) MergeStrategy.RESOLVE
 						.newMerger(repo);
 				merger.setWorkingTreeIterator(new FileTreeIterator(repo));
 				merger.setBase(srcCommit.getTree());
+				merger.setCommitNames(new String[] { "BASE", ourName, revertName }); //$NON-NLS-1$ //$NON-NLS-2$
 
+				String shortMessage = "Revert \"" + srcCommit.getShortMessage() //$NON-NLS-1$
+						+ "\""; //$NON-NLS-1$
+				String newMessage = shortMessage + "\n\n"
+						+ "This reverts commit " + srcCommit.getId().getName() //$NON-NLS-1$
+						+ ".\n"; //$NON-NLS-1$
 				if (merger.merge(headCommit, srcParent)) {
 					if (AnyObjectId.equals(headCommit.getTree().getId(), merger
 							.getResultTreeId()))
@@ -164,10 +180,6 @@ public class RevertCommand extends GitCommand<RevCommit> {
 							merger.getResultTreeId());
 					dco.setFailOnConflict(true);
 					dco.checkout();
-					String shortMessage = "Revert \"" + srcCommit.getShortMessage() + "\""; //$NON-NLS-2$
-					String newMessage = shortMessage + "\n\n" //$NON-NLS-1$
-							+ "This reverts commit " //$NON-NLS-1$
-							+ srcCommit.getId().getName() + ".\n"; //$NON-NLS-1$
 					newHead = new Git(getRepository()).commit()
 							.setMessage(newMessage)
 							.setReflogComment("revert: " + shortMessage).call(); //$NON-NLS-1$
@@ -183,6 +195,20 @@ public class RevertCommand extends GitCommand<RevCommit> {
 										srcParent.getId() },
 								MergeStatus.FAILED, MergeStrategy.RESOLVE,
 								merger.getMergeResults(), failingPaths, null);
+					else
+						failingResult = new MergeResult(null,
+								merger.getBaseCommit(0, 1),
+								new ObjectId[] { headCommit.getId(),
+										srcParent.getId() },
+								MergeStatus.CONFLICTING, MergeStrategy.RESOLVE,
+								merger.getMergeResults(), failingPaths, null);
+					if (!merger.failed() && !unmergedPaths.isEmpty()) {
+						String message = new MergeMessageFormatter()
+						.formatWithConflicts(newMessage,
+								merger.getUnmergedPaths());
+						repo.writeRevertHead(srcCommit.getId());
+						repo.writeMergeCommitMsg(message);
+					}
 					return null;
 				}
 			}
@@ -230,6 +256,26 @@ public class RevertCommand extends GitCommand<RevCommit> {
 				commit.copy()));
 	}
 
+	/**
+	 * @param ourCommitName
+	 *            the name that should be used in the "OURS" place for conflict
+	 *            markers
+	 * @return {@code this}
+	 */
+	public RevertCommand setOurCommitName(String ourCommitName) {
+		this.ourCommitName = ourCommitName;
+		return this;
+	}
+
+	private String calculateOurName(Ref headRef) {
+		if (ourCommitName != null)
+			return ourCommitName;
+
+		String targetRefName = headRef.getTarget().getName();
+		String headName = Repository.shortenRefName(targetRefName);
+		return headName;
+	}
+
 	/**
 	 * @return the list of successfully reverted {@link Ref}'s. Never
 	 *         <code>null</code> but maybe an empty list if no commit was

org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java

@@ -560,6 +560,9 @@ public final class Constants {
 	/** name of the file containing the commit msg for a squash commit */
 	public static final String SQUASH_MSG = "SQUASH_MSG";
 
+	/** name of the file containing the ID of a revert commit in case of conflicts */
+	public static final String REVERT_HEAD = "REVERT_HEAD";
+
 	/**
 	 * name of the ref ORIG_HEAD used by certain commands to store the original
 	 * value of HEAD

org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java

@@ -1119,6 +1119,19 @@ public abstract class Repository {
 			return RepositoryState.CHERRY_PICKING;
 		}
 
+		if (new File(getDirectory(), Constants.REVERT_HEAD).exists()) {
+			try {
+				if (!readDirCache().hasUnmergedPaths()) {
+					// no unmerged paths
+					return RepositoryState.REVERTING_RESOLVED;
+				}
+			} catch (IOException e) {
+				// fall through to REVERTING
+			}
+
+			return RepositoryState.REVERTING;
+		}
+
 		return RepositoryState.SAFE;
 	}
 
@@ -1362,6 +1375,27 @@ public abstract class Repository {
 		return ObjectId.fromString(raw, 0);
 	}
 
+	/**
+	 * Return the information stored in the file $GIT_DIR/REVERT_HEAD.
+	 *
+	 * @return object id from REVERT_HEAD file or {@code null} if this file
+	 *         doesn't exist. Also if the file exists but is empty {@code null}
+	 *         will be returned
+	 * @throws IOException
+	 * @throws NoWorkTreeException
+	 *             if this is bare, which implies it has no working directory.
+	 *             See {@link #isBare()}.
+	 */
+	public ObjectId readRevertHead() throws IOException, NoWorkTreeException {
+		if (isBare() || getDirectory() == null)
+			throw new NoWorkTreeException();
+
+		byte[] raw = readGitDirectoryFile(Constants.REVERT_HEAD);
+		if (raw == null)
+			return null;
+		return ObjectId.fromString(raw, 0);
+	}
+
 	/**
 	 * Write cherry pick commit into $GIT_DIR/CHERRY_PICK_HEAD. This is used in
 	 * case of conflicts to store the cherry which was tried to be picked.
@@ -1377,6 +1411,21 @@ public abstract class Repository {
 		writeHeadsFile(heads, Constants.CHERRY_PICK_HEAD);
 	}
 
+	/**
+	 * Write revert commit into $GIT_DIR/REVERT_HEAD. This is used in case of
+	 * conflicts to store the revert which was tried to be picked.
+	 *
+	 * @param head
+	 *            an object id of the revert commit or <code>null</code> to
+	 *            delete the file
+	 * @throws IOException
+	 */
+	public void writeRevertHead(ObjectId head) throws IOException {
+		List<ObjectId> heads = (head != null) ? Collections.singletonList(head)
+				: null;
+		writeHeadsFile(heads, Constants.REVERT_HEAD);
+	}
+
 	/**
 	 * Write original HEAD commit into $GIT_DIR/ORIG_HEAD.
 	 *
@@ -1514,4 +1563,5 @@ public abstract class Repository {
 			FileUtils.delete(headsFile, FileUtils.SKIP_MISSING);
 		}
 	}
+
 }

org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryState.java

@@ -173,6 +173,46 @@ public enum RepositoryState {
 		public String getDescription() { return JGitText.get().repositoryState_merged; }
 	},
 
+	/** An unfinished revert. Must resolve or reset before continuing normally
+	 */
+	REVERTING {
+		@Override
+		public boolean canCheckout() { return false; }
+
+		@Override
+		public boolean canResetHead() { return true; }
+
+		@Override
+		public boolean canCommit() { return false; }
+
+		@Override
+		public boolean canAmend() { return false; }
+
+		@Override
+		public String getDescription() { return JGitText.get().repositoryState_conflicts; }
+	},
+
+	/**
+	 * A revert where all conflicts have been resolved. The index does not
+	 * contain any unmerged paths.
+	 */
+	REVERTING_RESOLVED {
+		@Override
+		public boolean canCheckout() { return true; }
+
+		@Override
+		public boolean canResetHead() { return true; }
+
+		@Override
+		public boolean canCommit() { return true; }
+
+		@Override
+		public boolean canAmend() { return false; }
+
+		@Override
+		public String getDescription() { return JGitText.get().repositoryState_merged; }
+	},
+
 	/**
 	 * An unfinished rebase or am. Must resolve, skip or abort before normal work can take place
 	 */