bazelbuild / rules_android

Android rules for Bazel
Apache License 2.0
174 stars 37 forks source link

The Starlark implementaiton of process_deploy_jar fails to execute java.create_deploy_jar #212

Open Bencodes opened 6 months ago

Bencodes commented 6 months ago

The recently enabled implementation of desugaring fails to compile with types missing from the classpath.

I was able to repro this in the basic sample app https://github.com/bazelbuild/rules_android/compare/main...Bencodes:rules_android:single-jar-failure-repro?expand=1

Example building basic app:

➜ bazel build //... --enable_bzlmod=false
INFO: Analyzed 3 targets (0 packages loaded, 0 targets configured).
ERROR: /Users/blee/Projects/rules_android/examples/basicapp/java/com/basicapp/BUILD:3:15: Building deploy jar java/com/basicapp/basic_app_RESOURCES_DO_NOT_USE_migrated_deploy.jar failed: (Exit 2): singlejar_local failed: error executing JavaDeployJar command (from target //java/com/basicapp:basic_app_RESOURCES_DO_NOT_USE) external/remote_java_tools_darwin_arm64/java_tools/src/tools/singlejar/singlejar_local --output bazel-out/darwin_arm64-fastbuild/bin/java/com/basicapp/basic_app_RESOURCES_DO_NOT_USE_migrated_deploy.jar ... (remaining 54 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
singlejar_local: androidx/lifecycle/ViewModelProvider$Factory needed on the classpath for desugaring leakcanary/internal/ViewModelClearedWatcher$Companion$install$provider$1.  Please add the missing dependency to the target containing the latter.
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.214s, Critical Path: 0.06s
INFO: 3 processes: 3 internal.
ERROR: Build did NOT complete successfully

I confirmed that the view models are in fact on the classpath for the singlejar action looking at the subcommands:

bazel-out/darwin_arm64-fastbuild-android-ST-5b74a929aefd/bin/external/example_deps/_dx_migrated/androidx_lifecycle_lifecycle_viewmodel/classes_and_libs_merged.jar_desugared.jar

And I have confirmed that the jar itself contains the requested classes:

❯ zipinfo bazel-out/darwin_arm64-fastbuild-android-ST-5b74a929aefd/bin/external/example_deps/_dx_migrated/androidx_lifecycle_lifecycle_viewmodel/classes_and_libs_merged.jar_desugared.jar | grep "ViewModelProvider\$Factory.class"
-rw----     1.0 fat     1568 bx stor 69-Dec-31 16:00 androidx/lifecycle/ViewModelProvider$Factory.class
ahumesky commented 6 months ago

Thanks I'm able to reproduce the problem.

By the way the error here https://github.com/Bencodes/rules_android/blob/01a45fc62858722f7587de2e59c741380d16009d/examples/basicapp/.bazelrc#L10 (java.lang.NullPointerException: attempted to use --release, but JAVA_HOME is not set) is from requiring a newer rules_java with newer bazel

ahumesky commented 6 months ago

I sent a change out to update rules_java to 7.4.0

ahumesky commented 6 months ago

I think I see what the problem is. There're a few things going on.

1) It looks like some deps for aars are missing.

In the repro the error is: singlejar_local: androidx/lifecycle/ViewModelProvider$Factory needed on the classpath for desugaring leakcanary/internal/ViewModelClearedWatcher$Companion$install$provider$1. Please add the missing dependency to the target containing the latter.

leakcanary/internal/ViewModelClearedWatcher$Companion$install$provider$1 comes from this generated aar_import (bazel-basicapp/external/example_deps/BUILD):

aar_import(
    name = "com_squareup_leakcanary_leakcanary_object_watcher_android_androidx",
    aar = "v1/https/repo1.maven.org/maven2/com/squareup/leakcanary/leakcanary-object-watcher-android-androidx/2.12/leakcanary-object-watcher-android-androidx-2.12.aar",
    deps = [
        ":com_squareup_leakcanary_leakcanary_object_watcher_android_core",
        ":org_jetbrains_kotlin_kotlin_stdlib",
    ],
    tags = [
        "maven_coordinates=com.squareup.leakcanary:leakcanary-object-watcher-android-androidx:aar:2.12",
        "maven_url=https://maven.google.com/com/squareup/leakcanary/leakcanary-object-watcher-android-androidx/2.12/leakcanary-object-watcher-android-androidx-2.12.aar",
    ],
    visibility = ["//visibility:public"],
)

and the deps do not include anything from androidx. Indeed the pom file doesn't list androidx in its deps: https://repo1.maven.org/maven2/com/squareup/leakcanary/leakcanary-object-watcher-android-androidx/2.12/leakcanary-object-watcher-android-androidx-2.12.pom

Maybe that's not necessary for the android gradle plugin.

2) The jar that contains this class is in the list of jars for singlejar to merge, but this is kind of a red herring. The check for missing classes actually happens during desugaring of the aar_import, and then some metadata gets added to the jar in META-INF/desugar_deps (as a binary proto). For the jar in question desugar_deps this is

{
  missing_interface: {
    origin: {
      binary_name: "leakcanary/internal/AndroidXFragmentDestroyWatcher$fragmentLifecycleCallbacks$1"
    }
    target: {
      binary_name: "androidx/fragment/app/FragmentManager$FragmentLifecycleCallbacks"
    }
  }
  missing_interface: {
    origin: {
      binary_name: "leakcanary/internal/ViewModelClearedWatcher"
    }
    target: {
      binary_name: "androidx/lifecycle/ViewModel"
    }
  }
  missing_interface: {
    origin: {
      binary_name: "leakcanary/internal/ViewModelClearedWatcher$Companion$install$provider$1"
    }
    target: {
      binary_name: "androidx/lifecycle/ViewModelProvider$Factory"
    }
  }
}

The error is not reported by desugar when it desugars the aar_import (or an android_library or java_library for that matter I think) though. Singlejar reads all the metadata and reports the errors there at the android_binary level.

3) I'm not sure why this wasn't happening prior to https://github.com/bazelbuild/rules_android/commit/51465c2d64a22fe46e633676c71d62573b542178 because dexing and desugaring should have been done by native android_binary, and desugar deps checking is on by default there as well: https://github.com/bazelbuild/bazel/blob/2e6a8f0dd0108953960665ca5244b7239c8c28e6/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java#L377-L387

4) One might wonder what this check is for anyway -- there's an explanation here: https://github.com/bazelbuild/bazel/blob/2e6a8f0dd0108953960665ca5244b7239c8c28e6/src/tools/singlejar/desugar_checking.h#L28-L38

So it seems the fix is to add androidx to those maven deps.

A workaround would be to turn off desugar deps checking (though run the risk of having problems from neverlinked libraries), but unlinke the native rules which have --experimental_check_desugar_deps, at the moment the starlark rules are hardcoded to check the deps: https://github.com/bazelbuild/rules_android/blob/25e599ad18af589d55984211b7ea4fcdd4bfed6e/rules/java.bzl#L506

ahumesky commented 6 months ago

One more data point for item 3 above: switching the repro back to the native rules produces the same error, which makes sense (but doesn't answer why this error wasn't happening prior to https://github.com/bazelbuild/rules_android/commit/51465c2d64a22fe46e633676c71d62573b542178)

ahumesky commented 6 months ago

I have a change in review that will wire --experimental_check_desugar_deps to the starlark code for singlejar so that this check can at least be disabled as a workaround