Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava really necessary? #7129

Closed
3 tasks done
hick209 opened this issue Mar 27, 2024 · 2 comments
Closed
3 tasks done
Labels
type=enhancement Make an existing feature better

Comments

@hick209
Copy link

hick209 commented Mar 27, 2024

Context

I see that at some point in time we split some parts of Guava into external libraries.

One of the things extracted from it was the ListenableFuture class that was put into com.google.guava:listenablefuture:1.0

public interface ListenableFuture<V extends @Nullable Object> extends Future<V> {

Now in order not to have a conflict with the Guava lib itself (com.google.guava:guava), it was created an empty version of the library, without the ListenableFuture class, com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava and this was put as a dep of Guava, so during version resolution things would not break if com.google.guava:listenablefuture:1.0 was used somewhere else in your project, as version resolution would pick 9999.0-empty-to-avoid-conflict-with-guava.

The main question

Do we still need to keep things like that?

Could we instead actually remove the class ListenableFuture from com.google.guava:guava and have it depend on a non-empty version of com.google.guava:listenablefuture?

Referenced class

com.google.common.util.concurrent.ListenableFuture

Referenced libs

com.google.guava:guava
com.google.guava:listenablefuture:1.0
com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava

Why do we need it to be changed?

Reason 1: More standard practice

This way to fork things is non-conventional, which by itself might not be a strong enough reason to make a change, but here's another argument in favor of the change.

Reason 2: Keep "up-to-date-ness"

Since the split there was at least one nice improvement to ListenableFuture that improves nullability information of the class and this hasn't been synced into com.google.guava:listenablefuture despite the change having taken place 3 years ago.
4b02d3c

If we actually had com.google.guava:guava without ListenableFuture and depending on a non-empty version of com.google.guava:listenablefuture, this would not have happened, therefore it could result in less effort to keep libraries up to date.

Reason 3: Consistency

At the same time that com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava exists, we have com.google.guava:failureaccess which was also split out of Guava, but does not have the same treatment.
I assumed there might have been a good reason for it at the time, but what I ask here is that we revisit this decision.

My context

In my particular use case (and why I created this request) we use Buck as a build tool and we have a mono repo, which make having multiple versions of libraries co-exist tricky.

Current Behavior

Because of that, here's what we currently do:
We get both libs, guava and listenablefuture as they are both necessary by different parts of the codebase.
Since we use the non-empty version of listenablefuture we ended up patching the Guava lib, removing the ListenableFuture class from it, in order to make sure it won't clash with com.google.guava:listenablefuture:1.0.

The main reason for this request is that I dislike this patch and would like to kill it if possible.

Desired Behavior

No patch to Guava is necessary as it uses the same com.google.guava:listenablefuture as the rest of my repository, so there are no clashes.

In other words, com.google.guava:guava would depend on something like com.google.guava:listenablefuture:1.1 instead of com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava.

Checklist

@hick209 hick209 added the type=enhancement Make an existing feature better label Mar 27, 2024
@cpovirk
Copy link
Member

cpovirk commented Mar 28, 2024

Sadly, if ListenableFuture were in a separate artifact, then we'd be publishing 2 artifacts that have classes in the same package. That would cause problems for supporting Java modules. (I believe it would also cause problems for OSGi users, but I know less about that. We've even seen some surprises from splitting packages in "plain Java," but those might apply only to package-private APIs (which ListenableFuture doesn't have) and to package-info (which isn't currently very important for ListenableFuture).)

(I do think I officially regret failureaccess at this point :( And you make a good point that it's made worse by the difference between it and listenablefuture.)

@cpovirk cpovirk closed this as completed Mar 28, 2024
@hick209
Copy link
Author

hick209 commented Mar 28, 2024

If there's good reason to keep things this way I'll take your word for it.

Thanks for taking the time to look into and consider this regardless, @cpovirk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type=enhancement Make an existing feature better
Projects
None yet
Development

No branches or pull requests

2 participants