| Summary: | Upper Bound from com.google.inject | ||
|---|---|---|---|
| Product: | [Tools] GEF | Reporter: | Ro Mu <rouven.mueck> |
| Component: | GEF Zest | Assignee: | Alexander Nyßen <nyssen> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P3 | CC: | matthias.wienand, nyssen |
| Version: | unspecified | ||
| Target Milestone: | 5.0.0 (Oxygen) M7 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
|
Description
Ro Mu
What incompatibility do you observe? If there is one, why should be loosen our upper version constraint? It seems to be appropriate then. Concrete Problem is in org.eclipse.gef4.common.adapt.inject.AdapterInjector.AdapterMapInferrer.visit(ProviderInstanceBinding<? extends Object>) where this call is done Map<AdapterKey<?>, ?> adaptersOrProvidersByKeys = (Map<AdapterKey<?>, ?>) binding .getProviderInstance().get(); The concrete Provider is a RealMultibinderCollectionOfProvidersProvider In the com.google.inject 4 the get method returns a collection which leads to a ClassCast Exception(public Collection<Provider<T>> get()). For com.google.inject 3 there is no ClassCastException. The current version constraints within Zest are: com.google.inject;version="[1.3.0,2.0.0)", com.google.inject.binder;version="[1.3.0,2.0.0)", com.google.inject.multibindings;version="[1.3.0,2.0.0)", com.google.inject.name;version="[1.3.0,2.0.0)" This matches the package versions exported by Guice 3.0.0 and excludes everything that is incompatible, like Guice 4.0.0. If you want to be able to use GEF with Inject 4.0.0 that is a feature request, not a critical bug. Please open a different Bugzilla if that is your concern. I am resolving this as INVALID. I don´t want to run it with Guice 4 but in my product there is a guice 3 and a guice 4 running. The binder binds the version 4 which has exports packages with 1.4. This fits in the upper bound of 2. The fix i mean should be to set the bounds to [1.3.0,1.4.0) Then there will be no problem at all. Ok, I see. We will then have to investigate whether we use non-API Ort whether the 1.4 packages are incompatible to the 1.3 ones (not semantically versioned ). Guice is semantically versioned, i.e. an increase of the minor version represents API compatible changes, e.g. additions to the API were made, so that, from a client perspective, the API is still compatible with previous APIs of the same major version. Since we are using the 1.3.0 API, the 1.3+ versions should work, and the 2+ versions could possibly fail. I would guess that the type cast to Map is unsafe, i.e. it is not guaranteed by the Guice API that a Map is returned at that point. However, the GEF code you are referring to is quite old, and I cannot find that call in the current code base. Alexander, do you think we should release a fix for GEF4? I had not looked into the current code base yet. Indeed we have fixed several issues in the AdapterInjector. @Matthias: the Neon stream is finalized. If the current code base does not show this misbehavior, we can resolve this as fixed. Where did you obtain the Guice 4 bundle from? It's not provided in Orbit yet. I imported the Guice 4.1 bundles. It seems the is an API break within Guice Multibindings 4.1: a method was added to the MultibindingsTargetVisitor. I created https://github.com/google/guice/issues/1099 for the Guice issue (violation of semantic versioning). Apart from the Guice Multbindings API semantic versioning violation, it seems everything is now fine. I constrained the version of the com.google.guice.multibindings package import within org.eclipse.gef.common to [1.3.0, 1.4.0) because of the API break included in this package. All other bundles are not affected. Resolving this as fixed in 5.0.0 M7. If i apply that constraint to 4.1 only for the org.eclipse.gef(4).common I will get a Linkage Error while creating the ZestContentViewer. ava.lang.LinkageError: loader constraint violation: loader (instance of org/eclipse/osgi/internal/loader/EquinoxClassLoader) previously initiated loading for a different type with name "com/google/inject/binder/AnnotatedBindingBuilder" at org.eclipse.gef4.mvc.MvcModule.bindIUndoContext(MvcModule.java:203) at org.eclipse.gef4.mvc.MvcModule.configure(MvcModule.java:213) at org.eclipse.gef4.mvc.fx.MvcFxModule.configure(MvcFxModule.java:1118) at org.eclipse.gef4.zest.fx.ZestFxModule.configure(ZestFxModule.java:503) at org.eclipse.gef4.zest.fx.jface.ZestFxJFaceModule.configure(ZestFxJFaceModule.java:46) I guess at least the borders for com.google.inject should be patched as well. I changed the constraints to exclude Guice 4 (i.e. 1.4 packages) in general. The fix will be available with 5.0.0 M7. |