Bugzilla – Full Text Bug Listing
| Summary: | In E4 Template changing the Activator's package name causes an error | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Olivier Prouvost <olivier.prouvost> | ||||
| Component: | UI | Assignee: | Olivier Prouvost <olivier.prouvost> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | daniel_megert, Lars.Vogel, noopur_gupta, Vikas.Chandra | ||||
| Version: | 4.5 | Flags: | noopur_gupta:
review+
Vikas.Chandra: review+ |
||||
| Target Milestone: | 4.5 RC2 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| See Also: |
https://git.eclipse.org/r/48238 https://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=ec9e7488d467797447f77273a775dade71b8fa17 |
||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Olivier Prouvost
The Activator code for Eclipse 3 templates is generated in org.eclipse.pde.internal.ui.wizards.plugin.PluginClassCodeGenerator.generatePluginClass(String, String, PrintWriter) and whether the Activator extends org.eclipse.ui.plugin.AbstractUIPlugin or org.eclipse.core.runtime.Plugin is determined by the method org.eclipse.pde.internal.ui.wizards.plugin.PluginFieldData.isUIPlugin(). See bug 296604 for details on Activator. Tthe E4 Application Activator generation is already fixed in org.eclipse.pde.internal.ui.templates.e4.E4ApplicationNewWizard.performFinish(IProject, IPluginModelBase, IProgressMonitor) in the way described: old style Activator and imports removed, new style added. A proper way would be not to override this generation but to modify the PluginFieldData and PluginClassCode Generator to be able to generate a bare Activator for Eclipse 4 applications. Using Eclipse 4.5.0M7 I tried to reproduce by using the E4 RCP template, the E4 RCP template with changed package name, and the E4 RCP template with changed package name and sample content and in all cases the code generated implements org.osgi.framework.BundleActivator so it was apparently fixed elsewhere without closing this bug. Propose closing as WORKSFORME, unless we want to review Acitvator generation in general. Attaching Mylyn context. Created attachment 253493 [details]
mylyn/context/zip
Most likely fixed by Bug 466269. Thanks Maarten for the check. *** This bug has been marked as a duplicate of bug 466269 *** As explained in the bug #463822 comment #21 this bug is not yet fixed and renaming the package name for the activator still produce an error... The way to generate the activator should not depend only on AbstractUI flag but it should be selected by the template... New Gerrit change created: https://git.eclipse.org/r/48238 The Gerrit change (patch set 1) fixes the issues mentioned in bug 463822 comment #21 and bug 463822 comment #22. The code changes look good. - Parameter name in PluginFieldData.setE4Plugin(boolean fE4Plugin) could be changed to 'e4Plugin' as 'f' as a prefix is used for a field. - Copyright year and contributors list should be updated for files changed in pde.ui before releasing the change. Noopur, I updated the code with your remarks but unfortunately the gerrit/jenkins is down :( (In reply to Olivier Prouvost from comment #7) > Noopur, I updated the code with your remarks but unfortunately the > gerrit/jenkins is down :( Also, noticed that NewPluginTemplateWizard#setE4Plugin is a new method. So it should have the @since tag and the bundle version of org.eclipse.pde.ui should be updated. This will need the PMC approval. Please check if it is possible to set PluginFieldData#setE4Plugin in E4ApplicationNewWizard itself instead of creating the method NewPluginTemplateWizard#setE4Plugin. Ok I will add the @since and change the version. But what version number should I set ? It is today 3.8.200.qualifier... -> 3.8.201 ? 3.8.300 ? There was a discouraged access if I used directly a PluginFieldData in pde.ui.templates project (outside of pde.ui project). So I had the setE4Plugin on NewPluginTemplateWizard to avoid this and it sounds good because it is then reusable for other future templates. Anyway, if you prefer to remove this method it is possible by changing the package visibility for org.eclipse.pde.internal.ui.wizards.plugin but it is internal so ... Or by adding the setE4Plugin method on the IPluginFieldData but it is an interface... The two solutions I see are not better... Do you have any other idea ? Hi Olivier, Can you please verify bug 466269 , bug 463822 ,bug 466680 ,bug 463821 and bug 441331 with this fix and confirm on the bug. Yes those bugs are fixed with this fix. (In reply to Olivier Prouvost from comment #9) > Ok I will add the @since and change the version. But what version number > should I set ? It is today 3.8.200.qualifier... -> 3.8.201 ? 3.8.300 ? > > There was a discouraged access if I used directly a PluginFieldData in > pde.ui.templates project (outside of pde.ui project). So I had the > setE4Plugin on NewPluginTemplateWizard to avoid this and it sounds good > because it is then reusable for other future templates. > > Anyway, if you prefer to remove this method it is possible by changing the > package visibility for org.eclipse.pde.internal.ui.wizards.plugin but it is > internal so ... Or by adding the setE4Plugin method on the IPluginFieldData > but it is an interface... The two solutions I see are not better... Do you > have any other idea ? Vikas and I discussed about it and have uploaded patch set 4 on the gerrit change https://git.eclipse.org/r/#/c/48238. Please have a look. Yes it works :). I commented the review in path 4 Hi Olivier, The following things were changed a) setE4Plugin was put in E4ApplicationNewWizard as logically it belongs there. b) package org.eclipse.pde.internal.ui.wizards.plugin in pde.ui has a friend plugin - org.eclipse.pde.ui.templates. This is not the 1st time a package in pde.ui has had org.eclipse.pde.ui.templates as its friend. Ideally we would want all e4 related things in a separate package ( if not plugin) but right now things are quite mixed. ( refactoring in future wont be a bad idea) Rest of the changes are same. Can you please have a look? Also make sure no other change as mentioned above has been made unintentionally. Also a quick recheck of bugs in comment#10 wont hurt. If you want, you can also touch 1 file so that the author is assigned to you in gerrit. I get this warning "The file "Application.e4xmi" does not exist in the workspace" after creating the plugin with or without the patch. That warrants a separate bug ( for 4.6 I guess). (In reply to Vikas Chandra from comment #14) > Rest of the changes are same. Can you please have a look? Also make sure no > other change as mentioned above has been made unintentionally. Also a quick > recheck of bugs in comment#10 wont hurt. If you want, you can also touch 1 > file so that the author is assigned to you in gerrit. How much code is from Olivier, Noopur and Vikas? The main author must be set in the latest change, plus all other authors must be declared in the commit message using Also-by: For details see https://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions#Multiple_Authors Ok for the changes. It is ok for me. I wanted to do it as you said but I did not know that I could change the private visibility between the 2 plugins... I can retouch a file and add the Also By for Vikas and Noopur in the commit message... And it will be done I guess. Olivier, Yes that should be fine. Dani, primary author is Olivier - minor refactoring is done by us. Gerrit change https://git.eclipse.org/r/48238 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=ec9e7488d467797447f77273a775dade71b8fa17 See previous comment for commit id. verified in Version: Mars (4.5) Build id: I20150520-2000 opened Bug 467819. See last para of comment#14 |