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

Suppliers methods should accept Java Supplier #2758

Open
markelliot opened this issue Mar 8, 2017 · 8 comments
Open

Suppliers methods should accept Java Supplier #2758

markelliot opened this issue Mar 8, 2017 · 8 comments

Comments

@markelliot
Copy link

Now that Guava Supplier extends Java Supplier the Suppliers utility methods (e.g. Suppliers#memoize(Supplier) should accept a Java Supplier instead of a Guava one. From Guava 21 onwards, this shouldn't be a breaking change, because a Guava Supplier is a Java Supplier.

I'd be willing to submit a PR for this change if this seems acceptable.

@kevinb9n
Copy link
Contributor

kevinb9n commented Mar 8, 2017 via email

@ogregoire
Copy link

ogregoire commented Mar 14, 2017

Is it possible that breaking binary compatibility isn't a real problem anymore in the modern world? I admit I'm out of touch with that and would love some edification.

@kevinb9n

The binary compatibility of Guava is greater and greater problem because Guava has becomes immensely popular and we're stuck with long-dead dependencies that rely on unsupported versions of Guava. So we're stuck with a dependency we need, but can't rewrite ourselves, without being given time.

What we do to fix it internally is grab a copy of the source code of the project and compile it against a more recent version of Guava (and fix its Guava usage if Guava changed its API in between).

Though this might be frightening, the process is usually easy. It's just annoyingly long sometimes, especially if the build is non-standard (meaning, not Maven or Gradle) or if the usage of Guava is intensive.

@markelliot
Copy link
Author

Admittedly, I hadn't considered the scope of the backcompat issues here.

One alternative: a new utility class that duplicates the methods but changes signatures to accept and return Java Supplier instead.

@cpovirk
Copy link
Member

cpovirk commented Mar 14, 2017

Normally we'd skirt that by just keeping both overloads, but that
would be a disaster here, as target typing would fail for your lambda
expressions.

I'm pretty sure that target typing still "works": Java is smart enough to pick the more specific type. But "the more specific type" is our Function, Supplier, etc., so we're accumulating more binary references that will break if/when we finally do remove those types :(

(Of course, your larger point and your various particular points still hold: There is plenty to cause problems.)

A JavaSuppliers class is one possibility. Things get a little trickier with classes like Maps and Futures. Do we have Maps.transformValuesWithJavaFunction or FuturesUsingJavaFunctions.transform? Perhaps some of the lower-value utilities will just go away, or we'll provide equivalents that use Stream instead... but we have a lot to work out here.

@kevinb9n
Copy link
Contributor

kevinb9n commented Mar 14, 2017

@ogregoire Thanks for the patient explanation. To be clear, we have always worked hard to avoid any binary incompatibility, so I hate that that silly comment of mine is up there now looking to all the world like evidence that Guava developers don't get it. :-) We get it. Some of us just get momentarily confused and think "wait a minute, maybe it's really source incompatibility that causes all the bad problems". Derp.

(I'll freely admit that we're terribly spoiled by our internal everything-is-up-to-date-at-all-times environment.)

@ogregoire
Copy link

@kevinb9n Actually I was trying to explain how we do outside of the "everything-is-up-to-date-at-all-times" world ;)

@kashike
Copy link

kashike commented Jul 12, 2018

Any update on this?

@ghost
Copy link

ghost commented May 15, 2019

I think this caused a lot of trouble for me. It seems like it's there in 27.1-jre and not in 27.1-android.
My stackoverflow question related to this: https://stackoverflow.com/questions/56133193/how-to-properly-use-google-cloud-storage-in-scala/56145166#56145166

Edit: After reading the issue again, maybe this is actually something else entirely. But I'll leave it here in case it helps others, if that's okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants