jvm_import ignores embedded ProGuards #672
Comments
Background: JAR files can bundle ProGuard specs under `META-INF/proguard/` [See https://developer.android.com/studio/build/shrink-code] Problem: Bazel previously erroniously ignored these ProGuard specs, leading to failures with, for example, androidx.annotation.Keep. Bad times. There was previously a parallel issue with aar_import. [Fixed in bazelbuild#12749] Solution: This change causes the previously ignored, embedded proguards to be extracted, validated, and then bubbled up correctly via the ProguardSpecProvider. There's also a minor fix to aar_import, adding proguard validation and slightly simplifying the resulting code. For reasoning behind why library proguards should be validated, see the module docstring of proguard_whitelister.py Remaining issues: JAR files brought down from Maven via rules_jvm_external bypass java_import in favor of rolling their own jvm_import, since java_import apparently been broken for Kotlin for years. That'll need a subsequent fix, since this only fixes java_import. For context on the Kotlin breakage, see bazelbuild#4549. For the status on fixes in rules_jvm_external, see bazel-contrib/rules_jvm_external#672
Background: JAR files can bundle ProGuard specs under `META-INF/proguard/` [See https://developer.android.com/studio/build/shrink-code] Problem: Bazel previously erroniously ignored these ProGuard specs, leading to failures with, for example, androidx.annotation.Keep. Bad times. There was previously a parallel issue with aar_import. [Fixed in bazelbuild#12749] Solution: This change causes the previously ignored, embedded proguards to be extracted, validated, and then bubbled up correctly via the ProguardSpecProvider. There's also a minor fix to aar_import, adding proguard validation and slightly simplifying the resulting code. For reasoning behind why library proguards should be validated, see the module docstring of proguard_whitelister.py Remaining issues: JAR files brought down from Maven via rules_jvm_external bypass java_import in favor of rolling their own jvm_import, since java_import apparently been broken for Kotlin for years. That'll need a subsequent fix, since this only fixes java_import. For context on the Kotlin breakage, see bazelbuild#4549. For the status on fixes in rules_jvm_external, see bazel-contrib/rules_jvm_external#672
Problem: java_import has been unusably broken for years for JARs with Kotlin interfaces, since ijar strips out important information. This has caused multiple dependent projects (includng official Bazel ones) to abandon java_import in favor of rolling their own versions, which themselves contain issues that are getting fixed in java_import. Fragmentation is bad, and fragmentation of bugs and fixes is worse. For more, see https://github.com/bazelbuild/rules_jvm_external/blob/master/private/rules/jvm_import.bzl bazelbuild#4549 bazelbuild#14966 bazel-contrib/rules_jvm_external#672 Temporary solution: Until such time as ijar is fixed for Kotlin, this adds a toggle that enables/disables ijar on java_import. This should unblock use of java_import for libraries that might contain Kotlin, so implementations can reunify. It also restores java_import to a state where it works correctly by default. Per the user manual, ijars are a build performance optimization to allow caching of actions that use JARs whose implementations change frequenly. Imported (externally compiled) JARs shouldn't be changing very often, meaning that the build performance cost of disabling ijars for these prebuilt JARs should be relatively low. Therefore, the ijar toggle is off by default, so that the build is correct by default. But ijar is still available though the toggle, just in case someone is importing a Java-interface-only JAR that they change all the time.
Problem: java_import has been unusably broken for years for JARs with Kotlin interfaces, since ijar strips out important information. This has caused multiple dependent projects (includng official Bazel ones) to abandon java_import in favor of rolling their own versions, which themselves contain issues that are getting fixed in java_import. Fragmentation is bad, and fragmentation of bugs and fixes is worse. For more, see https://github.com/bazelbuild/rules_jvm_external/blob/master/private/rules/jvm_import.bzl bazelbuild#4549 bazelbuild#14966 bazel-contrib/rules_jvm_external#672 Temporary solution: Until such time as ijar is fixed for Kotlin, this adds a toggle that enables/disables ijar on java_import. This should unblock use of java_import for libraries that might contain Kotlin, so implementations can reunify. It also restores java_import to a state where it works correctly by default. Per the user manual, ijars are a build performance optimization to allow caching of actions that use JARs whose implementations change frequenly [1]. Imported (externally compiled) JARs shouldn't be changing very often, meaning that the build performance cost of disabling ijars for these prebuilt JARs should be relatively low. Therefore, the ijar toggle is off by default, so the build is correct by default. But ijar is still made available though the toggle, just in case someone is importing a Java-interface-only JAR that they change all the time. [1] https://docs.bazel.build/versions/main/user-manual.html#flag--use_ijars
|
cc-ing @jin, @cheister, @shs96c, because I think you all been the most involved in this :) I also tossed up a quick PR that might unblock using java_import for you guys with Kotlin in bazelbuild/bazel#14967. Would love it if you'd weigh in, lmk what you think, or add to it! |
Problem: java_import has been unusably broken for years for JARs with Kotlin interfaces, since ijar strips out important information. This has caused multiple dependent projects (includng official Bazel ones) to abandon java_import in favor of rolling their own versions, which themselves contain issues that are getting fixed in java_import. Fragmentation is bad, and fragmentation of bugs and fixes is worse. For more, see https://github.com/bazelbuild/rules_jvm_external/blob/master/private/rules/jvm_import.bzl bazelbuild#4549 bazelbuild#14966 bazel-contrib/rules_jvm_external#672 Temporary solution: Until such time as ijar is fixed for Kotlin, this adds a toggle that enables/disables ijar on java_import. This should unblock use of java_import for libraries that might contain Kotlin, so implementations can reunify. It also restores java_import to a state where it works correctly by default. Per the user manual, ijars are a build performance optimization to allow caching of actions that use JARs whose implementations change frequenly [1]. Imported (externally compiled) JARs shouldn't be changing very often, meaning that the build performance cost of disabling ijars for these prebuilt JARs should be relatively low. Therefore, the ijar toggle is off by default, so the build is correct by default. But ijar is still made available though the toggle, just in case someone is importing a Java-interface-only JAR that they change all the time. [1] https://docs.bazel.build/versions/main/user-manual.html#flag--use_ijars
|
Please also see the parallel discussion developing over at rules_kotlin! bazelbuild/rules_kotlin#697 I'm really hoping that the fix to java_import might be able to let you move back to using it--allowing you to get this fix (and other future fixes!) for free! |
|
If we didn't have to use a custom import rule, that'd be great. I'm just mostly concerned with introducing churn to the native Java rules, which AFAIK, is undergoing Starlarkification. Happy to review a PR that would improve |
|
Last I saw (a couple days ago) rules_java was still just calling through to the native implementations. I'd also heard they'd kinda stopped prioritizing starlarkification of the native rules. But you may well be privy to more internal discussions that I. Any updates you think I should know around starlarkification? Otherwise, I guess we just wait for things to land, and see what gets through! Unless you'd want to express your support over there. In which case I'd of course love that. |
Problem: java_import has been unusably broken for years for JARs with Kotlin interfaces, since ijar strips out important information. This has caused multiple dependent projects (including official Bazel ones) to abandon java_import in favor of rolling their own versions, which themselves contain issues that are getting fixed in java_import. Fragmentation is bad, and fragmentation of bugs and fixes is worse. For more, see https://github.com/bazelbuild/rules_jvm_external/blob/master/private/rules/jvm_import.bzl bazelbuild#4549 bazelbuild#14966 bazel-contrib/rules_jvm_external#672 Temporary solution: Until such time as ijar is fixed for Kotlin, this adds a toggle that optionally disables ijar on java_import. This should unblock use of java_import for libraries that might contain Kotlin and allow implementations to reunify.
|
A status update, after some discussion:
|
|
Heads that the fix that would enable using java_import (instead of a custom import rule, per @jin) seems likely to land in 5.2: bazelbuild/bazel#15355 |
|
Thanks for the update! We may not be able to immediately use java_import if that means imposing a Bazel 5.2 restriction on using rules_jvm_external however, for compatibility reasons. |
|
Totally! Figured I should at least let you know :) |
|
@jin, I was evaluating having it conditionally call java_import for bazel version where the fix has landed. Is there a place where compatibility target is documented? It occurred that the most responsible implementation would include a note there, so the code could be removed when the transition period had ended. [The other fix possibility remains: separately implementing bubbling up proguard specs in jvm_import. Could do, but I assume you'd want the ijar optimizations there, now fixed?] |
Hi wonderful Bazel folks,
I'd noticed that Bazel core was ignoring ProGuard files embedded in JARs, rather than bubbling them up in ProguardSpecProvider, so I'd pulled together a PR to fix it bazelbuild/bazel#14966.
But that doesn't automatically fix the issue in this repo, since it looks like rules_jvm_external rolls its own java_import, java_import having been badly broken for Kotlin for years. Sad times.
So, I thought I should give a heads, especially since the majority of JARs with these embedded ProGuard specs are probably being pulled from Maven. It seems like the right fix is probably to fix java_import and use that, rather than getting deeper into duplicating its essential logic. But you could also re-roll all the java_import proguard logic in jvm_import, using on the extractor tool I wrote in the PR (once it lands) and the Bazel proguard spec validator already available. Thoughts?
Thanks for reading,
Chris
(ex-Googler)
The text was updated successfully, but these errors were encountered: