Fix order of deletion for files/dirs in ResolveMerger

Before, the paths to delete were stored in a HashMap, which doesn't have
a particular order. So when e.g. both the file "a/b" and the directory
"a" were to be deleted, it would sometimes try to delete "a" first. This
resulted in a failed path because File#delete() fails when a directory
isn't empty.

With this change, an ArrayList is used for storing the paths to delete.
The list contains the paths in a top-down order, as defined by the order
of processEntry. When the files are deleted, the list is iterated in
reverse, ensuring that all files of a directory are deleted before the
directory itself.

Bug: 354099
Change-Id: I6b2ce96b3932ca84ecdfbeab457ce823c95433fb
Signed-off-by: Robin Stocker <robin@nibor.org>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java
index c6875b4832b890721baffd1779f57f418ce9a56c..9effe6022dd854b695c9ee43de0cabfb3414962f 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/MergeCommandTest.java
@@ -1037,6 +1037,51 @@ public class MergeCommandTest extends RepositoryTestCase {
                assertFalse(folder2.exists());
        }
 
+       @Test
+       public void testMergeRemovingFoldersWithoutFastForward() throws Exception {
+               File folder1 = new File(db.getWorkTree(), "folder1");
+               File folder2 = new File(db.getWorkTree(), "folder2");
+               FileUtils.mkdir(folder1);
+               FileUtils.mkdir(folder2);
+               File file = new File(folder1, "file1.txt");
+               write(file, "folder1--file1.txt");
+               file = new File(folder1, "file2.txt");
+               write(file, "folder1--file2.txt");
+               file = new File(folder2, "file1.txt");
+               write(file, "folder--file1.txt");
+               file = new File(folder2, "file2.txt");
+               write(file, "folder2--file2.txt");
+
+               Git git = new Git(db);
+               git.add().addFilepattern(folder1.getName())
+                               .addFilepattern(folder2.getName()).call();
+               RevCommit base = git.commit().setMessage("adding folders").call();
+
+               recursiveDelete(folder1);
+               recursiveDelete(folder2);
+               git.rm().addFilepattern("folder1/file1.txt")
+                               .addFilepattern("folder1/file2.txt")
+                               .addFilepattern("folder2/file1.txt")
+                               .addFilepattern("folder2/file2.txt").call();
+               RevCommit other = git.commit()
+                               .setMessage("removing folders on 'branch'").call();
+
+               git.checkout().setName(base.name()).call();
+
+               file = new File(folder2, "file3.txt");
+               write(file, "folder2--file3.txt");
+
+               git.add().addFilepattern(folder2.getName()).call();
+               git.commit().setMessage("adding another file").call();
+
+               MergeResult result = git.merge().include(other.getId())
+                               .setStrategy(MergeStrategy.RESOLVE).call();
+
+               assertEquals(MergeResult.MergeStatus.MERGED,
+                               result.getMergeStatus());
+               assertFalse(folder1.exists());
+       }
+
        @Test
        public void testFileModeMerge() throws Exception {
                if (!FS.DETECTED.supportsExecute())

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
index 6130cc72c89633ca6046637aad9f0484c0eb2339..2410d6fe04ca3a211bd2f40df0d386fb01a6161e 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
@@ -126,6 +126,8 @@ public class ResolveMerger extends ThreeWayMerger {
 
        private Map<String, DirCacheEntry> toBeCheckedOut = new HashMap<String, DirCacheEntry>();
 
+       private List<String> toBeDeleted = new ArrayList<String>();
+
        private Map<String, MergeResult<? extends Sequence>> mergeResults = new HashMap<String, MergeResult<? extends Sequence>>();
 
        private Map<String, MergeFailureReason> failingPaths = new HashMap<String, MergeFailureReason>();
@@ -240,16 +242,21 @@ public class ResolveMerger extends ThreeWayMerger {
                        for (Map.Entry<String, DirCacheEntry> entry : toBeCheckedOut
                                        .entrySet()) {
                                File f = new File(db.getWorkTree(), entry.getKey());
-                               if (entry.getValue() != null) {
-                                       createDir(f.getParentFile());
-                                       DirCacheCheckout.checkoutEntry(db, f, entry.getValue(), r);
-                               } else {
-                                       if (!f.delete())
-                                               failingPaths.put(entry.getKey(),
-                                                               MergeFailureReason.COULD_NOT_DELETE);
-                               }
+                               createDir(f.getParentFile());
+                               DirCacheCheckout.checkoutEntry(db, f, entry.getValue(), r);
                                modifiedFiles.add(entry.getKey());
                        }
+                       // Iterate in reverse so that "folder/file" is deleted before
+                       // "folder". Otherwise this could result in a failing path because
+                       // of a non-empty directory, for which delete() would fail.
+                       for (int i = toBeDeleted.size() - 1; i >= 0; i--) {
+                               String fileName = toBeDeleted.get(i);
+                               File f = new File(db.getWorkTree(), fileName);
+                               if (!f.delete())
+                                       failingPaths.put(fileName,
+                                                       MergeFailureReason.COULD_NOT_DELETE);
+                               modifiedFiles.add(fileName);
+                       }
                } finally {
                        r.release();
                }
@@ -441,7 +448,7 @@ public class ResolveMerger extends ThreeWayMerger {
                        } else if (modeT == 0 && modeB != 0) {
                                // we want THEIRS ... but THEIRS contains the deletion of the
                                // file
-                               toBeCheckedOut.put(tw.getPathString(), null);
+                               toBeDeleted.add(tw.getPathString());
                                return true;
                        }
                }