-
Notifications
You must be signed in to change notification settings - Fork 10.9k
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
30.1-android triggers Java 8 desugaring checks #5358
Comments
I didn't touch Protobuf and Netty; we upgrade those individually. Below are issues I encountered that caused me to not upgrade (further). Guava 30.1-android fails to build with Android without enabling desugaring. google/guava#5358 Robolectric 4.4 breaks AndroidChannelBuilderTest. grpc#7731 Opencensus 0.28.1+ is incompatible with gRPC. census-instrumentation/opencensus-java#2069 grpc#7732 Truth now defines the asm dependency as "compile" although it is still optional. But asm appears to have accidentally included incorrect gradle module metadata in their release (I see they've disabled the metadata on master) which make gradle think it requires Java 8. We could asm everywhere, but that's is annoying. It seems likely this will resolve itself. Mockito can be upgraded to 3.4.0, but it deprecates initMocks, which causes more code churn than I wanted in this commit. I still synchronized the example versions on 3.4.0, though, as it was already being used in some examples and the examples don't use initMocks.
Ouch, sorry. Thanks for letting us know. I think I was under the impression that desugaring of language features was on essentially universally nowadays, so this is information that I needed, though I certainly didn't want to discover it only by breaking stuff. Do you know more about the state of adoption? For example:
(I will note in the release notes that we need to figure this out.) |
In case it wasn't obvious, this was in the gRPC project. I'm not too concerned about requiring desugaring of language features for Android. Although I sort of expected to start doing that when we drop Java 7 support; it is a bit harder to explain before that point. I sincerely hope that gRPC's down-stream users are already desugaring; it is pretty painless and is quite helpful. But this would also be a new requirement, and I think you're all to aware of how those can be surprising 😄. If Java 7 is really close to dead, I'd expect there to be more Android users without desugaring than Java 7 users. So this change might impact more users in a small way than the Java 7 drop later which will impact few users in a large way. I'm very willing to follow your lead here. But I also thought you probably weren't aware of the issue. I think we'd message this as a pre-migration, making sure that Android users will suffer no ill effects from the Java 7 drop. |
I didn't touch Protobuf and Netty; we upgrade those individually. Below are issues I encountered that caused me to not upgrade (further). Guava 30.1-android fails to build with Android without enabling desugaring. google/guava#5358 Robolectric 4.4 breaks AndroidChannelBuilderTest. #7731 Opencensus 0.28.1+ is incompatible with gRPC. census-instrumentation/opencensus-java#2069 #7732 Truth now defines the asm dependency as "compile" although it is still optional. But asm appears to have accidentally included incorrect gradle module metadata in their release (I see they've disabled the metadata on master) which make gradle think it requires Java 8. We could asm everywhere, but that's is annoying. It seems likely this will resolve itself. Mockito can be upgraded to 3.4.0, but it deprecates initMocks, which causes more code churn than I wanted in this commit. I still synchronized the example versions on 3.4.0, though, as it was already being used in some examples and the examples don't use initMocks.
Thanks, that's all reassuring. Your "more users in a small way" / "few users in a large way" framing sounds like the right way of looking at it. A pre-migration is indeed what we're going for. But I had hoped we could ease into it with some runtime warnings and then escalate from there, not immediately force compile-time errors on people.... Then again, I did specifically call out the possibility of compile-time errors with this change, so I guess I got my wish :) It would just be nice if we had a way to turn off this one error until later in the process. Maybe I should have started with Martin Buchholz's suggestion to check java.class.version at runtime. It's not yet clear to me whether we're a lead worth following or cautionary example. No one else has reported problems, but I can't imagine many people have adopted the new version yet. (Someday, adoption data may be available through deps.dev, but it's hard to conclude much from its existing data without seeing historical trends: From its snapshot data, I can see that the majority of Maven projects are on Guava 20.0 or earlier, which presumably means it's pulling in a lot of dead projects. We could probably get better data if we were sufficiently motivated.) I will probably sit on this for a little: If someone turns up tomorrow and tells us that it's a big problem, it will be easy to decide to release 30.1.1 without the problematic class. If everything is still fine on Monday, then I should think harder about this. |
🤣
Sounds fair. gRPC will be use 30.0 for the moment, which was still an upgrade for us. But our release isn't even scheduled for another month, and even then it seems fine to use 30.0. But we'll still want to make sure we figure out things soon-ish so that we can get users into a solid state to avoid being in a lurch if there is a security vulnerability, Guava's Important New Feature, or the like. |
Unless we hear more reports of problems, I'm going to call this Working as Intended-ish. I've updated our announcement and release notes to mention the change. Some non-eventful updates: I've been doing daily web searches from problems from this, and so far, they've only led me back here. Plus, today, I did some searches of GitHub issues:
I went through 10 pages each from the first and third searches. I found one failure related to lack of desugaring. I left a comment there for the maintainer. If anyone reading this does have a problem with the new requirement of desugaring, please let us know here. |
I didn't touch Protobuf and Netty; we upgrade those individually. Below are issues I encountered that caused me to not upgrade (further). Guava 30.1-android fails to build with Android without enabling desugaring. google/guava#5358 Robolectric 4.4 breaks AndroidChannelBuilderTest. grpc#7731 Opencensus 0.28.1+ is incompatible with gRPC. census-instrumentation/opencensus-java#2069 grpc#7732 Truth now defines the asm dependency as "compile" although it is still optional. But asm appears to have accidentally included incorrect gradle module metadata in their release (I see they've disabled the metadata on master) which make gradle think it requires Java 8. We could asm everywhere, but that's is annoying. It seems likely this will resolve itself. Mockito can be upgraded to 3.4.0, but it deprecates initMocks, which causes more code churn than I wanted in this commit. I still synchronized the example versions on 3.4.0, though, as it was already being used in some examples and the examples don't use initMocks.
Closing to indicate that we're not currently planning to change anything in response to the gRPC report specifically. But please continue to report any discoveries here so that we can change course as needed. |
…e to 30.1.1-jre Updated to addres an identified issue - `io`: Deprecated `Files.createTempDir()`. ([fec0dbc](google/guava@fec0dbc)) ([CVE-2020-8908](https://nvd.nist.gov/vuln/detail/CVE-2020-8908); continuing discussion in [#4011](google/guava#4011)) - Increased the aggressiveness of [Guava 30.1](https://github.com/google/guava/releases/tag/v30.1)'s warning log message for running `guava-android` under a Java 7 VM. (Android VMs are unaffected.) If the warning _itself_ causes you trouble, you can eliminate it by silencing the logger for `com.google.common.base.Preconditions` (which is used _only_ for this warning). This warning prepares for [removing support for Java 7 in 2021](google/guava#5269). Please report any problems. We have tried to make the warning as safe as possible, but anytime a common library logs, especially as aggressively as we do in this new release, there is the potential for [`NullPointerException`](https://stackoverflow.com/a/41017717/28465) or even [deadlock](https://stackoverflow.com/a/48009613/28465). (To be clear, Guava will not log under Java 8 or Android, but it will under Java 7.) ([00c25e9](google/guava@00c25e9)) - `cache`: Fixed compatibility between `asMap().compute(...)` and a load. ([42bf4f4](google/guava@42bf4f4)) - `cache`: Added `@CheckReturnValue` to some APIs. ([a5ef129](google/guava@a5ef129)) - `collect`: Added `@DoNotCall` to the mutator methods on immutable types ([6ae9532](google/guava@6ae9532)) - `hash`: Removed `@Beta` from `HashCode`. ([2c9f161](google/guava@2c9f161)) - `io`: Removed `@Beta` from `CountingOutputStream`. ([d394bac](google/guava@d394bac847467039530f514f880ecca27263d0ff))i - If you use guava-android in an Android project (as opposed to from a Java VM), you will need to [enable desugaring of Java 8 _language features_](https://developer.android.com/studio/write/java8-support.html#supported_features) if you have not already done so. (And if you are releasing an Android _library_, then anyone who uses that library will also have to enable desugaring.) We expect for nearly all Android projects to have already enabled desugaring. But if this causes problems for you, please let us know on [issue #5358](google/guava#5358). The purpose of this change is to detect potential problems for users now so that we can plan to use Java 8 language features in our implementation later this year. - Introduced a warning log message when running `guava-android` under a Java 7 VM. (Android VMs are unaffected, aside from the need to use desugaring, described in the previous bullet.) This warning is not _guaranteed_ to be logged when running under Java 7, so please don't rely on it as your only warning about future problems. If the warning _itself_ causes you trouble, you can eliminate it by silencing the logger for `com.google.common.base.MoreObjects$ToStringHelper` (which is used _only_ for this warning). This warning prepares for [removing support for Java 7 in 2021](google/guava#5269). Please report any problems. We have tried to make the warning as safe as possible, but anytime a common library logs, there is the potential for [`NullPointerException`](https://stackoverflow.com/a/41017717/28465) or even [deadlock](https://stackoverflow.com/a/48009613/28465). (To be clear, Guava will _not_ log under Java 8 or Android, but it _may_ log under Java 7.) ([dc52e6e](google/guava@dc52e6e)) - Note that we subsequently made this warning more aggressive in [Guava 30.1.1](https://github.com/google/guava/releases/tag/v30.1.1), including changing the logger it uses to `com.google.common.base.Preconditions`. - `base`: Deprecated `StandardSystemProperty.JAVA_EXT_DIRS`. We do not plan to remove the API, but note that, under recent versions of Java, that property always has a value of `null`. ([38abf07](google/guava@38abf07)) - `net`: Added `HttpHeaders` constants for `Origin-Isolation` and `X-Request-ID`. ([a48fb4f](google/guava@a48fb4f), [8319d20](google/guava@8319d20)) - `reflect`: Added `ClassInfo.isTopLevel()`. ([4106272](google/guava@4106272)) - `util.concurrent`: Added `ClosingFuture.submitAsync(AsyncClosingCallable)`. ([c5e2d8d](google/guava@c5e2d8d)) - [Guava types can no longer be sent over GWT-RPC.](https://groups.google.com/d/msg/guava-announce/zHZTFg7YF3o/rQNnwdHeEwAJ) Even the earlier, temporary way to reenable support (`guava.gwt.emergency_reenable_rpc`) no longer has an effect. ([0cb89dd](google/guava@0cb89dd)) - `cache`: Fixed memory leak in `LocalCache` under [j2objc](https://developers.google.com/j2objc). ([5e519d9](google/guava@5e519d9)) - `collect`: Added two-element `min` and `max` methods to `Comparators`. ([958186c](google/guava@958186c)) - `collect`: Removed `@Beta` from `Multimaps.toMultimap`. ([b6b4dc4](google/guava@b6b4dc4)) - `collect`: Made the set returned by `ImmutableMap<K, V>.keySet()` serializable as long as `K` is serializable, even if `V` is not (and similarly for `values()`). ([f5a69c3](google/guava@f5a69c3)) - `collect`: Fixed bug in `powerSet.equals(otherPowerSet)` would erroneously return `false` if the two power sets' underlying sets were equal but had a different iteration order. ([215b1f0](google/guava@215b1f0)) - `collect`: Eliminated [j2objc](https://developers.google.com/j2objc) retain-cycle in `SingletonImmutableBiMap`. ([0ad38b8](google/guava@0ad38b8)) - `eventbus`: Prevented `@Subscribe` from being applied to a method that takes a primitive, as that will never be called. ([554546c](google/guava@554546c)) - `graph`: Made `Traverser.breadthFirst()` lazier, and optimized `Traverser` more generally. ([32f2d77](google/guava@32f2d77), [b5210ca](google/guava@b5210ca)) - `graph`: Added `@DoNotMock` to `Traverser`. ([6410f18](google/guava@6410f18)) - `io`: Deprecated `Files.createTempDir()`. ([fec0dbc](google/guava@fec0dbc)) ([CVE-2020-8908](https://nvd.nist.gov/vuln/detail/CVE-2020-8908); continuing discussion in [#4011](google/guava#4011)) - `io`: Upgraded `ByteStreams.copy(InputStream, OutputStream)` to use the faster `FileChannel` if possible. ([a1e9a0b](google/guava@a1e9a0b)) [update: My mistake: This was [rolled back](google/guava@e839f94), so it did not make 30.0.] - `math`: Added `roundToDouble` to `BigDecimalMath`, `BigIntegerMath`, and `LongMath`. ([bee4f3c](google/guava@bee4f3c), [2b5c096](google/guava@2b5c096), [633abf2](google/guava@633abf2)) - `net`: Added `MediaType` constants for several font/ types. ([571cf66](google/guava@571cf66)) - `net`: Added `HttpHeaders` constants for `Cross-Origin-Embedder-Policy(-Report-Only)?`. ([c3bf731](google/guava@c3bf731)) - `testing`: Made `EqualsTester` test that non-`String` objects are not equal to their `String` representations. ([c9570ea](google/guava@c9570ea)) - `util.concurrent`: Added `ClosingFuture`. ([52e048e](google/guava@52e048e)) - `util.concurrent`: Removed the deprecated 1-arg `ServiceManager.addListener(Listener)`. Use the 2-arg `addListener(Listener, Executor)` overload, setting the executor to `directExecutor()` for equivalent behavior. ([dfb0001](google/guava@dfb0001)) - `util.concurrent`: Changed `AbstractFuture.toString()` to no longer include the `toString()` of the result. ([2ebf27f](google/guava@2ebf27f)) - `util.concurrent`: Added `awaitTerminationUninterruptibly`. ([f07b954](google/guava@f07b954))
…e to 30.1.1-jre (#28) Updated to addres an identified issue - `io`: Deprecated `Files.createTempDir()`. ([fec0dbc](google/guava@fec0dbc)) ([CVE-2020-8908](https://nvd.nist.gov/vuln/detail/CVE-2020-8908); continuing discussion in [#4011](google/guava#4011)) - Increased the aggressiveness of [Guava 30.1](https://github.com/google/guava/releases/tag/v30.1)'s warning log message for running `guava-android` under a Java 7 VM. (Android VMs are unaffected.) If the warning _itself_ causes you trouble, you can eliminate it by silencing the logger for `com.google.common.base.Preconditions` (which is used _only_ for this warning). This warning prepares for [removing support for Java 7 in 2021](google/guava#5269). Please report any problems. We have tried to make the warning as safe as possible, but anytime a common library logs, especially as aggressively as we do in this new release, there is the potential for [`NullPointerException`](https://stackoverflow.com/a/41017717/28465) or even [deadlock](https://stackoverflow.com/a/48009613/28465). (To be clear, Guava will not log under Java 8 or Android, but it will under Java 7.) ([00c25e9](google/guava@00c25e9)) - `cache`: Fixed compatibility between `asMap().compute(...)` and a load. ([42bf4f4](google/guava@42bf4f4)) - `cache`: Added `@CheckReturnValue` to some APIs. ([a5ef129](google/guava@a5ef129)) - `collect`: Added `@DoNotCall` to the mutator methods on immutable types ([6ae9532](google/guava@6ae9532)) - `hash`: Removed `@Beta` from `HashCode`. ([2c9f161](google/guava@2c9f161)) - `io`: Removed `@Beta` from `CountingOutputStream`. ([d394bac](google/guava@d394bac847467039530f514f880ecca27263d0ff))i - If you use guava-android in an Android project (as opposed to from a Java VM), you will need to [enable desugaring of Java 8 _language features_](https://developer.android.com/studio/write/java8-support.html#supported_features) if you have not already done so. (And if you are releasing an Android _library_, then anyone who uses that library will also have to enable desugaring.) We expect for nearly all Android projects to have already enabled desugaring. But if this causes problems for you, please let us know on [issue #5358](google/guava#5358). The purpose of this change is to detect potential problems for users now so that we can plan to use Java 8 language features in our implementation later this year. - Introduced a warning log message when running `guava-android` under a Java 7 VM. (Android VMs are unaffected, aside from the need to use desugaring, described in the previous bullet.) This warning is not _guaranteed_ to be logged when running under Java 7, so please don't rely on it as your only warning about future problems. If the warning _itself_ causes you trouble, you can eliminate it by silencing the logger for `com.google.common.base.MoreObjects$ToStringHelper` (which is used _only_ for this warning). This warning prepares for [removing support for Java 7 in 2021](google/guava#5269). Please report any problems. We have tried to make the warning as safe as possible, but anytime a common library logs, there is the potential for [`NullPointerException`](https://stackoverflow.com/a/41017717/28465) or even [deadlock](https://stackoverflow.com/a/48009613/28465). (To be clear, Guava will _not_ log under Java 8 or Android, but it _may_ log under Java 7.) ([dc52e6e](google/guava@dc52e6e)) - Note that we subsequently made this warning more aggressive in [Guava 30.1.1](https://github.com/google/guava/releases/tag/v30.1.1), including changing the logger it uses to `com.google.common.base.Preconditions`. - `base`: Deprecated `StandardSystemProperty.JAVA_EXT_DIRS`. We do not plan to remove the API, but note that, under recent versions of Java, that property always has a value of `null`. ([38abf07](google/guava@38abf07)) - `net`: Added `HttpHeaders` constants for `Origin-Isolation` and `X-Request-ID`. ([a48fb4f](google/guava@a48fb4f), [8319d20](google/guava@8319d20)) - `reflect`: Added `ClassInfo.isTopLevel()`. ([4106272](google/guava@4106272)) - `util.concurrent`: Added `ClosingFuture.submitAsync(AsyncClosingCallable)`. ([c5e2d8d](google/guava@c5e2d8d)) - [Guava types can no longer be sent over GWT-RPC.](https://groups.google.com/d/msg/guava-announce/zHZTFg7YF3o/rQNnwdHeEwAJ) Even the earlier, temporary way to reenable support (`guava.gwt.emergency_reenable_rpc`) no longer has an effect. ([0cb89dd](google/guava@0cb89dd)) - `cache`: Fixed memory leak in `LocalCache` under [j2objc](https://developers.google.com/j2objc). ([5e519d9](google/guava@5e519d9)) - `collect`: Added two-element `min` and `max` methods to `Comparators`. ([958186c](google/guava@958186c)) - `collect`: Removed `@Beta` from `Multimaps.toMultimap`. ([b6b4dc4](google/guava@b6b4dc4)) - `collect`: Made the set returned by `ImmutableMap<K, V>.keySet()` serializable as long as `K` is serializable, even if `V` is not (and similarly for `values()`). ([f5a69c3](google/guava@f5a69c3)) - `collect`: Fixed bug in `powerSet.equals(otherPowerSet)` would erroneously return `false` if the two power sets' underlying sets were equal but had a different iteration order. ([215b1f0](google/guava@215b1f0)) - `collect`: Eliminated [j2objc](https://developers.google.com/j2objc) retain-cycle in `SingletonImmutableBiMap`. ([0ad38b8](google/guava@0ad38b8)) - `eventbus`: Prevented `@Subscribe` from being applied to a method that takes a primitive, as that will never be called. ([554546c](google/guava@554546c)) - `graph`: Made `Traverser.breadthFirst()` lazier, and optimized `Traverser` more generally. ([32f2d77](google/guava@32f2d77), [b5210ca](google/guava@b5210ca)) - `graph`: Added `@DoNotMock` to `Traverser`. ([6410f18](google/guava@6410f18)) - `io`: Deprecated `Files.createTempDir()`. ([fec0dbc](google/guava@fec0dbc)) ([CVE-2020-8908](https://nvd.nist.gov/vuln/detail/CVE-2020-8908); continuing discussion in [#4011](google/guava#4011)) - `io`: Upgraded `ByteStreams.copy(InputStream, OutputStream)` to use the faster `FileChannel` if possible. ([a1e9a0b](google/guava@a1e9a0b)) [update: My mistake: This was [rolled back](google/guava@e839f94), so it did not make 30.0.] - `math`: Added `roundToDouble` to `BigDecimalMath`, `BigIntegerMath`, and `LongMath`. ([bee4f3c](google/guava@bee4f3c), [2b5c096](google/guava@2b5c096), [633abf2](google/guava@633abf2)) - `net`: Added `MediaType` constants for several font/ types. ([571cf66](google/guava@571cf66)) - `net`: Added `HttpHeaders` constants for `Cross-Origin-Embedder-Policy(-Report-Only)?`. ([c3bf731](google/guava@c3bf731)) - `testing`: Made `EqualsTester` test that non-`String` objects are not equal to their `String` representations. ([c9570ea](google/guava@c9570ea)) - `util.concurrent`: Added `ClosingFuture`. ([52e048e](google/guava@52e048e)) - `util.concurrent`: Removed the deprecated 1-arg `ServiceManager.addListener(Listener)`. Use the 2-arg `addListener(Listener, Executor)` overload, setting the executor to `directExecutor()` for equivalent behavior. ([dfb0001](google/guava@dfb0001)) - `util.concurrent`: Changed `AbstractFuture.toString()` to no longer include the `toString()` of the result. ([2ebf27f](google/guava@2ebf27f)) - `util.concurrent`: Added `awaitTerminationUninterruptibly`. ([f07b954](google/guava@f07b954))
I'm seeing a compilation failure with 30.1-android. I don't see it with 30.0-android.
dc52e6e looks suspicious. It seems the code expected the build to ignore the Java 8 bytecode and fail at runtime. A minute of glancing didn't show a way to ignore the error without enabling desugaring.
The release notes didn't mention this new requirement. Is this an accident or on purpose? Clearly when Java 7 support is dropped desugaring would be required.
The text was updated successfully, but these errors were encountered: