Add support for Android data-binding when building with Kotlin

[BenHenning]

There's a lot of context behind this PR, but to summarize: data-binding v2 is currently in the process of being introduced in upstream Bazel (see bazelbuild/bazel#10795). While this solution works correctly with Java-based Android apps, the Kotlin macros seem to break for similar Kotlin + data-binding apps. With trivial data-binding projects, the generated databinding file fails to build due to missing dependencies. This seems to be happening because the Kotlin macro splits up the library into a native android_library target, and a separate kt_jvm_library to actually build the sources. When data-binding is enabled, android_library may also run the Javac compiler which means the dependencies passed into the top-level kt_android_library also need to be passed into the native android_library to satisfy dependencies.

There's one issue with the above: it's not valid for android_library to have deps but no sources (hence why the warning suggests using exports instead). However, it's valid to do this in exactly the data-binding case per https://docs.bazel.build/versions/master/be/android.html (see enable_data_binding).

This solution is a bit hacky, so please advise on:

1. A cleaner solution, if any
2. Any suggested comments and/or documentation I should add for future maintainers
3. Any tests that I should add
4. Any tests that I should run to verify this doesn't regress any existing system behaviors

[BenHenning]

NB: this actually fixes building the "_base" target, but "_kt" still fails with an error:

error: cannot access android.databinding.DataBindingComponent

I'll keep this PR open for early feedback if that's fine with maintainers, but I'm going to continue chipping away at the new issue.

[BenHenning]

So it turns out the issue above occurs when specifically compiling headers for the base android_library. When disabling headers (using --java_header_compilation=false per bazelbuild/bazel#8585) the entire APK can be built. It's not clear to me yet what additional changes are needed to get the header compilation step to pass.

Further, this leads to runtime errors when trying to open the app:

2020-07-06 16:20:27.608 25714-25714/? E/AndroidRuntime: FATAL EXCEPTION: main
    Process: *.*.*.bindingtest, PID: 25714
    java.lang.NoClassDefFoundError: Failed resolution of: Landroid/databinding/DataBinderMapperImpl;
        at android.databinding.DataBindingUtil.<clinit>(DataBindingUtil.java:32)
        at android.databinding.DataBindingUtil.setContentView(DataBindingUtil.java:284)
        at *.*.*.bindingtest.MainActivity.onCreate(MainActivity.kt:15)
        at android.app.Activity.performCreate(Activity.java:7802)
        at android.app.Activity.performCreate(Activity.java:7791)
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1306)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3245)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3409)
        at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:83)
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2016)
        at android.os.Handler.dispatchMessage(Handler.java:107)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7356)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)
     Caused by: java.lang.ClassNotFoundException: Didn't find class "android.databinding.DataBinderMapperImpl" on path: DexPathList[[dex file "/data/local/tmp/incrementaldeployment/*.*.*.bindingtest/dex/incremental_classes1.dex"],nativeLibraryDirectories=[/data/user/0/*.*.*.bindingtest/lib, /system/lib64, /vendor/lib64, /system/product/lib64]]
        at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:196)
        at com.google.devtools.build.android.incrementaldeployment.IncrementalClassLoader$DelegateClassLoader.findClass(IncrementalClassLoader.java:57)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:379)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:312)
        at android.databinding.DataBindingUtil.<clinit>(DataBindingUtil.java:32) 
        at android.databinding.DataBindingUtil.setContentView(DataBindingUtil.java:284) 
        at com.google.android.bindingtest.MainActivity.onCreate(MainActivity.kt:15) 
        at android.app.Activity.performCreate(Activity.java:7802) 
        at android.app.Activity.performCreate(Activity.java:7791) 
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1306) 
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3245) 
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3409) 
        at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:83) 
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135) 
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95) 
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2016) 
        at android.os.Handler.dispatchMessage(Handler.java:107) 
        at android.os.Looper.loop(Looper.java:214) 
        at android.app.ActivityThread.main(ActivityThread.java:7356) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930) 

Which seems like another issue of a dependency not being included somewhere.

[BenHenning]

