Fix stash apply using merge logic

Instead of the complicated strange stuff, implement staah
apply as cherry-pick.

Provided there are no conflicts and it is requested that
the index should be applied, perform yet another cherry-pick,
but discard tha results thereof it that would result in conflicts.

Bug: 376035
Change-Id: I553f3a753e0124b102a51f8edbb53ddeff2912e2

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

@@ -54,7 +54,7 @@ import java.text.MessageFormat;
 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.errors.CheckoutConflictException;
+import org.eclipse.jgit.api.errors.StashApplyFailureException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
@@ -314,14 +314,15 @@ public class StashApplyCommandTest extends RepositoryTestCase {
 
 		Status status = git.status().call();
 		assertTrue(status.getAdded().isEmpty());
-		assertTrue(status.getChanged().isEmpty());
+		assertEquals(1, status.getChanged().size());
+		assertTrue(status.getChanged().contains(PATH));
 		assertTrue(status.getConflicting().isEmpty());
-		assertTrue(status.getMissing().isEmpty());
+		assertEquals(1, status.getMissing().size());
+		assertTrue(status.getMissing().contains(PATH));
 		assertTrue(status.getModified().isEmpty());
 		assertTrue(status.getUntracked().isEmpty());
 
-		assertEquals(1, status.getRemoved().size());
+		assertTrue(status.getRemoved().isEmpty());
-		assertTrue(status.getRemoved().contains(PATH));
 	}
 
 	@Test
@@ -366,9 +367,66 @@ public class StashApplyCommandTest extends RepositoryTestCase {
 		try {
 			git.stashApply().call();
 			fail("Exception not thrown");
-		} catch (JGitInternalException e) {
+		} catch (StashApplyFailureException e) {
-			assertTrue(e.getCause() instanceof CheckoutConflictException);
  		}
+		assertEquals("content3", read(PATH));
+	}
+
+	@Test
+	public void stashedContentMerge() throws Exception {
+		writeTrashFile(PATH, "content\nmore content\n");
+		git.add().addFilepattern(PATH).call();
+		git.commit().setMessage("more content").call();
+
+		writeTrashFile(PATH, "content\nhead change\nmore content\n");
+		git.add().addFilepattern(PATH).call();
+		git.commit().setMessage("even content").call();
+
+		writeTrashFile(PATH, "content\nstashed change\nmore content\n");
+
+		RevCommit stashed = git.stashCreate().call();
+		assertNotNull(stashed);
+		assertEquals("content\nhead change\nmore content\n",
+				read(committedFile));
+		assertTrue(git.status().call().isClean());
+
+		writeTrashFile(PATH, "content\nmore content\ncommitted change\n");
+		git.add().addFilepattern(PATH).call();
+		git.commit().setMessage("committed change").call();
+
+		try {
+			git.stashApply().call();
+			fail("Expected conflict");
+		} catch (StashApplyFailureException e) {
+		}
+		Status status = new StatusCommand(db).call();
+		assertEquals(1, status.getConflicting().size());
+		assertEquals(
+				"content\n<<<<<<< HEAD\n=======\nstashed change\n>>>>>>> stash\nmore content\ncommitted change\n",
+				read(PATH));
+	}
+
+	@Test
+	public void workingDirectoryContentMerge() throws Exception {
+		writeTrashFile(PATH, "content\nmore content\n");
+		git.add().addFilepattern(PATH).call();
+		git.commit().setMessage("more content").call();
+
+		writeTrashFile(PATH, "content\nstashed change\nmore content\n");
+
+		RevCommit stashed = git.stashCreate().call();
+		assertNotNull(stashed);
+		assertEquals("content\nmore content\n", read(committedFile));
+		assertTrue(git.status().call().isClean());
+
+		writeTrashFile(PATH, "content\nmore content\ncommitted change\n");
+		git.add().addFilepattern(PATH).call();
+		git.commit().setMessage("committed change").call();
+
+		git.stashApply().call();
+		assertEquals(
+				"content\nstashed change\nmore content\ncommitted change\n",
+				read(committedFile));
 	}
 
 	@Test
