Suppliers methods should accept Java Supplier

[markelliot]

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]

Thanks, this is indeed one of the categories of transition tasks that we have kept back-burnering.

Your change should be among the simplest of these, but even it has a few issues:

• It's not binary compatible.
  • 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.
  • 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.
• It's not completely source-compatible. For example, you can pass Suppliers::memoize to a method that accepts Function<cgcb.Supplier<Long>, cgcb.Supplier<Long>>, but that would break with the change.
  • Though this is probably the kind of source incompatibility that we have been okay with in the past and will need to continue to be...
• We have been trying to limit differences between the main and Android forks of Guava to only differences of implementation, and APIs that are present in one and absent in the other. APIs that exist in both but with different signatures bring a new kind of trouble....

Thoughts?

On Wed, Mar 8, 2017 at 2:24 PM, Mark Elliot notifications@github.com wrote:

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.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/guava/issues/2758, or mute the thread https://github.com/notifications/unsubscribe-auth/AA5ClxfEQijwtFucBEaMycABrqI4v278ks5rjyqsgaJpZM4MXY7g .

-- Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb@google.com

[ogregoire]

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]

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]

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]

@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]

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

[kashike]

Any update on this?

[ghost]

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.