@ahumesky has been helping me navigate these latest errors. It seems the error is caused when you reference a data-binding-enabled library from a non-data-binding-enabled binary. However, after enabling data-binding for both I'm now running into a cannot find symbol error for the generated DataBindingMapperImpl for the library. Hoping to make more progress on that tomorrow. Note that the findings for this message correspond to a Java-only Bazel project, but I suspect the same issues will translate over to Kotlin for this specific part of the build process.

[BenHenning]

So per investigations today & feedback from @ahumesky it seems that the top-level android_binary also needs to have enable_data_binding set, but this requires the kt_android_library to be in a separate package (since one limitation in data-binding is that you can't have multiple libraries of the same package with data-binding enabled due to a package-level metadata file generated by data-binding causing conflicts). This solution + bazelbuild/bazel#2694 (comment) leads to a working Kotlin + data-binding solution (though the headers are still causing issues).

Another problem I'm noticing is that custom binding adapters are causing some issues in the Kotlin world where we split the android_library & kt_jvm_library file sets. At the surface this seems to be expected because android_library when built with data-binding is expecting all files relevant to the build to be included (including binding adapters). Forcibly adding binding adapter Java code to the base Android library leads to a working binding adapter situation, but this isn't ideal since we'd like the adapters to be in Kotlin. There may be one bug in data-binding at the moment that prevents binding adapters from being shared across library boundaries. Even without that bug, we still have the issue that some source files/deps have to be included in the android library vs. Kotlin JVM library. This might be a stronger indication that the two-step approach for kt_android_library is not robustly compatible with Android data-binding given the circular dependency present here.

[restingbull]

Sadly, because we don't have access to the native android providers, the kotlin android integration is pretty bad. And really quite inefficient.

Might that change soon?

[BenHenning]

@restingbull I'm definitely not in a position to answer that question. :) I'm actually wondering: given that the current integration isn't ideal, how much can kt_android_library be changed to support data-binding in the interim? There may be a few more workarounds like the one proposed in this PR required to make it possible, or at least caveats documented with the macro indicating that there are specific limitations in the current kt_android_library implementation.

[ahumesky]

So there's --experimental_enable_android_migration_apis which enables many of the Android APIs, for example:
https://github.com/bazelbuild/bazel/blob/3e3c831d7ce3deb0f052cd623389e62ae17101c5/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android/ValidatedAndroidDataApi.java

We could enable the databinding provider in starlark with that flag too:
https://github.com/bazelbuild/bazel/blob/bc74d1d5d68a569ccab39d1aabed82b3321bf3c6/src/main/java/com/google/devtools/build/lib/rules/android/databinding/DataBindingV2Provider.java#L35

Of course, having to rely on an experimental flag isn't great, but we don't have any plans to remove that flag until the starlark versions are available

[restingbull]

I have a deep, abiding dislike for using experimental flags in production code. Extra actions, I'm looking at you.

[BenHenning]

I think this is the best effort we can make short of introducing a dedicated kt_android_library rule that properly integrates with data-binding. While this approach seems to work fairly transparently for simple data-binding cases, the more complex case of custom binding adapters results in some issues that don't seem to have a similarly easy workaround:

• Android + data-binding: binding adapters aren't reusable across android_library boundaries rules_android#318 is tracking one underlying issue wherein you must add a manifest file to the underlying android_library to get it to run the data-binding step even when no resource files are included (which is needed in this case because the custom binding adapters need to be in their own library, and we need the corresponding data-binding metadata to be generated so downstream libraries know the adapters are available)
• Generating said metadata requires the sources to be included within the native android_library which isn't possible if the adapters are in Kotlin with the current implementation, so custom binding adapters must be written in Java until proper support is added for Kotlin & data-binding

Please let me know if this solution is acceptable to you @restingbull and others. While having binding adapters in Java isn't ideal, being able to actually use data-binding with Bazel in a project that heavily makes use of both data-binding and Kotlin is still a huge win for us.

[restingbull]

Don't pull from kwargs -- add the attribute and propagate as necessary.

[BenHenning]

Done.

[BenHenning]

Thanks @restingbull--I addressed your comment. PTAL.

[BenHenning]

Thanks @restingbull!