Bazel AndroidManifest merging doesn't preserve the correct order

[nkoroste]

The main difference between Gradle and Bazel manifest merging order is "If you have multiple libraries, their manifest priorities match the order they appear in your Gradle dependencies block." (doc) That means the manifest entries in library modules have higher priority than its dependency, which is not the case in bazel.

[nkoroste]

Related: #190

[nkoroste]

cc @ahumesky @ted-xie

[ahumesky]

To make sure we're talking about the same things, I adapted the basicapp in examples:

BUILD:

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

android_binary(
    name = "basic_app",
    manifest = "AndroidManifest.xml",
    deps = [":basic_lib"],
)

android_library(
    name = "basic_lib",
    srcs = ["BasicActivity.java"],
    manifest = "AndroidManifest.xml",
    resource_files = glob(["res/**"]),
    deps = [":manifest_AAA"],
)

android_library(
    name = "manifest_AAA",
    manifest = "AndroidManifestBar.xml",
    deps = [":manifest_ZZZ"],
    exports_manifest = True,
)

android_library(
    name = "manifest_ZZZ",
    manifest = "AndroidManifestBaz.xml",
    exports_manifest = True,
)

AndroidManifest.xml:

<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    package="com.basicapp" >

    <uses-sdk
        android:minSdkVersion="21"
        android:targetSdkVersion="30" />

</manifest>

AndroidManifestBar.xml:

<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android">

    <meta-data android:name = "data" android:value = "bar"/>

</manifest>

AndroidManifestBaz.xml:

<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android">

    <meta-data android:name = "data" android:value = "baz"/>

</manifest>

And then the merged manifest is:

<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
    package="com.basicapp" >

    <uses-sdk
        android:minSdkVersion="21"
        android:targetSdkVersion="30" />

    <meta-data
        android:name="data"
        android:value="bar" />

    <application />

</manifest>

With "bar" from AndriodManifestBar.xml as expected.

If the the names of the libraries are swapped (but not their dependency order, i.e. it's still AndroidManifest.xml -> AndroidManifestBar.xml -> AndroidManifestBaz.xml):

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

android_binary(
    name = "basic_app",
    manifest = "AndroidManifest.xml",
    deps = [":basic_lib"],
)

android_library(
    name = "basic_lib",
    srcs = ["BasicActivity.java"],
    manifest = "AndroidManifest.xml",
    resource_files = glob(["res/**"]),
    deps = [":manifest_ZZZ"],
)

android_library(
    name = "manifest_ZZZ",
    manifest = "AndroidManifestBar.xml",
    deps = [":manifest_AAA"],
    exports_manifest = True,
)

android_library(
    name = "manifest_AAA",
    manifest = "AndroidManifestBaz.xml",
    exports_manifest = True,
)

then the merged manifest is:

<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
    package="com.basicapp" >

    <uses-sdk
        android:minSdkVersion="21"
        android:targetSdkVersion="30" />

    <meta-data
        android:name="data"
        android:value="baz" />

    <application />

</manifest>

It contains "baz" from the lower-level android_library.

As changed in #190 and #193, this results from the sorting here:

rules_android/rules/busybox.bzl

Line 787 in 3414610

ordered_manifests = sorted(manifests.to_list(), key = _manifest_short_path)

Are there other cases (besides the naming of the targets) that results in unexpected merging priorities?

Interstingly, disabling the sorting and using "dependency order" has actually been available in the native rules since early 2019:
bazelbuild/bazel@1fcf614

but that option was added mostly incidentally in the course of adjusting things for changes related to configuration paths and didn't make it in the Starlark migration of the relevant code. I traced the origin of sorting the manifests to see why we do this -- I found the change in late 2014 (prior to open sourcing bazel) that doesn't give much explanation (I suspect it might have just been overly enthusiastic application of "sorting is deterministic")

[ahumesky]

I had considered just making "dependency order" the default for the bazel version of the rules (as #190 does, but adjusting so that internally we keep the sorting behavior) but it occurs to me that this would create a potential incompatibility between the native rules and the starlark rules, which could make migrating from native to starlark more complicated -- so we might need a switch in bazel anyway (as #193 does)

[ahumesky]

This should be addressed with f13f245

Note that the default is now to merge manifests in "dependency" order instead of what we're now calling "legacy" order (roughly alphabetical). The previous behavior can be enabled with --@rules_android//rules/flags:manifest_merge_order=legacy