-
Notifications
You must be signed in to change notification settings - Fork 29.7k
add default-flavor field to flutter pubspec, which will be used as the flavor in flutter build/run if --flavor is not provided
#147968
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
add default-flavor field to flutter pubspec, which will be used as the flavor in flutter build/run if --flavor is not provided
#147968
Conversation
|
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. |
|
This probably needs to be reflected in https://github.com/flutter/website |
|
looks like the failing windows test OOMed: This should not be related to my PR |
|
ok, I realized now that this doesn't really work on iOS. The |
|
the solution to the ios problem is to rename the default build configuration from |
There was a problem hiding this 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.
packages/flutter_tools/test/general.shard/flutter_manifest_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart
Show resolved
Hide resolved
packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart
Outdated
Show resolved
Hide resolved
|
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 flutter/packages/flutter_tools/lib/src/runner/flutter_command.dart Lines 1093 to 1100 in b5f6df7
What do you think about adding another line here that says something similar to |
introduced in flutter/flutter#147968
|
Thanks @andrewkolos for the thorough review. I applied all your suggested changes. Also, I added a (minor) mention to the |
_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.
andrewkolos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Looks like there are failing tests here |
christopherfujino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM
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). |
default-flavor field to flutter pubspec, which will be used as the flavor in flutter build/run if --flavor is not provided
|
@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 |
introduced in flutter/flutter#147968
|
@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: |
_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.
…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) ...
|
@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 |
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 |
…used as the flavor in `flutter build/run` if `--flavor` is not provided (flutter/flutter#147968)
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.
Good idea. I've added a note about this idea on #139286 so that it doesn't get lost to history.
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! |
* 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) ...
* 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) ...
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
…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
|
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 :
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 :
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 |
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. |
…used as the flavor in `flutter build/run` if `--flavor` is not provided (flutter/flutter#147968)
|
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 |
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 |
This PR adds a new flag
default-flavorin thefluttersection ofpubspec.yaml. It allows developers of multi-flavor android apps to specify a default flavor to be used forflutter run,flutter buildetc.Using
flutter runon flavored apps already works without specifying--flavoralready works on iOS (it defaults to therunnerschema), 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.