FileUtils: improve delete (Windows)

Ensure files are writable before trying to delete them.

Bug: 408846
Change-Id: I930a547594bba853c33634ae54bd64d236afade3
Signed-off-by: Alexander Nittka <alex@nittka.de>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FileUtilsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FileUtilsTest.java
index f9ec5d8d6ff552a54761a90e88c4d8e98f4972e0..2b1fb2ef04b36f1d92e7d43a3ddda039ff040c8b 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FileUtilsTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FileUtilsTest.java
@@ -78,6 +78,15 @@ public class FileUtilsTest {
                }
        }
 
+       @Test
+       public void testDeleteReadOnlyFile() throws IOException {
+               File f = new File(trash, "f");
+               FileUtils.createNewFile(f);
+               assertTrue(f.setReadOnly());
+               FileUtils.delete(f);
+               assertFalse(f.exists());
+       }
+
        @Test
        public void testDeleteRecursive() throws IOException {
                File f1 = new File(trash, "test/test/a");
@@ -338,6 +347,34 @@ public class FileUtilsTest {
                assertFalse(e.exists());
        }
 
+       @Test
+       public void testDeleteNonRecursiveTreeNotOk() throws IOException {
+               File t = new File(trash, "t");
+               FileUtils.mkdir(t);
+               File f = new File(t, "f");
+               FileUtils.createNewFile(f);
+               try {
+                       FileUtils.delete(t, FileUtils.EMPTY_DIRECTORIES_ONLY);
+                       fail("expected failure to delete f");
+               } catch (IOException e) {
+                       assertTrue(e.getMessage().endsWith(t.getAbsolutePath()));
+               }
+               assertTrue(f.exists());
+               assertTrue(t.exists());
+       }
+
+       @Test
+       public void testDeleteNonRecursiveTreeIgnoreError() throws IOException {
+               File t = new File(trash, "t");
+               FileUtils.mkdir(t);
+               File f = new File(t, "f");
+               FileUtils.createNewFile(f);
+               FileUtils.delete(t,
+                               FileUtils.EMPTY_DIRECTORIES_ONLY | FileUtils.IGNORE_ERRORS);
+               assertTrue(f.exists());
+               assertTrue(t.exists());
+       }
+
        @Test
        public void testRenameOverNonExistingFile() throws IOException {
                File d = new File(trash, "d");

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
index 4831fbb64effd5f1bddb130cc14a727c45e5beb4..c43956e53d1b85b18b91c8c0246b4b70889fcea4 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
@@ -20,6 +20,7 @@ import java.io.IOException;
 import java.nio.channels.FileChannel;
 import java.nio.file.AtomicMoveNotSupportedException;
 import java.nio.file.CopyOption;
+import java.nio.file.DirectoryNotEmptyException;
 import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
 import java.nio.file.LinkOption;
@@ -180,21 +181,31 @@ public class FileUtils {
                }
 
                if (delete) {
-                       Throwable t = null;
+                       IOException t = null;
                        Path p = f.toPath();
-                       try {
-                               Files.delete(p);
-                               return;
-                       } catch (FileNotFoundException e) {
-                               if ((options & (SKIP_MISSING | IGNORE_ERRORS)) == 0) {
-                                       throw new IOException(MessageFormat.format(
-                                                       JGitText.get().deleteFileFailed,
-                                                       f.getAbsolutePath()), e);
+                       boolean tryAgain;
+                       do {
+                               tryAgain = false;
+                               try {
+                                       Files.delete(p);
+                                       return;
+                               } catch (NoSuchFileException | FileNotFoundException e) {
+                                       handleDeleteException(f, e, options,
+                                                       SKIP_MISSING | IGNORE_ERRORS);
+                                       return;
+                               } catch (DirectoryNotEmptyException e) {
+                                       handleDeleteException(f, e, options, IGNORE_ERRORS);
+                                       return;
+                               } catch (IOException e) {
+                                       if (!f.canWrite()) {
+                                               tryAgain = f.setWritable(true);
+                                       }
+                                       if (!tryAgain) {
+                                               t = e;
+                                       }
                                }
-                               return;
-                       } catch (IOException e) {
-                               t = e;
-                       }
+                       } while (tryAgain);
+
                        if ((options & RETRY) != 0) {
                                for (int i = 1; i < 10; i++) {
                                        try {
@@ -210,11 +221,15 @@ public class FileUtils {
                                        }
                                }
                        }
-                       if ((options & IGNORE_ERRORS) == 0) {
-                               throw new IOException(MessageFormat.format(
-                                               JGitText.get().deleteFileFailed, f.getAbsolutePath()),
-                                               t);
-                       }
+                       handleDeleteException(f, t, options, IGNORE_ERRORS);
+               }
+       }
+
+       private static void handleDeleteException(File f, IOException e,
+                       int allOptions, int checkOptions) throws IOException {
+               if (e != null && (allOptions & checkOptions) == 0) {
+                       throw new IOException(MessageFormat.format(
+                                       JGitText.get().deleteFileFailed, f.getAbsolutePath()), e);
                }
        }