Bug 185477 - [API] Allow client to add arbitrary headers to a manifest.mf generated by Template Wizard
Summary: [API] Allow client to add arbitrary headers to a manifest.mf generated by Tem...
Status: RESOLVED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC All
: P3 enhancement (vote)
Target Milestone: 3.4 M4   Edit
Assignee: Les Jones CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-03 21:25 EDT by Linda Chan CLA
Modified: 2007-11-13 17:17 EST (History)
2 users (show)

See Also:


Attachments
'Straw man' solution - i.e. let me know this is on the right lines? (12.62 KB, patch)
2007-11-06 09:26 EST, Les Jones CLA
no flags Details | Diff
Patch to ConvertJavaToPluginOperation to support new manifest mechanism (3.18 KB, patch)
2007-11-06 09:29 EST, Les Jones CLA
no flags Details | Diff
Patch to support generic manifest header updates for PDE templates (4.78 KB, patch)
2007-11-07 05:05 EST, Les Jones CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Linda Chan CLA 2007-05-03 21:25:23 EDT
Add public PDE Template API to allow an extender to specifiy own headers for the generated manifest.mf.
The context of this request is based on suggestion described in bug 173393 comment #5.
Comment 1 Brian Bauman CLA 2007-05-04 10:04:52 EDT
I am definitely +1 for this bug.  This can help us refactor some of our own templates to be more consistent (Import-Package headers).  Probably something good to look at for 3.4.
Comment 2 Brian Bauman CLA 2007-06-14 12:36:24 EDT
I like this as a candidate for 3.4
Comment 3 Brian Bauman CLA 2007-11-02 11:53:33 EDT
Hey Les, you interested in this bug?  I saw you were working in the template area and wanted to get this in for 3.4, so I figured I would check to see if this is something you would like.
Comment 4 Les Jones CLA 2007-11-03 05:21:39 EDT
Yes, sure; I'll take a look.
Comment 5 Les Jones CLA 2007-11-05 07:44:16 EST
To be clear, this new mechanism would only be available to those templates that are generating into a plug-in using the newer OSGi based structure. Older v3.0 plug-ins that do not have a manifest will not have access to this model detail.

The initial implementation would provide a mechanism to do exactly what is requested - i.e. to be able to alter arbitrary headers to the manifest. There will be no additional logic to support more involved functionality for example, to deal  specifically with Bundle-localization, as implied by the bug 173393.

It may be useful, however, to provide a simple demonstration template of how this may be achieved using just the information available through the public API.
Comment 6 Brian Bauman CLA 2007-11-05 12:38:43 EST
Les, you are right on.  The functionality would only be available/called if the project has a Manifest (3.0+).  The support should also be very general and flexible.  I am thinking something like setHeader(String header, String value) so implementors can modify any header (even non-OSGi ones) they wish.
Comment 7 Les Jones CLA 2007-11-06 09:26:47 EST
Created attachment 82209 [details]
'Straw man' solution - i.e. let me know this is on the right lines?

Attached is a patch that is a proposed solution to this. In essence this introduces a new public interface into the API called IPluginManifestInfo that is accessible from IPluginModelBase.

As an example of how to use this new functionality, consider a scenario where you wanted to update the existing view template (in org.eclipse.pde.ui.templates) to support NLS (and have a couple of additional headers).

Add the following code at the beginning of the updateModel(...) method in org.eclipse.pde.internal.ui.template.ide.ViewTemplate :
IPluginManifestInfo manifestInfo = model.getPluginManifestInfo();

// Could be null if it's a v3.0 plug-in
if (manifestInfo != null) {
	manifestInfo.setHeader(Constants.BUNDLE_LOCALIZATION, "plugin");
	manifestInfo.setHeader(Constants.BUNDLE_VENDOR, "%foo.provider");
	manifestInfo.setHeader("Foo", "Bar");
}

Additionally, you'll need to drop a plugin.properties file into templates_3.0/view/bin/.

Another example will follow in the form of another patch file.
Comment 8 Les Jones CLA 2007-11-06 09:29:04 EST
Created attachment 82210 [details]
Patch to ConvertJavaToPluginOperation to support new manifest mechanism

This patch updates the ConvertJavaToPluginOperation to utilise the new proposed manifest update mechanism.
Comment 9 Les Jones CLA 2007-11-06 11:15:25 EST
You know, the more I think about this, the more I'm concerned that we don't need anything new here. Perhaps all that's needed is to consider exposing some of what's already available, e.g. IBundleModel and IBundle?
Comment 10 Brian Bauman CLA 2007-11-06 11:55:22 EST
Sorry Les for not giving out more guidance.  You are right, exposing the bundle classes as API would solve the problem.  My hesitance with that is that we really double the amount of API in pde.core to get this to work.

Instead, I was thinking about trying to keep it local to the template section.  For this, we could add new protected final methods in AbstractTemplateSection.  This would allow others to call the method to get/set Manifest headers in templates without having to create/support a bunch of new API.  The method could do an instanceof to confirm the model is a IBundlePluginModel, cast, and then call the appropriate methods to get/set headers.

What do you think?
Comment 11 Les Jones CLA 2007-11-06 12:04:31 EST
Totally agree, I was concerned that this fix was heading off into generic PDE model territory, which would have some potentially large testing repercussions.  :-(

I'll have a look and will pull this back into the template code.
Comment 12 Brian Bauman CLA 2007-11-06 12:15:16 EST
Thanks for your help Les!  And just so you know, you can skip the j-unit test case for this one ;-)
Comment 13 Les Jones CLA 2007-11-07 05:05:29 EST
Created attachment 82311 [details]
Patch to support generic manifest header updates for PDE templates

Attached is a patch that is constrained down to the template functionality, as suggested. Now, the only changed class is org.eclipse.pde.ui.templates.AbstractTemplateSection.

This time the equivalent change to the ViewTemplate (again at the beginning of the updateModel(...) method) would be :

// Could be false if it's a v3.0 plug-in
if (hasBundleManifest()) {
			
	setManifestHeader(Constants.BUNDLE_LOCALIZATION, "plugin");
	setManifestHeader(Constants.BUNDLE_VENDOR, "%foo.provider");
	setManifestHeader("Foo", "Bar");
}

N.B. The "hasBundleManifest()" check is optional, in a v3.0 plug-in setManifestHeader(...) does nothing.

The tests I've done (on 3.4M3) show that it works with all the available plug-in project types except v3.0, as expected - (i.e. it updates the manifest with v3.1, v3.2, v3.3, OSGi/Equinox, OSGi/Standard).
Comment 14 Brian Bauman CLA 2007-11-13 17:17:42 EST
Thanks Les!  Patch works great.  The javadoc was very well written also.  Good work!