Bugzilla – Full Text Bug Listing
| Summary: | Annotation @PostConstruct not found by injector | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] e4 | Reporter: | Paul Webster <pwebster> | ||||||||||
| Component: | UI | Assignee: | Paul Webster <pwebster> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||
| Severity: | blocker | ||||||||||||
| Priority: | P3 | CC: | aniefer, david_williams, emoffatt, hmalphettes, Mike_Wilson, ob1.eclipse, olivier.prouvost, patrick, remy.suen, tjwatson, tom.schindl | ||||||||||
| Version: | unspecified | Flags: | remy.suen:
review+
tjwatson: review+ |
||||||||||
| Target Milestone: | 4.1 RC4 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Linux | ||||||||||||
| See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=403814 | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Paul Webster
The 1.0.0 javax.annotation bundle has "Require-Bundle: system.bundle". Doesn't the 1.1.0 version require this as well? Or is this the case where javax.annotation 1.1.0 is not in 1.6, and so it shouldn't? PW Reference: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=5014 and https://bugs.eclipse.org/bugs/show_bug.cgi?id=344651 PW Andrew, how can I tell where this version is getting into our build? Our e4-orbit.map lists the correct one. plugin@javax.annotation,1.0.0=p2IU,id=javax.annotation,version=1.0.0.v20101115-0725 PW The ./org.eclipse.e4.ui.releng/maps/orbit.map is also being consulted here. One fix would be for the feature to restrict which version of javax.annotation it wants by specifying "1.0.0.qualifier" in the feature.xml Is Indigo supposed to have javax.annotation 1.1.0 in it? I don't see it marked as a singleton, which means it's possible to have both versions of the bundle installed at the same time. But won't this effect anybody else that does use 1.1.0 (sometime it'll pick java 6 JRE_LIB and sometimes it will pick the bundle)? PW Created attachment 197259 [details]
patch
I believe this is the simplest and most robust method of selecting a specific version of javax.annotation.
Currently the 4.1 SDK build uses map files from both org.eclipse.e4.sdk and org.eclipse.e4.ui.releng. Any other fix would involve separating the 4.1 map files from e4 map files leading to possibly having duplicate entries.
Created attachment 197279 [details]
test case patch
The reason the tests are getting wired to the system export of javax.annotation (version 0.0.0) is because of the use of Require-Bundle. org.eclipse.e4.ui.menu.tests bundle requires org.eclipse.core.runtime. The org.eclipse.core.runtime bundle requires org.eclipse.osgi and re-exports it! The org.eclipse.e4.ui.menu.tests also requires javax.annotation (bundle-version>=1.0.0).
The issue is OSGi class loading delegation happens in the order the bundles are required. So from the org.eclipse.e4.ui.menu.tests we delegate all class loads of javax.annotation first to org.eclipse.osgi and then to the javax.annotation bundle. Essentially javax.annotation is treated as a split package. All bundles should use Import-Package for the javax.annotation bundles to avoid this. This patch moves the require-bundle for javax.annotation to the beginning of the require-bundle list for the org.eclipse.e4.ui.menu.tests bundle. This allows it to load the annotation class from the bundle first.
Ahhhh, split packages and re-exporting of bundles, two of the more evil things that have been introduced to the modularity world.
I opened bug 348155 to talk to Virgo about the implications of javax.annotation 1.1.0 vs Java 1.6 and Java 1.7 (which includes 1.1.0) PW As I see it, we have 2 options for 4.1: ==== Option 1. specify javax.annotation 1.0 in our features and in Import-Package in our bundles, excluding 1.1. This will work on java 1.5 and 1.6. I'm not sure on java 7 AFAICT Virgo is not in Indigo, so javax.annotation 1.1 is not in the release train. When installing Virgo it (must) specifically request 1.1, although as it turns out if they depend on org.eclipse.core.runtime they have to make sure they have their dependency order correct as well, or only ever use Import-Package. ==== Option 2: specify javax.annotation 1.1 in our bundles and features. We'll never use the system bundle in java 1.5 and 1.6. I'm not sure about java 7, however, since the system provides the package javax.annotation 1.1 as well and there's some question about how the bundle will interact with java 7. PW I'm voting for Option 1: First we go through and make sure all of our bundles 1) use Import-Package and 2) use the version range [1.0.0,1.1.0) Then we specify the version 1.0.0 in our org.eclipse.e4.rcp feature. PW Sorry for the trouble Paul and thanks for formulating the issue so clearly. I volunteered to package javax.annotation for Indigo. I did not suspect all the subtleties going on behind Require-Bundle: system.bundle and singleton. Looking forward to hear what is the best solution to cope with those packages that are available in newer JDKs. Sorry it is too late to change the orbit version of this bundle. Otherwise I would think that corrections in the way that it is packaged are most welcome. Maybe it should look more like the javax.transaction bundle: Fragment-Host: system.bundle Export-Package: javax.transaction.xa;version="1.1", javax.transaction;version="1.1" Although that requires everyone to use Import-Package: javax.transaction;version="1.1.0" For jetty we consume the javax.annotation package and we need to work both when the boot-delegation is on and off. (In reply to comment #11) > Sorry for the trouble Paul and thanks for formulating the issue so clearly. > I volunteered to package javax.annotation for Indigo. It's just one of those OSGi things (modularity is hard :-) > Otherwise I would think that corrections in the way that it is packaged are > most welcome. > Maybe it should look more like the javax.transaction bundle: > Fragment-Host: system.bundle > Export-Package: javax.transaction.xa;version="1.1", > javax.transaction;version="1.1" > Tom, I see javax.transaction set itself up using "Fragment-Host: system.bundle". Does that mean even javax.annotation 1.0 is wrong, since it says "Require-Bundle: system.bundle"? PW (In reply to comment #12) > (In reply to comment #11) > > Tom, I see javax.transaction set itself up using "Fragment-Host: > system.bundle". Does that mean even javax.annotation 1.0 is wrong, since it > says "Require-Bundle: system.bundle"? > > PW The Fragment-Host system.bundle approach works nicely if all you are doing is adding more types to what is provided by the VM. The javax.transaction package is the classic case where the VM provides a very limited subset of the package (I think the VM only provides a few exception types that are thrown by other packages in the VM). I don't think the javax.annotation package falls into this category. Especially the 1.1 version since it needs add more attributes to an existing type (or annotation class). The only way it can do that is to be its own bundle. If it were to be a fragment of the system.bundle then the following delegation would occur: javax.annotation class load | V VM boot class path (Java5; nothing found, Java6; find 1.0; Java7; find 1.1) | V Framework class path (find content from javax.annotation fragment) As you can see this approach would probably be ok for the 1.0 version of javax.annotation because we would either: 1) On Java 5 or less; find the content from the fragment 2) On Java 6; find the content from the VM and use it as 1.0 3) On Java 7; find the content from the VM and use it as 1.0 even though the VM provides 1.1 version of the package. This would not be a good idea for the 1.1 version of the javax.annotation bundle when on Java 6 because the VM classes would always be loaded first and override what the fragment bundle provides. Created attachment 197302 [details]
import package v01
This patch switches all uses of javax.annotation and javax.inject to import package, version="1.0.0"
It also sets our org.eclipse.e4.rcp and org.eclipse.e4.ui.feature to include the javax.annotation plugin at 1.0.0.qualifier
PW
Remy, Tom, could you please review this patch? Thanx, PW (In reply to comment #15) > Remy, Tom, could you please review this patch? Looks good to me after having picked Tom's brain for classloading intel. I noticed we didn't change the javax.inject stuff in the feature.xml files but since there's only one lying around in Orbit it seems okay. I found missing Import-Package statements for javax.annotation and javax.inject on: org.eclipse.e4.ui.workbench.renderers.swt.cocoa Otherwise patch looks good. Forget about org.eclipse.e4.ui.workbench.renderers.swt.cocoa it is a fragment of org.eclipse.e4.ui.workbench.renderers.swt which correct imports the packages. There is no need to add additional imports since they are already imported from the host. +1 to patch. Released for I20110603-1145 PW Created attachment 231119 [details]
Sample app showing PostConstruct issue
This problem has reappeared for me on Kepler releases. I had no problems with Juno, but after switching my target to Kepler M7 I need to add an Import-Package statement for javax.annotation to my bundles. If the import is missing, the PostConstruct annotation cannot be found (isAnnotationPresent returns false).
Running on Java 6, Mac OS X. Also using the e4 Bridge, but I doubt this is relevant to resolving the PostConstruct class. I've uploaded a sample RCP app that causes the problem to occur for me. Remove the javax.annotation import to reproduce.
(In reply to comment #20) > Created attachment 231119 [details] > Sample app showing PostConstruct issue Yes, you must use import-package for javax.annotation and javax.inject or we won't find @PostConstruct. It's required. PW (In reply to comment #21) > Yes, you must use import-package for javax.annotation and javax.inject or we > won't find @PostConstruct. It's required. > > PW Is there a reason this "just worked" in Juno? I'm guessing this is going to come up pretty frequently as people migrate to Kepler. Thank you for this crucial information ! Paul, do you know if there is a documentation that mention this trick ? I could not find it when I wrote my talk about E4 injection or my trainings... (In reply to comment #21) > Yes, you must use import-package for javax.annotation and javax.inject or we > won't find @PostConstruct. It's required. > > PW FWIW, I didn't have to add an import for javax.inject to get the @PostConstruct working, which makes sense. Are you saying that e4 developers should just get used to adding those two imports so that all of the Java annotations are available? (In reply to comment #21) > (In reply to comment #20) > > Created attachment 231119 [details] > > Sample app showing PostConstruct issue > > Yes, you must use import-package for javax.annotation and javax.inject or we > won't find @PostConstruct. It's required. > > PW If we create an E4 Application Project with some content (using the wizard), only the import-package is generated for javax.annotations and the javax.inject is declared as a required plugin. Is that right ? See bug 403814 comment #8 http://wiki.eclipse.org/Eclipse4/RCP/FAQ#Why_isn.27t_my_.40Inject-able.2F.40PostConstruct_methods_being_injected.3F PW I fixed the wiki yesterday !! The javax.annotation must be imported as a package and with version 1.1.0. With Kepler and the latest E4 tools the default E4 application project import javax.annotation with release 1.0.0 and when you run the application, the @PostConstruct is still no called ! It is due to this version number ! I will open an bug for that. |