Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 514764

Summary: Upper Bound from com.google.inject
Product: [Tools] GEF Reporter: Ro Mu <rouven.mueck>
Component: GEF ZestAssignee: 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 CLA 2017-04-05 07:46:15 EDT
As in Manifests seen the major version bound for com.google.inject packages is set to exclusively  2.0.0 . In com.google.inject version 4.0 some interfaces changed which are extended by, for example gef4.mvc which leads to an incompatibility. Perhaps there is a need of changing the bounds correctly.
Comment 1 Alexander Nyßen CLA 2017-04-05 15:10:48 EDT
What incompatibility do you observe? If there is one, why should be loosen our upper version constraint? It seems to be appropriate then.
Comment 2 Ro Mu CLA 2017-04-06 03:06:35 EDT
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.
Comment 3 Alexander Nyßen CLA 2017-04-06 07:33:52 EDT
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.
Comment 4 Ro Mu CLA 2017-04-07 08:47:21 EDT
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.
Comment 5 Alexander Nyßen CLA 2017-04-07 09:17:40 EDT
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 ).
Comment 6 Matthias Wienand CLA 2017-04-07 09:33:41 EDT
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?
Comment 7 Alexander Nyßen CLA 2017-04-07 09:57:19 EDT
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.
Comment 8 Alexander Nyßen CLA 2017-05-04 13:02:59 EDT
Where did you obtain the Guice 4 bundle from? It's not provided in Orbit yet.
Comment 9 Alexander Nyßen CLA 2017-05-04 13:30:59 EDT
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.
Comment 10 Alexander Nyßen CLA 2017-05-04 13:44:47 EDT
I created https://github.com/google/guice/issues/1099 for the Guice issue (violation of semantic versioning).
Comment 11 Alexander Nyßen CLA 2017-05-04 13:46:35 EDT
Apart from the Guice Multbindings API semantic versioning violation, it seems everything is now fine.
Comment 12 Alexander Nyßen CLA 2017-05-04 13:59:34 EDT
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.
Comment 13 Ro Mu CLA 2017-05-05 07:14:10 EDT
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.
Comment 14 Alexander Nyßen CLA 2017-05-05 11:28:19 EDT
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.