rules_android
rules_android copied to clipboard
Changes to resource require all upstream targets to rebuild
Given:-
android_binary("app") -> android_library("libA") -> android_library("libB) -> android_library("libc") -> "res/values/strings.xml"
bazel build //:app
Adding a new string resource in "libc" causes a rebuild of all upstream targets ("app", "libA', "libB").
We're using android_strict_resource_deps as well as the namespaced_r_classes https://github.com/bazelbuild/bazel/pull/11385 I have open.
As I expect there is no resource merging or linking for "libA' + "libB", just a single AndroidResourceCompiler
for "libC" and a single AndroidAapt2
action to link the changed resource into the android_binary.
What I am surprised about is that all upstream targets have a KotlinCompile
action associated with them. This is because the new resource has changed the R.class and libC_resource.jar which is an input to all upstream targets.
This is where my knowledge of Bazel is a little hazy, but is it expected that the libC_resource.jar output is given as an input to ALL upstream targets (i.e: "app", "libA", "libB")?
I think, what we want, since we have strict_java_deps is that the change to the resource would only require recompiling the Java/Kotlin for libC + libB (since B is permitted to depend on libC.R.class as its a direct dependency)
I noticed that all transitive resource jars are collected and added to AndroidLibraryResourceClassJarProvider
here:-
https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java#L233
Commenting out the collection of transitive resource jars and only including the locally produced resource jar has no affect though.
Of is this is correct, do we need to put some kind of ijar in place for upsteam targets that are not direct parents?
@donaldchai - would appreciate your thoughts here. I'm happy submit a PR if you think this is valid change and can give a few pointers.
Thanks, Jonathan
is it expected that the libC_resource.jar output is given as an input to ALL upstream targets?
Unfortunately, yes. With regular Java code, I expect "interface JARs" (ijars/hjars) containing declarations of non-private members to be propagated to all upstream targets. The resource JARs containing classes produced by https://github.com/bazelbuild/bazel/blob/master/src/tools/android/java/com/google/devtools/build/android/PlaceholderIdFieldInitializerBuilder.java are effectively already that, except they contain nonzero field initializers (I wish they were always 0, but someone's tests may rely on that misfeature).
So improving incremental builds will require a different ijar model for R classes. Here's what we can do:
-
Create ijars which omit package-private entities, to be used by targets in other packages. With private resources and https://github.com/bazelbuild/bazel/commit/2e817e7cd1a549623e73ffaca90d337b5cbab79a this means that adding a private resource will not trigger a full rebuild. In builds which use merging for R classes (i.e. not bazelbuild/bazel#11385) R classes should also be package-private.
-
Omit resource JARs/ijars entirely for indirect parents, which is basically what you're suggesting. This is a mild inconvenience w.r.t. any possible warnings/errors regarding violating java_strict_deps, but worth it if you rely on R classes from other packages.
I suspect that the line you point to is unrelated to Java compilation (see https://github.com/bazelbuild/bazel/commit/bfb3de3809a550da08c39f95d64d2a6f707f8add where it was added).
I can offer thoughts on overall direction, but since I'm also an external contributor I can't help with pull requests.
Thanks for the explanation @donaldchai
I'm looking into the second option because we currently build with Buck that does not support merging R files and as such we have many targets that reference the R class from their direct dependencies and so need to maintain compatibility until we're able to fully migrate to Bazel.
It looks like this is the point where the resource jar is provided to consuming rules:-
https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java#L434
I don't see a way to only make it available for direct parents however, any ideas?
I think this is also the case because the Kotlin rules don't currently have access to the Android Skylark APIs (although I've been playing around with exposing the ones we need for resource processing) but right now they create a sandwich jar that wraps a kt_jvm_library and a source-less android_library for resource processing so the resources do still need to be provided to the direct parent at least.
The second option seems like the way to implement public/private resource support more fully in Bazel and once we're off Buck, or could make Buck use merged R classes I could implement this feature too, I guess here we would only provide the ijar for the R class containing public resources only.