The main difference between Gradle and Bazel manifest merging order is "If you have mu

Related: <a class="issue-link js-issue-link" data-error-text="Failed to load title" da

cc <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="

This should be addressed with <a class="commit-link" data-hovercard-type="commit" data

Bazel AndroidManifest merging doesn't preserve the correct order about rules_android HOT 5 CLOSED

nkoroste commented on September 14, 2024

Bazel AndroidManifest merging doesn't preserve the correct order

from rules_android.

Comments (5)

nkoroste commented on September 14, 2024

Related: #190

from rules_android.

nkoroste commented on September 14, 2024

cc @ahumesky @ted-xie

from rules_android.

ahumesky commented on September 14, 2024

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")

from rules_android.

ahumesky commented on September 14, 2024

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)

from rules_android.

ahumesky commented on September 14, 2024

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

from rules_android.

Bazel AndroidManifest merging doesn't preserve the correct order about rules_android HOT 5 CLOSED

Comments (5)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent