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 e5a73e5 to c89defa (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 bc1345b to e5a73e5 (3 revisions) (flutter#148807)
  Add test for undo_history_controller.0.dart (flutter#148205)
  Roll Flutter Engine from a8872c8 to bc1345b (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 8a352f0 to a8872c8 (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 e5a73e5 to c89defa (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 bc1345b to e5a73e5 (3 revisions) (flutter#148807)
  Add test for undo_history_controller.0.dart (flutter#148205)
  Roll Flutter Engine from a8872c8 to bc1345b (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 8a352f0 to a8872c8 (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 e5a73e5 to c89defa (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 bc1345b to e5a73e5 (3 revisions) (flutter#148807)
  Add test for undo_history_controller.0.dart (flutter#148205)
  Roll Flutter Engine from a8872c8 to bc1345b (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 8a352f0 to a8872c8 (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 102401667+Dimilkalathiya@users.noreply.github.com `CupertinoDialogRoute` leak fix (flutter/flutter#148774)
2024-05-22 fluttergithubbot@gmail.com Marks Windows plugin_test to be flaky (flutter/flutter#148835)
2024-05-22 sokolovskyi.konstantin@gmail.com Add tests for actions.0.dart API example. (flutter/flutter#148678)
2024-05-22 tessertaha@gmail.com Introduce `WidgetStateBorderSide.lerp` (flutter/flutter#148122)
2024-05-22 holzgeist@users.noreply.github.com 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 katelovett@google.com [wiki migration] Pages under docs/postmortems/ (flutter/flutter#148798)
2024-05-22 engine-flutter-autoroll@skia.org Roll Flutter Engine from e5a73e520e89 to c89defa55801 (2 revisions) (flutter/flutter#148812)
2024-05-22 victorsanniay@gmail.com Make hover tests functional and cleanup mouse pointers in Material toggleables (flutter/flutter#148808)
2024-05-21 amirpanahandeh@yahoo.com Fix two dimensional viewport unexpected null exception when no child is laid out (flutter/flutter#148256)
2024-05-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from bc1345b6b50a to e5a73e520e89 (3 revisions) (flutter/flutter#148807)
2024-05-21 32538273+ValentinVignal@users.noreply.github.com Add test for undo_history_controller.0.dart (flutter/flutter#148205)
2024-05-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from a8872c8915a2 to bc1345b6b50a (6 revisions) (flutter/flutter#148802)
2024-05-21 polinach@google.com Fix test that leaks images. (flutter/flutter#148494)
2024-05-21 34871572+gmackall@users.noreply.github.com Fix warnings in `dependency_version_checker.gradle.kts` (flutter/flutter#148699)
2024-05-21 katelovett@google.com [wiki migration] Android team pages (flutter/flutter#148585)
2024-05-21 polinach@google.com Fix leaky test. (flutter/flutter#148788)
2024-05-21 bruno.leroux@gmail.com Add DropdownButton.menuWidth (flutter/flutter#148125)
2024-05-21 82763757+NobodyForNothing@users.noreply.github.com Add test for focus example 2 (flutter/flutter#147624)
2024-05-21 34871572+gmackall@users.noreply.github.com Add a migrator to remove `FlutterMultiDexApplication.java` (flutter/flutter#148515)
2024-05-21 katelovett@google.com [wiki migration] Infra team pages (flutter/flutter#148718)
2024-05-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8a352f01e503 to a8872c8915a2 (1 revision) (flutter/flutter#148776)
2024-05-21 jacksongardner@google.com Fix the output of the CDN test. (flutter/flutter#148730)
2024-05-21 katelovett@google.com [wiki migration] Release team pages (flutter/flutter#148723)
2024-05-21 ian@hixie.ch Remove hidden dependencies on LocalProcessManager (flutter/flutter#148096)
2024-05-21 pateltirth454@gmail.com Adds Missing `onHover` & `onFocusChange` for `OutlinedButton.icon` (flutter/flutter#144374)
2024-05-21 82763757+NobodyForNothing@users.noreply.github.com Adds tests to NestedScrollView examples (flutter/flutter#148170)
2024-05-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from c2ef01f6f1ab to 8a352f01e503 (18 revisions) (flutter/flutter#148766)
2024-05-21 nate.w5687@gmail.com `switch` expressions: finale (flutter/flutter#148711)
2024-05-21 matej.knopp@gmail.com [iOS] specify minimum os version for native asset frameworks (flutter/flutter#148504)
2024-05-21 59215665+davidhicks980@users.noreply.github.com Removed brand references from MenuAnchor.dart (flutter/flutter#148760)
2024-05-21 zanderso@users.noreply.github.com 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 rmistry@google.com,stuartmorgan@google.com,tarrinneal@google.com 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