This PR rolls in a number of breaking changes from dart-lang/native:
* `BuildMode` is no longer part of the protocol, so Flutter no longer
passes it in.
* This means all code dealing with the name conflict between
`native_assets_cli.BuildMode` and `flutter_tools.BuildMode` has been
cleaned up.
* Also, the logs no longer mention the build mode.
* The tests still exercise both modes, because linking only happens in
release mode.
* `OS` is no longer part of the main protocol, but of the "code"
"protocol extension".
* The code now aligns more with `OS?` being nullable in a bunch of
places, since it is nullable if there's no code assets.
* The OS-specific config is nested in an object per OS.
* `CCompilerConfig`s fields are non-nullable now.
* So instead of passing an object with nullable fields around, a null
instead of the object is returned in various places.
* `FileSystem` is now passed in to the native assets builder.
This PR contains no feature changes.
This PR will need to be followed up by restricting what environment
variables are passed in (similar to
https://github.com/dart-lang/native/pull/1764), I will do this in a
follow up PR.
Tests:
* All existing features should be covered by existing tests.
In the future a hook may be invoked multiple times with different
`supportedAssetTypes` (soon to be renamed to `buildAssetTypes`).
The hook should only emit those asset types that are in
`supportedAssetTypes` - anything else is an error. Right now flutter
happens to invoke hooks only with `Code` asset types, but more asset
types are coming, for which this PR is a preparation for.
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>
First part of https://github.com/flutter/flutter/issues/160379.
This should be a NOP, because without the
`explicit-package-dependencies` feature flag, all dependencies
(including dev_dependencies) are considered, for the purposes of
plugins, non-dev dependencies.
This change prepares for flipping that flag, so when the flip occurs, it
is _also_ a NOP for these tests.
<!-- 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>
Almost all of the code is just adopting to changes to the APIs of
`package:native_assets_builder`, `package:native_assets_cli` and
`package:native_toolchain_c`
There's only two semantic changes
* Removes a test that checks for a verification error if a build hook
produces a static library if the preferred linking mode is dynamic:
=> The test is written in a very hacky way. By monkey patching the build
config.json that flutter build actually made. This monkey patching
relies on package:cli_config which is now no longer used.
=> The actual code that checks for this mismatch lives in
dart-lang/native repository and is tested there. So there's really no
need to duplicate that.
* The `package:native_assets_builder` no longer knows about code assets.
This is something a user of that package (e.g. flutter tools) adds. Now
the dry-run functionality will invoke build hooks who produce code
assets without an architecture.
=> The `package:native_assets_builder` used to expand such a code asset
to N different code assets (one for each supported architecture)
=> This logic was now moved to flutter tools. => In the near future
we're going to this dry-run complexity, which will then also get rid of
this uglyness (of expanding to all archs of an OS).
We added `use_modular_headers!` to our `Podfile`s as we originally planned to phase out `use_frameworks!` (see https://github.com/flutter/flutter/pull/42204). However, our plans have now changed and we are instead phasing out CocoaPods entirely in favor of Swift Package Manager.
CocoaPods's `use_frameworks!` and `use_modular_headers!` are two different overlapping options that should not be used together. This change removes the `use_modular_headers!` from the macOS `Podfile` and the iOS Swift `Podfile` (the iOS Objective-C template was recently deprecated https://github.com/flutter/flutter/pull/155867).
This change only affects _new_ Flutter apps. This change does not include an automatic migration as that could break existing apps. Instead, users are encouraged to migrate from CocoaPods to Swift Package Manager.
https://github.com/flutter/flutter/issues/156259
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.
This is simply removing unnecessary parenthesis from various places. This change is because of a change to the unnecessary_parentesis lint that will trigger there. Here is the CL https://dart-review.googlesource.com/c/sdk/+/390161.
- https://github.com/dart-lang/linter/issues/4996
If anything else is needed please let me know.
I'd like to ask for this PR to wait a bit until the bots are run again on that CL so that I can be sure nothing else will trigger, I will come back here and update this whenever everything is complete. Thanks!
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.
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)
*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].*
