Fix StashApply regarding handling of untracked files

There was a bug regarding how JGit handled untracked files when applying
a stash. Problem was that untracked files are applied by doing a merge
of HEAD and untrackedFiles commit with a merge base of the stashed HEAD.
That's wrong because the untrackedFiles commit has no parent and
contains only the untracked files. Using stashed HEAD as merge base
leads to unneccessary conflicts on files not event included in the
untrackedFiles commit.

Imagine this graph directly before you want to apply a stash which was
based on 0. You want to apply the stash on current HEAD commit 5.

  5 (HEAD,master)
 /
0---+
 \   \
  1---3 (WIP on master)
     /
    2 (untracked files on master)

Imagine for a specific (tracked) file f
- commit 0 contains X
- HEAD contains Y
- commit 2 (the untracked files) does not contain file f

A merge of 2 and 5 with a merge base of 0 leads to a conflict. The 5
commit wants to modify the file and the 2 commit wants to delete the
file -> conflict.

If no merge base is set then the semantic is correct.

Thanks to Bow for finding this bug and providing the test case.

Bug: 485467
Change-Id: I453fa6ec337f81b2a52c4f51f23044faeec409e6
Also-by: Bow Ruggeri <bow@bow.net>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java
index ff7066e8bf29c051e2ef1349e0a0ddf0c48539eb..a526fdaa7ebad7a9abb399e5f444320e77a6b710 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java
@@ -58,6 +58,7 @@ import java.util.concurrent.Callable;
 import org.eclipse.jgit.api.CreateBranchCommand.SetupUpstreamMode;
 import org.eclipse.jgit.api.MergeResult.MergeStatus;
 import org.eclipse.jgit.api.errors.NoHeadException;
+import org.eclipse.jgit.junit.JGitTestUtil;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
@@ -184,6 +185,40 @@ public class PullCommandTest extends RepositoryTestCase {
                                .getRepositoryState());
        }
 
+       @Test
+       public void testPullWithUntrackedStash() throws Exception {
+               target.pull().call();
+
+               // change the source file
+               writeToFile(sourceFile, "Source change");
+               source.add().addFilepattern("SomeFile.txt").call();
+               source.commit().setMessage("Source change in remote").call();
+
+               // write untracked file
+               writeToFile(new File(dbTarget.getWorkTree(), "untracked.txt"),
+                               "untracked");
+               RevCommit stash = target.stashCreate().setIndexMessage("message here")
+                               .setIncludeUntracked(true).call();
+               assertNotNull(stash);
+               assertTrue(target.status().call().isClean());
+
+               // pull from source
+               assertTrue(target.pull().call().isSuccessful());
+               assertEquals("[SomeFile.txt, mode:100644, content:Source change]",
+                               indexState(dbTarget, CONTENT));
+               assertFalse(JGitTestUtil.check(dbTarget, "untracked.txt"));
+               assertEquals("Source change",
+                               JGitTestUtil.read(dbTarget, "SomeFile.txt"));
+
+               // apply the stash
+               target.stashApply().setStashRef(stash.getName()).call();
+               assertEquals("[SomeFile.txt, mode:100644, content:Source change]",
+                               indexState(dbTarget, CONTENT));
+               assertEquals("untracked", JGitTestUtil.read(dbTarget, "untracked.txt"));
+               assertEquals("Source change",
+                               JGitTestUtil.read(dbTarget, "SomeFile.txt"));
+       }
+
        @Test
        public void testPullLocalConflict() throws Exception {
                target.branchCreate().setName("basedOnMaster").setStartPoint(
@@ -576,4 +611,4 @@ public class PullCommandTest extends RepositoryTestCase {
                                fis.close();
                }
        }
-}
+}
\ No newline at end of file

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashApplyCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashApplyCommand.java
index 1699b9f3d7d27cf5d077ef19e27ac5ce5aecd0db..b8ee1ec0b390a96b0c1bed1503a350e29727584c 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashApplyCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashApplyCommand.java
@@ -223,8 +223,13 @@ public class StashApplyCommand extends GitCommand<ObjectId> {
                                        ResolveMerger untrackedMerger = (ResolveMerger) strategy
                                                        .newMerger(repo, true);
                                        untrackedMerger.setCommitNames(new String[] {
-                                                       "stashed HEAD", "HEAD", "untracked files" }); //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$
-                                       untrackedMerger.setBase(stashHeadCommit);
+                                                       "null", "HEAD", "untracked files" }); //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$
+                                       // There is no common base for HEAD & untracked files
+                                       // because the commit for untracked files has no parent. If
+                                       // we use stashHeadCommit as common base (as in the other
+                                       // merges) we potentially report conflicts for files
+                                       // which are not even member of untracked files commit
+                                       untrackedMerger.setBase(null);
                                        boolean ok = untrackedMerger.merge(headCommit,
                                                        untrackedCommit);
                                        if (ok)