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

Implement resource merging for flavored source sets - Part 2 #165

Closed

Conversation

arunkumar9t2
Copy link
Contributor

@arunkumar9t2 arunkumar9t2 commented Apr 9, 2024

#89 introduced ability to merge multiple flavored source sets similar to what is done on Gradle. However it only supported resources (blocks few variants internally), this PR however supports assets as well as manifests. This PR also deprecates the resource_files attribute for unified macro which can be noisy especially when the resources layout don't match it's layout.

This PR updates the API to be more descriptive as updated in the tests. This brings feature parity to AGP variants.

android_binary(
    name = "android_binary_sample",
    srcs = glob([
        "src/main/java/**/*.kt",
    ]),
    custom_package = "com.grab.test",
    manifest = "src/main/AndroidManifest.xml",
    resource_sets = {
        "flavor": {
            "res": "src/flavor/res",
            "manifest": "src/flavor/AndroidManifest.xml",
            "assets": "src/flavor/assets",
        },
        "main": {
            "res": "src/main/res",
            "manifest": "src/main/AndroidManifest.xml",
            "assets": "src/main/assets",
        },
    },
)

The new resource_sets is the source of truth of all resources and follows Gradle semantics. The entries are sorted based on priority with the top one being higher priority.

Implementation notes:

  • New manifest merger is implemented in resource merger which allows manifest merging while retaining placeholders. This is the same impl AGP uses to merge manifests
  • Updated resources.bzl to consider assets and manifest

Alternative approaches

  • Instead of preprocessing, we could create multiple android_library targets per folder and have the default rules handle the merging however this is inefficient as well as does not handle manifests properly. The resource merger used here is multiplex compatible and is much more efficient than creating multiple internal targets.

Grazel changes to generate this new format will follow.

@arunkumar9t2 arunkumar9t2 changed the title WIP: Resource merging Implement resource merging for flavored source sets - Part 2 Apr 9, 2024
@@ -19,6 +19,7 @@ android_binary(
"minSdkVersion": "21",
"targetSdkVersion": "31",
"applicationId": "com.grab.test",
"orientation": "portrait",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sample for demonstrating manifest merge while retaining placeholder

@arunkumar9t2 arunkumar9t2 force-pushed the arun/fix-resource-merging branch from 77f9249 to 3f0ad81 Compare November 6, 2024 03:21
@arunkumar9t2 arunkumar9t2 force-pushed the arun/fix-resource-merging branch from 3f0ad81 to d5b11a8 Compare November 6, 2024 03:52
@arunkumar9t2 arunkumar9t2 force-pushed the arun/fix-resource-merging branch from d5b11a8 to 10ecaed Compare November 11, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants