Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [eclipse.org-architecture-council] Discussing Hard Requirement for Guava 21 in Oxygen 
We discussed your scenarios. Please find the answers inline. To other readers: You may replace Code Recommenders with "any project that uses Guava classes in their public APIs”.




> On 9. May 2017, at 11:41, Gunnar Wagenknecht <gunnar@xxxxxxxxxxxxxxx> wrote:
> 
> Marcel,
> 
> I think there is some confusion with the actual implications. It's my understanding that the proposed solution only affects the Oxygen release train *and* (more specific) is only relevant to the Oxygen release train repository and the Eclipse packages. Any commercial vendor is still able to continue shipping the specific version of Guava it prefers, as long as the projects using Guava guarantee compatibility to that version. When a commercial vendor decides that it wants to adopt Oxygen then it will need to test and may need to touch its code. Isn't that expected behavior? 
> 
> Please keep in mind that Eclipse API has a commitment to backwards compatibility. However, I don't think that commitment covers 3rd party API. If clients are using 3rd party API, then they need to retest and ensure that their code is working with a specific version.
> 
> Can you please be more specific about the expected breakage? 
> 
> Here in an example:
> - Code Recommenders increase the version range to Guava [15, 22).
> - Oxygen ships *only* Guava 21
> - Code Recommenders Extension ABC defines a narrower dependency version range
> - user installs Code Recommenders Extension ABC 
> - p2 will install a lower Guava version
> a) Extension will work as expected, Code Recommenders *and* Extension will use lower Guava version

That will not happen since Equinox compute bundle wirings only once. 

> b) Extension will not work because of …?


Let's suppose that ABC uses Guava [15, 16) and Code Recommenders declares a uses constraint of some Guava package:

1. The user installs an Oxygen EPP package which includes Code
  Recommenders and Guava 21
2. The user installs ABC
  - This causes p2 to pull in Guava 15 as well.
3. The user restarts Eclipse
  - Equinox will fail to resolve ABC due to a uses conflict.
  - This conflict is solvable with "-clean", but by default Equinox
    will not cut the Code Recommenders -> Guava 21 to replace it by a
    wire to Guava 15.
4. The (average) user is confused and blames ABC for not working/showing
  up in the UI.

FWIW, if Code Recommenders did not declare a uses constraint, ABC would
resolve but the user would get a ClassCastException or similar at
runtime if Guava objects are passed between Code Recommenders and ABC.

If I were the developer of ABC, it would be upset about a minor release
of Code Recommenders breaking my extension in this way. That's why I
think Code Recommenders would need a major version increase to signal to
extenders that this is *not* a drop-in replacement.


> c) Code Recommenders and others break because…?

Only the extension will break.

> d) Eclipse installation will be hosed

No.



> 
> Here in another example:
> - Oxygen ships *only* Guava 21
> - User installs Plug-in "ABC", which defines a narrower dependency version range
> => p2 will install a lower Guava version
> a) Plug-in will work as expected, Code Recommenders and others continue to work using the newer Guava version
> b) Plug-in will work as expected, Code Recommenders and others continue to work using the older Guava version
> c) Plug-in will work as expected, Code Recommenders and others break because...?
> d) Plug-in will not work, everything else continues to work
> e) Eclipse installation will be hosed
> 


Assuming that Code Recommenders is already installed and ABC is not a
Code Recommenders extension (as in the previous example), this will IMHO
trigger option a), as Equinox leaves Code Recommenders wired to Guava 21.


> 
> I'm also not sure I understand the major version requirement update. Are you re-exporting the *full* Guava bundle or are you re-exporting a specific Guava package? I was under the impression that this major version increase should only be done when you are actually re-exporting things. That's why re-exporting is not recommended 

We have Guava classes like Optional, Predicate, and ImmutableSet in our
public API (e.g. method return values, parameters, subclasses). Here's an excerpt from bundle org.eclipse.recommenders.utils:

Import-Package: com.google.common.annotations;version="[15.0.0,16.0.0)",
com.google.common.base;version="[15.0.0,16.0.0)",
com.google.common.collect;version="[15.0.0,16.0.0)",
com.google.common.hash;version="[15.0.0,16.0.0)",
com.google.common.io;version="[15.0.0,16.0.0)",
com.google.common.util.concurrent;version="[15.0.0,16.0.0)"
...
Export-Package: org.eclipse.recommenders.utils;
 uses:="com.google.common.base,
  com.google.common.collect,
  org.eclipse.core.runtime,
  org.eclipse.recommenders.utils.names,
  org.osgi.framework"
...

Thus, we do not re-export anything, but importers of package
org.eclipse.recommenders.utils would in all likelihood, if they call a
method returning an ImmutableSet, for example, need to Import-Package
com.google.common.collect. The uses constraint then ensures that both
org.eclipse.recommenders.utils and the importer a wired to the same
provider of com.google.common.collect.


As an additional example: Consider a class of Guava changes in an incompatible way (e.g. a method is removed that clients of Code Recommenders rely on). The Code Recommenders extension will break at runtime because the method could not be found. 

Or consider the client uses a class that was present in Guava 15 but is not in Guava 21. Then we’ll experience ClassNotFoundExceptions.


> 
> FWIW, Text is working hard on allowing their lib bundle to work with Guava 14-21.
> https://github.com/eclipse/xtext-lib/issues/30
> 
> FYI, here is an older discussion and a case where re-export was added back to retain binary backwards compatibility:
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=156134#c20
> 
> 
> It sounds to me, as we should also make general recommendation for any post-Oxygen release: Get rid of Guava API pollution in Eclipse API. :) Guava seems to be a continuous source of frustration and a huge sink for developer time, which is causing terrible maintenance costs btw.


There is truth in that.

Having a class com.google.common.base.Optional in your public API has
obvious benefits, but Guava's rapid-fire (major) releases are a problem.

Now, most of these nice-to-have classes have since made their way into
Java 8, so if we had to introduce a new major version of Code
Recommenders and cause API breakage, we would probably just make Code
Recommenders require Java 8, switch to java.lang.Optional and friends,
and be relieved of most of our worries. But give that we only planned on
doing a minor release with Oxygen, that's way to big a change this late
in the game.



Marcel

Back to the top