Skip to content

Conversation

@auto-submit
Copy link
Contributor

@auto-submit auto-submit bot commented Dec 8, 2023

Reverts #132985
Initiated by: christopherfujino
This change reverts the following previous change:
Original Description:
Provides support for conditional bundling of assets through the existing --flavor option for flutter build and flutter run. Closes #21682. Resolves #136092

Change

Within the assets section pubspec.yaml, the user can now specify one or more flavors that an asset belongs to. Consider this example:

# pubspec.yaml
flutter:
  assets:
    - assets/normal-asset.png
    - path: assets/vanilla/ice-cream.png
      flavors: 
        - vanilla
    - path: assets/strawberry/ice-cream.png
      flavors:
        - strawberry

With this pubspec,

  • flutter run --flavor vanilla will not include assets/strawberry/ice-cream.png in the build output.
  • flutter run --flavor strawberry will not include assets/vanilla/ice-cream.png.
  • flutter run will only include assets/normal-asset.png.

Open questions

  • Should this be supported for all platforms, or should this change be limited to ones with documented --flavor support (Android, iOS, and (implicitly) MacOS)? This PR currently only enables this feature for officially supported platforms.

Design thoughts, what this PR does not do, etc.

This does not provide an automatic mapping/resolution of asset keys/paths to others based on flavor at runtime.

The implementation in this PR represents a simplest approach. Notably, it does not give Flutter the ability to dynamically choose an asset based on flavor using a single asset key. For example, one can't use Image.asset('config.json') to dynamically choose between different "flavors" of config.json (such as dev-flavor/config.json or prod-flavor/config.json). However, a user could always implement such a mechanism in their project or in a library by examining the flavor at runtime.

When multiple entries affect the same file and 1) at least one of these entries have a flavors list provided and 2) these lists are not equivalent, we always consider the manifest to be ambiguous and will throw a ToolExit.

Details For example, these manifests would all be considered ambiguous:
assets:
  - assets/
  - path: assets/vanilla.png
    flavors: 
      - vanilla

assets:
  - path: assets/vanilla/
    flavors: 
      - vanilla
  - path: assets/vanilla/cherry.png
     flavor:
      - cherry

# Thinking towards the future where we might add glob/regex support and more conditions other than flavor:
assets:
  - path: assets/vanilla/**
    flavors:
      - vanilla
  - path: assets/**/ios/**
    platforms: 
       - ios

# Ambiguous in the case of assets like "assets/vanilla/ios/icon.svg" since we 
# don't know if flavor `vanilla` and platform `ios` should be combined using or-logic or and-logic.

See this review comment thread for the full story on how I arrived at this decision.

This does not support Android's multidimensional flavors feature (in an intuitive way)

Details

Conder this excerpt from a Flutter project's android/app/build.gradle file:

android {
    // ...

    flavorDimensions "mode", "api"

    productFlavors {
        free {
            dimension "mode"
            applicationIdSuffix ".free"
        }

        premium {
            dimension "mode"
            applicationIdSuffix ".premium"
        }

        minApi23 {
            dimension "api"
            versionNameSuffix "-minApi23"
        }

        minApi21 {
            dimension "api"
            versionNameSuffix "-minApi21"
        }
    }
}

In this setup, the following values are valid --flavor are valid freeMinApi21, freeMinApi23, premiumMinApi21, and premiumMinApi23. We call these values "flavor combinations". Consider the following from the Android documentation1:

In addition to the source set directories you can create for each individual product flavor and build variant, you can also create source set directories for each combination of product flavors. For example, you can create and add Java sources to the src/demoMinApi24/java/ directory, and Gradle uses those sources only when building a variant that combines those two product flavors.

Source sets you create for product flavor combinations have a higher priority than source sets that belong to each individual product flavor. To learn more about source sets and how Gradle merges resources, read the section about how to create source sets.

This feature will not behave in this way. If a user utilizes this feature and also Android's multidimensional flavors feature, they will have to list out all flavor combinations that contain the flavor they want to limit an asset to:

assets:
  - assets/free/
    flavors:
      - freeMinApi21
      - freeMinApi23

This is mostly due to a technical limitation in the hot-reload feature of flutter run. During a hot reload, the tool will try to update the asset bundle on the device, but the tool does not know the flavors contained within the flavor combination (that the user passes to --flavor). Gradle is the source of truth of what flavors were involved in the build, and flutter run currently does not access to that information since it's an implementation detail of the build process. We could bubble up this information, but it would require a nontrivial amount of engineering work, and it's unclear how desired this functionality is. It might not be worth implementing.

See https://flutter.dev/go/flavor-specific-assets for the (outdated) design document.

Pre-launch Checklist
  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

Footnotes

  1. https://developer.android.com/build/build-variants#flavor-dimensions

@auto-submit auto-submit bot added the revert of Bot Only: Tracking label for bot. Tracks new revert of pull requests. label Dec 8, 2023
@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Dec 8, 2023
@auto-submit auto-submit bot merged commit 21766a4 into master Dec 8, 2023
@auto-submit auto-submit bot deleted the revert_016eb85177c9cef03f62c2ec2ad797398952e310 branch December 8, 2023 06:40
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

revert of Bot Only: Tracking label for bot. Tracks new revert of pull requests. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

asset manifest parsing code is duplicated for parsing the deferred components manifest Support different set of assets depending on --flavor xxx

2 participants