Community
Participate
Working Groups
Created attachment 123700 [details] a patch to illustrate the solution The touchpoint.eclipse bundle has optional requirements on some publisher classes. It seems however that these classes are in fact hard requirements. Two issues - the referenced classes are mentioned in a method of a class (EclipseTouchpoint) that must be loaded for p2 to function effectively. This may well cause NoClassDeFoundErrors when loadind EclipseTouchpoint in the absence of the publisher. I always get the behaviour of the verifier at classload time wrong but it is likely that that createBundleIU method (the one that contains all the references to the publisher) should be put out in a separate class that is not run/called unless we know we have the publisher installed. - the method prepareIU has the following code c = Class.forName("org.eclipse.equinox.p2.publisher.eclipse.BundlesAction"); //$NON-NLS-1$ if (c != null) c = Class.forName("org.eclipse.osgi.service.resolver.PlatformAdmin"); //$NON-NLS-1$ } catch (ClassNotFoundException e) { throw new IllegalStateException(NLS.bind(Messages.generator_not_available, e.getMessage())); } So basically if the publisher class is not found an ISE is thrown. The caller of this method is not setup in any way to catch this exception so we end up unwinding all the way up. Probably here we should be returning null.
Ideally the eclipse touchpoint should have no dependency on the publisher as it would be better to handle the partial IU expansion earlier. With that said, the current code looks ok to me from a runtime verifier perspective. The publisher classes are not exposed in fields, statics or method signatures so should only attempt to load if we detect a partial IU (normally from an old-style update site) and call createBundleIU. An ISE maybe heavy handed however the reason it throws it is to avoid installing a partial IU into the profile. If we log and return null my concern is that the IU will get installed and we'll be in a bad shape in terms of resolving future metadata and potentially our configured runtime state. As is the call fails and the transaction rollsback which in this case is the intent.
Indeed you are rigth wrt the class loading. I setup a test case of an agent without the publisher and was able to install stuff. I recall a case that Oleg pointed out where this sort of structure was insufficient isolation. Go figure. As for the ISE, the desire to avoid installing a partial IU makes sense. However, the only caller of prepareIU is initializeOperand() and it does IInstallableUnit preparedIU = prepareIU(iu, profile); if (preparedIU == null) return Util.createError(NLS.bind(Messages.failed_prepareIU, iu)); So it really ought not happen. BTW, why is prepareIU public? It is called by a test but that is not really a great reason to make things public.
Yep. I see what you're saying wrt returning null now and also agree there is no need to make the method public. The test can just as easily run the same logic via initializeOperand.
What is the thinking on this? put it in? This is the only thing directly requiring that hte publisher be shipped as part of an "agent". Note that bug 262332 is affected as it would normally not include the publisher.
The installer is currently also broken because of this: java.lang.NoClassDefFoundError: org/eclipse/equinox/p2/publisher/IPublisherAdvice at java.lang.Class.getDeclaredConstructors0(Native Method) at java.lang.Class.privateGetDeclaredConstructors(Unknown Source) at java.lang.Class.getConstructor0(Unknown Source) at java.lang.Class.newInstance0(Unknown Source) at java.lang.Class.newInstance(Unknown Source) at org.eclipse.core.internal.registry.osgi.RegistryStrategyOSGI.createExecutableExtension(RegistryStrategyOSGI.java:170) at org.eclipse.core.internal.registry.ExtensionRegistry.createExecutableExtension(ExtensionRegistry.java:874) at org.eclipse.core.internal.registry.ConfigurationElement.createExecutableExtension(ConfigurationElement.java:243) at org.eclipse.core.internal.registry.ConfigurationElementHandle.createExecutableExtension(ConfigurationElementHandle.java:51) at org.eclipse.equinox.internal.p2.engine.TouchpointManager$TouchpointEntry.getTouchpoint(TouchpointManager.java:46) at org.eclipse.equinox.internal.p2.engine.TouchpointManager.getTouchpoint(TouchpointManager.java:119) at org.eclipse.equinox.internal.p2.engine.TouchpointManager.getTouchpoint(TouchpointManager.java:99)
Created attachment 126353 [details] Possible fix. (not tested)
(That fix didn't work)
*** Bug 267246 has been marked as a duplicate of this bug. ***
I spent a bit of time on this bug tonight trying to understand when the VM will link in classes. What I found was that static references to a class inside the body of a method will not force early linking however use of a constructor will. So, in prepareIU the call to BundlesAction.createBundleIU does not cause BundleAction to be linked until the last moment. On the other hand the calls to new PublisherInfo and new AdviceFileAction will force the immediate linking of the classes from the Publisher (and trigger the CND error). What this means is that we should do something like Jeff originally suggested and isolate the use of the publisher in a separate support class.
Thanks Jeff. Fixed in HEAD.
cool. thanks
