Fixes#162107
Commands that were helpful when working on this pr.
`dart dev/tools/bin/generate_gradle_lockfiles.dart
--no-gradle-generation` from flutter/flutter root.
`./gradlew test` from packages/flutter_tools/gradle.
`git add -- ":*.lockfile"` for adding only lockfile changes.
`../../bin/cache/dart-sdk/bin/dart bin/test_runner.dart test -t
android_java11_dependency_smoke_tests` from dev/devicelab
`ktlint --editorconfig=dev/bots/test/analyze-test-input/.editorconfig
--baseline=dev/bots/test/analyze-test-input/ktlint-baseline.xml
packages/flutter_tools/gradle/src/ --format` formatting kotlin code.
Need ktlint 1.5
## 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.
- [x] All existing and new tests are passing.
## `integration_ui_keyboard_resize` flakiness reduction
This PR addresses flakiness in the `integration_ui_keyboard_resize` test
(issue #161300) and slightly improves its performance.
**Issue summary:**
The `integration_ui_keyboard_resize` test occasionally fails because the
device keyboard does not reliably open at the start of the test. This PR
mitigates one cause of this flakiness, though other potential underlying
issues may still exist. See #161300 for a detailed investigation.
**Changes and rationale:**
1. **Improved keyboard detection logic (flakiness reduction):**
* The core change moves `await driver.tap(defaultTextField);` *inside*
the loop that checks for layout changes. This ensures the test
repeatedly attempts to open the keyboard *while* waiting for the
expected layout shift, significantly improving reliability.
* **Local testing results (MacBook Pro M1 + Pixel 8 Pro):**
* **Original implementation:** Consistent failures within the first 15
iterations.
* **This PR:** Ran consistently for at least 300 iterations (out of
2000) before any failure, with some runs exceeding 900 iterations.
2. **`enableTextEntryEmulation` configuration improvement:**
* The `keyboard_resize.dart` app is *exclusively* used for testing and
requires `enableTextEntryEmulation: false` for proper device keyboard
interaction.
* This PR sets `enableTextEntryEmulation: false` directly within the
app's `enableFlutterDriverExtension` setup. This simplifies the test
setup, avoids cross-thread communication, and prevents the app from
appearing broken when run independently (as manual text input wouldn't
work otherwise).
3. **UI enhancements for manual testing:**
* Added `SafeArea` and `InputDecoration` to `keyboard_resize.dart`. This
ensures the `TextField` is fully visible and usable during manual
testing (it was previously obscured by the Android status bar). This
improves the developer experience when debugging the app directly.
4. **Optimized polling for speed:**
* Reduced the polling interval and increased the number of polling
iterations (maintaining the overall timeout). This change resulted in a
~2-second speed improvement per test iteration during local testing.
**Further flakiness improvement discussion and ideas**
See the discussion at https://github.com/flutter/flutter/issues/161300
**Testing and reproduction:**
To reproduce the original flakiness and verify the fix (reproducibility
may vary based on setup; see
https://github.com/flutter/flutter/issues/161300#issuecomment-2618952501):
1. **Disable automatic reboots:** Comment out the `await
checkForRebootRequired();` line in
9e273d5e6e/dev/devicelab/lib/framework/framework.dart (L240-L243)
2. **Disable automatic retries:** Set `static const int retryNumber =
0;` in
9e273d5e6e/dev/devicelab/lib/framework/cocoon.dart (L56-L57)
3. **Install `hyperfine`:** This is a command-line benchmarking tool
(e.g., `brew install hyperfine` on macOS).
4. **Connect a device/emulator:** Connect a physical Android device or
start an emulator.
5. **Run repeated tests:** Use `hyperfine` to run the test multiple
times and capture the output:
```bash
hyperfine -r 100 'dart bin/test_runner.dart test -t
integration_ui_keyboard_resize > log.txt'
```
Related to #149836
Find all jvmTarget definitions that do not use JavaVersion.* then update
them.
While editing those files align the usages with
docs/contributing/Android-API-And-Related-Versions.md.
Documentation source that this pr follows
https://github.com/flutter/flutter/pull/164198/files#diff-ee6ec18be8d752e2696c8ccc8bec2f202dfc29a43b3b4f9d8041aa6bc3e852a1
This pr is expected to cause no behavioral changes.
This pr makes logical sense after
https://github.com/flutter/flutter/pull/164198 but can be landed in any
order.
## 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 `///`).
- [ ] 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.
- [x] All existing and new tests are passing.
Related to #149836
- Remove last usages of minSdkVersion/compileSdkVersion/targetSdkVersion
in favor of minSdk/compileSdk/targetSdk. That do not also use
`flutter.minSdkVersion` etc. (that will happen in another pass)
- Update the files that used "SdkVersion" to use equals for setting new
values. [Not yet landed documentation
change](https://github.com/flutter/flutter/pull/164195/files#diff-ee6ec18be8d752e2696c8ccc8bec2f202dfc29a43b3b4f9d8041aa6bc3e852a1)
- align sourceCompatibility and targetCompatibility across repo to use
JavaVersion.* and update android version documentation.
This pr updates `kotlinOptions jvmTarget` documentation but does not fix
all existing usages.
This pr is expected to cause no behavioral changes.
This pr makes logical sense after
https://github.com/flutter/flutter/pull/164195 but can be landed in any
order.
## 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 `///`).
- [ ] 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.
- [x] All existing and new tests are passing.
Related to #149836
Last of the non api 35 references in flutter/flutter for targetSdk and
compileSdk.
`find . -type f -name "build.gradle" | xargs grep -e "targetSdk" | tr -d
'=' | tr -s ' ' | grep -v flutter\.targetSdkVersion | grep -v
engine/src/flutter/third_party/ | grep -v "targetSdk 35"`
`find . -type f -name "build.gradle" | xargs grep -e "compileSdk" | tr
-d '=' | tr -s ' ' | grep -v flutter\.compileSdkVersion | grep -v
engine/src/flutter/third_party/ | grep -v "compileSdk 35"`
rewrite of https://github.com/flutter/flutter/pull/163622 after this
test was updated to run against a newer version of gradle and agp in
https://github.com/flutter/flutter/pull/163849/files#diff-83b6ad7c016bffbb682664eb65c576a7ebf9c312fc60727c0e0e20f5641cbc2aR462
Android-API-And-Related-Versions.md was updated to reflect that after
further investigation that android is using "=" in their documentation
and that the space syntax is discouraged. Equals is setting a property
and space is the equivalent to a function call like `compileSdk(35)` and
assignment is prefered.
The use of equals for proprty assignment also aligns with
https://docs.gradle.org/current/userguide/migrating_from_groovy_to_kotlin_dsl.html#prepare_your_groovy_scripts
## 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.
- [x] All existing and new tests are passing.
Fixes#163750
Adds local.properties in any directory to the repo wide gitignore.
Test changes to make the test easier to debug, specifically identifying
the difference between a debug and release failure.
## 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.
- [x] All existing and new tests are passing.
- **update pure_android_host_apps/android_custom_host_app to
compileSdk/targetSdk 35, newest stable agp and gradle versions, update
source compatibility to java 17, updated dependencies**
- **Update documentation and migrate compileSdkVersion to compileSdk**
- **Update gitignore to ignore specific module folder name and
local.properties repo wide**
Related to #149836
## 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.
- [x] All existing and new tests are passing.
[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
Closes https://github.com/flutter/flutter/issues/160257.
~~This will sit on CI for a bit and give me a better idea of how much
work there is to do.~~ Done!
## IMPORTANT: Do **not** revert on 1-2 isolated post-submit failures.
While I've spent the last several weeks getting this passing all
presubmit tests, and some integration tests I suspected might be
affected, it is possible that there are 1-2 integration tests that will
fail as a result of landing this PR. I'll disable the flag
(`--no-explicit-package-dependencies`) if the failures look obvious
enough, otherwise I'll revert.
Part of #149836
## 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.
- [x] All existing and new tests are passing.
Related to #163228
## 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.
- [ ] 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].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.
Fixes https://github.com/flutter/flutter/issues/162615
Can test 2 different ways.
On a mac (or linux machine) with adb on the path (or android sdk set in
ANDROID_HOME) and an emulator running (or physical device attached) that
is api 30 or higher.
```
cd dev/devicelab
dart bin/test_runner.dart test -t android_display_cutout
```
OR
```
dev/integration_tests/display_cutout_rotation
flutter drive integration_test/display_cutout_test.dart
```
Proof the test ran successfully
```
[2025-02-12 08:08:22.069817] [STDOUT] Removing Synthetic notch...
[2025-02-12 08:08:22.071147] [STDOUT] Executing "/b/s/w/ir/cache/android/sdk/platform-tools/adb -s emulator-5554 shell cmd overlay disable com.android.internal.display.cutout.emulation.tall" in "/b/s/w/ir/x/w/rc/tmpk3k3yhhp/flutter sdk/dev/integration_tests/display_cutout_rotation/" with environment {BOT: true, LANG: en_US.UTF-8}
[2025-02-12 08:08:22.862219] [STDOUT] Checking for reboot
[android_defines_test] Process terminated with exit code 0.
Task result:
{
"success": true,
"data": null,
"detailFiles": [],
"benchmarkScoreKeys": [],
"reason": "success"
}
```
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8723125792202374961/+/u/run_android_defines_test/stdout
All checks passed
https://github.com/flutter/flutter/pull/162641/checks?check_run_id=36991537539
## 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.
- [x] All existing and new tests are passing.
When there are no more platform views, make sure the overlay layer is
hidden. When a platform view is added again, show the overlay. The
show/hide allows us to avoid continually recreating and destroying the
overlay surface + swapchain when a platform view slides in and out of
frame.
To further reduce memory usage, we could do a delayed de-allocation of
the overlay layer (say after 50 frames of no platform view, destroy it).
But I'm leaving this to a follow up.
Developers will need to _conditionally_ use HCPP (or the framework will
need to handle it automatically). This requires the ability to query at
runtime whether HCPP mode is enabled + supported.
Add a message channel to do so, and add the usage of this to the
android_engine_test. Does not yet add any automatic selection.
---------
Co-authored-by: Matan Lurey <matanlurey@users.noreply.github.com>
No bug adding to documentation.
## 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].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [x] 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].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.
Another large test suite that has never been running.
The underlying thing I _think_ it wanted tested, which was hybrid
composition, is now tested by `android_engine_test`. I also added a red
square on top of each platform view so that, like
`hybrid_android_views`, we can make sure layering is working as
expected.
Any other ideas? Can also be follow-up PRs?
Someone could always resurrect it via `git`-magic (or GitHub), but let's
not maintain (including Gradle lock files, Dart analysis, etc) unused
code that hasn't run in 12+ months.
/cc @johnmccutchan
Use the PlatformViewController2 to register a platform view, allow the
dart side platform view logic to opt into this new platform view. Wires
up an integration test with android_engine_test.
Powered by the new and amazing `unnecessary_ignore` lint.
We're not enabling this lint by default because being able to
temporarily use ignores that don't ignore anything is a powerful tool to
enable migrations. We should turn this lint on locally periodically,
though, and clean up all outdated ignores.
## 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].
- [ ] 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].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.
Trying to figure out if flake reports
https://github.com/flutter/flutter/issues/162362 and
https://github.com/flutter/flutter/issues/162363 are related to
https://github.com/flutter/flutter/issues/162310.
I was unable to reproduce locally:
```sh
dart tool/deflake.dart lib/platform_view/hybrid_composition_platform_view_main.dart
```
... so either the Mac Android emulators don't show this behavior, or
perhaps its just the slowness of CI VMs.
Either way, this should try taking a screenshot 2 more times (which has
built-in waits/sleeps), so if the problem persists that means there is a
more critical HC problem (the screen is _always_ drawn black sometimes)
versus a more transient problem (it "takes longer").
A few changes in this PR (could have been split up, but that would take
an extra 4 hours - 2 days in CI time):
- Removed the old `flutter_driver_android_test` shards and references
- Override `AndroidManifest.xml` per backend, forcing that backend to be
used (no fallbacks) or it fails
- Bumps the Android emulator to 35 due to the use of magic strings, I
think
The check for the Impeller backend actually being used is the honor
system right now, as a bug in `flutter drive`
(https://github.com/flutter/flutter/issues/162087) prevents me from
seeing error output, meaning that it's impossible to be sure a
particular Impeller backend is used (or understand why a crash happens),
so I guess I'll work on that next.
Due to https://github.com/flutter/flutter/issues/162088, we were
accidentally running our Vulkan tests as OpenGLES 🤦🏼 .
Test that the position of a cutout as reported by the Android engine
repositions based on screen orientation.
Related to https://github.com/flutter/engine/pull/55992
Part of https://github.com/flutter/flutter/issues/155658
to test run flutter drive integration_test/display_cutout_test.dart
from dev/integration_tests/display_cutout_rotation
Pr also force upgrades pub dependencies because I was getting presubmit
failure in version solve.
## 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.
- [x] All existing and new tests are passing.
Towards https://github.com/flutter/flutter/issues/161333.
This adds the initial `.ci.yaml` configuration (marked `bringup: true`),
without changing the existing shard to avoid the problems we ran into
last time when I renamed the shard atomically, as well as piping for
providing context in which the test will run (Vulkan or OpenGLES), which
can't be introspected in the _running_ app.
In a follow-up PR I'll:
- Edit `AndroidManifest.xml` before the suite, setting `ImpellerBackend`
appropriately
- Move the bringup nodes to presubmit
- Delete the existing (legacy) node
The goal here is to have a great standalone `android_engine_test` suite
that [replaces
`scenario_app/android`](https://github.com/flutter/flutter/pull/160992).
No test is _functionally_ changed in this PR, but overview of changes:
- Finished renaming the suite `android_engine_tests` instead of
`flutter_driver_android`
- Added instructions and an environment variable for local generation of
golden-files (`UPDATE_GOLDENS=1`)
- Added explanations of the individual tests, where they live, and how
to run them locally
- Added a hybrid-composition (HC, not TLHC, which is already tested)
test
- Renamed "other_smiley" to "surface_texture_smiley" (and renamed the
original to "surface_producer_smiley")
- Removed unnecessary ".android" suffix (we will not run this on
anything but Android)
- Added a `tool/deflake.dart` to run a test suite 10x (or custom) times
locally to try and determine flakiness
After this PR, I'll add flags to let you control variants and name the
screenshots accordingly, i.e.:
- API v34 or v35
- OpenGLES or Vulkan (will require an `AndroidManifest.xml` edit during
the test instrumentation)
Towards https://github.com/flutter/flutter/issues/161261 (before I add
more tests).
This is a 1:1 move, with massaging of
`run_flutter_driver_android_tests.dart` to be more automated as tests
are added.
/cc @reidbaker
imperative apply has been deprecated since #139690
part of #121541
Doing this will make things easier for us during the conversion of
`flutter.groovy` into Kotlin.
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.
