Bug 340640 - Manifest Editor unstable in 3.7
Summary: Manifest Editor unstable in 3.7
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 3.7 RC1   Edit
Assignee: Dani Megert CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2011-03-22 06:57 EDT by Laurent Goubet CLA
Modified: 2011-05-16 16:37 EDT (History)
2 users (show)

See Also:
curtis.windatt.public: review+


Attachments
log file (24.70 KB, application/octet-stream)
2011-03-22 06:58 EDT, Laurent Goubet CLA
no flags Details
Fix (935 bytes, patch)
2011-05-09 08:45 EDT, Dani Megert CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laurent Goubet CLA 2011-03-22 06:57:24 EDT
I know this bug is vague ... but I didn't really manage to pinpoint a particular bug or steps to reproduce them; the issue is : I've been messing around with a plugin.xml for the past 10 hours in order to try and understand how a particular extension point works ... and my plugin.xml got corrupted no less than 5 times (forcing me into manually changing the XML to supress the warning/errors).

The actions ending up corrupting the XML are usually drag & drop or deletions. Attached is a .log containing the worse I had : drag & dropping an element (a "test" in the "enablement" of the "org.eclipse.core.resources.modelProviders" extension point) inside an other (an "or" in the same "enablement") ended up preventing me from even saving the plugin.xml.

There aren't any exceptions logged when the plugin.xml ends up corrupted ... and the corruption happens randomly. Not the best bug to raise, but I at least wanted this instability to be known if it isn't already.

PS : as a matter of fact ... I managed to find "reproduction" steps. However, the plugin.xml gets corrupted waaaay before I drag & dropped ... Here are the steps and what they appear to do :

1 - Create a new "Plug-in Project", I named it "test" for my example
2 - go to the "extensions" tab of the manifest editor and add the extension "org.eclipse.core.resources.modelProviders" (add the dependency when prompted). Save.
3 - Optional : Open the "plugin.xml" file in a text editor and put it beside the manifest editor in order to easily witness the issue
4 - right-click the extension point and add a new "enablement" children. Save.
5 - right-click the enablement and add a new "adapt" children to it. Save. At this point, the plugin.xml is already corrupted (see that it has _two_ "enablement" tags instead of one ... even though the manifest editor shows no issue. any further modification from within the manifest editor will corrupt the file even more).



6 - any furter modification will corrupt the plugin.xml even more... but as the editor shows nothing, the unsuspecting user will carry on : right-click the "enablement" clause and add a new "test" children to it. Save.
7 - right-click the "enablement" clause and add a new "or" children to it; drag & drop the "test" inside the "or". Save.

At this point, the manifest editor "sometimes" refresh the view, thus notifying the user that something went terribly awry ... but it is too late. Nothing save for manually editing the plugin.xml file will prevent any further damage.

This is extremely problematic for users : I corrupted "big" plugin.xml files many times and have been forced to go back in time through CVS history each time in order to try and get a functional plugin back.
Comment 1 Laurent Goubet CLA 2011-03-22 06:58:01 EDT
Created attachment 191665 [details]
log file
Comment 2 Laurent Goubet CLA 2011-03-22 06:58:49 EDT
I'm currently downloading the M6 build, will update as soon as I've tested the reproduction steps
Comment 3 Laurent Goubet CLA 2011-03-22 07:53:29 EDT
In 3.7M6 ... I can't add a single extension to my plugin :p, and am thus blocked at step 3. Whatever the extension point I select in step 2, the manifest editor displays the error "No Schema found for the 'xxx.xxx.xxx' extension point" ...
Comment 4 Laurent Goubet CLA 2011-03-22 08:50:57 EDT
Note about comment #c3 : this is with an Eclipse 3.7M6 64bits classic download, under Java 1.6.0_17 . A colleague of mine tested this with an Eclipse 3.7M6 in linux 32 bits without reproducing the "no schema" thing ... he still reproduced the plugin.xml corruption though.
Comment 5 Curtis Windatt CLA 2011-03-22 12:12:49 EDT
Probably a dupe of Bug 331485.  We haven't been able to find a reproducable case or a possible cause.
Comment 6 Laurent Goubet CLA 2011-03-22 12:28:56 EDT
The steps outlined in comment 1 allow for 100% reproduction rate. I have checked this in :

- Windows 7 64bits - Eclipse 3.7M5 64bits
- Windows 7 64bits - Eclipse 3.7M6 64bits
- Ubuntu 32bits - Eclipse 3.7M6 32bits

With a "clean" Eclipse (download the "classic" zip, unzip, use reproduction steps).

For the record, 3.7 is the first release since 3.3 where I corrupt my plugin.xml that easily. i.e: the steps from comment 1 will not end up in a corrupt plugin.xml in 3.6 and 3.5.
Comment 7 Curtis Windatt CLA 2011-03-22 14:41:08 EDT
This would be a really nice fix for 3.7, but I don't have time to work on it for M7.  Might have time in RC1.
Comment 8 Dani Megert CLA 2011-05-06 05:59:29 EDT
I cannot reproduce the issues with the steps from comment 0 using http://download.eclipse.org/eclipse/downloads/drops/I20110505-0800/index.php. 

