bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.21k stars 4.06k forks source link

Android + data-binding: binding adapters aren't reusable across android_library boundaries #11745

Open BenHenning opened 4 years ago

BenHenning commented 4 years ago

Description of the problem:

When building an Android project with data-binding enabled, metadata about binding adapters don't seem to be shared across android_library boundaries in some circumstances.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Or, just build the following project:

demo-project-java-dagger-databinding.zip

What operating system are you running Bazel on?

Ubuntu

What's the output of bazel info release?

development version

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

bazel build //src:bazel //tools/android/runtime_deps:android_tools.tar.gz for locally checked out Bazel (git hash: 82e6b6a3b9589b93f4abcf3eb0f14cd1d50008ff). This is needed since recently submitted data-binding functionality hasn't yet been included in a public release of Bazel.

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

https://github.com/bazelbuild/bazel.git
82e6b6a3b9589b93f4abcf3eb0f14cd1d50008ff
82e6b6a3b9589b93f4abcf3eb0f14cd1d50008ff

Have you found anything relevant by searching the web?

No--this was filed in conjunction with an ongoing discussion with @ahumesky.

Any other information, logs, or outputs that you want to share?

The error log when building the demo project includes:

ERROR: Cannot find a setter for <android.widget.TextView app:temperature> that accepts parameter type 'java.lang.String'

If a binding adapter provides the setter, check that the adapter is annotated correctly and that the parameter type matches.

Note that merging the binding adapter library with the corresponding databinding library that uses it results in a project that cleanly builds.

github-actions[bot] commented 1 year ago

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

github-actions[bot] commented 1 year ago

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

BenHenning commented 1 year ago

Note that this issue still exists in Bazel 6.2.0.

BenHenning commented 1 year ago

NB: I found one workaround to this: one make an adapter "databinding-capable" by enabling databinding for that build unit and then depending on it in another library that requires the adapter. If, like my team, you have warnings-as-errors enabled then you can workaround this by generating an empty layout file for the databinding library so that it has something to process (I use an empty FrameLayout for this purpose).

Edit: Correction: the above only works for adapters using the "android" namespace. "app" namespace adapters don't seem to be found across library-boundaries, though I'm still looking into trying to mimic whatever works with the "android" namespace.

Edit 2: This all seems wrong. Even in Bazel, the databinding compiler doesn't always produce consistent results. I have seem that sometimes custom adapters build and sometimes they don't, but I think largely this issue still exist for all custom adapters (I have yet to find a viable workaround).

Edit 3: Actually, I was able to find a workaround. My "NB" solution above does seem to work. I'm not sure why I was suddenly seeing issues with it, but it actually does seem to be working now.

Edit 4: All of the above is true. I'm able to sometimes get a passing incremental build with shared adapters, but non-incremental builds don't seem to work. It really seems that something in android_library isn't deterministic across incremental builds, at least when it comes to databinding.

Edit 5: I found the reason for my issue, though I don't know what in android_library is causing the difference in behavior between incremental and non-incremental builds. It seems that when I try to use kt_android_library, it will succeed after a previous successful build. However, upon non-incrementally building, it will fail to find the adapters. I suspect this has something to do with the fact that kt_android_library exports the actual android_library holding the adapters and generating databinding metadata: https://github.com/bazelbuild/rules_kotlin/blob/ebef8c560a0faf29eed54810df084b30c32c6e6e/kotlin/internal/jvm/android.bzl#L83. Perhaps there's an issue with library exporting? The good news is that this means adapters can be split across multiple libraries so long as enable_data_binding is true, but they can't be in Kotlin (currently).

Edit 6: I suspect it makes sense outright why kt_android_library can't work. It seems that the databinding system requires adapter sources in order to actually generate the necessary databinding metadata for downstream dependencies to generate necessary adapter bridge code. This still shouldn't result in the increment build inconsistencies mentioned in edit (4), but it provides some explanation as to the underlying problem. Since android_library doesn't support Kotlin sources, I don't see this ever working for Kotlin adapters without a more invasive version of kt_android_library that depends on android_library internals to ensure the sources, rather than binaries, are processed by the databinding compiler. Much of this is just my supposition since I haven't dug too deeply into android_library's implementation to understand what files are actually passed to the databinding compiler, but it does seem that binary classes aren't considered.

BenHenning commented 1 year ago

Okay, following up with an actual comment rather than just an edit.

Per the findings above, I finally found a viable workaround. It seems that the individual databinding library units only seem to work well with Kotlin if there's still Java code present in the compilation step of the android_library that requires databinding. Perhaps with a databinding provider it might be possible to make this work a bit better with rules_kotlin and kt_android_library.

That all being said, my workaround involves a custom rule and utility that generates Java bridge code into a srcjar that's passed to the android_library that contains the reference bindings. This will result in the actual adapters that databinding depends on being duplicated, but the bridges are simple call-throughs to the actual adapters written in Kotlin.