-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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 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) ...
|
@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 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) ...
* 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) ...
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
…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.