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>

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");

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();
+			boolean tryAgain;
+			do {
+				tryAgain = false;
 				try {
 					Files.delete(p);
 					return;
-			} catch (FileNotFoundException e) {
+				} catch (NoSuchFileException | FileNotFoundException e) {
-				if ((options & (SKIP_MISSING | IGNORE_ERRORS)) == 0) {
+					handleDeleteException(f, e, options,
-					throw new IOException(MessageFormat.format(
+							SKIP_MISSING | IGNORE_ERRORS);
-							JGitText.get().deleteFileFailed,
+					return;
-							f.getAbsolutePath()), e);
+				} catch (DirectoryNotEmptyException e) {
-				}
+					handleDeleteException(f, e, options, IGNORE_ERRORS);
 					return;
 				} catch (IOException e) {
+					if (!f.canWrite()) {
+						tryAgain = f.setWritable(true);
+					}
+					if (!tryAgain) {
 						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) {
+			handleDeleteException(f, t, options, IGNORE_ERRORS);
-				throw new IOException(MessageFormat.format(
+		}
-						JGitText.get().deleteFileFailed, f.getAbsolutePath()),
-						t);
 	}
+
+	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);
 		}
 	}