@@ -387,9 +445,9 @@ public class StashApplyCommandTest extends RepositoryTestCase {
 		try {
 			git.stashApply().call();
 			fail("Exception not thrown");
-		} catch (JGitInternalException e) {
+		} catch (StashApplyFailureException e) {
-			assertTrue(e.getCause() instanceof CheckoutConflictException);
 		}
+		assertEquals("content2", read(PATH));
 	}
 
 	@Test

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

@@ -442,8 +442,11 @@ sourceRefNotSpecifiedForRefspec=Source ref not specified for refspec: {0}
 squashCommitNotUpdatingHEAD=Squash commit -- not updating HEAD
 staleRevFlagsOn=Stale RevFlags on {0}
 startingReadStageWithoutWrittenRequestDataPendingIsNotSupported=Starting read stage without written request data pending is not supported
+stashApplyConflict=Applying stashed changes resulted in a conflict
+stashApplyConflictInIndex=Applying stashed index changes resulted in a conflict. Dropped index changes.
 stashApplyFailed=Applying stashed changes did not successfully complete
 stashApplyOnUnsafeRepository=Cannot apply stashed commit on a repository with state: {0}
+stashApplyWithoutHead=Cannot apply stashed commit in an empty repository or onto an unborn branch
 stashCommitMissingTwoParents=Stashed commit ''{0}'' does not have two parent commits
 stashDropDeleteRefFailed=Deleting stash reference failed with result: {0}
 stashDropFailed=Dropping stashed commit failed

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

@@ -42,7 +42,6 @@
  */
 package org.eclipse.jgit.api;
 
-import java.io.File;
 import java.io.IOException;
 import java.text.MessageFormat;
 
@@ -50,77 +49,47 @@ 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.StashApplyFailureException;
 import org.eclipse.jgit.api.errors.WrongRepositoryStateException;
 import org.eclipse.jgit.dircache.DirCache;
+import org.eclipse.jgit.dircache.DirCacheBuilder;
 import org.eclipse.jgit.dircache.DirCacheCheckout;
-import org.eclipse.jgit.dircache.DirCacheEditor;
-import org.eclipse.jgit.dircache.DirCacheEditor.DeletePath;
-import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit;
 import org.eclipse.jgit.dircache.DirCacheEntry;
 import org.eclipse.jgit.dircache.DirCacheIterator;
-import org.eclipse.jgit.errors.CheckoutConflictException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.RepositoryState;
+import org.eclipse.jgit.merge.MergeStrategy;
+import org.eclipse.jgit.merge.ResolveMerger;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.treewalk.AbstractTreeIterator;
-import org.eclipse.jgit.treewalk.CanonicalTreeParser;
 import org.eclipse.jgit.treewalk.FileTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
