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 16:05, Gunnar Wagenknecht <gunnar@xxxxxxxxxxxxxxx> wrote:

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.

As Matthias said, it does not.


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?

Yes and no. Recomputing bundle wirings whenever a new plugin was installed may break your Eclipse installation (b/c of missing uses constraints elsewhere). I lean towards saying that this might be the technical correct solution but may break many  installations and thus may cause new harm.


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.

I tend to disagree. 

1. Currently I have the means to prevent those runtime resolution issues myself. But the planning council decision eliminates that means.
2. Taking this to the extreme: Since the planning council can force all projects to use a newer version of any 3rd party library at any time, every project may have to update its code and test if this will brake its clients. Especially if my code depends on a class that exists in, say, Guava 15 but not in Guava 2, I have to work around this somehow (that’s actually something we have to do for this particular situation).

If I can’t use version ranges on my dependencies to guarantee that my code will work I question the value of package imports and uses...



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. 


Sorry, we just refining your scenario to make sure we understood it right. Regarding the wiring issues, there is no assumption.


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. 

This might be true for this particular example. I used this to illustrate another problem with that approach. You never know what Guava 21+x will do.

I still don't think you need a major version update.

I guess this depends on how people interpret semantic versioning. 
Independent of that discussion: In any case, I have to update all uses directives, and change my code using classes that do not exist anymore in Guava 21.

Again, why do we have uses constraints if (even) we deny to properly make use of them?



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.

Not sure we speak about the same things. But if I extend such a class, then this method becomes part of my 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


What is +1’ed?

+1 for: its too late - or
+1 for: replacing with Java 8 types?


In any case, I’m still not happy that the inability of others to properly use version constraints becomes my problem forcing me to update my code whenever someone thinks we need to update to library Y in version X+1 (Guava today, something else tomorrow).


Marcel








Back to the top