Permit disabling resource merging

[cgruber]

Description of the problem / feature request:

Now that name-spaced resources are supported, teams that exclusively use this feature have no need for resource merging work, which ends up being a substantial fraction of a clean (or low incrementality) build.

Feature requests: what underlying problem are you trying to solve with this feature?

Reduce unnecessary work during an android build.

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

Write any trivial app which consumes .aars, and depend on a namespaced resource directly (rather than in the local R.class). Then build. All the resources are merged into the current library, even though the current library doesn't require local references to these.

What operating system are you running Bazel on?

Mac OS X 10.14.5

What's the output of bazel info release?

release 0.28.1

Have you found anything relevant by searching the web?

Discussion with @jin has confirmed that the resource merging is (presently) mandatory.

For context, we turned this off in gradle and achieved about a 30% reduction in build times. Bazel's sensitivity to caching/incrementality will reduce this gain in high-incrementality situations, but might increase it in low-incrementality scenarios. Our observations from profiles are that we spend about a substantial fraction of our build, front-loaded, pegging our workers with this work.

Note: Copied from bazelbuild/bazel#8577 which I'm closing.

[jongerrish]

How can I build a development version of Bazel that includes my modifications to ResourceProcessorBusyBox?

I've added a new Action that invokes a new tool in the busybox that I want to test that basically -

1. Run aapt2 compile - outputs = intermediate resource .flat files
2. Generate R.java, R.class, R.txt directly from those .flat files (R files only include resources local to that library, i.e: no direct or transitive dependencies)

However, the local version of bazel I build with bazelisk build //src:bazel keeps replacing bazel-bin/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox with the original version that doesn't contain my changes.

Any pointers on how to test this? @timpeut @jin @djwhang ?

[jin]

@jongerrish ResourceProcessorBusyBox and a few other Android tooling are decoupled from the Bazel binary, in the @android_tools repo.

To run with a local copy of the @android_tools repo, run:

$ bazel build //tools/android/runtime_deps:android_tools

which gives you bazel-bin/tools/android/runtime_deps/android_tools.tar.gz.

Then you can un-tar this into a directory and use --override_repository=android_tools=/path/to/android_tools to use it. You can also use local_repository, but I find --override_repository to be more useful for development.

[jongerrish]

Thanks @jin - that worked, you're awesome! :-D

[jongerrish]

@cgruber - What do you think the expected behavior for contents of generated R files is with namespaced resources?

If I have a library foo that depends on bar, and bar has a public resource baz, should I be able to refer to it via a.R.baz or must it be b.R.baz?

We're not using namespaced resources (yet) so I'm wondering if I'd need to do a migration of all references to resources in other modules ie.: a.R.baz -> b.R.baz or if the right thing to do is merge public resources up into R files of depending libraries.

Thoughts?

[jongerrish]

@jin @djwhang @timpeut - removing library.ap_ from the outputs of AndroidResourceLink actions saves us ~1.7GB of outputs. As a first step would you accept a PR to remove libraries.ap_ from the outputs? I don't see a reason in exporting these outputs, there are no consumers of these intermediate artifacts. If so, would you want it behind a flag, e.g: [no]output_library_linked_resources?

I'm also skeptical of the need to export R.java source jars for android_library targets since Android Studio links R code references to the XML resources and their size for our builds is another ~600MB but I'd be interested to hear your thoughts on that.

Ultimately I think we can add the original feature @cgruber requested, I have a proof of concept for this too.

[jongerrish]

bazelbuild/bazel#11253 can mitigate the problem by removing the library.ap_ files from the Action outputs.

[jongerrish]

@jin I think bazelbuild/bazel#11294 is probably a better approach than bazelbuild/bazel#11253. For us this saves around ~900 actions and about 2.5GB on intermediate outputs.

[jin]

@jongerrish That's a great result! I'll let @djwhang @timpeut and @donaldchai decide what's the best step to take here (I don't work on the Android rules anymore)

[jongerrish]

@djwhang @timpeut @donaldchai - thoughts?

I also have another change in our fork that implements rudimentary namespace support for R classes as per the Gradle feature android.namespacedRClass=true - this allows us to disable resource merging all together for much smaller R files.

[donaldchai]

I'm a bit wary of referencing resources with fully-qualified names of R classes, since that's not done in XML. (And switching to that model for XML will cause problems.)

FYI, Bazel has (undocumented) support to create R classes with only the direct dependencies of a rule, rather than the transitive closure. You can see that used in this test: https://github.com/bazelbuild/bazel/blob/0de69d0d6d6479a2d5dfc666d43c8e1391875efd/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java#L1771
(Unfortunately, I chose a poor name for the flag since it only affects R class generation.).

We've found that this (in conjunction with --experimental_use_rtxt_from_merged_resources) significantly reduces R class size.

A caveat here on the implementation is that there's a a misfeature in
https://github.com/bazelbuild/bazel/blob/f7d726f88f20c871d274b75a01f6a64d186403dc/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java#L181 which causes providers to be exported (i.e. transitive deps become direct deps). We've worked around that in Starlark, and haven't changed the behavior in Bazel---this is a bit of a breaking change since it exposes resource conflicts which weren't previously reported.

[jongerrish]

@donaldchai Yeah, we've enabled android_resources_strict_deps and that definitely yielded improvements. I know it's switched on by default in G3 for a while.

I agree, re: fully-qualified R classes, it did result in wins for us, but I understand the option in Gradle is experimental (its probably the precursor to proper support for public vs private R symbols) so that's why I put it behind a flag and if you're supportive we could mark that as experimental also.

The biggest issue for us is that the Kotlin rules Android support is severely hamstringed at the moment, because it has to wrap Kotlin compilation and Android resource processing into separate synthetic rules and forces exporting the results (breaking strict deps).

https://github.com/bazelbuild/rules_kotlin/blob/master/kotlin/internal/jvm/android.bzl#L53

So what is the situation regarding open sourcing the internal Android Skylark rules? Even if it's an alpha or some kind of early access would be a game changer for us. What we really need here is access to the underlying actions + providers for things like resource processing and the android.jar to make the Kotlin rules performant for Android.

Migrating to Bazel is a priority for us, but it's a hard sell to our developers at the moment because performance for Android really lags behind Buck. If we're able to get some semi-well supported APIs for the resource processing I'm more than happy and able to dedicate some signifiant time to working on both Kotlin + Android Skylark rules.

[donaldchai]

I think it's reasonable to allow disabling resource merging with an experimental option. It's worth being explicit that this (as well as android_resources_strict_deps) can surface ODR violations on R classes and thus will be experimental for some time. }

breaking strict deps
https://github.com/bazelbuild/rules_kotlin/blob/master/kotlin/internal/jvm/android.bzl#L53

The line you point to looks fine to me. I filed bazelbuild/rules_kotlin#331

the situation regarding open sourcing the internal Android Skylark rules

@djwhang @timpeut I think the timeline on the doc attached to bazelbuild/bazel#8391 needs to be updated.