Handle missing "ours" stage in WorkingTreeIterator.hasCrLfInIndex()

In a delete-modify conflict with the deletion as "ours" there may be
no stage 2 in the index. Add appropriate null checks. Add a new test
for this case, and verify that the file gets added with a single LF
after conflict resolution with core.autocrlf=true. This matches the
behavior of canonical git for this case.

Bug: 547724
Change-Id: I1bafdb83d9b78bf85294c78325e818e72fae53bc
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CommitCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CommitCommandTest.java
index 4bfac1505a102829469d434114567e04afd297b4..18fed4bff7ed2f249e385094797e07eaa3151acd 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CommitCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CommitCommandTest.java
@@ -661,6 +661,54 @@ public class CommitCommandTest extends RepositoryTestCase {
                }
        }
 
+       @Test
+       public void testDeletionConflictWithAutoCrlf() throws Exception {
+               try (Git git = new Git(db)) {
+                       // Commit a file with CR/LF into the index
+                       FileBasedConfig config = db.getConfig();
+                       config.setString("core", null, "autocrlf", "false");
+                       config.save();
+                       File file = writeTrashFile("file.txt", "foo\r\n");
+                       git.add().addFilepattern("file.txt").call();
+                       git.commit().setMessage("Initial").call();
+                       // Switch to side branch
+                       git.checkout().setCreateBranch(true).setName("side").call();
+                       assertTrue(file.delete());
+                       git.rm().addFilepattern("file.txt").call();
+                       git.commit().setMessage("Side").call();
+                       // Switch on autocrlf=true
+                       config.setString("core", null, "autocrlf", "true");
+                       config.save();
+                       // Switch back to master and commit a conflict
+                       git.checkout().setName("master").call();
+                       writeTrashFile("file.txt", "foob\r\n");
+                       git.add().addFilepattern("file.txt").call();
+                       assertEquals("[file.txt, mode:100644, content:foob\r\n]",
+                                       indexState(CONTENT));
+                       writeTrashFile("g", "file2.txt", "anything");
+                       git.add().addFilepattern("g/file2.txt");
+                       RevCommit master = git.commit().setMessage("Second").call();
+                       // Switch to side branch again so that the deletion is "ours"
+                       git.checkout().setName("side").call();
+                       // Cherry pick master: produces a delete-modify conflict.
+                       CherryPickResult pick = git.cherryPick().include(master).call();
+                       assertEquals("Expected a cherry-pick conflict",
+                                       CherryPickStatus.CONFLICTING, pick.getStatus());
+                       // XXX: g/file2.txt should actually be staged already, but isn't.
+                       git.add().addFilepattern("g/file2.txt").call();
+                       // Resolve the conflict by taking the master version
+                       writeTrashFile("file.txt", "foob\r\n");
+                       git.add().addFilepattern("file.txt").call();
+                       git.commit().setMessage("Cherry").call();
+                       // We expect this to be committed with a single LF since there is no
+                       // "ours" stage.
+                       assertEquals(
+                                       "[file.txt, mode:100644, content:foob\n]"
+                                                       + "[g/file2.txt, mode:100644, content:anything]",
+                                       indexState(CONTENT));
+               }
+       }
+
        private void testConflictWithAutoCrlf(String baseLf, String lf)
                        throws Exception {
                try (Git git = new Git(db)) {

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
index c0c24872b2ac3b5ff20f4f6ff0b41bb5eda0898e..3efa66459c0d22096b450fc97ce62407c512277e 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
@@ -1516,6 +1516,7 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
                        ObjectId blobId = entry.getObjectId();
                        if (entry.getStage() > 0
                                        && entry.getStage() != DirCacheEntry.STAGE_2) {
+                               blobId = null;
                                // Merge conflict: check ours (stage 2)
                                byte[] name = entry.getRawPath();
                                int i = 0;
@@ -1523,7 +1524,8 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
                                        dirCache.next(1);
                                        i++;
                                        entry = dirCache.getDirCacheEntry();
-                                       if (!Arrays.equals(name, entry.getRawPath())) {
+                                       if (entry == null
+                                                       || !Arrays.equals(name, entry.getRawPath())) {
                                                break;
                                        }
                                        if (entry.getStage() == DirCacheEntry.STAGE_2) {
@@ -1533,17 +1535,20 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
                                }
                                dirCache.back(i);
                        }
-                       try (ObjectReader reader = repository.newObjectReader()) {
-                               ObjectLoader loader = reader.open(blobId, Constants.OBJ_BLOB);
-                               try {
-                                       return RawText.isCrLfText(loader.getCachedBytes());
-                               } catch (LargeObjectException e) {
-                                       try (InputStream in = loader.openStream()) {
-                                               return RawText.isCrLfText(in);
+                       if (blobId != null) {
+                               try (ObjectReader reader = repository.newObjectReader()) {
+                                       ObjectLoader loader = reader.open(blobId,
+                                                       Constants.OBJ_BLOB);
+                                       try {
+                                               return RawText.isCrLfText(loader.getCachedBytes());
+                                       } catch (LargeObjectException e) {
+                                               try (InputStream in = loader.openStream()) {
+                                                       return RawText.isCrLfText(in);
+                                               }
                                        }
+                               } catch (IOException e) {
+                                       // Ignore and return false below
                                }
-                       } catch (IOException e) {
-                               // Ignore and return false below
                        }
                }
                return false;