This auto-formats all *.dart files in the repository outside of the
`engine` subdirectory and enforces that these files stay formatted with
a presubmit check.
**Reviewers:** Please carefully review all the commits except for the
one titled "formatted". The "formatted" commit was auto-generated by
running `dev/tools/format.sh -a -f`. The other commits were hand-crafted
to prepare the repo for the formatting change. I recommend reviewing the
commits one-by-one via the "Commits" tab and avoiding Github's "Files
changed" tab as it will likely slow down your browser because of the
size of this PR.
---------
Co-authored-by: Kate Lovett <katelovett@google.com>
Co-authored-by: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com>
These tests depend on line numbers that `dart format` is changing.
Pre-format the files in question and adjust the tests so they continue
to pass when the entire repo is formatted.
See https://github.com/flutter/flutter/issues/159154.
See https://github.com/flutter/flutter/pull/159169.
Before this PR, it appeared we were accidentally leaking (keeping
active) `flutter_tester` instances (or any test device) after `flutter
run` completion, even if the runner was not explicitly detached. I
_think_ this is a bug, but I'll check with the tools team and possibly
@jonahwilliams before finalizing this.
/cc @jason-simmons
---------
Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
Cleans up https://github.com/flutter/flutter/pull/155800. In summary, `ResidentRunner`/`FlutterDevice` have branching behavior around logging that depends on the type of `DeviceLogReader` on the `FlutterDevice` instance. Let's instead move this behavior to the `DeviceLogReader` implementations.
My apologies for the large diff. Much of this is a refactor that was a bit too difficult to separate into its own commits.
Here are the main two changes
* Replaces the mutable `connectedVmService` field on the `DeviceLogReader` class with a new method `provideVmService`. This serves largely the same purpose as the mutable field, but it allows for asynchronous code. This is where we put the logic that used to exist in `FlutterDevice.tryInitLogReader`.
* Removes the `tryInitLogReader` method from `FlutterDevice`. This method served to set the `appPid` field on the `FlutterDevice`'s `DeviceLogReader` instance. This was only used in the case of Android to filter out logs unrelated to the flutter app coming from the device, so we can move this logic to `AdbLogReader`'s implementation of `provideVmService`.
Closes https://github.com/flutter/flutter/issues/158012.
This is (effectively) a user-facing NOP, which is exchanging an
on-by-default command-line argument (`--implicit-pubspec-resolution`)
for an off-by-default global feature flag
(`explicit-package-dependencies`). It matches the mental model better,
is less painstaking to maintain and feed throughout, and will be easier
to globally flip on/off in a future PR.
---------
Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
Work towards https://github.com/flutter/flutter/issues/157819. **No behavior changes as a result of this PR**.
Based on a proof of concept by @jonahwilliams (https://github.com/flutter/flutter/pull/157818).
The existence of this flag (which for the time being, defaults to `true`) implies the following:
1. The (legacy, deprecated) `.flutter-plugins` file is not generated:
https://docs.flutter.dev/release/breaking-changes/flutter-plugins-configuration
2. The (legacy, deprecated) `package:flutter_gen` is not synthetically generated:
https://github.com/flutter/website/pull/11343
(awaiting website approvers, but owners approve this change)
This change creates `useImplicitPubspecResolution` and plumbs it through as a required variable, parsing it from a `FlutterCommand.globalResults` where able. In tests, I've defaulted the value to `true` 100% of the time - except for places where the value itself is acted on directly, in which case there are true and false test-cases (e.g. localization and i10n based classes and functions).
I'm not extremely happy this needed to change 50+ files, but is sort of a result of how inter-connected many of the elements of the tools are. I believe keeping this as an explicit (flagged) argument will be our best way to ensure the default behavior changes consistently and that tests are running as expected.
Fixes https://github.com/flutter/flutter/issues/154903
This PR contains some refactoring. To make the actual change easier to figure out, I've tried to separate parts of the change into multiple commits for easier reviewing ð.
**I plan on cherry-picking this change to stable.**
Reverts: flutter/flutter#152358
Initiated by: zanderso
Reason for reverting: Speculative revert to determine whether this PR is related to https://github.com/flutter/flutter/issues/153026
Original PR Author: andrewkolos
Reviewed By: {christopherfujino}
This change reverts the following previous change:
Contributes to fixing https://github.com/flutter/flutter/issues/137184.
Cleaned up version of earlier PR, https://github.com/flutter/flutter/pull/152187.
This PR guards all the writes to `Process::stdin` by wrapping them with `ProcessUtils.writelnToStdinUnsafe`. This way, if any writes fail, we should at least get a stacktrace in our crash reporting.
This reverts commit 7cdc23b3e1bae2bc7bc2d1f34773eaa3629d4fcc.
The failure in the `native_assets_test` integration test on Windows was caused by the DevTools process not being shutdown by the `ColdRunner` when running the profile mode portion of the test. This resulted in the test being unable to clean up the project created by the test as DevTools was still holding onto a handle within the directory. This PR adds back the mistakenly removed DevTools shutdown logic in the `ColdRunner`.
Reverts: flutter/flutter#146593
Initiated by: zanderso
Reason for reverting: Consistently failing `Windows_android native_assets_android` as in https://ci.chromium.org/ui/p/flutter/builders/prod/Windows_android%20native_assets_android/2533/overview
Original PR Author: bkonyi
Reviewed By: {christopherfujino, kenzieschmoll}
This change reverts the following previous change:
This change is a major step towards moving away from shipping DDS via Pub.
The first component of this PR is the move away from importing package:dds to launch DDS. Instead, DDS is launched out of process using the `dart development-service` command shipped with the Dart SDK. This makes Flutter's handling of DDS consistent with the standalone Dart VM.
The second component of this PR is the initial work to prepare for the removal of instances of DevTools being served manually by the flutter_tool, instead relying on DDS to serve DevTools. This will be consistent with how the standalone Dart VM serves DevTools, tying the DevTools lifecycle to a live DDS instance. This will allow for the removal of much of the logic needed to properly manage the lifecycle of the DevTools server in a future PR. Also, by serving DevTools from DDS, users will no longer need to forward a secondary port in remote workflows as DevTools will be available on the DDS port.
There's two remaining circumstances that will prevent us from removing DevtoolsRunner completely:
- The daemon's `devtools.serve` endpoint
- `flutter drive`'s `--profile-memory` flag used for recording memory profiles
This PR also includes some refactoring around `DebuggingOptions` to reduce the number of debugging related arguments being passed as parameters adjacent to a `DebuggingOptions` instance.
This change is a major step towards moving away from shipping DDS via
Pub.
The first component of this PR is the move away from importing
package:dds to launch DDS. Instead, DDS is launched out of process using
the `dart development-service` command shipped with the Dart SDK. This
makes Flutter's handling of DDS consistent with the standalone Dart VM.
The second component of this PR is the initial work to prepare for the
removal of instances of DevTools being served manually by the
flutter_tool, instead relying on DDS to serve DevTools. This will be
consistent with how the standalone Dart VM serves DevTools, tying the
DevTools lifecycle to a live DDS instance. This will allow for the
removal of much of the logic needed to properly manage the lifecycle of
the DevTools server in a future PR. Also, by serving DevTools from DDS,
users will no longer need to forward a secondary port in remote
workflows as DevTools will be available on the DDS port. This code is currently
commented out and will be enabled in a future PR.
There's two remaining circumstances that will prevent us from removing
DevtoolsRunner completely:
- The daemon's `devtools.serve` endpoint
- `flutter drive`'s `--profile-memory` flag used for recording memory
profiles
This PR also includes some refactoring around `DebuggingOptions` to
reduce the number of debugging related arguments being passed as
parameters adjacent to a `DebuggingOptions` instance.
This pull request aims for improved readability, based on issue #146600.
```dart
// before
List<SupportedPlatform> getSupportedPlatforms({bool includeRoot = false}) {
final List<SupportedPlatform> platforms = includeRoot
? <SupportedPlatform>[SupportedPlatform.root]
: <SupportedPlatform>[];
if (android.existsSync()) {
platforms.add(SupportedPlatform.android);
}
if (ios.exists) {
platforms.add(SupportedPlatform.ios);
}
if (web.existsSync()) {
platforms.add(SupportedPlatform.web);
}
if (macos.existsSync()) {
platforms.add(SupportedPlatform.macos);
}
if (linux.existsSync()) {
platforms.add(SupportedPlatform.linux);
}
if (windows.existsSync()) {
platforms.add(SupportedPlatform.windows);
}
if (fuchsia.existsSync()) {
platforms.add(SupportedPlatform.fuchsia);
}
return platforms;
}
// after
List<SupportedPlatform> getSupportedPlatforms({bool includeRoot = false}) {
return <SupportedPlatform>[
if (includeRoot) SupportedPlatform.root,
if (android.existsSync()) SupportedPlatform.android,
if (ios.exists) SupportedPlatform.ios,
if (web.existsSync()) SupportedPlatform.web,
if (macos.existsSync()) SupportedPlatform.macos,
if (linux.existsSync()) SupportedPlatform.linux,
if (windows.existsSync()) SupportedPlatform.windows,
if (fuchsia.existsSync()) SupportedPlatform.fuchsia,
];
}
```
In service of https://github.com/flutter/flutter/issues/146879 and https://github.com/flutter/flutter/issues/145812. In these issues, we see what appears to be the flutter tool getting stuck somewhere during hot reload. It may help if we knew were exactly where we are getting stuck (preparing assets, writing them to device, etc.).
This PR adds a new parameter to `FlutterTestDriver::run`, `verbose`. When verbose is set, `FlutterTestDriver` will run `flutter` with `--verbose` in its tests. Keep in mind that `FlutterTestDriver` only prints logs from `flutter` when a test fails, so this shouldn't spam the logs of passing tests.
This PR sets the parameter when invoking the flaky tests from https://github.com/flutter/flutter/issues/146879 and #145812, so we should see more detailed logs in future flakes.
While this is a low risk PR, you can verify the change by intentionally breaking hot reload code, clearing the cached tool binaries, and then running either of these tests.
This adds support for adding the `--wasm` flag to `flutter run` and `flutter drive`
* Emits errors if you attempt to use the skwasm renderer without the `--wasm` flag
* Emits errors if you try to use `--wasm` when not using a web device
* Uses the skwasm renderer by default if you pass `--wasm` and no `--web-renderer`
Reverts: flutter/flutter#144752
Initiated by: andrewkolos
Reason for reverting: compilation issue has turned the tree red
Original PR Author: andrewkolos
Reviewed By: {christopherfujino}
This change reverts the following previous change:
In service of https://github.com/flutter/flutter/issues/143348
When invoking a package to transform an asset, we set `FLUTTER_BUILD_MODE` to the CLI name of the build mode being used. Inspired by https://github.com/flutter/flutter/issues/101077#issuecomment-1890379501:
> Do transformers know whether they get executed in debug or release mode? I kinda imagine that being useful. Ex: There's a transformer that optimizes the file size of images. Depending on the amount and size of the images, that could take a significant amount of time. Therefore, I might want to only execute it in release builds.
Note for the reviewer: the interesting part of this change can be found in the commit [set environment variable to build mode when running asset transformer…](579912d470). The rest of the change is updating call sites with a new argument.
In service of https://github.com/flutter/flutter/issues/143348
When invoking a package to transform an asset, we set `FLUTTER_BUILD_MODE` to the CLI name of the build mode being used. Inspired by https://github.com/flutter/flutter/issues/101077#issuecomment-1890379501:
> Do transformers know whether they get executed in debug or release mode? I kinda imagine that being useful. Ex: There's a transformer that optimizes the file size of images. Depending on the amount and size of the images, that could take a significant amount of time. Therefore, I might want to only execute it in release builds.
Note for the reviewer: the interesting part of this change can be found in the commit [set environment variable to build mode when running asset transformerâ¦](579912d470). The rest of the change is updating call sites with a new argument.
Partial implementation of https://github.com/flutter/flutter/issues/143348
This enables asset transformation during hot reload (except for web, because that has its own implementation of `DevFS` ð). Asset transformers will be reapplied after changing any asset and performing a hot reload during `flutter run`.
This is an attempt at a reland of https://github.com/flutter/flutter/pull/141396
The main changes here that are different than the original PR is fixes to wire up the `flutter test` command properly with the web renderer.
Dual Web Compile has had some issues where `flutter test` is not respecting the `--web-renderer` flag for some reason. I haven't gotten entirely to the bottom of the issue, but for now we need to rever these changes while I investigate. This reverts the following PRs:
https://github.com/flutter/flutter/pull/143128https://github.com/flutter/flutter/pull/141396
While doing this revert, I had a few merge conflicts with https://github.com/flutter/flutter/pull/142760, and I tried to resolve the merge conflicts within the spirit of that PR's change, but @chingjun I might need your input on whether the imports I have modified are okay with regards to the change you were making.
Add a new `BuildTargets` class that provides commonly used build targets. And avoid importing files from `build_system/targets` except from the top level entrypoints or from top level commands.
Also move `scene_importer.dart` and `shader_compiler.dart` into `build_system/tools` because they are not `Target` classes, but wrapper for certain tools.
With this change, we can ignore all files in `build_system/targets` internally and make PR #142709 easier to land internally. See cl/603434066 for the corresponding internal change.
Related to:
https://github.com/flutter/flutter/pull/142709https://github.com/flutter/flutter/issues/142041
Also note that I have opted to add a new variable in `globals.dart` for `BuildTargets` in this PR, but I know that we are trying to get rid of globals. Several alternatives that I was considering:
1. Add a new field in `BuildSystem` that returns a `BuildTargets` instance. Since `BuildSystem` is already in `globals`, we can access build targets using `globals.buildSystem.buildTargets` without adding a new global variable.
2. Properly inject the `BuildTargetsImpl` instance from the top level `executable.dart` and top level commands.
Let me know if you want me to do one of the above instead. Thanks!
This implements dual compile via the newly available flutter.js bootstrapping APIs for intelligent build fallback.
* Users can now use the `FlutterLoader.load` API from flutter.js
* Flutter tool injects build info into the `index.html` of the user so that the bootstrapper knows which build variants are available to bootstrap
* The semantics of the `--wasm` flag for `flutter build web` have changed:
- Instead of producing a separate `build/web_wasm` directory, the output goes to the `build/web` directory like a normal web build
- Produces a dual build that contains two build variants: dart2wasm+skwasm and dart2js+CanvasKit. The dart2wasm+skwasm will only work on Chrome in a cross-origin isolated context, all other environments will fall back to dart2js+CanvasKit.
- `--wasm` and `--web-renderer` are now mutually exclusive. Since there are multiple build variants with `--wasm`, the web renderer cannot be expressed via a single command-line flag. For now, we are hard coding what build variants are produced with the `--wasm` flag, but I plan on making this more customizable in the future.
* Build targets now can optionally provide a "build key" which can uniquely identify any specific parameterization of that build target. This way, the build target can invalidate itself by changing its build key. This works a bit better than just stuffing everything into the environment defines because (a) it doesn't invalidate the entire build, just the targets which are affected and (b) settings for multiple build variants don't translate well to the flat map of environment defines.
This PR increases Android's `minSdkVersion` to 21.
There are two changes in this PR aside from simply increasing the number
from 19 to 21 everywhere.
First, tests using `flutter_gallery` fail without updating the
lockfiles. The changes in the PR are the results of running
`dev/tools/bin/generate_gradle_lockfiles.dart` on that app.
Second, from
[here](https://developer.android.com/build/multidex#mdex-pre-l):
> if your minSdkVersion is 21 or higher, multidex is enabled by default
and you don't need the multidex library.
As a result, the `multidex` option everywhere is obsolete. This PR
removes all logic and tests related to that option that I could find.
`Google testing` and `customer_tests` pass on this PR, so it seems like
this won't be too breaking if it is at all. If needed I'll give this
some time to bake in the framework before landing the flutter/engine
PRs.
Context: https://github.com/flutter/flutter/issues/138117,
https://github.com/flutter/flutter/issues/141277, b/319373605
It's now possible to natively compile a flutter app for windows-arm64. Cross-compilation is not yet implemented.
Uses arm64 artifacts now available for Dart/Flutter. Platform detection is based on Abi class, provided by Dart. Depending if Dart is an arm64 or x64 binary, the Abi is set accordingly. Initial bootstrap of dart artifacts (update_dart_sdk.ps1) is checking PROCESSOR_ARCHITECTURE environment variable, which is the way to detect host architecture on Windows.
This is available only for master channel (on other channels, it fallbacks to windows-x64).
On windows-x64, it produces an x64 app. On windows-arm64, it produces an arm64 app.
Reverts flutter/flutter#137618
Initiated by: Jasguerrero
This change reverts the following previous change:
Original Description:
It's now possible to natively compile a flutter app for
windows-arm64. Cross-compilation is not yet implemented.
Uses arm64 artifacts now available for Dart/Flutter.
Platform detection is based on Abi class, provided by Dart. Depending if
Dart is an arm64 or x64 binary, the Abi is set accordingly.
Initial bootstrap of dart artifacts (update_dart_sdk.ps1) is checking
PROCESSOR_ARCHITECTURE environment variable, which is the way to detect
host architecture on Windows.
This is available only for master channel (on other channels, it
fallbacks to windows-x64).
On windows-x64, it produces an x64 app. On windows-arm64, it produces an
arm64 app.
It's now possible to natively compile a flutter app for
windows-arm64. Cross-compilation is not yet implemented.
Uses arm64 artifacts now available for Dart/Flutter.
Platform detection is based on Abi class, provided by Dart. Depending if
Dart is an arm64 or x64 binary, the Abi is set accordingly.
Initial bootstrap of dart artifacts (update_dart_sdk.ps1) is checking
PROCESSOR_ARCHITECTURE environment variable, which is the way to detect
host architecture on Windows.
This is available only for master channel (on other channels, it
fallbacks to windows-x64).
On windows-x64, it produces an x64 app. On windows-arm64, it produces an
arm64 app.
