Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pre-release] Statically linking fails with custom styles that depend on resources in an aar #39

Open
Bencodes opened this issue Aug 3, 2021 · 6 comments · May be fixed by #52
Open
Assignees
Labels
bug Something isn't working

Comments

@Bencodes
Copy link
Contributor

Bencodes commented Aug 3, 2021

We did another round of testing against the data bindings patches with the alpha branch of rules_android. We got further but hit a new failure case where custom styles that references Android resource attributes provided by 3rd party dependencies aren't being found during static linking.

Tested using rules_jvm_external and bazel HEAD.

Original report: bazelbuild/bazel#11497 (comment)

Exception in thread "main" java.lang.RuntimeException: Error during Statically linking com.google.devtools.build.android.aapt2.CompiledResources@35aea049:
Command: bazel-out/host/bin/external/androidsdk/aapt2_binary\
        link\
        --static-lib\
        --manifest\
        /var/folders/ts/25b588x11_n7f1k_2361ybr80000gp/T/android_resources_tmp17478694338343542457/manifest-aapt-dummy/AndroidManifest.xml\
        --no-static-lib-packages\
        --no-version-vectors\
        -R\
        /var/folders/ts/25b588x11_n7f1k_2361ybr80000gp/T/android_resources_tmp17478694338343542457/filtered/bazel-out/darwin-fastbuild/bin/_migrated/lib_symbols/symbols.zip\
        -I\
        external/androidsdk/platforms/android-30/android.jar\
        --auto-add-overlay\
        -o\
        /var/folders/ts/25b588x11_n7f1k_2361ybr80000gp/T/android_resources_tmp17478694338343542457/lib.apk\
        --java\
        /var/folders/ts/25b588x11_n7f1k_2361ybr80000gp/T/android_resources_tmp17478694338343542457/java\
        --output-text-symbols\
        /var/folders/ts/25b588x11_n7f1k_2361ybr80000gp/T/android_resources_tmp17478694338343542457/R.txt
Output:
res/values/styles.xml:5: error: style attribute 'attr/autoSizeMinTextSize (aka com.demo.app:attr/autoSizeMinTextSize)' not found.
error: failed linking references.

        at com.google.devtools.build.android.CommandHelper.execute(CommandHelper.java:42)
        at com.google.devtools.build.android.AaptCommandBuilder.execute(AaptCommandBuilder.java:297)
        at com.google.devtools.build.android.aapt2.ResourceLinker.linkStatically(ResourceLinker.java:272)
        at com.google.devtools.build.android.ValidateAndLinkResourcesAction.main(ValidateAndLinkResourcesAction.java:207)
        at com.google.devtools.build.android.ResourceProcessorBusyBox$Tool$7.call(ResourceProcessorBusyBox.java:105)
        at com.google.devtools.build.android.ResourceProcessorBusyBox.processRequest(ResourceProcessorBusyBox.java:234)
        at com.google.devtools.build.android.ResourceProcessorBusyBox.main(ResourceProcessorBusyBox.java:177)
Target //:lib failed to build

BUILD

load("@rules_android//rules:rules.bzl", "android_library")

android_library(
    name = "lib",
    custom_package = "com.demo.app",
    # ... manifest, visibility, ...
    resource_files = glob(["res/**/*.xml"]),
    deps = [
        "@maven//:androidx_appcompat_appcompat",
        "@maven//:androidx_appcompat_appcompat_resources",
    ],
)

res/values/styles.xml

<?xml version="1.0" encoding="utf-8"?>
<resources>

    <style name="FooTextAppearance">
        <item name="autoSizeMinTextSize">1sp</item>
    </style>

</resources>
@ahumesky
Copy link
Collaborator

ahumesky commented Aug 5, 2021

Thanks for the detailed report.
I was able to reproduce this problem, and I was able to get past it by specifying the android namespace in the item name:

<item name="android:autoSizeMinTextSize">1sp</item>

Can you try this and see if it works?

@alexjlockwood
Copy link

