When running `dart format` over these lines the `// ignore` ended up on
a line where it wasn't properly ignoring the lint. This adjusts the
placement of `// ignore`s so they will continue to ignore the right
thing even after the code is auto formatted.
I am hoping that if we do this now the large PR that formats the entire
repo will go in smoother without manual intervention.
<!-- start_original_pr_link -->
Reverts: flutter/flutter#159756
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: gmackall
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: Made the tree red due to some std out.
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: gmackall
<!-- end_original_pr_author -->
<!-- start_reviewers -->
Reviewed By: {reidbaker}
<!-- end_reviewers -->
<!-- start_revert_body -->
This change reverts the following previous change:
In preparation for changing engine builds to be unstripped by default
https://github.com/flutter/engine/pull/52852, which will allow us to
make progress towards resolving
https://github.com/flutter/flutter/issues/60240.
Tricks AGP in to downloading the NDK when building a flutter app (that
uses the FGP, which to my knowledge is all ways of building flutter
apk/aab/aar).
I want to follow this up by modifying the tool to search for the log
line that the NDK is missing (making it throw an error in that case) as
a safeguard, because that would be the last line of defense before we
accidentally build a bloated app. The safeguard won't work for add to
app, from what I understand, because while they use the FGP (so they
should be forced to be download the NDK) they don't invoke the flutter
tool, and therefore won't invoke the custom error handling of
[`gradle_errors.dart`](https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/android/gradle_errors.dart)
Some details
1. Respects existing values for `externalNativeBuild.cmake.path` for
apps that actually use it.
2. Silences some warnings that would otherwise appear for add to app
builds or builds that manually invoke gradle:
```
C/C++: CMake Warning (dev) in CMakeLists.txt:
C/C++: No project() command is present. The top-level CMakeLists.txt file must
C/C++: contain a literal, direct call to the project() command. Add a line of
C/C++: code such as
C/C++: project(ProjectName)
C/C++: near the top of the file, but after cmake_minimum_required().
C/C++: CMake is pretending there is a "project(Project)" command on the first
C/C++: line.
C/C++: This warning is for project developers. Use -Wno-dev to suppress it.
C/C++: CMake Warning:
C/C++: Manually-specified variables were not used by the project:
C/C++: CMAKE_EXPORT_COMPILE_COMMANDS
C/C++: CMAKE_LIBRARY_OUTPUT_DIRECTORY
C/C++: CMAKE_RUNTIME_OUTPUT_DIRECTORY
```
3. Our ci installs the NDK at an abnormal place that AGP can't find
without help. I've modified all the `build.gradle`s that we have checked
in to point to the pre-installed path. **But some of our tests make a
new app from the templates, and those tests will now start downloading
the NDK** (as they won't be able to find it at it's current path from
templates). We could resolve this by actually fixing
https://github.com/flutter/flutter/issues/136666. This would be a very
significant lift from what I understand - we rely on this hardcoding in
a lot of places in our infra.
Fixes https://github.com/flutter/flutter/issues/155576
## Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
<!-- end_revert_body -->
Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>
In preparation for changing engine builds to be unstripped by default
https://github.com/flutter/engine/pull/52852, which will allow us to
make progress towards resolving
https://github.com/flutter/flutter/issues/60240.
Tricks AGP in to downloading the NDK when building a flutter app (that
uses the FGP, which to my knowledge is all ways of building flutter
apk/aab/aar).
I want to follow this up by modifying the tool to search for the log
line that the NDK is missing (making it throw an error in that case) as
a safeguard, because that would be the last line of defense before we
accidentally build a bloated app. The safeguard won't work for add to
app, from what I understand, because while they use the FGP (so they
should be forced to be download the NDK) they don't invoke the flutter
tool, and therefore won't invoke the custom error handling of
[`gradle_errors.dart`](https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/android/gradle_errors.dart)
Some details
1. Respects existing values for `externalNativeBuild.cmake.path` for
apps that actually use it.
2. Silences some warnings that would otherwise appear for add to app
builds or builds that manually invoke gradle:
```
C/C++: CMake Warning (dev) in CMakeLists.txt:
C/C++: No project() command is present. The top-level CMakeLists.txt file must
C/C++: contain a literal, direct call to the project() command. Add a line of
C/C++: code such as
C/C++: project(ProjectName)
C/C++: near the top of the file, but after cmake_minimum_required().
C/C++: CMake is pretending there is a "project(Project)" command on the first
C/C++: line.
C/C++: This warning is for project developers. Use -Wno-dev to suppress it.
C/C++: CMake Warning:
C/C++: Manually-specified variables were not used by the project:
C/C++: CMAKE_EXPORT_COMPILE_COMMANDS
C/C++: CMAKE_LIBRARY_OUTPUT_DIRECTORY
C/C++: CMAKE_RUNTIME_OUTPUT_DIRECTORY
```
3. Our ci installs the NDK at an abnormal place that AGP can't find
without help. I've modified all the `build.gradle`s that we have checked
in to point to the pre-installed path. **But some of our tests make a
new app from the templates, and those tests will now start downloading
the NDK** (as they won't be able to find it at it's current path from
templates). We could resolve this by actually fixing
https://github.com/flutter/flutter/issues/136666. This would be a very
significant lift from what I understand - we rely on this hardcoding in
a lot of places in our infra.
Fixes https://github.com/flutter/flutter/issues/155576
## Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
---------
Co-authored-by: Gray Mackall <mackall@google.com>
In the future, it will be possible for Swift Package Manager to be enabled on one but not all platforms (see https://github.com/flutter/flutter/issues/151567#issuecomment-2455941279).
This updates the `.flutter-plugin-dependencies` file format to separate iOS's and macOS's SwiftPM enablement. For now, these platforms will always have the same value.
This `.flutter-plugin-dependencies` file is read by our CocoaPods scripts to determine whether we should use CocoaPods or not to inject plugins.
Part of https://github.com/flutter/flutter/issues/151567
Fix the `--ab` option in the benchmark harness:
- Make `--local-engine-host` optional. The web engine doesn't need it, so it doesn't build it. But the tool crashes by failing to find it.
- Disable icon tree shaking because `--ab` runs against local engine build, whose Dart kernel version is out of sync with the framework, which crashes the const finder.
Reverts: flutter/flutter#157032
Initiated by: gmackall
Reason for reverting: https://github.com/flutter/flutter/pull/157032#issuecomment-2436336078
Original PR Author: gmackall
Reviewed By: {reidbaker, bartekpacia}
This change reverts the following previous change:
I recently noticed the following log when building an app in verbose mode:
```
This Android Gradle plugin (8.1.0) was tested up to compileSdk = 33 (and compileSdkPreview = "UpsideDownCakePrivacySandbox").
You are strongly encouraged to update your project to use a newer
Android Gradle plugin that has been tested with compileSdk = 35.
```
It looks like AGP would like us to use a newer AGP version if we want to use compileSdk 35 (which we do). This pr upgrades the tests, in advance of updating the templates.
I recently noticed the following log when building an app in verbose mode:
```
This Android Gradle plugin (8.1.0) was tested up to compileSdk = 33 (and compileSdkPreview = "UpsideDownCakePrivacySandbox").
You are strongly encouraged to update your project to use a newer
Android Gradle plugin that has been tested with compileSdk = 35.
```
It looks like AGP would like us to use a newer AGP version if we want to use compileSdk 35 (which we do). This pr upgrades the tests, in advance of updating the templates.
Fixes https://github.com/flutter/flutter/issues/157359
Prevents Flutter.xcframework.dSYM from being copied into the App.framework folder. I am not 100% positive if there are cases where it's valid to have multiple dSYMs in that folder, so I'm just string matching and filtering out `Flutter.xcframework.dSYM`
Reverts: flutter/flutter#154061
Initiated by: QuncCccccc
Reason for reverting: might be the reason that cause Framework tree to red
Original PR Author: bartekpacia
Reviewed By: {gmackall, reidbaker}
This change reverts the following previous change:
This PR resolves#151166
Allows applying of `include_flutter.groovy` via the `apply from:` syntax, which allows using a host app that is using the Gradle Kotlin DSL (the default these days when creating an Android app in AS).
Explanation: The `include_flutter.groovy` script is currently not able to be called by Kotlin gradle files, because it is [intended to be invoked with the following lines](https://docs.flutter.dev/add-to-app/android/project-setup#depend-on-the-modules-source-code):
```
setBinding(new Binding([gradle: this])) // new
evaluate(new File( // new
settingsDir.parentFile, // new
'flutter_module/.android/include_flutter.groovy' // new
))
```
`setBinding` isn't part of the Kotlin gradle DSL, and there isn't (that I can find) an easy Kotlin equivalent. If this binding isn't set, the reference to `gradle` in `include_flutter.groovy` is wrong, which breaks the script.
This PR modifies `include_flutter.groovy` to also support being invoked through the standard way of invoking a script via the Gradle Groovy/Kotlin DSLs, which is `apply from:` (or it's slightly different Kotlin syntax). The start of the script identifies which of the two approaches is being used by checking if the binding is set, and then initializes some variables differently depending on the case.
If we land this, I believe we should update the example Gradle files for both the `kts` and `groovy` cases to prefer the `apply from` syntax as I think this is the syntax most developers would be more familiar with already seeing in their Gradle files.
Fixes https://github.com/flutter/flutter/issues/156456
There is a known long running test that has been bumping up against the default timeout. This adjusts the perTestTimeout for the devicelab tests to add 15 seconds to stop this from flaking.
The test is question: 82ebb74c64/dev/devicelab/test/run_test.dart (L76)
`test/run_test.dart: run.dart script prints a message after a few seconds when failing to connect (this test takes >10s)`
This test waits on the application to print a message before
continuting, but that message does not reach the test post UI/platform
thread merge.
The test otherwise seems to function fine, and the app isn't crashing.
Reverts: flutter/flutter#156440
Initiated by: zanderso
Reason for reverting: Failing in post submit with
```
[2024-10-08 18:00:22.743647] [STDOUT] stdout: [!] CocoaPods could not find compatible versions for pod "Google-Mobile-Ads-SDK":
[2024-10-08 18:00:22.743695] [STDOUT] stdout: In Podfile:
[2024-10-08 18:00:22.743718] [STDOUT] stdout: google_mobile_ads (from `.symlinks/plugins/google_mobile_ads/ios`) was resolved t
Original PR Author: flutter-pub-roller-bot
Reviewed By: {fluttergithubbot}
This change reverts the following previous change:
This PR was generated by `flutter update-packages --force-upgrade`.
This pull request aims to improve code readability, based on feedback gathered in a recent design doc.
<br>
There are two factors that hugely impact how easy it is to understand a piece of code: **verbosity** and **complexity**.
Reducing **verbosity** is important, because boilerplate makes a project more difficult to navigate. It also has a tendency to make one's eyes gloss over, and subtle typos/bugs become more likely to slip through.
Reducing **complexity** makes the code more accessible to more people. This is especially important for open-source projects like Flutter, where the code is read by those who make contributions, as well as others who read through source code as they debug their own projects.
<hr>
<br>
The following examples show how pattern-matching might affect these two factors:
<details> <summary><h3>Example 1 (GOOD)</h3> [click to expand]</summary>
```dart
if (ancestor case InheritedElement(:final InheritedTheme widget)) {
themes.add(widget);
}
```
Without using patterns, this might expand to
```dart
if (ancestor is InheritedElement) {
final InheritedWidget widget = ancestor.widget;
if (widget is InheritedTheme) {
themes.add(widget);
}
}
```
Had `ancestor` been a non-local variable, it would need to be "converted" as well:
```dart
final Element ancestor = this.ancestor;
if (ancestor is InheritedElement) {
final InheritedWidget inheritedWidget = ancestor.widget;
if (widget is InheritedTheme) {
themes.add(theme);
}
}
```
</details>
<details> <summary><h3>Example 2 (BAD) </h3> [click to expand]</summary>
```dart
if (widget case PreferredSizeWidget(preferredSize: Size(:final double height))) {
return height;
}
```
Assuming `widget` is a non-local variable, this would expand to:
```dart
final Widget widget = this.widget;
if (widget is PreferredSizeWidget) {
return widget.preferredSize.height;
}
```
<br>
</details>
In both of the examples above, an `if-case` statement simultaneously verifies that an object meets the specified criteria and performs a variable assignment accordingly.
But there are some differences: Example 2 uses a more deeply-nested pattern than Example 1 but makes fewer useful checks.
**Example 1:**
- checks that `ancestor` is an `InheritedElement`
- checks that the inherited element's `widget` is an `InheritedTheme`
**Example 2:**
- checks that `widget` is a `PreferredSizeWidget`
(every `PreferredSizeWidget` has a `size` field, and every `Size` has a `height` field)
<br>
<hr>
I feel hesitant to try presenting a set of cut-and-dry rules as to which scenarios should/shouldn't use pattern-matching, since there are an abundance of different types of patterns, and an abundance of different places where they might be used.
But hopefully the conversations we've had recently will help us converge toward a common intuition of how pattern-matching can best be utilized for improved readability.
<br><br>
- resolves https://github.com/flutter/flutter/issues/152313
- Design Doc: [flutter.dev/go/dart-patterns](https://flutter.dev/go/dart-patterns)
Prior to this fix, `_TaskRunner.run` would immediately cleanup the
keep-alive port once the task completed, which would result in the
isolate shutting down as soon as the task result was returned from
`ext.cocoonRunTask` callback in the form of a
`ServiceExtensionResponse`. Since the service extension response is
processed by the service isolate, it was possible for the VM to start
shutting down before the service isolate could send the task result data
back to the task runner.
This change introduces a new service extension,
`ext.cocoonTaskResultReceived`, that the task runner invokes after it
receives the task result from `ext.cocoonRunTask`, notifying the task
process that it can close the keep-alive port and shutdown.
Fixes https://github.com/flutter/flutter/issues/155475
This PR addresses an issue where the `--target-platform` flag was not being respected when building APKs in debug mode. Previously, debug builds would always include `x86` and `x64` architectures, regardless of the specified target platform. This change ensures that the `--target-platform` flag is honored across all build modes, including debug.
To achieve this, `BuildApkCommand` has been slightly changed to become responsible for list of archs that should be built in the current run,rather than just parsing arguments. Previously, this responsibility was distributed to gradle, which could be frustrating (in my opinion)
Fixes#153359
*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*
*List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.*
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
https://github.com/flutter/flutter/pull/151675 bumped module templates to AGP 8.1.
In doing so, I tried to work around a behavior change [that was new in AGP 8.0](https://developer.android.com/build/releases/past-releases/agp-8-0-0-release-notes):
> AGP 8.0 creates no SoftwareComponent by default. Instead AGP creates SoftwareComponents only for variants that are configured to be published using the publishing DSL.
by using AGP's publishing DSL to define which variants to publish in the module's ephemeral gradle files:
```
android.buildTypes.all {buildType ->
if (!android.productFlavors.isEmpty()) {
android.productFlavors.all{productFlavor ->
android.publishing.singleVariant(productFlavor.name + buildType.name.capitalize()) {
withSourcesJar()
withJavadocJar()
}
}
} else {
android.publishing.singleVariant(buildType.name) {
withSourcesJar()
withJavadocJar()
}
}
}
```
The problem is that this doesn't get applied to the plugin projects used by the module, so if a module uses any plugin it breaks. This PR fixes that by applying similar logic, but to each project (not just the module's project).
Tested manually with https://github.com/gmackall/GrayAddToApp, and also re-enabled an old test that tested this use case as a part of the PR.
Fixes: https://github.com/flutter/flutter/issues/154371