kt_jvm_import ignores embedded ProGuards?

[cpsauer]

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 in bazelbuild/bazel#14966.

I know rules_kotlin won't automatically get that fix because it don't currently draw on java_import in the implementation of kt_jvm_import, presumably because java_import was broken for Kotlin (bazelbuild/bazel#4549). So I'm also working to land a quick fix to java_import in bazelbuild/bazel#14967 that should make it usable for Kotlin, if you'd like it, say, in the implementation of kt_jvm_import.

Anyway I figured I should give a friendly heads that this ProGuard issue is out there in rules_java and its Kotlin-supporting brethren.

And that (hopefully) you'll soon be able to get the fix on the cheap by leaning on java_import for implementation. Or by sharing code one layer deeper and drawing on the extractor tool in the PR (once it lands) and the Bazel proguard spec validator (already available).

Thanks for reading and considering,
Chris
(ex-Googler)

P.S. cc'ing @kevin1e100, just because we'd been discussing these PRs and the state of ijar stuff.

[Bencodes]

I know rules_kotlin won't automatically get that fix because it don't currently draw on java_import in the implementation of kt_jvm_import, presumably because java_import was broken for Kotlin (bazelbuild/bazel#4549).

Thanks @cpsauer for looking into all this! It shouldn't be too hard for us to mirror this behavior once those tools are in Bazel core. We can finally look into flipping the experimental_google_legacy_api flag which will expose the ProguardSpecProvider provider.

So I'm also working to land a quick fix to java_import in bazelbuild/bazel#14967 that should make it usable for Kotlin, if you'd like it, say, in the implementation of kt_jvm_import.

What's blocking us from building proper Kotlin support into ijar so that Kotlin metadata isn't being stripped out anymore? This would also let rules_jvm_external use java_import directly as well without having to compromise by disabling some of the nice-to-have build optimizations that ijar providers (I know there have been some discussions in the past, but forgetting where those landed).

[cpsauer]

You're very welcome. Thanks for looking at it, too, @Bencodes!

Lots of text back, but trying to do a good job addressing what you wrote :)

Re ijar supporting Kotlin:

Not sure what's hung up the fix to bazelbuild/bazel#4549 for 4+ years beyond what's in that thread.

I, too, would like to see ijar fixed for Kotlin if someone has a good handle on how, and, all else being equal, certainly like that fix more than my own! @kevin1e100 was pulled in originally to see if it might merit a higher priority. But it seems like it requires enough Kotlin/JVM expertise that someone else would have comparative advantage over me there.

bazelbuild/bazel#14967 is just an 80/20 fix to try to get things unbroken, unblocked, and hopefully defragmented in the meantime, until ijar gets fixed. When I encountered bazelbuild/bazel#4549, I happened to have the java_import implementation warm in my mental cache from having just fixed the ProGuard issue, so I figured I'd toss up the quick fix. I'm not a particularly heavy Kotlin user or anything, but I figured I'd do my part to leave things better than I found them.

I'd be happy to abandon the PR if folks (or you?) want to do the full fix to ijar right away, but otherwise, with java_import, it seems like we've been letting the perfect be the enemy of the good for a long time now. If the best time to have quickly fixed java_import was 4 years ago, then the second best time is right now :)

More generally than java_import, you probably have way more context than I do about how a working ijar might be helpful for rules_kotlin! Would it heavily benefit kt_jvm_library the same way it heavily benefits java_library (but not java_import)? Or do you guys have another fix for not needing to recompile when dependencies make frequent changes to their implementations but not to their interfaces If a Kotlin-compatible ijar would be a big boon to rules_kotlin then maybe join me

Re rules_jvm_external using java_import

I'm hoping bazelbuild/bazel#4549 will already be enough for them to switch to java_import. The ijar performance optimization doesn't much apply to prebuilt jars anyway. And it should be a strict upgrade, since they aren't bothering with ijar in their workaround implementation, either. I'm hoping they'll just be able to move over and get the ProGuard fix for free.

As a cross-ref: I filed a parallel issue over at bazel-contrib/rules_jvm_external#672

Re mirroring ProGuard fixes to jt_jvm_import

Delighted to hear that you're open to reusing the tools to pick up ProGuards, and maybe even excited about it :)

Am I right that you mean flipping --experimental_google_legacy_api generally to true-by-default? (If so, awesome. I'd love that!)

It makes me a little sad that we're even partially reimplementing these things, rather than unifying implementations. But maybe that's the consequence of the stalled Starlark rewrite of rules_android. (You know about the pre-alpha branch of rules_android and all that, right?) Is there any way we could bring the implementations closer together between Java and Kotlin, so that that a fix for one is more likely to be a fix for all?

Thanks for reading and for your thoughtfulness!
Chris

[cpsauer]

A status update, after some discussion:

1. cushon just landed bazelbuild/bazel@a32a0fd, which should fix kotlin_rules and Ijars: Update the Ijar tooling to do the right thing. bazel#4549 and make java_import play nice with Kotlin at long last. Yay!
   Maybe this is useful to you guys? I know @Bencodes has been following along.
2. Over in Export ProGuard specs from java_import. bazel#14966, we've been discussing, and this ProGuard problem might get resolved by Bazel switching wholesale to R8 this quarter.

[cpsauer]

Heads that the fix that would enable reusing ijar (or java_import more broadly) seems likely to land in 5.2: bazelbuild/bazel#15355