Community
Participate
Working Groups
I have noticed that in 3.3M6, if I delete a project that has read-only files in it, it will prompt if I want to delete the files even if I select "Do not delete contents". The files are not actually deleted, but the error dialog should not be presented to the user since the files are not actually being deleted. I don't remember seeing this in 3.3M4 or earlier.
Created attachment 62805 [details] Test project I have attached a test product that shows this bug. This project is just a generic project with a single text file in it that is marked as read-only.
I've encountered this several times, and the first couple of times it frightened me enough for me to make a backup copy of the projects I was trying to get out of my workspace. I now know that the dialog lies so it doesn't block me, but others are bound to hit this and think that there is no way for them to delete a project without deleting its contents.
I find this behavior very annoying. I will try to investigate this issue and create a patch.
This seem to be a duplicate of the <url=https://bugs.eclipse.org/bugs/show_bug.cgi?id=178846>178846</url>
Tod: is a fix this going to make 3.3? I've seen several users get thrown by this and what they end up doing is copying their entire project on disk because they don't trust that Eclipse won't delete things. What's worse is that it's weird enough that I didn't report it the first 5 times it happened to me because as an advanced user I know that this won't happen. That won't be the case when non-early adopters start getting confused by this. As such I think that having this go out in 3.3 would cause a lot of confusion and some mistrust of how Eclipse interacts with the filesystem. Also, it is a regression of 3.2 and I think that it's too big a regression not to fix.
This is not currently on my list of must fix bugs for 3.3 but if there is a patch I can look at it. If we get time early enough in the RC1 cycle we can try ot get something in.
The problem seems to be that AbstractResourcesOperation.computeDeleteStatus() always checks the read-only status of all files in the directory tree. If I want to make a patch for this, would it be better to add a boolean parameter to this method to determine whether to check the resources tree, or to add a separate method called computeDeleteStatusNoReadOnlyCheck (or something similar) and call this method from the DeleteResourcesOperation if deleteContent is false?
This is in Undo code (thanks for checking). Moving to Susan for comment.
I'll take this one for RC1. This was introduced during M4 due to fixing bug #162920.
Yikes, there are two distinct problems right now with Undo Delete Project when a read-only child is present. 1) If you aren't deleting the contents, it prompts you when there is a read-only child (the bug originally reported) 2) If you ARE deleting the contents, it doesn't prompt you when there is a read-only child, and something fails along the way that makes the operation not undoable. Both of these need to be fixed for RC1. #1 is easily fixed, I'm looking into why #2 is failing right now...
Created attachment 66384 [details] org.eclipse.ide.patch Here is a proposed patch to org.eclipse.ui.ide. Tod, can you check it please? Issue 1) is fixed by virtue of pulling out the read only check into a separate method. DeleteResourcesOperation overrides this method and does not do a read only check on projects if the content is not to be deleted. A more general fix would be to change the ReadOnlyStateChecker to take a parameter as to whether to check the children of containers when checking a container, but this will do for now. Issue 2) is fixed by calling the computeExecutionStatus() method manually from the DeleteResourceAction. This ensures that the read only check is performed when content is to be deleted. The fact that the delete was not undoable was as designed, because if you delete content it cannot be restored (and the dialog tells you that it's not undoable).
DeleteResourceAction is doing a syncExec inside of a job which is generally a pretty dangerous idea.DeleteResourcesOperation is specced to be called from a background Thread if you like. I think it would be better to have this as two jobs where the current Job joins on the job running the operation.
Created attachment 66469 [details] Suggested changes Here is a version using UIJobs (yes you did need the UI thread) and joins rather than a syncExec
thanks, Tod. Yes, the UI thread is needed because the computeExecutionStatus methods access the UI. It's not really spec'ed whether they can/should or not, but at this point the safest change is to ensure that the method runs in the UI thread. I rechecked the scenarios and reran the UI tests, so I think we are good to go. cc'ing Kim for code review. Please see Tod's suggested changes as the proposed patch. And Tod, can you +1 for fixing this for RC1?
+1
released in HEAD >20070509
Excellent, this will preempt a lot of confusion.
verified in I20070517-0010, WinXP