-import org.eclipse.jgit.treewalk.filter.TreeFilter;
-import org.eclipse.jgit.util.FileUtils;
 
 /**
  * Command class to apply a stashed commit.
  *
+ * This class behaves like <em>git stash apply --index</em>, i.e. it tries to
+ * recover the stashed index state in addition to the working tree state.
+ *
  * @see <a href="http://www.kernel.org/pub/software/scm/git/docs/git-stash.html"
  *      >Git documentation about Stash</a>
+ *
  * @since 2.0
  */
 public class StashApplyCommand extends GitCommand<ObjectId> {
 
 	private static final String DEFAULT_REF = Constants.STASH + "@{0}"; //$NON-NLS-1$
 
-	/**
-	 * Stash diff filter that looks for differences in the first three trees
-	 * which must be the stash head tree, stash index tree, and stash working
-	 * directory tree in any order.
-	 */
-	private static class StashDiffFilter extends TreeFilter {
-
-		@Override
-		public boolean include(final TreeWalk walker) {
-			final int m = walker.getRawMode(0);
-			if (walker.getRawMode(1) != m || !walker.idEqual(1, 0))
-				return true;
-			if (walker.getRawMode(2) != m || !walker.idEqual(2, 0))
-				return true;
-			return false;
-		}
-
-		@Override
-		public boolean shouldBeRecursive() {
-			return false;
-		}
-
-		@Override
-		public TreeFilter clone() {
-			return this;
-		}
-
-		@Override
-		public String toString() {
-			return "STASH_DIFF"; //$NON-NLS-1$
-		}
-	}
-
 	private String stashRef;
 
+	private boolean applyIndex = true;
+
 	/**
 	 * Create command to apply the changes of a stashed commit
 	 *
@@ -144,63 +113,6 @@ public class StashApplyCommand extends GitCommand<ObjectId> {
 		return this;
 	}
 
-	private boolean isEqualEntry(AbstractTreeIterator iter1,
-			AbstractTreeIterator iter2) {
-		if (!iter1.getEntryFileMode().equals(iter2.getEntryFileMode()))
-			return false;
-		ObjectId id1 = iter1.getEntryObjectId();
-		ObjectId id2 = iter2.getEntryObjectId();
-		return id1 != null ? id1.equals(id2) : id2 == null;
-	}
-
-	/**
-	 * Would unstashing overwrite local changes?
-	 *
-	 * @param stashIndexIter
-	 * @param stashWorkingTreeIter
-	 * @param headIter
-	 * @param indexIter
-	 * @param workingTreeIter
-	 * @return true if unstash conflict, false otherwise
-	 */
-	private boolean isConflict(AbstractTreeIterator stashIndexIter,
-			AbstractTreeIterator stashWorkingTreeIter,
-			AbstractTreeIterator headIter, AbstractTreeIterator indexIter,
-			AbstractTreeIterator workingTreeIter) {
-		// Is the current index dirty?
-		boolean indexDirty = indexIter != null
-				&& (headIter == null || !isEqualEntry(indexIter, headIter));
-
-		// Is the current working tree dirty?
-		boolean workingTreeDirty = workingTreeIter != null
-				&& (headIter == null || !isEqualEntry(workingTreeIter, headIter));
-
-		// Would unstashing overwrite existing index changes?
-		if (indexDirty && stashIndexIter != null && indexIter != null
-				&& !isEqualEntry(stashIndexIter, indexIter))
-			return true;
-
-		// Would unstashing overwrite existing working tree changes?
-		if (workingTreeDirty && stashWorkingTreeIter != null
-				&& workingTreeIter != null
-				&& !isEqualEntry(stashWorkingTreeIter, workingTreeIter))
-			return true;
-
-		return false;
-	}
-
-	private ObjectId getHeadTree() throws GitAPIException {
-		final ObjectId headTree;
-		try {
-			headTree = repo.resolve(Constants.HEAD + "^{tree}"); //$NON-NLS-1$
-		} catch (IOException e) {
-			throw new JGitInternalException(JGitText.get().cannotReadTree, e);
-		}
-		if (headTree == null)
-			throw new NoHeadException(JGitText.get().cannotReadTree);
-		return headTree;
-	}
-
 	private ObjectId getStashId() throws GitAPIException {
 		final String revision = stashRef != null ? stashRef : DEFAULT_REF;
 		final ObjectId stashId;
@@ -216,91 +128,19 @@ public class StashApplyCommand extends GitCommand<ObjectId> {
 		return stashId;
 	}
 
-	private void scanForConflicts(TreeWalk treeWalk) throws IOException {
-		File workingTree = repo.getWorkTree();
-		while (treeWalk.next()) {
-			// State of the stashed index and working directory
-			AbstractTreeIterator stashIndexIter = treeWalk.getTree(1,
-					AbstractTreeIterator.class);
-			AbstractTreeIterator stashWorkingIter = treeWalk.getTree(2,
-					AbstractTreeIterator.class);
-
-			// State of the current HEAD, index, and working directory
-			AbstractTreeIterator headIter = treeWalk.getTree(3,
-					AbstractTreeIterator.class);
-			AbstractTreeIterator indexIter = treeWalk.getTree(4,
-					AbstractTreeIterator.class);
-			AbstractTreeIterator workingIter = treeWalk.getTree(5,
-					AbstractTreeIterator.class);
-
-			if (isConflict(stashIndexIter, stashWorkingIter, headIter,
-					indexIter, workingIter)) {
-				String path = treeWalk.getPathString();
-				File file = new File(workingTree, path);
-				throw new CheckoutConflictException(file.getAbsolutePath());
-			}
-		}
-	}
-
-	private void applyChanges(TreeWalk treeWalk, DirCache cache,
-			DirCacheEditor editor) throws IOException {
-		File workingTree = repo.getWorkTree();
-		while (treeWalk.next()) {
-			String path = treeWalk.getPathString();
-			File file = new File(workingTree, path);
-
-			// State of the stashed HEAD, index, and working directory
-			AbstractTreeIterator stashHeadIter = treeWalk.getTree(0,
-					AbstractTreeIterator.class);
-			AbstractTreeIterator stashIndexIter = treeWalk.getTree(1,
-					AbstractTreeIterator.class);
-			AbstractTreeIterator stashWorkingIter = treeWalk.getTree(2,
-					AbstractTreeIterator.class);
-
-			if (stashWorkingIter != null && stashIndexIter != null) {
-				// Checkout index change
-				DirCacheEntry entry = cache.getEntry(path);
-				if (entry == null)
-					entry = new DirCacheEntry(treeWalk.getRawPath());
-				entry.setFileMode(stashIndexIter.getEntryFileMode());
-				entry.setObjectId(stashIndexIter.getEntryObjectId());
-				DirCacheCheckout.checkoutEntry(repo, file, entry,
-						treeWalk.getObjectReader());
-				final DirCacheEntry updatedEntry = entry;
-				editor.add(new PathEdit(path) {
-
-					public void apply(DirCacheEntry ent) {
-						ent.copyMetaData(updatedEntry);
-					}
-				});
-
-				// Checkout working directory change
-				if (!stashWorkingIter.idEqual(stashIndexIter)) {
-					entry = new DirCacheEntry(treeWalk.getRawPath());
-					entry.setObjectId(stashWorkingIter.getEntryObjectId());
-					DirCacheCheckout.checkoutEntry(repo, file, entry,
-							treeWalk.getObjectReader());
-				}
-			} else {
-				if (stashIndexIter == null
-						|| (stashHeadIter != null && !stashIndexIter
-								.idEqual(stashHeadIter)))
-					editor.add(new DeletePath(path));
-				FileUtils
-						.delete(file, FileUtils.RETRY | FileUtils.SKIP_MISSING);
-			}
-		}
-	}
-
 	/**
 	 * Apply the changes in a stashed commit to the working directory and index
 	 *
-	 * @return id of stashed commit that was applied
+	 * @return id of stashed commit that was applied TODO: Does anyone depend on
+	 *         this, or could we make it more like Merge/CherryPick/Revert?
 	 * @throws GitAPIException
 	 * @throws WrongRepositoryStateException
+	 * @throws NoHeadException
+	 * @throws StashApplyFailureException
 	 */
 	public ObjectId call() throws GitAPIException,
-			WrongRepositoryStateException {
+			WrongRepositoryStateException, NoHeadException,
+			StashApplyFailureException {
 		checkCallable();
 
 		if (repo.getRepositoryState() != RepositoryState.SAFE)
@@ -308,73 +148,114 @@ public class StashApplyCommand extends GitCommand<ObjectId> {
 					JGitText.get().stashApplyOnUnsafeRepository,
 					repo.getRepositoryState()));
 
-		final ObjectId headTree = getHeadTree();
-		final ObjectId stashId = getStashId();
-
 		ObjectReader reader = repo.newObjectReader();
 		try {
 			RevWalk revWalk = new RevWalk(reader);
+
+			ObjectId headCommit = repo.resolve(Constants.HEAD);
+			if (headCommit == null)
+				throw new NoHeadException(JGitText.get().stashApplyWithoutHead);
+
+			final ObjectId stashId = getStashId();
 			RevCommit stashCommit = revWalk.parseCommit(stashId);
 			if (stashCommit.getParentCount() != 2)
 				throw new JGitInternalException(MessageFormat.format(
 						JGitText.get().stashCommitMissingTwoParents,
 						stashId.name()));
 
-			RevTree stashWorkingTree = stashCommit.getTree();
+			ObjectId headTree = repo.resolve(Constants.HEAD + "^{tree}");
-			RevTree stashIndexTree = revWalk.parseCommit(
+			ObjectId stashIndexCommit = revWalk.parseCommit(stashCommit
-					stashCommit.getParent(1)).getTree();
+					.getParent(1));
-			RevTree stashHeadTree = revWalk.parseCommit(
+			ObjectId stashHeadCommit = stashCommit.getParent(0);
-					stashCommit.getParent(0)).getTree();
+
+			ResolveMerger merger = (ResolveMerger) MergeStrategy.RESOLVE
+					.newMerger(repo);
+			merger.setCommitNames(new String[] { "stashed HEAD", "HEAD",
+					"stash" });
+			merger.setBase(stashHeadCommit);
+			merger.setWorkingTreeIterator(new FileTreeIterator(repo));
+			if (merger.merge(headCommit, stashCommit)) {
+				DirCache dc = repo.lockDirCache();
+				DirCacheCheckout dco = new DirCacheCheckout(repo, headTree,
+						dc, merger.getResultTreeId());
+				dco.setFailOnConflict(true);
+				dco.checkout(); // Ignoring failed deletes....
+				if (applyIndex) {
+					ResolveMerger ixMerger = (ResolveMerger) MergeStrategy.RESOLVE
+							.newMerger(repo, true);
+					ixMerger.setCommitNames(new String[] { "stashed HEAD",
+							"HEAD", "stashed index" });
+					boolean ok = ixMerger.merge(headCommit, stashIndexCommit);
+					if (ok) {
+						resetIndex(revWalk
+								.parseTree(ixMerger.getResultTreeId()));
+					} else {
+						throw new StashApplyFailureException(
+								JGitText.get().stashApplyConflict);
+					}
+				}
+			} else {
+				throw new StashApplyFailureException(
+						JGitText.get().stashApplyConflict);
+			}
+			return stashId;
 
-			CanonicalTreeParser stashWorkingIter = new CanonicalTreeParser();
+		} catch (JGitInternalException e) {
-			stashWorkingIter.reset(reader, stashWorkingTree);
+			throw e;
-			CanonicalTreeParser stashIndexIter = new CanonicalTreeParser();
+		} catch (IOException e) {
-			stashIndexIter.reset(reader, stashIndexTree);
+			throw new JGitInternalException(JGitText.get().stashApplyFailed, e);
-			CanonicalTreeParser stashHeadIter = new CanonicalTreeParser();
+		} finally {
-			stashHeadIter.reset(reader, stashHeadTree);
+			reader.release();
-			CanonicalTreeParser headIter = new CanonicalTreeParser();
+		}
-			headIter.reset(reader, headTree);
+	}
+
+	/**
+	 * @param applyIndex
+	 *            true (default) if the command should restore the index state
+	 */
+	public void setApplyIndex(boolean applyIndex) {
+		this.applyIndex = applyIndex;
+	}
 
-			DirCache cache = repo.lockDirCache();
+	private void resetIndex(RevTree tree) throws IOException {
-			DirCacheEditor editor = cache.editor();
+		DirCache dc = repo.lockDirCache();
+		TreeWalk walk = null;
 		try {
-				DirCacheIterator indexIter = new DirCacheIterator(cache);
+			DirCacheBuilder builder = dc.builder();
-				FileTreeIterator workingIter = new FileTreeIterator(repo);
 
-				TreeWalk treeWalk = new TreeWalk(reader);
+			walk = new TreeWalk(repo);
-				treeWalk.setRecursive(true);
+			walk.addTree(tree);
-				treeWalk.setFilter(new StashDiffFilter());
+			walk.addTree(new DirCacheIterator(dc));
+			walk.setRecursive(true);
 
-				treeWalk.addTree(stashHeadIter);
+			while (walk.next()) {
-				treeWalk.addTree(stashIndexIter);
+				AbstractTreeIterator cIter = walk.getTree(0,
-				treeWalk.addTree(stashWorkingIter);
+						AbstractTreeIterator.class);
-				treeWalk.addTree(headIter);
+				if (cIter == null) {
-				treeWalk.addTree(indexIter);
+					// Not in commit, don't add to new index
-				treeWalk.addTree(workingIter);
+					continue;
+				}
 
-				scanForConflicts(treeWalk);
+				final DirCacheEntry entry = new DirCacheEntry(walk.getRawPath());
+				entry.setFileMode(cIter.getEntryFileMode());
+				entry.setObjectIdFromRaw(cIter.idBuffer(), cIter.idOffset());
 
-				// Reset trees and walk
+				DirCacheIterator dcIter = walk.getTree(1,
-				treeWalk.reset();
+						DirCacheIterator.class);
-				stashWorkingIter.reset(reader, stashWorkingTree);
+				if (dcIter != null && dcIter.idEqual(cIter)) {
-				stashIndexIter.reset(reader, stashIndexTree);
+					DirCacheEntry indexEntry = dcIter.getDirCacheEntry();
-				stashHeadIter.reset(reader, stashHeadTree);
+					entry.setLastModified(indexEntry.getLastModified());
-				treeWalk.addTree(stashHeadIter);
+					entry.setLength(indexEntry.getLength());
-				treeWalk.addTree(stashIndexIter);
+				}
-				treeWalk.addTree(stashWorkingIter);
 
-				applyChanges(treeWalk, cache, editor);
+				builder.add(entry);
-			} finally {
-				editor.commit();
-				cache.unlock();
 			}
-		} catch (JGitInternalException e) {
+
-			throw e;
+			builder.commit();
-		} catch (IOException e) {
-			throw new JGitInternalException(JGitText.get().stashApplyFailed, e);
 		} finally {
-			reader.release();
+			dc.unlock();
+			if (walk != null)
+				walk.release();
 		}
-		return stashId;
 	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/api/errors/StashApplyFailureException.java

@@ -0,0 +1,19 @@
+package org.eclipse.jgit.api.errors;
+
+import org.eclipse.jgit.api.errors.GitAPIException;
+
+/**
+ * Thrown from StashApplyCommand when stash apply fails
+ */
+public class StashApplyFailureException extends GitAPIException {
+
+	/**
+	 * Create a StashApplyFailedException
+	 *
+	 * @param message
+	 */
+	public StashApplyFailureException(final String message) {
+		super(message);
+	}
+
+}

org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java

@@ -503,7 +503,10 @@ public class JGitText extends TranslationBundle {
 	/***/ public String squashCommitNotUpdatingHEAD;
 	/***/ public String staleRevFlagsOn;
 	/***/ public String startingReadStageWithoutWrittenRequestDataPendingIsNotSupported;
+	/***/ public String stashApplyConflict;
+	/***/ public String stashApplyConflictInIndex;
 	/***/ public String stashApplyFailed;
+	/***/ public String stashApplyWithoutHead;
 	/***/ public String stashApplyOnUnsafeRepository;
 	/***/ public String stashCommitMissingTwoParents;
 	/***/ public String stashDropDeleteRefFailed;