Bug 197984 - [Manifest][Validation]More consistent R4 validation
Summary: [Manifest][Validation]More consistent R4 validation
Status: RESOLVED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 M3   Edit
Assignee: PDE-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: bugday, contributed
Depends on:
Blocks:
 
Reported: 2007-07-26 11:51 EDT by Brian Bauman CLA
Modified: 2011-06-20 13:42 EDT (History)
1 user (show)

See Also:


Attachments
Patch (4.58 KB, patch)
2007-09-28 15:14 EDT, Nobody - feel free to take it CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Bauman CLA 2007-07-26 11:51:58 EDT
In the BundleErrorReporter, we need to validate R4 syntax in the presence of "Bundle-ManifestVersion: 2".  Currently, we validate the R4 syntax for the singleton attribute/directive based on the target's version >= 3.1.  This is incorrect since with Bundle-ManifestVersion: 1 (or no version), the syntax is correct.

In addition, it might be nice to add a new warning (not sure if it correlates to any existing compiler settings) if the Manifest's has a Bundle-ManifestVersion of 1 (or no version) and the target's version >= 3.1.  This way users are aware they are running the older format.  As extra credit it would be nice to have a quick fix that will make any format changes as required.
Comment 1 Chris Aniszczyk CLA 2007-09-21 11:13:28 EDT
this one may be interesting for you Gary, this is another one where all things start with the BundleErrorReporter :)

btw, we need a picture of you for our "hall of fame" page
http://www.eclipse.org/pde/pde-ui/committers/committers.php
Comment 2 Nobody - feel free to take it CLA 2007-09-24 09:03:40 EDT
I'd be more than happy to look into this Chris, if you want to mark me as the assignee.
Comment 3 Chris Aniszczyk CLA 2007-09-24 09:07:00 EDT
it's yours ;)
Comment 4 Brian Bauman CLA 2007-09-28 12:20:08 EDT
Hi Gary, Chris suggested you might need some more explanation.

Here is the easiest way to reproduce what I am talking about:
1. Create a new plug-in project.  Don't choose any templates.
2. Change the Bundle-ManifestVersion to 1
3. If the Bundle-SymbolicName has a singleton attr/directive, remove it.
4. Go to the Overview Page and select the "This plug-in is a singleton" check box.
5. Save the file.

Notice how PDE generates an warning about the singleton attribute being deprecated.  This is incorrect.  For a Manifest with a Bundle-ManifestVersion of 1, the singleton value has to be defined as an attribute.

The general idea is, if someone has a valid Manifest with a Bundle-ManifestVersion of 1, PDE should validate it as such and not validate it against the Bundle-ManifestVersion of 2 just because the target platform's version.  I could have sworn there were more cases, but until I can find/remember some more, once you fix that problem we can mark the bug as fixed.
Comment 5 Nobody - feel free to take it CLA 2007-09-28 15:14:02 EDT
Created attachment 79421 [details]
Patch

Thanks for your input Brian! 

Attached is a patch as per your recommendations in the previous post. The logic now is:

- For manifests with "Bundle-ManifestVersion" 1 declared (or none) and "Bundle-SymbolicName" singleton attributes in use, no warning is generated

- For manifests with "Bundle-ManifestVersion" 1 declared and "Bundle-SymbolicName" singleton directives in use a warning is declared that singleton directives are not applicable with "Bundle-ManifestVersion" 1 format & syntax. The quickfix will then change the directive to an attribute.
Comment 6 Brian Bauman CLA 2007-09-28 18:20:36 EDT
Thanks Gary for the patch.  It worked great.  Users will no longer get into incorrect states based on PDE's validation :)

I tweaked the patch just a little bit.  I remembered we read the Bundle-ManifestHeader in validateBundleManifestHeader() and set fOSGiR4 = true if the value was > 1.  So instead of reading the header a second time, I used the fOSGiR4 variable.  In order to make this work correctly, I had to do a little reordering, by calling validateBundleManifestHeader() just before we validate the singleton attribute/directive.

I also updated the old wording of pre-31 to the new verbiage you come up with, I liked it a bit better.  Thanks again!