| Summary: | deleting project imported from another workspace raise exception | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Bozier jerome <jerome.bozier> | ||||||||
| Component: | TPTP | Assignee: | Bozier jerome <jerome.bozier> | ||||||||
| Status: | CLOSED FIXED | QA Contact: | Kathy Chan <kathy> | ||||||||
| Severity: | normal | ||||||||||
| Priority: | P2 | CC: | jptoomey, paulslau | ||||||||
| Version: | unspecified | Flags: | jptoomey:
review+
paulslau: review+ paulslau: review+ |
||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | adopter | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Bozier jerome
Jerome, You indicated that this is a regression coming from bug 282145 (which was fixed in release 4.6.2). So was this scenario working in 4.6.1? How common is the problem? Adding Joe to the CC list to assess impact to consumer. I'm tentatively targeting this to 4.7.1 unless this turns out to be a serious problem affecting many consumers. Although this is a regression, the symptoms seem to be very minor. No dialog box pops up for the exception (it just gets logged in the error log), and the project appears to be deleted successfully. In light of this, I don't expect RPT will request a fix for this in 4.7.0, and agree with the target of 4.7.1 I discovered this bug while working on 228262 (and also 315078) I only entered them to not forget to fix them This configuration (deleting project that was imported in another workspace) was not covered by 282145 manual test : I did not thought of it when I wrote it because it is not a classic way of using eclipse. It is dangerous and I don't expect users to work like this (I erased 1 week of work like this by making and error...) In conclusion, I don't think it is vital to fix it for 4.7.1 Updating estimated time to fix it Created attachment 170630 [details]
patch
this patch fix 2 problems :
. for resource location, it use resource.getLocation() to have its physical location on disk instead of building it with current workspace location + full path (that does not work on resource from project that are not in default location)
. fix an exception that sometime occur on project import, inside resource listener (listener expect an IFile instead of a IProject, and cast was a bit brutal)
updating worked hours manual test + allTest added under CVS (will update wiki when patch will be under CVS) Joe, could you review this patch please ? i made some test this morning (without the patch) and found that you can lose some source code if patch is not there if you import a project from another workspace that contains package and java source, when you it them without clicking on "delete project content on disk", the package and the java file may be physically erased (it's how i lost 1 week of coding some day ago). the exception raised during the delete seems to mix up the checkbox values in the refactoring tree it is why i change my mind and really think it's needed to push it under 4.7 Jerome, A couple of comments: 1) I reviewed your changes, and they seem fine. 2) I am not about to reproduce the condition where files are accidentally deleted. We have a couple of developers on our team who had seen this issue before, but it was believed to have been fixed by https://bugs.eclipse.org/bugs/show_bug.cgi?id=284071 , and it had not been seen recently. I tried to reproduce this defect based on your description using the adopting product, and I was unable to reproduce it. (i.e. From the test navigator, I imported an existing project without copying it into the workspace and then deleted it without checking the "delete files on sile system" checkbox.) I see the two exceptions in the error log, but the project still exists on the file system. I need to understand exactly when files will be deleted erroneously. If we really delete users files erroneously, then I agree we should try to fix this in 4.7.0, but otherwise I'd prefer to wait for 4.7.1. Can you please clarify exactly how to reproduce the data loss, and perhaps also explain how "the exception raised during the delete seems to mix up the checkbox values in the refactoring tree". Thanks! after some testing, it can not reproduce the problem of unwanted deleted resource anymore (with or without the patch) my thinking is that debug launching + some hot code replacing leaded to an unstable configuration. in conclusion, i finally agree with Joe to defer it to 4.7.1 Created attachment 171075 [details]
Patch V2
this patch replace previous one
previous patch was fixing IFile/IFolder deleting when they where located on a project form another workspace
this version add the same kind of fix for physical delete of a IProject imported from another workspace
it also add some code cleaning (remove of unnecessary field + removing of compilation warning by typing the used maps)
updating worked hours Requires adding /org.eclipse.hyades.test.ui.navigator.tests/manual/Test.UI.TestNavigator_bugzilla_315093.testsuite to /org.eclipse.hyades.test.ui.navigator.tests/manual/AllTests.testsuite and http://wiki.eclipse.org/TPTP_Test_Tools_Project_Tests#Manual. Of interested but not formally requested by a consuming product. As such, a candidate defect for TPTP 4.7.0.1. Patch reviewed and approved. . patch pushed under CVS head . allTest updated . wiki updated closing Requires verification before closing. ok, test failed on latest build.
here is the problem :
inside DeleteLogicalProjectChange : perform
try {
try {
TrashBox.instance().trashProject(project, false);
} catch (Exception e) {
UiPlugin.logError("error while putting project into trashbox : "+project.getName());
UiPlugin.logError(e);
}
project.delete(false, true, new SubProgressMonitor(pm, 1));
} catch (CoreException e) {
UiPlugin.logError(e);
}
Change undo = null;
try {
undo = TrashBox.instance().createUntrashChange(project);
} catch (Exception e) {
UiPlugin.logError("error while creating undo delete for project : "+project.getName());
UiPlugin.logError(e);
}
return undo;
problem is that undo is calculated after the "project.delete". so, when we make a "getLocation" inside createUntrashChange, the result is null...
new patch incomming
Created attachment 177326 [details]
patch V3
this patch complete previous one
now, undo change is calculated before the physical delete, so that we are able to retrieve physical location (it is too late if done after the delete)
Paul, could you review this small patch please ? many thanks in advance (re-opening the bug until this final fix is under CVS) fix delivered under CVS after approval This defect had been resolved as FIXED for more than 1 month. Please verify with the latest TPTP 4.7.2 driver. If this defect is still left unverified by February 25, we'll close it on the originator's behalf. TPTP 4.7.2 driver can be downloaded from: http://www.eclipse.org/tptp/home/downloads/?ver=4.7.2 closing |