Bugzilla – Full Text Bug Listing
| Summary: | [LocalFile][Win32] Read-only files, and projects containing them, cannot be deleted | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Baltasar Belyavsky <bbelyavsky> | ||||||
| Component: | Resources | Assignee: | Andrey Loskutov <loskutov> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | major | ||||||||
| Priority: | P3 | CC: | daniel_megert, eclipse.sprigogin, IngedevTeam.fr, Lars.Vogel, loskutov, marcin.swiezawski, Platform-UI-Inbox, sptaszkiewicz | ||||||
| Version: | 4.6 | Flags: | Lars.Vogel:
pmc_approved+
|
||||||
| Target Milestone: | 4.6.2 | ||||||||
| Hardware: | PC | ||||||||
| OS: | All | ||||||||
| See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=180758 https://git.eclipse.org/r/80437 https://bugs.eclipse.org/bugs/show_bug.cgi?id=26294 https://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=e56853d1b0dc0cf6cfa0d4c6cef552ab46093b2a https://git.eclipse.org/r/83380 https://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=87958f118eb55d11bf9fb884653843af5ea28054 |
||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
I can't reproduce (but I'm on Linux). Please attach the full stack trace from your error and the steps to reproduce the issue. Created attachment 263797 [details]
Workspace log
I see this problem on Windows 7, following these steps: 1. Download any Eclipse Neon package(eg. http://www.eclipse.org/downloads/download.php?file=/technology/epp/downloads/release/neon/R/eclipse-java-neon-R-win32.zip ) 2. Download any Java 1.8 (eg. the latest build - 1.8.0_101) 3. Launch Eclipse, and create any project (eg. General > Project) 4. Create any file in that project, right-click on it, and set the "Read-only" attribute. 5. Select the file, and try to delete it. Or try to delete the project (having the "Delete project contents on disk" checked). Just wondering if this is related to the space in the file path. Contains: Could not delete: E:\Downloads\CCS\Eclipse\Eclipse SDK\eclipse-java-neon-R-win32\eclipse\workspace\a\aaa.txt. java.nio.file.AccessDeniedException: E:\Downloads\CCS\Eclipse\Eclipse SDK\eclipse-java-neon-R-win32\eclipse\workspace\a\aaa.txt at sun.nio.fs.WindowsException.translateToIOException(Unknown Source) at sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source) at sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source) at sun.nio.fs.WindowsFileSystemProvider.implDelete(Unknown Source) at sun.nio.fs.AbstractFileSystemProvider.deleteIfExists(Unknown Source) at java.nio.file.Files.deleteIfExists(Unknown Source) at org.eclipse.core.internal.filesystem.local.LocalFile.internalDelete(LocalFile.java:218) at org.eclipse.core.internal.filesystem.local.LocalFile.delete(LocalFile.java:136) at org.eclipse.core.internal.resources.ResourceTree.internalDeleteFile(ResourceTree.java:310) at org.eclipse.core.internal.resources.ResourceTree.standardDeleteFile(ResourceTree.java:796) at org.eclipse.core.internal.resources.Resource.unprotectedDelete(Resource.java:1828) at org.eclipse.core.internal.resources.Resource.delete(Resource.java:782) at org.eclipse.ltk.core.refactoring.resource.DeleteResourceChange.perform(DeleteResourceChange.java:163) I've repeated the same steps with a shorted path, with no spaces. Same result: Contains: Could not delete 'E:\Downloads\eclipse\workspace\a\b.txt'. org.eclipse.core.runtime.CoreException: Problems encountered while deleting files. at org.eclipse.core.internal.filesystem.local.LocalFile.delete(LocalFile.java:138) at org.eclipse.core.internal.resources.ResourceTree.internalDeleteFile(ResourceTree.java:310) at org.eclipse.core.internal.resources.ResourceTree.standardDeleteFile(ResourceTree.java:796) at org.eclipse.core.internal.resources.Resource.unprotectedDelete(Resource.java:1828) at org.eclipse.core.internal.resources.Resource.delete(Resource.java:782) ... Hello everyone, Is it possible to plan a fix for SR1 ? Thanks in advance Regards Arnaud IngeDev Team Ingenico Group I can confirm on Windows 10. I remember in the old good working days of Eclipse 3.8 there was a validateEdit() operation somewhere, which also checked if a file which is coming from a project NOT managed by version control plugin is a read only and popped a dialog if the user really really wants to delete this file. This dialog is not shown for me anymore. (In reply to Andrey Loskutov - on the beach till 12.09 from comment #7) > there was a validateEdit() operation somewhere, It is in IWorkspace > which also checked if a file which is > coming from a project NOT managed by version control plugin is a read only > and popped a dialog if the user really really wants to delete this file. > This dialog is not shown for me anymore. Arrgh! The dialog is shown if one edits the read-only file OR if one tries to delete this file from *Package* Explorer. I had luck to use my debug workspace on Windows 10 where I've tested *Project* Explorer before, so that it is most likely also doesn't work on Linux. So this is solely a Project explorer issue, not OS related. I will check if I can see why it is broken now or if this was always broken (I never use Project Explorer, so I don't know if it is introduced recently). A-ha. The Package Explorer uses org.eclipse.jdt.internal.ui.refactoring.reorg.DeleteAction which uses RefactoringExecutionStarter.startDeleteRefactoring() which finally triggers org.eclipse.jdt.internal.corext.refactoring.reorg.JavaDeleteProcessor.confirmDeletingReadOnly(). The Project Explorer uses org.eclipse.ui.actions.DeleteResourceAction which doesn't care about read only files - if user confirmed initial question about deleting files, it simply tries to remove them. Interestingly, it has similar code to check read-only projects, but this code isn't used somehow. I'm investigating. OK, there are two problems: 1) org.eclipse.ui.actions.DeleteResourceAction doesn't check if resources to delete contain read-only files. This was probably never implemented. The related code is in org.eclipse.ltk.internal.core.refactoring.resource.DeleteResourcesProcessor.checkFinalConditions(IProgressMonitor, CheckConditionsContext) and should perform similar checks as org.eclipse.jdt.internal.corext.refactoring.reorg.JavaDeleteProcessor.checkFinalConditions(IProgressMonitor, CheckConditionsContext) does via recalculateElementsToDelete() -> confirmDeletingReadOnly(). It looks as it was always broken. 2) The actual regression is coming from commit https://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=b8088ae7aa4bc6fa23a9e5dfc3836ad776e8ff3b (https://git.eclipse.org/r/#/c/66577/). The replacement of java.io.File.delete() with java.nio.file.Files.deleteIfExists(Path) is not equivalent. The former one can delete read-only files, latter one can't => exception. I think the problem 2) is severe one, since it affects all users of org.eclipse.core.internal.filesystem.local.LocalFile.delete(int, IProgressMonitor), is a regression and should be considered for 4.6.2. New Gerrit change created: https://git.eclipse.org/r/80437 (In reply to Eclipse Genie from comment #11) > New Gerrit change created: https://git.eclipse.org/r/80437 This fixes the problem on Windows but causes test failures on Linux (where it was expected that deleting read-only files is not possible, see https://hudson.eclipse.org/platform/job/eclipse.platform.resources-Gerrit/428/): junit.framework.AssertionFailedError: 2.0 - should have failed at junit.framework.Assert.fail(Assert.java:57) at junit.framework.TestCase.fail(TestCase.java:227) at org.eclipse.core.tests.resources.regression.Bug_026294.testDeleteFolderLinux(Bug_026294.java:435) junit.framework.AssertionFailedError: 2.0 - should have failed at junit.framework.Assert.fail(Assert.java:57) at junit.framework.TestCase.fail(TestCase.java:227) at org.eclipse.core.tests.resources.regression.Bug_026294.testDeleteClosedProjectLinux(Bug_026294.java:310) junit.framework.AssertionFailedError: 2.0 - should have failed at junit.framework.Assert.fail(Assert.java:57) at junit.framework.TestCase.fail(TestCase.java:227) at org.eclipse.core.tests.resources.regression.Bug_026294.testDeleteOpenProjectLinux(Bug_026294.java:164) That's strange. How "delete" ever worked for Project explorer on Linux then? Or was it always broken? Unfortunately I have only my windoof notebook here and can't check the old code on Linux. Actually we have 3 problems: 1) [since ever] inconsistent implementation of LocalFile.delete operation on different platforms 2) [regression] behavior change of LocalFile.delete in 4.6 on windows (previously it was possible to delete read-only files, not it is not) 3) [since ever] + [regression] DeleteResourcesProcessor doesn't check back on deletion on read-only files and so delete fails from Project Explorer/Navigator due 2). In spirit of bug 26294 we should make best effort on deleting resources, means: if we can delete files (by removing the "read-only" attribute), we should delete them and don't throw an exception. This will fix 1) and partially 3) but will introduce behavior change on Linux (where it was expected that deleting read-only files is not possible). This seem to be a better case as if we add extra fails on Windows. I will modify the patch and tests accordingly. Gerrit change https://git.eclipse.org/r/80437 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=e56853d1b0dc0cf6cfa0d4c6cef552ab46093b2a +1 from PMC member to downport (In reply to Andrey Loskutov from comment #10) > 2) The actual regression is coming from commit > https://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/ > ?id=b8088ae7aa4bc6fa23a9e5dfc3836ad776e8ff3b > (https://git.eclipse.org/r/#/c/66577/). The replacement of > java.io.File.delete() with java.nio.file.Files.deleteIfExists(Path) is not > equivalent. The former one can delete read-only files, latter one can't => > exception. Just for my understanding, if the commit you mentioned caused this bug and it did not fix any apparent bug but was supposed to be "Code streamlining", then why it is better to fix this bug in a way other than reverting the commit which caused regression? I'm having hard time to justify why target.delete() was expected to be wrong and had to be replaced with Files.deleteIfExists(target.toPath()) only to realize later in this bug that it actually has to be Files.deleteIfExists(target.toPath()) and target.delete() as a fallback. If I compare the original code and current master than current master is more complicated and does not seem to be any better. (In reply to Szymon Ptaszkiewicz from comment #16) Not to mentioned that modifying tests to make sure the new code does not cause test failures looks very fishy... (In reply to Szymon Ptaszkiewicz from comment #16) > (In reply to Andrey Loskutov from comment #10) > > 2) The actual regression is coming from commit > > https://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/ > > ?id=b8088ae7aa4bc6fa23a9e5dfc3836ad776e8ff3b > > (https://git.eclipse.org/r/#/c/66577/). The replacement of > > java.io.File.delete() with java.nio.file.Files.deleteIfExists(Path) is not > > equivalent. The former one can delete read-only files, latter one can't => > > exception. > > Just for my understanding, if the commit you mentioned caused this bug and > it did not fix any apparent bug but was supposed to be "Code streamlining", > then why it is better to fix this bug in a way other than reverting the > commit which caused regression? I'm having hard time to justify why > target.delete() was expected to be wrong and had to be replaced with > Files.deleteIfExists(target.toPath()) only to realize later in this bug that > it actually has to be Files.deleteIfExists(target.toPath()) and > target.delete() as a fallback. If I compare the original code and current > master than current master is more complicated and does not seem to be any > better. Szymon, I assumed there were good reasons to change the code. Since I'm not deeply involved in resources project development, I can not judge what was the motivation behind the original fix. (In reply to Szymon Ptaszkiewicz from comment #17) > (In reply to Szymon Ptaszkiewicz from comment #16) > > Not to mentioned that modifying tests to make sure the new code does not > cause test failures looks very fishy... I've only added new test, the old one was simply never executed, therefore the change to make it actually test something. (In reply to Andrey Loskutov from comment #18) > Szymon, I assumed there were good reasons to change the code. Since I'm not > deeply involved in resources project development, I can not judge what was > the motivation behind the original fix. Right. I didn't mean to ask you this question only to refer to the comment in which you were able to identify the cause of regression - thank you for that! I hope that Sergey will answer the concerns I raised in comment 16. (In reply to Szymon Ptaszkiewicz from comment #16) The current code with the Andrey's fix is more efficient than the original code before https://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=b8088ae7aa4bc6fa23a9e5dfc3836ad776e8ff3b. The original code was always executing two I/O operations: File.delete() and either File.exists() or File.isDirectory(). The current code is executing one I/O operation in most cases and issues the second I/O operation only if the file is readonly. Hello everybody, i would like to know if this issue will be in SR2 release train as it seems to be planned ? Thanks Arnaud LUCIEN Ingenico New Gerrit change created: https://git.eclipse.org/r/83380 (In reply to Arnaud Lucien from comment #21) Thank you for the reminder. The answer to your question is yes. Gerrit change https://git.eclipse.org/r/83380 was merged to [R4_6_maintenance]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=87958f118eb55d11bf9fb884653843af5ea28054 Verified on Win 7 with build id: M20161116-1100 |
Created attachment 263785 [details] Screenshot of the popup error. When Eclipse 4.6 is used in combination with JRE 1.8, read-only files cannot be deleted from the Project Explorer view. Projects, containing any read-only files, also cannot be deleted. User gets a popup error, as shown in the attached screenshot, and the file/project remains in the workspace (and in the filesystem). In older versions of Eclipse, There would be a popup question, asking the user if he is sure he wants to delete read-only resources.