New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 834979 link

Starred by 1 user
Status: Fixed
Owner:
agrieve@chromium.org
Closed: Apr 2018
Cc:
█ doughorn@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

dist_jar / dist_aar with direct_deps_only should not contain R.class files of transitive deps

Project Member Reported by agrieve@chromium.org, Apr 19 2018 
E.g.:

dist_jar("test_dist_jar") {
    output = "${root_out_dir}/lib.java/test.jar"
    direct_deps_only = true
    use_unprocessed_jars = true
    requires_android = true
    deps = [
      "//ui/android:ui_utils_java",
    ]
}

The resulting test.jar will have R.class files from support library.

Comment 1 by agrieve@chromium.org, Apr 19 2018 

The cause here, is that for android resources, we currently compile all transitive R.java files into each target.

At some point in time, we would filter these out via jar_excluded_patterns, but this works only for use_unprocessed_jars = false.

Furthermore, for dist_aar(), you actually do want to include R.class files for your own libraries, as that makes it easier on IDEs to resolve your R symbols (existing .aar files started to do this, although I haven't checked lately if they continued with it or not).

So, what I think we should do is just not include the transitive R.class files, but rather just the direct ones.
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 23 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48b395f6dd3891b98a309066fe30b1687266df6f

commit 48b395f6dd3891b98a309066fe30b1687266df6f
Author: Andrew Grieve <agrieve@chromium.org>
Date: Mon Apr 23 11:05:01 2018

Android: Compile only "owned" R.java srcjars in java targets

The ownership model here isn't perfect due to prebuilts sometime not
bundling their R.class files. You can also have multiple java_libraries
depend on the same android_resources(), in which case they would both
"own" it. Shared ownership doesn't actually cause any problems though.

This fixes a bug where dist_jar() / dist_aar() with direct_deps_only=true
would have all transitive R.class files packaged in them.

This should also speed builds marginally due to fewer R.java compiles.

TBR=digit  # lgtm'ed

Bug:  834979 
Change-Id: I3e2bde2f7619c51a3aa894d66bfbf443418b9a37
Reviewed-on: https://chromium-review.googlesource.com/1020289
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552668}
[modify] https://crrev.com/48b395f6dd3891b98a309066fe30b1687266df6f/build/android/gradle/generate_gradle.py
[modify] https://crrev.com/48b395f6dd3891b98a309066fe30b1687266df6f/build/android/gyp/write_build_config.py
[modify] https://crrev.com/48b395f6dd3891b98a309066fe30b1687266df6f/build/config/android/internal_rules.gni

Comment 3 by agrieve@chromium.org, Apr 23 2018

Status: Fixed (was: Started)
►

Sign in to add a comment