Community
Participate
Working Groups
For ages the "platform" URI scheme followed by the "plugin" segment has been used to identify a resource inside a plug-in. So opening a stream on something like 'platform:/plugin/foo/icons/me.jpg' allows for reading 'me.jpg' regardless of where the plug-in 'foo' is located or of how it is deployed (jarred or as a directory). For more details, check this entry on my blog: http://lmap.blogspot.com/2008/03/platform-scheme-uri.html . This is NOT how the value of the handler attribute on E4 UI elements are resolved. A string like 'platform:/plugin/foo/org.example.Test' should not be used to encode the class 'org.example.Test' in plug-in 'foo'". All that said, I do think having a simple, easy way to specify a class in a bundle is very useful. So I'd suggest defining a new "first segment" for the 'platform' scheme to cover this use. Something like 'platform:/classpath/<plug-id>/<fully classified name of a class>' would probably do the trick. Btw, like the others, this new use should be implemented and registered to ensure it can be reused by other parts of Eclipse - like 'org.eclipse.core.runtime.FileLocator.resolve(URL) for example. See 'org.eclipse.core.internal.runtime.PlatformURLPluginConnection' for details.
Sounds fine to me. I chose the format because it was already in use and I don't like re-inventing the wheel (but will certainly accept a new wheel as long as it's still round...;-). Any pointers as to what's involved to properly implement and register the new 'classpath' segment? I fear we've already missed the boat for getting this into 3.6 but perhaps we can do something in the e4 space (where it's needed most).
I would put forth that we should change to a different scheme. While technically a URL is a URI, all of the other uses of "platform:/*" are URLs, which expect to return an InputStream. We never want to do that in our case, we want simply to look up the Class object. Maybe something like: bundleclass:/org.eclipse.e4.demo.e4photo/org.eclipse.e4.demo.e4photo.ExitHandler PW
(In reply to comment #2) Although I don't see a problem using the platform schema, I think I prefer what you are suggesting. However, if a new scheme is indeed created, we should probably the "technically accurate" and set the bundle id as the URI's authority: bundleclass://org.eclipse.e4.demo.e4photo/org.eclipse.e4.demo.e4photo.ExitHandler
(In reply to comment #2) I hit "commit" too fast on my last comment and then was dragged by other issues. So, if a new scheme is create, I would assume that there wouldn't be any need to implement a URI handler for it, at least at this moment, right? If one day someone wants to get the "byte" contents of a class file (a crazy bytecode inspection tool, for example), such a support could be added.
(In reply to comment #4) > So, if a new scheme is create, I would assume that there wouldn't be any need > to implement a URI handler for it, at least at this moment, right? Right, we wouldn't provide a URI handler for it since we need the information it specifies to call bundle.loadClass(*). If we needed a protocol to load the class bytes I would probably like to go back to platform: ... but I don't think we would need that. PW
You caught my attention in bug 305586 comment 35: bundleclass://<bundle id>/<fully qualified name of the class> Bundle id is not a good choice. Reading through this bug it's the BSN. bundleclass://<bundle symbolic name>/<fully qualified name of the class> Anyway, I was wondering if a version range is necessary here? For example, if multiple versions of a bundle are running... Not sure if such a use case is supported, though.
(In reply to comment #6) > You caught my attention in bug 305586 comment 35: > > bundleclass://<bundle id>/<fully qualified name of the class> > > Bundle id is not a good choice. Reading through this bug it's the BSN. > > bundleclass://<bundle symbolic name>/<fully qualified name of the class> oh, this second form is I believe what we mean. bundleclass://org.eclipse.e4.ide.application/org.eclipse.e4.ide.application.ExitHandler > Anyway, I was wondering if a version range is necessary here? For example, if > multiple versions of a bundle are running... Not sure if such a use case is > supported, though. while I'm not against supporting this in the future, this use is currently equivalent to using extension points and singleton plugins. PW
hehe..I think I should apologize for using the term "bundle id"... Old habits die hard ;-) I definitely meant "bundle symbolic name". Regarding adding version range support, I'd side with Paul and keep it simple for now. Btw, if there is indeed such a need in the future, this information doesn't need to be encoded as a segment of the path. Probably the query portion would do the trick: bundleclass://org.eclipse.e4.ide.application/org.eclipse.e4.ide.application.ExitHandler?vli=3;vhe=4 On this "not terribly well thought" example, 'vli' and 'vhe' stands for 'version low inclusive' and 'version high exclusive', respectively.
(In reply to comment #7) > while I'm not against supporting this in the future, this use is currently > equivalent to using extension points and singleton plugins. Ok, I see. I just thought that it might be a good idea to think a little bit about this because I recall there were some discussions about removing such a limitation. Despite, it could be also useful to support this for contract reasons. But from an URI point of view we have all the possibilities to defer this to a later point. bundleclass://<bundle symbolic name>[:OSGi version range]/<fully qualified name of the class>
I'm not sure what the uri-concept brings us in the way we are using it for our contributions would it be much better to model the contribution-definition as a real object? Then we don't have to come up which such arbritary paths, ...
See my proposal in bug 305586 comment 34
Oleg, is this something we can address before RC1? While I'm not looking for more work, we're currently mis-representing the plaform://plugin URL. PW
(In reply to comment #12) > Oleg, is this something we can address before RC1? While I'm not looking for > more work, we're currently mis-representing the plaform://plugin URL. > PW My inclanation would be not to change it for the initial release. 1) I don't see it as a real issue - nothing is broken, this is more of a question of "purity". Which is cool in itself, if it were to come at no cost. 2) The UR{I/L}s we use to save model on disk will very likley change in 4.1 if we get time to work on model persistance. 3) The same URIs are used by parts: contributionURI="platform:/plugin/org.eclipse.e4.demo.e4photo/org.eclipse.e4.demo.e4photo.Thumbnails so those would have to change as well. 4) There was a comment above about not needing a URI handler for the new type. I am not sure if this is correct as handlers used by EMF to convert absolute paths (C:/abc/myWorkspace/org.acme/a.class) into "platform:/plugin/org.acme...". This might or might not be an issue, but needs to be verified. So, unless I am missing something big, this is for "after initial release". So far the only real issue was "what if somebody tries to open a stream on it
(In reply to comment #13) > > 3) The same URIs are used by parts: > > contributionURI="platform:/plugin/org.eclipse.e4.demo.e4photo/org.eclipse.e4.demo.e4photo.Thumbnails > Yes, they'll all have to change. The issue is that we're wrong on all fronts. It's not a platform: URL, which is published in the help. The format is wrong, and if used in the system handlers will throw an exception. There's nothing wrong with using a URI to specify this: bundleclass:/org.eclipse.e4.demo.e4photo/org.eclipse.e4.demo.e4photo.ExitHandler Using our own scheme allows us to define how it is used, and the makeup of the URI. But it is most definitely not a platform: URI. But if you want to defer this to post 4.0 I'm OK with that. PW
OK, if we want to do this now is the time. Dean, could you have a look at this? PW
Released the fix as at: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=77b20e2d7323577b6ceee6984e10160c822e48ce Modified contributionURIs to use the format of bundleclass://<bundle symbolic name>/<fully qualified class name> Fixed 1) All Eclipse uses of the contributionURI to use the new form. These appeared in the xmi files, hard coded in source as well as generated in source. 2) Fixed the resolver to load using the new syntax 3) Fixed the model reconciler so that it can read the old format from persisted models and convert to the new format. Any uses of the old contribution URI in client code will cause bundle load errors to be sent to the log.
Fixed
The org.eclipse.e4.ui.workbench.renderers.swt.cocoa has some code that generates platform:/plugin URLs, but it was obfuscated and missed. Fix committed in 49c869c3e1f3c93c026044114f023e96c1bc4949.
Re-opened as org.eclipse.platform's LegacyXMI.e4xmi seems to have been missed.
Oh, and the XMLModelReconciler change doesn't seem to correct platform-style URIs in the original e4xmi file. I think we should re-think the backwards compatibility story here. Any bundle that programmatically adds to the model (e.g., through an addon) that has not been fixed up will cause the application to abort. The following stacktrace is what I just had happen as to the live model editor not being fixed up yet. !ENTRY org.eclipse.osgi 4 0 2012-01-18 19:06:07.746 !MESSAGE Application error !STACK 1 java.lang.IllegalArgumentException at org.eclipse.osgi.framework.internal.core.PackageAdminImpl.getBundles(PackageAdminImpl.java:583) at org.eclipse.e4.ui.internal.workbench.Activator.getBundleForName(Activator.java:78) at org.eclipse.e4.ui.internal.workbench.ReflectionContributionFactory.getBundle(ReflectionContributionFactory.java:135) at org.eclipse.e4.ui.internal.workbench.ReflectionContributionFactory.doCreate(ReflectionContributionFactory.java:61) at org.eclipse.e4.ui.internal.workbench.ReflectionContributionFactory.create(ReflectionContributionFactory.java:53) at org.eclipse.e4.ui.internal.workbench.E4Workbench.processHierarchy(E4Workbench.java:166) at org.eclipse.e4.ui.internal.workbench.E4Workbench.init(E4Workbench.java:117) at org.eclipse.e4.ui.internal.workbench.E4Workbench.<init>(E4Workbench.java:68) at org.eclipse.e4.ui.internal.workbench.swt.E4Application.createE4Workbench(E4Application.java:291) at org.eclipse.ui.internal.Workbench$4.run(Workbench.java:546) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:532) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:352) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:624) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:579) at org.eclipse.equinox.launcher.Main.run(Main.java:1433) at org.eclipse.equinox.launcher.Main.main(Main.java:1409)
Created attachment 209742 [details] Scan of eclipse.platform.ui Dean, could you please verify the files listed in the attachment? Thanx, PW
Created attachment 209743 [details] Scan of o.e.e4.ui and o.e.e4.tools These files have to be checked as well. PW
Created attachment 209753 [details] Patch for eclipse.platform Attaching a patch for fixes to the eclipse.platform repo since I don't have commit rights there. Paul will apply them for me.
(In reply to comment #23) > Created attachment 209753 [details] > Patch for eclipse.platform > Released to the repo PW
Created attachment 209757 [details] Patch for org.eclipse.e4.ui repo
Created attachment 209758 [details] Patch for org.eclipse.e4.tools repo
OK, I think we just have one area in the tests that has been missed: http://download.eclipse.org/eclipse/downloads/drops4/I20120119-1310/results/html/org.eclipse.e4.ui.menu.tests_linux.gtk.x86_64.html PW
Did anyone fixed the URIs created by the e4xmi-Editor? I'm currently traveling so can't have a look.
(In reply to comment #28) > Did anyone fixed the URIs created by the e4xmi-Editor? I'm currently traveling > so can't have a look. I believe we did, and we're just waiting for a good build. We'll confirm tomorrow morning. PW
Dean, could you run the org.eclipse.e4.ui.menu.tests.MenuTestSuite. We're still getting 37 failures: http://download.eclipse.org/eclipse/downloads/drops4/I20120119-2200/results/html/org.eclipse.e4.ui.menu.tests_linux.gtk.x86_64.html PW
(In reply to comment #30) > Dean, could you run the org.eclipse.e4.ui.menu.tests.MenuTestSuite. We're > still getting 37 failures: > > http://download.eclipse.org/eclipse/downloads/drops4/I20120119-2200/results/html/org.eclipse.e4.ui.menu.tests_linux.gtk.x86_64.html Seems like we don't have a valid URI authority, and we're passing in null. I've released some logging to find out what we're working with when this happens. java.lang.IllegalArgumentException at org.eclipse.osgi.framework.internal.core.PackageAdminImpl.getBundles(PackageAdminImpl.java:583) at org.eclipse.e4.ui.internal.workbench.Activator.getBundleForName(Activator.java:78) at org.eclipse.e4.ui.internal.workbench.ReflectionContributionFactory.getBundle(ReflectionContributionFactory.java:135) at org.eclipse.e4.ui.internal.workbench.ReflectionContributionFactory.doCreate(ReflectionContributionFactory.java:61) at org.eclipse.e4.ui.internal.workbench.ReflectionContributionFactory.create(ReflectionContributionFactory.java:53) at org.eclipse.e4.ui.internal.workbench.E4Workbench.instantiateRenderer(E4Workbench.java:99)
A few more commits to pick up stray pieces: Add backwards-compat code to rewrite platform:/plugin URIs, and log an error http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=dc905740f41cb8395770bccb8363ef24d98d67f8 A tweak to avoid NPEs for the alternative language-factory form of URIs http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a919eab013eb17ae2b5703644816e4ed85714f5a Update model editor's class-open actions for new bundleclass URIs http://git.eclipse.org/c/e4/org.eclipse.e4.tools.git/commit/?id=6596f726fee162692eeb74043b74257512696579 Fix up org.eclipse.e4.tools.compat's ReflectionContributionFactory to handle bundleclass:// style URIs http://git.eclipse.org/c/e4/org.eclipse.e4.tools.git/commit/?id=11aa5b86017f2495fd82b6b13e6dd24da0c12361
(In reply to comment #32) > Add backwards-compat code to rewrite platform:/plugin URIs, and log an error > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=dc905740f41cb8395770bccb8363ef24d98d67f8 I did a mass rename of the content in my deltas.xml and Eclipse started up. Good. However, I did not get anything logged for some reason.
(In reply to comment #33) > > I did a mass rename of the content in my deltas.xml and Eclipse started up. > Good. However, I did not get anything logged for some reason. the deltas took care of your problem. AFAIK this fix is for RCP apps with an Application.e4xmi that they haven't updated yet. PW
(In reply to comment #34) > (In reply to comment #33) > > I did a mass rename of the content in my deltas.xml and Eclipse started up. > > Good. However, I did not get anything logged for some reason. > > the deltas took care of your problem. AFAIK this fix is for RCP apps with an > Application.e4xmi that they haven't updated yet. Right, of course. I took more draconian measures and confirmed that diff is in I20120126-1300. platform-style URIs deprecated for referencing types: platform:/plugin/org.eclipse.e4.ui.workbench/org.eclipse.e4.ui.internal.workbench.handlers.SaveAllHandler URI rewritten as: bundleclass://org.eclipse.e4.ui.workbench/org.eclipse.e4.ui.internal.workbench.handlers.SaveAllHandler
Marking this as closed since M5 is now out the door. Any other issues should be logged as new bugs. Thanks all for putting this into place!
