Bug 500306 - Read-only files, and projects containing them, cannot beI20160927-0800

deleted

If the LocalFile.delete() fails with AccessDeniedException, retry the
operation via old API which doesn't care about read-only flags. This
makes the LocalFile.delete() behavior more predictable across different
platforms.

Change-Id: I33b010469c4e3d6906d1aa2a66e51bf4e53f9f30
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

3 files changed, 42 insertions, 10 deletions

diff --git a/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java
index 7b34b5cec..f521b7c4f 100644
--- a/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java
+++ b/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java
@@ -11,13 +11,13 @@
  * 	   Martin Lippert (VMware) - [394607] Poor performance when using findFilesForLocationURI
  * 	   Sergey Prigogin (Google) - [433061] Deletion of project follows symbolic links
  *                                [464072] Refresh on Access ignored during text search
+ * 	   Andrey Loskutov (loskutov@gmx.de) - [500306] Read-only files, and projects containing them, cannot be deleted
  *******************************************************************************/
 package org.eclipse.core.internal.filesystem.local;
 
 import java.io.*;
 import java.net.URI;
-import java.nio.file.DirectoryNotEmptyException;
-import java.nio.file.Files;
+import java.nio.file.*;
 import org.eclipse.core.filesystem.*;
 import org.eclipse.core.filesystem.URIUtil;
 import org.eclipse.core.filesystem.provider.FileInfo;
@@ -25,6 +25,7 @@ import org.eclipse.core.filesystem.provider.FileStore;
 import org.eclipse.core.internal.filesystem.Messages;
 import org.eclipse.core.internal.filesystem.Policy;
 import org.eclipse.core.runtime.*;
+import org.eclipse.core.runtime.Path;
 import org.eclipse.osgi.util.NLS;
 
 /**
@@ -54,7 +55,7 @@ public class LocalFile extends FileStore {
 
 	/**
 	 * Creates a new local file.
-	 * 
+	 *
 	 * @param file The file this local file represents
 	 */
 	public LocalFile(File file) {
@@ -66,7 +67,7 @@ public class LocalFile extends FileStore {
 	 * This method is called after a failure to modify a file or directory.
 	 * Check to see if the parent is read-only and if so then
 	 * throw an exception with a more specific message and error code.
-	 * 
+	 *
 	 * @param target The file that we failed to modify
 	 * @param exception The low level exception that occurred, or <code>null</code>
 	 * @throws CoreException A more specific exception if the parent is read-only
@@ -83,7 +84,7 @@ public class LocalFile extends FileStore {
 	 * This method is called after a failure to modify a directory.
 	 * Check to see if the target is not writable (e.g. device doesn't not exist) and if so then
 	 * throw an exception with a more specific message and error code.
-	 * 
+	 *
 	 * @param target The directory that we failed to modify
 	 * @param exception The low level exception that occurred, or <code>null</code>
 	 * @throws CoreException A more specific exception if the target is not writable
@@ -214,9 +215,18 @@ public class LocalFile extends FileStore {
 			throw new OperationCanceledException();
 		}
 		try {
-			// First try to delete - this should succeed for files and symbolic links to directories.
-			Files.deleteIfExists(target.toPath());
-			return true;
+			try {
+				// First try to delete - this should succeed for files and symbolic links to directories.
+				Files.deleteIfExists(target.toPath());
+				return true;
+			} catch (AccessDeniedException e) {
+				// If the file is read only, it can't be deleted via Files.deleteIfExists()
+				// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=500306
+				if (target.delete()) {
+					return true;
+				}
+				throw e;
+			}
 		} catch (DirectoryNotEmptyException e) {
 			monitor.subTask(NLS.bind(Messages.deleting, target));
 			String[] list = target.list();
diff --git a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/filesystem/DeleteTest.java b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/filesystem/DeleteTest.java
index 1584ce2a2..9f7255a2e 100644
--- a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/filesystem/DeleteTest.java
+++ b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/filesystem/DeleteTest.java
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.core.tests.filesystem;
 
+import java.io.File;
 import org.eclipse.core.filesystem.EFS;
 import org.eclipse.core.filesystem.IFileStore;
 import org.eclipse.core.runtime.CoreException;
@@ -52,4 +53,25 @@ public class DeleteTest extends FileSystemTest {
 		}
 		assertTrue("1.1", !dir.fetchInfo().exists());
 	}
+
+	public void testDeleteReadOnlyFile() throws Exception {
+		ensureExists(localFileBaseStore, true);
+		IFileStore file = localFileBaseStore.getChild("child");
+		ensureExists(file, false);
+		assertTrue("1.0", file.fetchInfo().exists());
+		ensureReadOnlyLocal(file);
+		file.delete(EFS.NONE, getMonitor());
+		// success: we expect that read-only files can be removed
+		assertTrue("1.1", !file.fetchInfo().exists());
+	}
+
+	/**
+	 * Ensures that the provided store is read-only
+	 */
+	protected void ensureReadOnlyLocal(IFileStore store) throws Exception {
+		File localFile = store.toLocalFile(0, getMonitor());
+		boolean readOnly = localFile.setReadOnly();
+		assertTrue("1.0", readOnly);
+		assertFalse("1.1", localFile.canWrite());
+	}
 }
diff --git a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_026294.java b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_026294.java
index 48ac6c090..424ada2c5 100644
--- a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_026294.java
+++ b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_026294.java
@@ -278,7 +278,7 @@ public class Bug_026294 extends ResourceTest {
 	 * TODO: enable this test once bug 48321 is fixed.
 	 */
 	public void testDeleteClosedProjectLinux() {
-		if (!(Platform.getOS().equals(Platform.OS_LINUX) && isReadOnlySupported()))
+		if (!(Platform.getOS().equals(Platform.OS_LINUX)))
 			return;
 
 		IProject project = null;
@@ -410,7 +410,7 @@ public class Bug_026294 extends ResourceTest {
 	 * Works only for Linux with natives.
 	 */
 	public void testDeleteFolderLinux() {
-		if (!(Platform.getOS().equals(Platform.OS_LINUX) && isReadOnlySupported()))
+		if (!(Platform.getOS().equals(Platform.OS_LINUX)))
 			return;
 
 		IProject project = null;