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

On 9. May 2017, at 15:12, Marcel Bruch <marcel.bruch@xxxxxxxxxxxxxx> wrote:
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. 

To be fair, I don't know Equinox caching internals. I was under the impression that it would recompute the wirings when something changes in the system. Thus, I would expect Equinox to also re-wire recommenders if it detects usage conflict.

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.

So this is an Equinox issue then, isn't it? Assuming t works with -clean but not without. In such a case I think we are fighting symptoms with the Guava issue. Shouldn't we instead wait for an Equinox fix?

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.

Hmm .... it all sounds not your fault. You are not breaking someone because you increased the version range of supported versions. It really sounds like a runtime resolution issue that can occur with any library and any time.

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.

Frankly, there is a lot assumptions in this discussion. In general, I don't like basing such widespread decisions on assumptions. 

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:

You don't re-export any of the packages and as fas as I'm aware of, none of those classes changed in Guava. Thus, it shouldn't really matter what version of Guava you consume and re-use in your API. I still don't think you need a major version update.

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.

Yes, that's why uses are important for the runtime resolution.

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.

Again, that is only your fault if the method is part of your API but it's not. It looks like it's a dependency of your API.


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

In such a case you would ensure the version range of Code Recommenders being tight so that the class is included. But how can a class be missing when it's part of your API? I'm confused. And btw, if a client uses a class only present in a specific version of Guava then he better ensures that his dependency information is correct in the client's manifest. The Import statement is the client's responsibility - not yours. We then have p2 to ensure that every dependency is installed.

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.

Well, I challenge that this as painful in non-OSGi Maven/Gradle projects as it is for us.

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.

+1

-Gunnar


Back to the top