mobile-install fails with dependencies from rules_jvm_external #232

fhilgers · 2024-05-18T17:02:52Z

When trying to use mobile-install on the example/basicapp I am getting the following stacktrace after launching the application: stacktrace.txt

I am using rules_jvm_external with an overridden rules_android to the version from rules_android git repository.

This is a patch to apply to the example:

diff --git a/examples/basicapp/MODULE.bazel b/examples/basicapp/MODULE.bazel
index aac81cc..37a891e 100644
--- a/examples/basicapp/MODULE.bazel
+++ b/examples/basicapp/MODULE.bazel
@@ -4,6 +4,14 @@ module(
 
 bazel_dep(name = "rules_java", version = "7.4.0")
 bazel_dep(name = "bazel_skylib", version = "1.3.0")
+bazel_dep(name = "rules_jvm_external", version = "6.1")
+local_path_override(
+    module_name = "rules_jvm_external",
+    path = "../rules_jvm_external/"
+)
+
+remote_android_extensions = use_extension("@rules_android//bzlmod_extensions:android_extensions.bzl", "remote_android_tools_extensions")
+use_repo(remote_android_extensions, "android_gmaven_r8", "android_tools")
 
 bazel_dep(
     name = "rules_android",
@@ -15,7 +23,21 @@ local_path_override(
     path = "../../",
 )
 
-register_toolchains(
-    "@rules_android//toolchains/android:android_default_toolchain",
-    "@rules_android//toolchains/android_sdk:android_sdk_tools",
+maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven")
+
+maven.install(
+    aar_import_bzl_label = "@rules_android//android:rules.bzl",
+    artifacts = [
+       "androidx.appcompat:appcompat:1.6.1",
+       "androidx.appcompat:appcompat-resources:1.6.1",
+    ],
+    fail_if_repin_required = True,
+    lock_file = "//:manifest_install.json",
+    repositories = [
+        "https://maven.google.com",
+        "https://repo1.maven.org/maven2",
+    ],
+    resolver = "maven",
+    use_starlark_android_rules = True,
 )
+use_repo(maven, "maven")
diff --git a/examples/basicapp/WORKSPACE.bzlmod b/examples/basicapp/WORKSPACE.bzlmod
index a69bd79..589a040 100644
--- a/examples/basicapp/WORKSPACE.bzlmod
+++ b/examples/basicapp/WORKSPACE.bzlmod
@@ -1,4 +1,10 @@
 load("@rules_android//rules:rules.bzl", "android_sdk_repository")
 android_sdk_repository(
     name = "androidsdk",
-)
\ No newline at end of file
+)
+
+new_local_repository(
+    name = "androidstudio",
+    path = "android-studio",
+    build_file = "//:androidstudio.BUILD",
+)
diff --git a/examples/basicapp/java/com/basicapp/BUILD b/examples/basicapp/java/com/basicapp/BUILD
index a8fb99d..80b401c 100644
--- a/examples/basicapp/java/com/basicapp/BUILD
+++ b/examples/basicapp/java/com/basicapp/BUILD
@@ -11,4 +11,7 @@ android_library(
     srcs = ["BasicActivity.java"],
     manifest = "AndroidManifest.xml",
     resource_files = glob(["res/**"]),
+    deps = [
+      "@maven//:androidx_appcompat_appcompat"
+    ]
 )
diff --git a/examples/basicapp/java/com/basicapp/BasicActivity.java b/examples/basicapp/java/com/basicapp/BasicActivity.java
index 03c9aef..98f4824 100644
--- a/examples/basicapp/java/com/basicapp/BasicActivity.java
+++ b/examples/basicapp/java/com/basicapp/BasicActivity.java
@@ -21,10 +21,12 @@ import android.view.View;
 import android.widget.Button;
 import android.widget.TextView;
 
+import androidx.appcompat.app.AppCompatActivity;
+
 /**
  * The main activity of the Basic Sample App.
  */
-public class BasicActivity extends Activity {
+public class BasicActivity extends AppCompatActivity {
 
   @Override
   protected void onCreate(Bundle savedInstanceState) {
diff --git a/mobile_install/tools.bzl b/mobile_install/tools.bzl
index 72dd5b6..c71fd58 100644
--- a/mobile_install/tools.bzl
+++ b/mobile_install/tools.bzl
@@ -37,7 +37,7 @@ TOOL_ATTRS = dict(
         ),
     ),
     _studio_deployer = attr.label(
-        default = "//tools/android:gen_fail", # TODO(#119): Studio deployer jar to be released
+        default = "@@androidstudio//:deployer_deploy.jar", # TODO(#119): Studio deployer jar to be released
         allow_single_file = True,
         cfg = "exec",
         executable = True,

Edit: I forgot to mention that building and manually installing the apk works fine.

The text was updated successfully, but these errors were encountered:

fhilgers · 2024-05-18T17:12:07Z

As far as I understood by looking over the implementation, mobile-install mainly uses the AndroidIdeInfo provider which has a merged R.java:

In the extract function from mobile-install/adapters/android_binary:

resource_src_jar = target[AndroidIdeInfo].resource_jar.source_jar,  # This is the R with real ids.

I am not really familiar with android but maybe this is helpful, as in the stacktrace the error is that the specific androidx/startup/R$string could not be found.

When looking at all the split apks that mobile install produces and extracting the dex files there is also only com/example/basicapp/R.class with all the merged resources.

fhilgers · 2024-05-19T01:45:57Z

Okay there are actually two reasons why it fails:

In mobile_install/adapters/android_binary.bzl all the resources files are filtered:

    return dict(
        debug_key = utils.only(ctx.rule.files.debug_key, allow_empty = True),
        debug_signing_keys = ctx.rule.files.debug_signing_keys,
        debug_signing_lineage_file = utils.only(ctx.rule.files.debug_signing_lineage_file, allow_empty = True),
        key_rotation_min_sdk = ctx.rule.attr.key_rotation_min_sdk,
        merged_manifest = target[AndroidIdeInfo].generated_manifest,
        native_libs = target[AndroidIdeInfo].native_libs,
        package = target[AndroidIdeInfo].java_package,
        resource_apk = target[AndroidIdeInfo].resource_apk,
        resource_src_jar = target[AndroidIdeInfo].resource_jar.source_jar,  # This is the R with real ids.
        aar_native_libs_info = MIAndroidAarNativeLibsInfo(
            transitive_native_libs = transitive_native_libs,
        ),
        android_dex_info = providers.make_mi_android_dex_info(
            dex_shards = dex(
                ctx,
                target[JavaInfo].runtime_output_jars +
                #filter_jars(
                #    ctx.label.name + "_resources.jar",
                #    target[JavaInfo].runtime_output_jars,
                #) +
                (
                    [
                        extension_registry_class_jar,
                    ] if extension_registry_class_jar else []
                ),
                target[JavaInfo].transitive_compile_time_jars,
            ),
            deps = providers.collect(MIAndroidDexInfo, ctx.rule.attr.deps),
        ),
        # TODO(djwhang): It wasteful to collect packages in
        # android_resources_info, rather we should be looking to pull them
        # from the resources_v3_info.
        android_resources_info = providers.make_mi_android_resources_info(
            package = target[AndroidIdeInfo].java_package,
            deps = providers.collect(
                MIAndroidResourcesInfo,
                ctx.rule.attr.deps,
            ),
        ),
        java_resources_info = providers.make_mi_java_resources_info(
            deps = providers.collect(
                MIJavaResourcesInfo,
                ctx.rule.attr.deps,
            ),
        ),
        apk = target[AndroidIdeInfo].signed_apk,
    )

Commenting that filtering out makes another problem apparent:

The rules_jvm_external ruleset uses jvm_import instead of java_import for importing jar files. I have not yet looked into the difference but changing that to java_import leads to everything working with mobile-install.

What is the reason all resources files are filtered out? Is it important or was it just overlooked that it filters out external resources. Should they be added somewhere else?

fhilgers · 2024-05-19T03:03:37Z

The reason why jvm_import does not work is that it has to be added to the adapters in the mobile-install aspects.

fhilgers · 2024-05-19T15:44:16Z

So the problem was threefold:

No aspect for jvm_import which is used by rules_jvm_external instead of java import -> The aspect for java_import can be reused for that
Package name is inferred by directory structure or the explicit attribute on aar_import. This is not set by rules_jvm_external. There is alread a Todo label to instead retrieve the package name from AndroidManifest.xml -> I have implemented the package name retrieval from manifest and that works fine.
The aar_import aspect ignores StarlarkAndroidResourcesInfo -> Try to use that if available

Let me know if this makes sense and whether these proposed fixes are appropriate, so that I can open a pr.

ted-xie · 2024-05-23T13:40:03Z

Hi, thanks for filing this issue!

I think jvm_import only exists in rules_jvm_external. I believe they had their own reasons to write their own rule for dealing with imported jars rather than re-using java_import.
Usually package name inferred by directory structure works well enough. Can you elaborate why this doesn't work for your case? I also don't see where you've implemented the package name retrieval in the patch you shared.
I think we will end up implementing this in the near future, as we finish up the Starlark migration effort for rules_android and fully delete both native Android rules and native Android providers from Bazel.

fhilgers · 2024-05-23T14:22:38Z

rules_jvm_external is not setting the package field in aar_import and as the BUILD file with all imports is in the root of the generated repository the package name is empty. My usecase was just importing any library from maven but because the package name is unset the resources are missing or in the wrong location for them to be found. The patch I shared was before I found out the problems, it was just importing packages from maven via rules_jvm_external where the problem happened. I can clean up a bit and make a pr so it can be looked at.

ajsinclair · 2024-08-18T01:15:09Z

The reason why jvm_import does not work is that it has to be added to the adapters in the mobile-install aspects.

This commit should resolve that issue.

fhilgers linked a pull request May 23, 2024 that will close this issue

Make mobile-install work with rules_jvm_external #233

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mobile-install fails with dependencies from rules_jvm_external #232

mobile-install fails with dependencies from rules_jvm_external #232

fhilgers commented May 18, 2024 •

edited

Loading

fhilgers commented May 18, 2024 •

edited

Loading

fhilgers commented May 19, 2024

fhilgers commented May 19, 2024

fhilgers commented May 19, 2024

ted-xie commented May 23, 2024

fhilgers commented May 23, 2024

ajsinclair commented Aug 18, 2024

mobile-install fails with dependencies from rules_jvm_external #232

mobile-install fails with dependencies from rules_jvm_external #232

Comments

fhilgers commented May 18, 2024 • edited Loading

fhilgers commented May 18, 2024 • edited Loading

fhilgers commented May 19, 2024

fhilgers commented May 19, 2024

fhilgers commented May 19, 2024

ted-xie commented May 23, 2024

fhilgers commented May 23, 2024

ajsinclair commented Aug 18, 2024

fhilgers commented May 18, 2024 •

edited

Loading

fhilgers commented May 18, 2024 •

edited

Loading