Laurent, can you try with that build? If you still see the problem, then please try to provide even more detailed steps (e.g. how do you save (menu, keyboard, ...).
Comment 9 Laurent Goubet CLA 2011-05-06 09:24:54 EDT
(In reply to comment #8)
> I cannot reproduce the issues with the steps from comment 0 using
> http://download.eclipse.org/eclipse/downloads/drops/I20110505-0800/index.php. 
> 
> Laurent, can you try with that build? If you still see the problem, then please
> try to provide even more detailed steps (e.g. how do you save (menu, keyboard,
> ...).

Currently downloading the build, but it will take a while (firefox announces more than 2 hours ...). I'll get back to you as soon as I could retest this.
Comment 10 Laurent Goubet CLA 2011-05-06 16:02:42 EDT
This can still be reproduced with the build you pointed me to. The reproduction steps I outlined in my first post are but one of the many use cases that fail (in fact, I stopped using the manifest editor to edit my plugin.xml files ... the text editor is less easy to use, but I found that I was faster using it instead of checking after each edit whether the file is still "green").

I'll start again, here are the exact steps I take. Before I start, here are this computer's specs :

Windows 7 64 bits
Java 1.6.0_17

(As mentionned in comment 4 , this had also been tested in Eclipse 3.7M6 with
linux 32 bits)

1. download the build you pointed (eclipse-SDK-I20110505-0800-win32-x86_64.zip as per my specs)
2. unzip in a "classic" folder with no whitespace or weird characters in its path (for the record : G:\development\eclipse-SDK-I20110505-0800-win32-x86_64\eclipse)
3. double click eclipse.exe
4. select ".\workspace" as the workspace
5. right click in the package explorer, select new > Other... > Plug-in Project
6. name it "test", hit "Next>" then "Finish". Don't switch perspective when asked to (stay on the Java perspective)
7. go to the "exensions" tab of the opened manifest editor
8. hit "Add..." and type in "*modelp" as the filter.
9. untick "Show only extension points from the required plugins"
10. double click the "org.eclipse.core.resources.modelProviders" point in the list and add the dependency when prompted (hit "Yes").
11. save (ctrl+s)
12. right-click the extension point, select New > enablement
13. save (ctrl+s)
14. right-click the "enablement" child, and select"New > adapt
15. save (ctrl+s)
16. close the manifest editor
17. double click the test/META-INF/MANIFEST.MF file
18. look at the _two_ "enablement" children of our extension point instead of a single one.

The issue seem to be triggered by us saving the file, as I cannot find any way to reproduce this if I _only_ save once and close the editor before doing any further modification.

As I mentionned, most of the basic manipulations we can make on the "extension" tab (and possibly others, I always realized this too late and had to revert to my last commit when the file was corrupt) will create duplicates in the plugin.xml.

A workaround would be to save only once and restart the editor ... but I tend to save very often, and closing editors when I know I still need them seem ... counter intuitive.
Comment 11 Curtis Windatt CLA 2011-05-06 16:48:54 EDT
This could still be a duplicate of bug 331485.  We have a planned 3.7 fix for that bug, so hopefully it will help with your issues.
Comment 12 Dani Megert CLA 2011-05-09 06:16:13 EDT
Thanks for the detailed steps. I can now reproduce the problem using latest I-build and also HEAD, hence it is not a duplicate of bug 331485 which we recently fixed.

I also verified that it works in 3.6.x. This is something we need to fix for 3.7.
Comment 13 Laurent Goubet CLA 2011-05-09 07:00:14 EDT
This is a wild guess ... and I don't know how the manifest editor (well, its "extension" tab at least) is implemented; but it feels like there is a buffer which is not properly flushed when saving : each time I make an edit and save after step 13 of my comment 10, a duplicate of my previous save is copied in the plugin.xml. No idea if that can help but... :p.

And indeed, 3.7 is the first release where I experience this. I develop Eclipse plugins since 3.2, but never saw such duplicates even when heavily modifying plugin.xml files.
Comment 14 Dani Megert CLA 2011-05-09 07:04:10 EDT
This is caused by the fix for bug 309104. I already commented there that the loading code is strange (see bug 309104 comment 3).
Comment 15 Dani Megert CLA 2011-05-09 08:45:19 EDT
Created attachment 195062 [details]
Fix
Comment 16 Dani Megert CLA 2011-05-09 08:46:32 EDT
The problem is that we load the model too often in some cases where we didn't before and then replace stuff. This happens because we missed to check for the fLoaded field as we did in 3.6.x.

Curtis please review.
Comment 17 Curtis Windatt CLA 2011-05-09 11:52:58 EDT
+1 Fixes the problem in the detailed steps and the code change makes sense.

Dani, please commit the patch.
Comment 18 Dani Megert CLA 2011-05-09 11:55:11 EDT
Committed to HEAD.
Comment 19 Curtis Windatt CLA 2011-05-16 16:37:35 EDT
Verified in I20110514-0800