Skip to content

Conversation

@holzgeist
Copy link
Contributor

@holzgeist holzgeist commented May 8, 2024

This PR adds a new flag default-flavor in the flutter section of pubspec.yaml. It allows developers of multi-flavor android apps to specify a default flavor to be used for flutter run, flutter build etc.
Using flutter run on flavored apps already works without specifying --flavor already works on iOS (it defaults to the runner schema), so I (and others in #22856) figured this would be nice to have.

fixes #22856

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@google-cla
Copy link

google-cla bot commented May 8, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label May 8, 2024
@holzgeist
Copy link
Contributor Author

This probably needs to be reflected in https://github.com/flutter/website
(related to me not yet checking "I updated/added relevant documentation (doc comments with ///)." above

@holzgeist holzgeist marked this pull request as ready for review May 8, 2024 08:00
@holzgeist
Copy link
Contributor Author

looks like the failing windows test OOMed:

[ +502 ms] > Task :app:packageDebug FAILED
[        ] AAPT2 aapt2-8.1.0-10154469-windows Daemon #0: shutdown
[        ] FAILURE: Build failed with an exception.
[        ] * What went wrong:
[        ] Execution failed for task ':app:packageDebug'.
[        ] > A failure occurred while executing com.android.build.gradle.tasks.PackageAndroidArtifact$IncrementalSplitterRunnable
[        ]    > java.lang.OutOfMemoryError (no error message)

This should not be related to my PR

@holzgeist
Copy link
Contributor Author

ok, I realized now that this doesn't really work on iOS. The runner schema is only applied if there is no flavor. If there is one due to the default value, the build doesn't work anymore. Any ideas? Is there a way to tell if we are building for Android in getBuildInfo in flutter_command?

@holzgeist
Copy link
Contributor Author

the solution to the ios problem is to rename the default build configuration from Debug, Profile and Release to Debug <Default Flavor>, etc.
Works like a charm :)

@andrewkolos andrewkolos self-requested a review May 16, 2024 20:16
Copy link
Contributor

@andrewkolos andrewkolos left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I think this feature will be simple enough to maintain and thus worth having in the tool. Everything in this PR looks correct. I just have a few nitpicks.

@andrewkolos
Copy link
Contributor

andrewkolos commented May 17, 2024

I think we should document this new pubspec entry. We could do so on the flutter.dev page, but we could start by mentioning in the help text of --flavor:

void usesFlavorOption() {
argParser.addOption(
'flavor',
help: 'Build a custom app flavor as defined by platform-specific build setup.\n'
'Supports the use of product flavors in Android Gradle scripts, and '
'the use of custom Xcode schemes.',
);
}

What do you think about adding another line here that says something similar to Overrides the value of the 'default-flavor' entry in the flutter pubspec.?

holzgeist added a commit to holzgeist/flutter_website that referenced this pull request May 21, 2024
@holzgeist
Copy link
Contributor Author

Thanks @andrewkolos for the thorough review. I applied all your suggested changes.

Also, I added a (minor) mention to the Flavors documentation page: flutter/website#10632

sfshaza2 pushed a commit to flutter/website that referenced this pull request May 21, 2024
_Description of what this PR is changing or adding, and why:_

Document the `default-flavor` entry in `pubspec.yaml` introduced in
flutter/flutter#147968

_Issues fixed by this PR (if any):_

## Presubmit checklist

- [x] This PR doesn’t contain automatically generated corrections
(Grammarly or similar).
- [x] This PR follows the [Google Developer Documentation Style
Guidelines](https://developers.google.com/style) — for example, it
doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person).
- [x] This PR uses [semantic line
breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks)
of 80 characters or fewer.
Copy link
Contributor

@andrewkolos andrewkolos left a comment

Choose a reason for hiding this comment

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

LGTM!

@christopherfujino
Copy link
Contributor

Looks like there are failing tests here

@andrewkolos
Copy link
Contributor

The test failures might be due to #148704.

Once the tree goes green again, I will update the PR branch to include b6bed5a, which will skip the flaky tests.

@christopherfujino
Copy link
Contributor

The test failures might be due to #148704.

Once the tree goes green again, I will update the PR branch to include b6bed5a, which will skip the flaky tests.

oh nice

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

RSLGTM

@andrewkolos
Copy link
Contributor

Also, I added a (minor) mention to the Flavors documentation page: flutter/website#10632

FYI flutter/website#10632 got merged and then reverted since the functionality from this PR won't be available to stable release users until the next major release. Feel free to re-draft the PR and include note in the description that the feature won't be available until then (and thus the PR shouldn't be merged until then).

@andrewkolos andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label May 22, 2024
@andrewkolos andrewkolos changed the title Feat add default flavor add default-flavor field to flutter pubspec, which will be used as the flavor in flutter build/run if --flavor is not provided May 22, 2024
@auto-submit auto-submit bot merged commit 4354835 into flutter:master May 22, 2024
@holzgeist
Copy link
Contributor Author

@andrewkolos oh, thanks. Sorry about that. I didn't think about that resp. I had no idea how fast documentation was released

I'll re-send the website PR

holzgeist added a commit to holzgeist/flutter_website that referenced this pull request May 22, 2024
@navaronbracke
Copy link
Contributor

navaronbracke commented May 22, 2024

@holzgeist Thanks for this! I did file #139286 in the past as well, which relates to the same problem from a tool perspective. If we get around to solving that issue - using a tool exit with a relevant message - perhaps we can include a reference to this fix in the message?

For example: The required flavor variant was not specified. Consider passing it as the "--flavor" option, or specifying the default flavor in your pubspec.yaml

sfshaza2 pushed a commit to flutter/website that referenced this pull request May 22, 2024
_Description of what this PR is changing or adding, and why:_

Document new feature (setting default flavor in pubspec.yaml) introduced
in flutter/flutter#147968

This is a new version of #10632,
please only merge once the PR above is released

_Issues fixed by this PR (if any):_

flutter/flutter#22856

## Presubmit checklist

- [x] This PR doesn’t contain automatically generated corrections
(Grammarly or similar).
- [x] This PR follows the [Google Developer Documentation Style
Guidelines](https://developers.google.com/style) — for example, it
doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person).
- [x] This PR uses [semantic line
breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks)
of 80 characters or fewer.
hello-coder-xu added a commit to hello-coder-xu/flutter that referenced this pull request May 22, 2024
…pdate

* master: (92 commits)
  Add tests for actions.0.dart API example. (flutter#148678)
  Introduce `WidgetStateBorderSide.lerp` (flutter#148122)
  add `default-flavor` field to flutter pubspec, which will be used as the flavor in `flutter build/run` if `--flavor` is not provided (flutter#147968)
  [wiki migration] Pages under docs/postmortems/ (flutter#148798)
  Roll Flutter Engine from e5a73e520e89 to c89defa55801 (2 revisions) (flutter#148812)
  Make hover tests functional and cleanup mouse pointers in Material toggleables (flutter#148808)
  Fix two dimensional viewport unexpected null exception when no child is laid out (flutter#148256)
  Roll Flutter Engine from bc1345b6b50a to e5a73e520e89 (3 revisions) (flutter#148807)
  Add test for undo_history_controller.0.dart (flutter#148205)
  Roll Flutter Engine from a8872c8915a2 to bc1345b6b50a (6 revisions) (flutter#148802)
  Fix test that leaks images. (flutter#148494)
  Fix warnings in `dependency_version_checker.gradle.kts` (flutter#148699)
  [wiki migration] Android team pages (flutter#148585)
  Fix leaky test. (flutter#148788)
  Add DropdownButton.menuWidth (flutter#148125)
  Add test for focus example 2 (flutter#147624)
  Add a migrator to remove `FlutterMultiDexApplication.java` (flutter#148515)
  [wiki migration] Infra team pages (flutter#148718)
  Roll Flutter Engine from 8a352f01e503 to a8872c8915a2 (1 revision) (flutter#148776)
  Fix the output of the CDN test. (flutter#148730)
  ...
@holzgeist
Copy link
Contributor Author

@andrewkolos it looks like my warning in the PR description wasn't enough, it got merged again: flutter/website#10642

Can you revert it again? I'll send a new PR once this feature hits stable

@holzgeist
Copy link
Contributor Author

@holzgeist Thanks for this! I did file #139286 in the past as well, which relates to the same problem from a tool perspective. If we get around to solving that issue - using a tool exit with a relevant message - perhaps we can include a reference to this fix in the message?

Yup, an early exit would be better than the "no output file found" message in the end. Unfortunately I have no idea how to handle that

@andrewkolos
Copy link
Contributor

@andrewkolos it looks like my warning in the PR description wasn't enough, it got merged again: flutter/website#10642

Can you revert it again? I'll send a new PR once this feature hits stable

Oh no 🤭! Sorry about that. I've went ahead and gotten it reverted again (flutter/website#10646). Per, flutter/website#10646 (comment), you are welcome to redraft it again now, but I would keep it marked as a Draft and include something like "[Next stable]" in the title.

@holzgeist Thanks for this! I did file #139286 in the past as well, which relates to the same problem from a tool perspective. If we get around to solving that issue - using a tool exit with a relevant message - perhaps we can include a reference to this fix in the message?

Good idea. I've added a note about this idea on #139286 so that it doesn't get lost to history.

Yup, an early exit would be better than the "no output file found" message in the end. Unfortunately I have no idea how to handle that

Fixing that issue would indeed be more complicated. I would love to do some digging and provide some helpful hints over on that thread, but alas I don't have the time at the moment 😞

Thanks again for the continued work here!

hello-coder-xu added a commit to hello-coder-xu/flutter that referenced this pull request May 23, 2024
* master: (29 commits)
  Add tests for actions.0.dart API example. (flutter#148678)
  Introduce `WidgetStateBorderSide.lerp` (flutter#148122)
  add `default-flavor` field to flutter pubspec, which will be used as the flavor in `flutter build/run` if `--flavor` is not provided (flutter#147968)
  [wiki migration] Pages under docs/postmortems/ (flutter#148798)
  Roll Flutter Engine from e5a73e520e89 to c89defa55801 (2 revisions) (flutter#148812)
  Make hover tests functional and cleanup mouse pointers in Material toggleables (flutter#148808)
  Fix two dimensional viewport unexpected null exception when no child is laid out (flutter#148256)
  Roll Flutter Engine from bc1345b6b50a to e5a73e520e89 (3 revisions) (flutter#148807)
  Add test for undo_history_controller.0.dart (flutter#148205)
  Roll Flutter Engine from a8872c8915a2 to bc1345b6b50a (6 revisions) (flutter#148802)
  Fix test that leaks images. (flutter#148494)
  Fix warnings in `dependency_version_checker.gradle.kts` (flutter#148699)
  [wiki migration] Android team pages (flutter#148585)
  Fix leaky test. (flutter#148788)
  Add DropdownButton.menuWidth (flutter#148125)
  Add test for focus example 2 (flutter#147624)
  Add a migrator to remove `FlutterMultiDexApplication.java` (flutter#148515)
  [wiki migration] Infra team pages (flutter#148718)
  Roll Flutter Engine from 8a352f01e503 to a8872c8915a2 (1 revision) (flutter#148776)
  Fix the output of the CDN test. (flutter#148730)
  ...
hello-coder-xu added a commit to hello-coder-xu/flutter that referenced this pull request May 23, 2024
* master: (92 commits)
  Add tests for actions.0.dart API example. (flutter#148678)
  Introduce `WidgetStateBorderSide.lerp` (flutter#148122)
  add `default-flavor` field to flutter pubspec, which will be used as the flavor in `flutter build/run` if `--flavor` is not provided (flutter#147968)
  [wiki migration] Pages under docs/postmortems/ (flutter#148798)
  Roll Flutter Engine from e5a73e520e89 to c89defa55801 (2 revisions) (flutter#148812)
  Make hover tests functional and cleanup mouse pointers in Material toggleables (flutter#148808)
  Fix two dimensional viewport unexpected null exception when no child is laid out (flutter#148256)
  Roll Flutter Engine from bc1345b6b50a to e5a73e520e89 (3 revisions) (flutter#148807)
  Add test for undo_history_controller.0.dart (flutter#148205)
  Roll Flutter Engine from a8872c8915a2 to bc1345b6b50a (6 revisions) (flutter#148802)
  Fix test that leaks images. (flutter#148494)
  Fix warnings in `dependency_version_checker.gradle.kts` (flutter#148699)
  [wiki migration] Android team pages (flutter#148585)
  Fix leaky test. (flutter#148788)
  Add DropdownButton.menuWidth (flutter#148125)
  Add test for focus example 2 (flutter#147624)
  Add a migrator to remove `FlutterMultiDexApplication.java` (flutter#148515)
  [wiki migration] Infra team pages (flutter#148718)
  Roll Flutter Engine from 8a352f01e503 to a8872c8915a2 (1 revision) (flutter#148776)
  Fix the output of the CDN test. (flutter#148730)
  ...
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 23, 2024
flutter/flutter@d02292d...73bf206

2024-05-22 [email protected] `CupertinoDialogRoute` leak fix (flutter/flutter#148774)
2024-05-22 [email protected] Marks Windows plugin_test to be flaky (flutter/flutter#148835)
2024-05-22 [email protected] Add tests for actions.0.dart API example. (flutter/flutter#148678)
2024-05-22 [email protected] Introduce `WidgetStateBorderSide.lerp` (flutter/flutter#148122)
2024-05-22 [email protected] add `default-flavor` field to flutter pubspec, which will be used as the flavor in `flutter build/run` if `--flavor` is not provided (flutter/flutter#147968)
2024-05-22 [email protected] [wiki migration] Pages under docs/postmortems/ (flutter/flutter#148798)
2024-05-22 [email protected] Roll Flutter Engine from e5a73e520e89 to c89defa55801 (2 revisions) (flutter/flutter#148812)
2024-05-22 [email protected] Make hover tests functional and cleanup mouse pointers in Material toggleables (flutter/flutter#148808)
2024-05-21 [email protected] Fix two dimensional viewport unexpected null exception when no child is laid out (flutter/flutter#148256)
2024-05-21 [email protected] Roll Flutter Engine from bc1345b6b50a to e5a73e520e89 (3 revisions) (flutter/flutter#148807)
2024-05-21 [email protected] Add test for undo_history_controller.0.dart (flutter/flutter#148205)
2024-05-21 [email protected] Roll Flutter Engine from a8872c8915a2 to bc1345b6b50a (6 revisions) (flutter/flutter#148802)
2024-05-21 [email protected] Fix test that leaks images. (flutter/flutter#148494)
2024-05-21 [email protected] Fix warnings in `dependency_version_checker.gradle.kts` (flutter/flutter#148699)
2024-05-21 [email protected] [wiki migration] Android team pages (flutter/flutter#148585)
2024-05-21 [email protected] Fix leaky test. (flutter/flutter#148788)
2024-05-21 [email protected] Add DropdownButton.menuWidth (flutter/flutter#148125)
2024-05-21 [email protected] Add test for focus example 2 (flutter/flutter#147624)
2024-05-21 [email protected] Add a migrator to remove `FlutterMultiDexApplication.java` (flutter/flutter#148515)
2024-05-21 [email protected] [wiki migration] Infra team pages (flutter/flutter#148718)
2024-05-21 [email protected] Roll Flutter Engine from 8a352f01e503 to a8872c8915a2 (1 revision) (flutter/flutter#148776)
2024-05-21 [email protected] Fix the output of the CDN test. (flutter/flutter#148730)
2024-05-21 [email protected] [wiki migration] Release team pages (flutter/flutter#148723)
2024-05-21 [email protected] Remove hidden dependencies on LocalProcessManager (flutter/flutter#148096)
2024-05-21 [email protected] Adds Missing `onHover` & `onFocusChange` for `OutlinedButton.icon` (flutter/flutter#144374)
2024-05-21 [email protected] Adds tests to NestedScrollView examples (flutter/flutter#148170)
2024-05-21 [email protected] Roll Flutter Engine from c2ef01f6f1ab to 8a352f01e503 (18 revisions) (flutter/flutter#148766)
2024-05-21 [email protected] `switch` expressions: finale (flutter/flutter#148711)
2024-05-21 [email protected] [iOS] specify minimum os version for native asset frameworks (flutter/flutter#148504)
2024-05-21 [email protected] Removed brand references from MenuAnchor.dart (flutter/flutter#148760)
2024-05-21 [email protected] Skip flaky test in expression_evaluation_test.dart (flutter/flutter#148737)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request May 31, 2024
…the flavor in `flutter build/run` if `--flavor` is not provided (flutter#147968)

This PR adds a new flag `default-flavor` in the `flutter` section of `pubspec.yaml`. It allows developers of multi-flavor android apps to specify a default flavor to be used for `flutter run`, `flutter build` etc.
Using `flutter run` on flavored apps already works without specifying `--flavor` already works on iOS (it defaults to the `runner` schema), so I (and others in flutter#22856) figured this would be nice to have.

fixes flutter#22856
@sbatezat
Copy link

sbatezat commented Jul 1, 2024

I don't get it.

Adding a "default-flavor" solves Android issue when it's not possible to "flutter build"/"flutter run" an app without flavor, but it introduces a new issue on iOS where, when uses, the iOS platform is now expecting the definition of a default-flavor (where it was not needed before).

Let's me explain further. Let's say i've got a "free" flavor and a normal app, like that :

flavorDimensions "default"

productFlavors {
    FREE {
        dimension "default"
    }
}

I can fix the build of "flutter build" / "flutter run" without the flavor flag on the cli by adding your "default-flavor: PAID" flag on the pubspec with "PAID" also on the gradle file :

flavorDimensions "default"

productFlavors {
    PAID {
        dimension "default"
    }
    FREE {
        dimension "default"
    }
}

But then, on iOS, it's not working anymore (and it was working), because the "PAID" flavor does not exists. So I need to create it whereas it was not needed before....

Taking this into consideration, I'm not sure this MR is the way to go

@andrewkolos
Copy link
Contributor

I don't get it.

Unless #22856 was requesting the ability to configure a default flavor exclusively for Android, this PR is a valid resolution to that issue. You are welcome to file a new feature request.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
…used as the flavor in `flutter build/run` if `--flavor` is not provided (flutter/flutter#147968)
@vashworth vashworth mentioned this pull request Mar 20, 2025
4 tasks
@antfitch
Copy link
Contributor

antfitch commented Mar 24, 2025

Out of curiosity, if a developer builds a Flutter app that ports to both iOS and Android, will this produce an error when the iOS app is built for that project?

@andrewkolos
Copy link
Contributor

Out of curiosity, if a developer builds a Flutter app that ports to both iOS and Android, will this produce an error when the iOS app is built for that project?

Assuming "this" refers to the default-flavor feature/field in the pubspec, it should work for both platforms. However, it's very likely this feature doesn't work when building a Flutter app directly from Android Studio/Xcode (as opposed to using the flutter CLI or Flutter IDE plugin).

@antfitch
Copy link
Contributor

Out of curiosity, if a developer builds a Flutter app that ports to both iOS and Android, will this produce an error when the iOS app is built for that project?

Assuming "this" refers to the default-flavor feature/field in the pubspec, it should work for both platforms. However, it's very likely this feature doesn't work when building a Flutter app directly from Android Studio/Xcode (as opposed to using the flutter CLI or Flutter IDE plugin).

I agree, it makes sense that this wouldn't work from Android Studio/Xcode. I was noticing the thread of messages around iOS issues and the default-flavor field. I wanted to make sure that default-flavor in the pubspec works with flavors for iOS. It sounds like it does. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

On Android when there are multiple flavors and no --flavor flag is specified then use a default one

6 participants