Towards https://github.com/flutter/flutter/issues/159000 as part of
debugging issues such as
https://github.com/flutter/flutter/issues/158560.
On a `bringup: true` shard, run a number of `flutter build apk` tests in
succession to try and routinely trigger timeouts and crashes, so we can
test other hypotheses on how to fix this problem (i.e. potentially
around increasing memory, changing daemon configuration, aggressively
killing processes, etc).
Currently the `NativeAsset` target in flutter tools is responsible for
two things:
* performing the dart build (in the app as well as all transitive pub
dependencies)
* taking output (shared libraries) from this build and copying them
around
This intermingling of responsibilities leads to more complex code and
potentially unnecessary work: If the source code changed (e.g. `.c`
files change) we have to run the dart build again. But doing so may
result in the same shared libraries (e.g. adding comments to the `.c`
code). Currently we're going to copy the shared libraries despite them
having not changed, which then may cause upstream things to be dirtied
(if it's based on timestamp of files) and re-built.
Instead this PR splits this `NativeAsset` into the two orthogonal pieces
* `DartBuild` target that is responsible for the dart build
* `InstallCodeAssets` that is responsible for copying shared libraries
to the right place and producing a `native_assets.yaml`.
This decoupling is also preparation for a future where a dart build can
produce other kinds of assets (e.g. data assets) and is used in the web
build as well. (The web build would use `DartBuild` but not
`InstalCodeAssets`).
After the dart build is done, the flutter tool has to bundle the
produced shared libraries, which it does that by copying them around.
Though the code assumed that all code assets are shared libraries to be
bundled, whereas in fact one can have code assets without any actual
code (ones that are installed on the target system already or artificial
code assets whose symbols get resolved from executable / process).
=> Using non-bundled code assets currently results in null pointer
exceptions and/or cast errors.
=> We update the copy code to only operate on code assets that have a
shared library to bundle.
We also update the copy routines by removing copy&past'ed - but slightly
different - printing code into the shared caller function.
In release builds linking of native assets is enabled. The build step is
only a temprary step, it's output is given to the link step which then
returns all final assets (effectively a map-reduce system). Assets that
aren't sent to a specific linker could be conceptually viewed as sent to
a linker that will emit it's input as-is.
=> The code currently took output of build & link step and therefore
accumulated assets that aren't explicitly sent to a linker twice.
=> This led to performing work twice for those (e.g. copying them twice)
This PR changes this such that if linking mode is enabled, we only rely
on the output of the link phase.
That in return means many tests that mock the native asset builds need
to be updated to mock the output of the link phase.
This started happening after moving DDS to launch from `dart
development-service` rather than `DartDevelopmentService` (see
33b402d24c2131dcf97e272ec3d59c0249762e7f) . This state error was
originally meant to be thrown when some string parsing failed, but is
currently wrapping the `DartDevelopmentServiceException`.
Fixes https://github.com/flutter/flutter/issues/158537
This adds support for web 1.x.x to be included in the plugin template, when creating a web plugin.
I don't have an issue referenced. Related to https://github.com/dart-lang/web/issues/316
I was poking around in logger.dart when I noticed a few classes have unnecessary exposed member variables. This PR reduces visibility of these in the pursuit of making these classes slightly easier to grok.
<details>
<summary> Pre-launch checklist </summary>
</details>
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>
It would be nice to have somewhere to iterate and experiment with what we can do to either fix frequent timeout problems we have with `flutter build apk` (across platforms, to be clear, though I've just started with Linux) or get more information on why the crashes/timeouts happen.
Open to other ways to doing this (though preferably _not_ LED).
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 moves the `usesSwiftPackageManager` property from the platform-agnostic `Project` to the platform-specific `IosProject` and `MacOSProject`.
This will allow the `IosProject` and `MacOSProject` to return different values for `usesSwiftPackageManager` in the future. For now, both of these projects will always return the same value.
Part of https://github.com/flutter/flutter/issues/151567
Closes https://github.com/flutter/flutter/issues/157629.
Avoids failures in the style:
```txt
00:37 +4597 ~3 -1: /Users/matanl/Developer/flutter/packages/flutter_tools/test/commands.shard/permeable/build_appbundle_test.dart: analytics logs success [E]
Expected: contains Event:<{"eventName":"flutter_command_result","eventData":{"commandPath":"create","result":"success","commandHasTerminal":false,"maxRss":147013632}}>
Actual: [
Event:{"eventName":"command_usage_values","eventData":{"workflow":"create","commandHasTerminal":false,"createProjectType":"app","createAndroidLanguage":"kotlin","createIosLanguage":"swift"}},
Event:{"eventName":"flutter_command_result","eventData":{"commandPath":"create","result":"success","commandHasTerminal":false,"maxRss":143261696}},
Event:{"eventName":"timing","eventData":{"workflow":"flutter","variableName":"create","elapsedMilliseconds":527,"label":"success"}},
Event:{"eventName":"command_usage_values","eventData":{"workflow":"appbundle","commandHasTerminal":false,"buildAppBundleTargetPlatform":"android-arm,android-arm64,android-x64","buildAppBundleBuildMode":"release"}},
Event:{"eventName":"flutter_command_result","eventData":{"commandPath":"appbundle","result":"success","commandHasTerminal":false,"maxRss":147013632}},
Event:{"eventName":"timing","eventData":{"workflow":"flutter","variableName":"appbundle","elapsedMilliseconds":10,"label":"success"}}
]
Which: does not contain Event:<{"eventName":"flutter_command_result","eventData":{"commandPath":"create","result":"success","commandHasTerminal":false,"maxRss":147013632}}>
```
I am guessing <https://api.flutter.dev/flutter/dart-io/ProcessInfo/maxRss.html> is not guaranteed to be stable while running, and as a result is not. This stabilizes the value for a test that I believe does not care about getting the "real" value anyway.
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
Fixes https://github.com/flutter/flutter/issues/153777.
To summarize that issue, `ErrorHandlingFileSystem.systemTempDirectory` calls [`LocalFileSystem.systemTempDirectory`](45c8881eb2/packages/flutter_tools/lib/src/base/file_system.dart (L229)), which makes a `Directory.createSync` call, which can throw exceptions that _should_ be handled and result in a graceful tool exit (e.g. insufficient storage). However, we aren't catching those, hence this issue.
All we need to do is wrap that call with the `FileSystemException`-handling logic we already have in the tool. See the diff.
I don't think I'll be cherry-picking this since 1) it's not an extremely common crash and 2) users can probably pick apart the crash message and figure out that they need to clear some storage space to proceed.
<details>
<summary> Pre-launch checklist </summary>
</details>
Removes duplicated constants and ensures consistency by using package:vm_service as a source of truth for RPC error codes for requests made with package:vm_service.
**PR Title:**
Remove block and line comments when detecting `'.flutter-plugins'` in `settings.gradle`
---
**Description:**
This PR modifies the `configureLegacyPluginEachProjects` function to remove block (`/* ... */`) and line (`// ...`) comments from the `settings.gradle` or `settings.gradle.kts` file content before checking for the presence of the `'.flutter-plugins'` string. This ensures that only uncommented, meaningful code is considered during the detection, preventing false positives when the string appears within comments.
**Why is this change necessary?**
In some cases, the `'.flutter-plugins'` string may be present inside comments in the `settings.gradle` file. The existing implementation does not account for this and may incorrectly detect the string even when it's commented out. This can lead to unintended behavior, such as configuring plugin projects when it is not necessary.
By removing comments before performing the check, we prevent false positives and ensure that the detection logic is accurate, only acting when the `'.flutter-plugins'` string is present in active code.
**Changes Made:**
- **Added comment removal logic:**
- Removed block comments (`/* ... */`) using the regular expression `/(?s)\/\*.*?\*\//`.
- The `(?s)` flag enables dot-all mode, allowing `.` to match newline characters.
- Removed line comments (`// ...`) using the regular expression `/(?m)\/\/.*$`.
- The `(?m)` flag enables multi-line mode, so `^` and `$` match the start and end of each line.
- Combined both comment removal steps into a single chain for efficiency.
- **Updated the string detection:**
- The check for `'.flutter-plugins'` is now performed on the uncommented content of the `settings.gradle` file.
- This ensures that only meaningful, uncommented code is considered during detection.
**Issue Fixed:**
- Fixes [#155484](https://github.com/flutter/flutter/issues/155484)
---
---
If you need any further assistance or have questions, feel free to reach out!
---
**Links:**
- [Contributor Guide]
- [Tree Hygiene]
- [Flutter Style Guide]
- [Features we expect every widget to implement]
- [CLA]
- [flutter/tests]
- [breaking change policy]
- [Discord]
- [Data Driven Fixes]
Work towards https://github.com/flutter/flutter/issues/56591.
I explicitly want an LGTM from @andrewkolos @jmagman @jonahwilliams before merging.
---
After this PR, `<Plugin>.isDevDependency` is resolved based on the following logic, IFF:
- The plugin comes from a package _A_ listed in the app's package's `dev_dependencies: ...`
- The package _A_ is not a normal dependency of any transitive non-dev dependency of the app
See [`compute_dev_dependencies_test.dart`](51676093a3/packages/flutter_tools/test/general.shard/compute_dev_dependencies_test.dart) for probably the best specification of this behavior.
We (still) do not write the property to disk (i.e. it never makes it to `.flutter-plugins-dependencies`), so there is no impact to build artifacts at this time; that would come in a follow-up PR (and then follow-up follow-up PRs for the various build systems in both Gradle and Xcode to actually use that value to omit dependencies).
Some tests had to be updated; for the most part it was updating the default `ProcessManager` because a call to `dart pub deps --json` is now made in code that computes what plugins are available, but there should be no change in behavior.
_/cc @jonasfj @sigurdm for FYI only (we talked on an internal thread about this; see https://github.com/dart-lang/sdk/issues/56968)._
_/cc @camsim99 @cbracken @johnmccutchan for visibility on the change._
The test was immediately checking the contents of stdout after the daemon indicated that the hot reload had completed. This could cause a race since the reloaded code may not have had time to execute.
Fixes https://github.com/flutter/flutter/issues/158245
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).
While doing some hacking on `Cache` in https://github.com/flutter/flutter/pull/158081, I noticed that [`Cache.test`](de93182753/packages/flutter_tools/lib/src/cache.dart (L139)) allows the caller to tell Cache to use some given directory as the flutter root (instead of depending on the static global [`Cache.flutterRoot`](4f3976a4f2/packages/flutter_tools/lib/src/cache.dart (L206))). This has a default value, `/cache`. However, `/cache` is an unintuitive name for the root directory of a Flutter installation.
This led to confusion when updating some tests. I wanted to create `/bin/cache/engine-dart-sdk.stamp` for tests, but in reality I needed to create `/cache/bin/cache/engine-dart-sdk.stamp`.
This PR changes this default to the current directory of the file system (which I'm guessing is `/` for all intents and purposes).
<details>
<summary> Pre-launch checklist </summary>
</details>
Currently the bot that runs `flutter update-packages` makes PRs that
fail due to native asset integration tests failing.
The root cause is due to incompatible versions on `package:logging`. The
bot tries to upgrade `package:logging` from `1.2.0` to `1.3.0`.
Here's what seems to happen:
* `flutter update-packages` will update
`dev/integration_tests/link_hook/pubspec.yaml` with `package:logging` to
`1.3.0` (as it does with all other `pubspec.yaml` files in the flutter
repository)
* `flutter create --template=package_ffi` will generate a template with
`package:logging` `^1.2.0`
* The test in question
* creates ffi template (which will use `^1.2.0`)
* make it depend on `dev/integration_tests/link_hook` (which uses
`=1.3.0`)
* changes logging dependency from the template from `^1.2.0` to `=1.2.0`
IMHO
* `flutter update-packages` is doing what it's supposed to
* `flutter create --template=package_ffi` can generate templates with
versions it determines (maybe there are use cases where we want to
generate templates with older versions)
* The problematic part is the test:
* it makes the generated template depend on `link_hook` and
* changes template generated pubspec to use pinned dependencies
This PR makes the test package (created via template) use the pinned
package versions from `dev/integration_tests/link_hook` (for
dependencies that are common among the two).
All other dependencies that the template has on top of
`dev/integration_tests/link_hook` it can pin as it does currently.
This will give us deterministic CI behavior (as we use flutter pined
packages and remaining deps being pinned via template) It avoids
changing the `flutter update-packages` and `flutter create
--template=package_ffi` (as their behavior seems reasonable)
Should fix https://github.com/flutter/flutter/issues/158135
removed public modifier from this methods :`getVersionCode` , `getVersionName`.
add static to :`pluginSupportsAndroidPlatform` ,`buildGradleFile`,`settingsGradleFile`
`getCompileSdkFromProject` ,`getAssembleTask`
refactor `==null` usage to `:?` to unify the usage
see https://github.com/flutter/flutter/issues/147122 for context
This is to handle Google testing failures for https://github.com/flutter/flutter/pull/158126.
We want to update G3 to provide this parameter before merging the full change.
Testing is not needed because the change is no-op.