@ahumesky The usage of autoSizeMinTextSize without the android: namespace is intentional (and definitely works with Gradle/Buck). The android.R.attr.autoSizeMinTextSize attribute was added in API 26 as part of an effort to introduce auto-resizing text to the Android native TextView. Since using android:autoSizeMinTextSize doesn't work on API 25 and older (since old platforms won't understand it), AppCompat added the ability In order to backport the ability to older devices by creating its own R.attr.autoSizeMinTextSize attribute. The idea being that developers would prefer using the app-namespaced attributes instead of the android-namespaced attributes so they can make use of the feature safely on old platforms.

You can read more about this specific attribute here. But the idea extends to many other attributes too. The support libraries rely on creating these app-name spaced attributes pretty heavily to backport new platform features, so avoiding android: is definitely intentional and works in other build systems like Buck/Gradle too.

I think the fact that <item name="android:autoSizeMinTextSize">1sp</item> worked but not <item name="autoSizeMinTextSize">1sp</item> is an indication that maybe something weird is going on specifically with app-namespaced resources.

@ahumesky
Copy link
Collaborator

Thanks for the details here.

Using the native android rules (i.e. the built-in / non-starlark rules), an app with name="autoSizeMinTextSize" and with appcompat dependencies builds, so it does look like something is missing in the starlark version of the rules.

One thing to note is that maven_install actually uses the native version of aar_import by default. These attributes enable the starlark version:

maven_install(
  ...
  use_starlark_android_rules = True,
  # the default is "@build_bazel_rules_android//android:rules.bzl"
  aar_import_bzl_label = "@rules_android//rules:rules.bzl",
)

And further, looking at the providers of native aar_import for appcompat:

$ ~/bazel/bazel-bin/src/bazel cquery @maven//:androidx_appcompat_appcompat --experimental_google_legacy_api --output=starlark --starlark:expr="'\n'.join(sorted(providers(target)))"
AndroidAssetsInfo
AndroidManifestInfo
AndroidNativeLibsInfo
AndroidResourcesInfo
DataBindingV2Info
FileProvider
FilesToRunProvider
InstrumentedFilesInfo
JavaInfo
OutputGroupInfo
ProguardSpecProvider

vs starlark aar_import:

<set use_starlark_android_rules = True on maven_install>
$ ~/bazel/bazel-bin/src/bazel cquery @maven//:androidx_appcompat_appcompat --experimental_google_legacy_api --output=starlark --starlark:expr="'\n'.join(sorted(providers(target)))"
AndroidIdeInfo
AndroidLibraryResourceClassJarProvider
AndroidNativeLibsInfo
FileProvider
FilesToRunProvider
JavaInfo
OutputGroupInfo
ProguardSpecProvider

AndroidResourcesInfo and others are missing.

So that means that starlark android_library + native aar_import also doesn't work (while native aar_import works with native android_library), and so perhaps there's something missing in android_library too. (On the other hand, it's not clear that we really need to support this mix of native and starlark rules). We'll go through the code to see what the disconnect is.

@Bencodes
Copy link
Contributor Author

@ahumesky I did some digging yesterday afternoon to try to figure out what's going on here. We found a few things:

  1. Inside rules_jvm_external the _get_aar_import_statement_or_empty_str macro seems to be returning an empty string even when aar_import_bzl_label and use_starlark_android_rules are set. Hard coding the return value to @rules_android//rules:rules.bzl causes the aar_import rule in rules_android alpha to get called.
  2. propagate_resources is False for all aar_import targets which results in the providers that would normally provide symbols.zip not being there (I'm able to confirm this looking at the symbols.zip being passed to aapt2 via -R during static linking).

@ahumesky
Copy link
Collaborator

I think I found another reason this is not working: a chunk of code in aar_import related to resource processing is not being exported to github, because that code relies on other parts of the native android rules in bazel that haven't been open sourced. We may need to refactor this part of resource processing, and that might take a bit of time to work through.

@kkpattern
Copy link

Hi, do we have an approximate expected time for pre-alpha to be officially merged into master?

Currently we encountered some problems when building Android app with bazel. Like bazelbuild/bazel#13295 bazelbuild/bazel#11497

seems the starlark version rule_android is key to solve these problems or at least make it easier.

Is there anything we can do to help speed up the process?

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants