Skip to content

Reland: [macos] add flavor options to tool commands #119564

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 31, 2023
Merged

Reland: [macos] add flavor options to tool commands #119564

merged 2 commits into from
Jan 31, 2023

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Jan 30, 2023 •
edited
Loading

Adds --flavor option to flutter run and flutter build. Running against preexisting devicelab flavor tests for feature parity between macOS, iOS, and Android.

This relands #118421 by alex-wallen which was reverted in #118858 due to the following test failures:

The bail-out with "Host and target are the same. Nothing to install." added in packages/flutter_tools/lib/src/commands/install.dart triggered failures in the following tests, which unconditionally attempt to install the built app, which is unsupported on desktop since the host and target are the same:

In order to fix the failure, the StartupTest has been updated to not install on desktop operating systems.

Fixes #64088

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.

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

@cbracken cbracken requested a review from keyonghan as a code owner January 30, 2023 21:44
@flutter-dashboard flutter-dashboard bot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Jan 30, 2023
@cbracken cbracken requested a review from jmagman January 30, 2023 21:45
@cbracken
Reland: [macos] add flavor options to tool commands
36de951 
Adds --flavor option to flutter run and flutter build. Running against
preexisting devicelab flavor tests for feature parity between macOS,
iOS, and Android.

This relands flutter#118421 by alex-wallen which was reverted in flutter#118858 due to
the following test failures:

The bail-out with "Host and target are the same. Nothing to install."
added in `packages/flutter_tools/lib/src/commands/install.dart`
triggered failures in the following tests, which unconditionally attempt
to install the built app, which is unsupported on desktop since the
host and target are the same:

* https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8791495589540422465/+/u/run_flutter_view_macos__start_up/test_stdout
* https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8791496218824259121/+/u/run_complex_layout_win_desktop__start_up/test_stdout
* https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8791496218165602641/+/u/run_flutter_gallery_win_desktop__start_up/test_stdout

Fixes flutter#64088
@jmagman
Copy link
Member

jmagman commented Jan 30, 2023

In order to fix the failure, the StartupTest has been updated to not install on desktop operating systems.

Instead it seems install should "succeed" on desktop with nothing to do, instead of tool exiting?

@cbracken
Copy link
Member Author

Hrm. On the fence on that; I suppose it depends on how we want to treat the idea of installation. e.g. For Windows UWP, when we had it, installation was a meaningful operation that involved registration etc. with the OS. On macOS, it's not so much as no-op as something that's a meaningless operation.

@cbracken
Copy link
Member Author

Summary of discussion: I'm going to yank the changes to the install command that were part of the original PR since they're orthogonal to flavour support. We can deal with that and figure out expected behaviour in a separate PR.

jmagman
jmagman approved these changes Jan 31, 2023
View reviewed changes
@cbracken
Partial revert: eliminate install check on desktop
ad62b37 
The original flavour support patch included a check that triggered a
failure when flutter install is run on desktop OSes. This was
intentional, since the host and target devices are the same and
installation is unnecessary to launch the app on currently-supported
desktop OSes.

Note that Windows UWP apps *do* require installation to run, and we used
to have an install command for those apps, though UWP is no longer
supported.

Since that part of the change was orthogonal to flavour support itself,
I'm reverting that component of the change and we can deal with it
separately if so desired.
@cbracken cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 31, 2023
@auto-submit auto-submit bot merged commit d272a3a into flutter:master Jan 31, 2023
@cbracken cbracken deleted the mac-flavours branch January 31, 2023 17:37
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 31, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 31, 2023
@engine-flutter-autoroll
d272a3a Reland: [macos] add flavor options to tool commands (flutter/…
80b5e7d 
…flutter#119564)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 31, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 31, 2023
@engine-flutter-autoroll
d272a3a Reland: [macos] add flavor options to tool commands (flutter/…
4ee360d 
…flutter#119564)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 31, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 31, 2023
@engine-flutter-autoroll
d272a3a Reland: [macos] add flavor options to tool commands (flutter/…
8bb6f42 
…flutter#119564)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 31, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 31, 2023
@engine-flutter-autoroll
d272a3a Reland: [macos] add flavor options to tool commands (flutter/…
965ac91 
…flutter#119564)
@cbracken cbracken mentioned this pull request Jan 31, 2023
Closed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 1, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 1, 2023
@engine-flutter-autoroll
d272a3a Reland: [macos] add flavor options to tool commands (flutter/…
41d08be 
…flutter#119564)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 1, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 1, 2023
@engine-flutter-autoroll
d272a3a Reland: [macos] add flavor options to tool commands (flutter/…
98a81de 
…flutter#119564)
auto-submit bot pushed a commit to flutter/plugins that referenced this pull request Feb 1, 2023
@engine-flutter-autoroll
Roll Flutter from 0a22a1d to d278808 (58 revisions) (#7078)
36d8066 
* 472b887 0ec8e2802 Roll Fuchsia Mac SDK from 9y7C2oamTv6Py4JSC... to EAFnGijD0l5QxaPxF... (flutter/engine#39236) (flutter/flutter#119461)

* 15cd00f a7bb0e410 Roll Fuchsia Linux SDK from 1D63BqURfJdG4r3CK... to xTXbcsPr5GJvFSLha... (flutter/engine#39238) (flutter/flutter#119482)

* 530c3f2 [Re-land#2] Button padding M3 (flutter/flutter#119498)

* 17eb2e8 Ability to disable the browser's context menu on web (flutter/flutter#118194)

* df8ad3d roll packages (flutter/flutter#119370)

* b68cebd roll packages (flutter/flutter#119530)

* 59d80dc [Android] Add explicit exported tag to Linux_android flavors test (flutter/flutter#117542)

* 458b298 Refactoring to use `ver` command instead of `systeminfo` (flutter/flutter#119304)

* 54405bf fixes PointerEventConverter to handle malformed scrolling event (flutter/flutter#118124)

* e69ea6d Support flipping mouse scrolling axes through modifier keys (flutter/flutter#115610)

* 92df6b4 396c7fd0b Reland "Remove references to Observatory (#38919)" (flutter/engine#39139) (flutter/flutter#119546)

* 7477d7a Reland "Add --serve-observatory flag to run, attach, and test (#118402)" (flutter/flutter#119529)

* 6c12e39 Introduce ParagraphBoundary subclass for text editing (flutter/flutter#116549)

* b227df3 Hint text semantics to be excluded in a11y read out if hintText is not visible. (flutter/flutter#119198)

* 18c7f8a Fix typo in --machine help text (flutter/flutter#119563)

* 329f86a Make a few values non-nullable in cupertino (flutter/flutter#119478)

* c4520bc b2efe0175 [web] Expose felt flag for building CanvasKit Chromium (flutter/engine#39201) (flutter/flutter#119567)

* 8898f4f Marks Mac_android run_debug_test_android to be unflaky (flutter/flutter#117468)

* 1f0b6fb Remove deprecated AppBar/SliverAppBar/AppBarTheme.textTheme member (flutter/flutter#119253)

* edaeec8 Roll Flutter Engine from b2efe01754ef to 5011144c0b46 (3 revisions) (flutter/flutter#119578)

* 865dc5c Roll Flutter Engine from 5011144c0b46 to daa8eeb7fc0b (2 revisions) (flutter/flutter#119584)

* 1148a2a Migrate EditableTextState from addPostFrameCallbacks to compositionCallbacks (flutter/flutter#119359)

* 2340902 Roll Flutter Engine from daa8eeb7fc0b to 77218818138f (3 revisions) (flutter/flutter#119586)

* 65900b7 Remove deprecated AnimatedSize.vsync parameter (flutter/flutter#119186)

* 5b6572f Add debug diagnostics to channels integration test (flutter/flutter#119579)

* 504e565 Roll Flutter Engine from 77218818138f to 9448f2966c11 (3 revisions) (flutter/flutter#119592)

* 7ba4406 Revert "[Re-land#2] Button padding M3 (#119498)" (flutter/flutter#119597)

* 2c34a88 Roll Flutter Engine from 9448f2966c11 to 72abe0e4b828 (3 revisions) (flutter/flutter#119603)

* df0ab40 Roll Plugins from ff84c44 to 9da327c (15 revisions) (flutter/flutter#119629)

* 67d07a6 [flutter_tools] Fix parsing of existing DDS URIs from exceptions (flutter/flutter#119506)

* d272a3a Reland: [macos] add flavor options to tool commands (flutter/flutter#119564)

* a16d82c aa00da3c1 Roll Skia from fc31f43cc40a to 3c6eb76a683a (1 revision) (flutter/engine#39280) (flutter/flutter#119605)

* f6b0c6d Use first Dart VM Service found with mDNS if there are duplicates (flutter/flutter#119545)

* d4c7485 Make Decoration.padding non-nullable (flutter/flutter#119581)

* 2fccf4d Remove MediaQuery from WidgetsApp (flutter/flutter#119377)

* 9b3b9cf Roll Flutter Engine from aa00da3c1612 to cd2e8885e491 (6 revisions) (flutter/flutter#119639)

* 6a54059 Make MultiChildRenderObjectWidget const (flutter/flutter#119195)

* e2b3d89 Fix CupertinoNavigationBar should create a backward compatible Annota… (flutter/flutter#119515)

* 7bf95f4 1aaf3db31 Roll Dart SDK from 4fdbc7c28141 to 9bcc1773ebf0 (1 revision) (flutter/engine#39290) (flutter/flutter#119640)

* 0e22aca Add support for image insertion on Android (flutter/flutter#110052)

* ff22813 separatorBuilder can't return null (flutter/flutter#119566)

* 60c1f29 2471f430f Update buildroot to c02da5072d1bb2. (flutter/engine#39292) (flutter/flutter#119645)

* fbe9ff3 Disable an inaccurate test assertion that will be fixed by an engine roll (flutter/flutter#119653)

* 8f90e2a Roll Flutter Engine from 2471f430ff4b to bb7b7006f4a3 (2 revisions) (flutter/flutter#119655)

* 3884381 Make gen-l10n error handling independent of logger state (flutter/flutter#119644)

* 198a51a Migrate the Material Date pickers to M3 Reprise (flutter/flutter#119033)

* dc86565 Roll Flutter Engine from bb7b7006f4a3 to 521b975449ba (4 revisions) (flutter/flutter#119670)

* 82df235 Undo making Flex,Row,Column const (flutter/flutter#119669)

* 6f9a896 Roll Flutter Engine from 521b975449ba to 38913c5484cf (2 revisions) (flutter/flutter#119675)

* 8d0af36 🥅 Produce warning instead of error for storage base url overrides (flutter/flutter#119595)

* 3894d24 1703a3966 Roll Skia from c29211525dac to 654f4805e8b8 (21 revisions) (flutter/engine#39309) (flutter/flutter#119683)

* a752c2f Expose enableIMEPersonalizedLearning on CupertinoSearchTextField (flutter/flutter#119439)

* e1f0b1d d92e23cb5 Roll Skia from 654f4805e8b8 to da41cf18f651 (1 revision) (flutter/engine#39311) (flutter/flutter#119686)

* 97d273c CupertinoThemeData equality (flutter/flutter#119480)

* 4167835 5b549950f Roll Fuchsia Linux SDK from 71lEeibIyrq0V8jId... to TFcelQ5SwrzkcYK2d... (flutter/engine#39312) (flutter/flutter#119688)

* b4a6e34 0d87b1562 Roll Dart SDK from 8b57d23a7246 to de03e1f41b50 (1 revision) (flutter/engine#39313) (flutter/flutter#119695)

* 3af30ff Roll Flutter Engine from 0d87b156265c to c08a286d60e9 (3 revisions) (flutter/flutter#119706)

* d278808 [Re-land] Exposed tooltip longPress (flutter/flutter#118796)
@jmagman jmagman mentioned this pull request Feb 2, 2023
Closed
@jmagman
Copy link
Member

jmagman commented Feb 2, 2023

The flavors_test_macos test isn't passing: #119782

@JasCodes JasCodes mentioned this pull request Feb 11, 2023
Closed
@nilsreichardt nilsreichardt mentioned this pull request Feb 21, 2023
Closed
@arnemolland arnemolland mentioned this pull request May 31, 2023
Merged
8 tasks
auto-submit bot pushed a commit that referenced this pull request Jun 14, 2023
@arnemolland
Fix inconsistently suffixed macOS flavored bundle directory (#127997)
6d2b5ea 
The current implementation of macOS flavor support (#119564) assumes a bundle directory that differs from both the iOS implementation and the official documentation. The [documentation](https://docs.flutter.dev/deployment/flavors) instructs developers to suffix their Xcode build configurations with `-<flavor>`, but the implementation assumes a space:

https://github.com/flutter/flutter/blob/5fd9ef4240d3fc239f042f49b8eb1ad24260091f/packages/flutter_tools/lib/src/macos/application_package.dart#L174-L178

Whereas the iOS implementation, which is the reference for the docs, assumes a `-<flavor>` suffix:

https://github.com/flutter/flutter/blob/a257efc2841ed7042322fbd043f0983e705d7da2/packages/flutter_tools/lib/src/ios/xcodeproj.dart#L482-L488

This change replaces the empty space with the `-` character which is in line with the documentation and iOS implementation, as well as removing the sentence-casing applied to the flavor name; every bundle built with a flavor keeps the original flavor name in its filename.

*List which issues are fixed by this PR. You must list at least one issue.*
#122684.

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
@jmagman jmagman mentioned this pull request Dec 7, 2023
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmagman jmagman jmagman approved these changes

@keyonghan keyonghan Awaiting requested review from keyonghan

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support flavors for macOS
2 participants
@cbracken @jmagman