mirror of
https://github.com/flutter/flutter.git
synced 2025-06-03 00:51:18 +00:00
b8de503ded
12 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Daco Harkes
|
9ded795e63
|
[native assets] Roll dependencies (#162017)
This PR rolls in a number of breaking changes for native assets: * Pub workspaces are now supported, this requires a refactoring to how the `NativeAssetsBuildRunner` is used. Most notably it requires being explicit about the `runPackageName`. Flutter does not seem to have that value, but it does have the `projectUri` which can be used to find the package name via the package config. * The API for build hooks has been redesigned. This PR updates the project template, test project files, and test project strings. ## 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]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- 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 |
||
|
Daco Harkes
|
da080e6976
|
[native assets] Cleanup dead code 2 (#161916)
This PR deletes dead code. The `FlutterNativeAssetsBuildRunnerImpl` has a `PackageConfig` argument, so the package config file must exist. This means the `hasPackageConfig` method is meaningless, it will always return `true`. The only case where it might have returned false was in the unit test mock. This unit test is now deleted. (It must be the case that `flutter_tools` internally ensures `pub get` has been run in the project before it reaches any code paths that try to build native assets.) ## 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]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] 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 |
||
|
Daco Harkes
|
27ba2f2790
|
[native assets] Roll dependencies (#160672)
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. |
||
|
Michael Goderbauer
|
5491c8c146
|
Auto-format Framework (#160545)
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> |
||
|
Daco Harkes
|
4aa2caef20
|
[native assets] Create NativeAssetsManifest.json instead of kernel embedding (#159322)
This PR introduces a `NativeAssetsManifest.json` next to the `AssetManifest.bin` and `FontManifest.json`. This removes the need for embedding the native assets mapping inside the kernel file and enables decoupling native assets building and bundling from the kernel compilation in flutter tools. This means `flutter run` no longer does a dry run of `hook/build.dart` hooks. (It also means all isolate groups will have the same native assets. However, since Flutter does not support `Isolate.spawnUri` from kernel files, this is not a regression.) Implementation details: * g3 is still using kernel embedding. https://github.com/flutter/flutter/pull/142016 introduced an argument to embed a `native_assets.yaml` inside `flutter attach` and `flutter run` (the outer flutter process), but it is not used in `flutter assemble` (the inner process when doing `flutter run`). So, those arguments need to still be respected. However, all other logic related to embedding a yaml encoding in the kernel file has been removed. * All dry-run logic has been removed. 🎉 * The `KernelSnapshot` target no longer depends on the `InstallCodeAssets` target. Instead, the various OS-specific "BundleAsset" targets now depend on the `InstallCodeAssets` target. The `InstallCodeAssets` invokes the build hooks and produces the `NativeAssetsManifest.json`. The various "BundleAsset" commands synchronize the `NativeAssetsManifest.json` to the app bundle. * `InstallCodeAssets` produces a `native_assets.json`, which is renamed to `NativeAssetsManifest.json` in the various "Bundle" targets. This means that all unit tests of the "Bundle" targets now need to create this file. (Similar to how `app.dill` is expected to exist because `KernelSnapshot` is a dependency of the "Bundle" targets.) * Because dynamic libraries need to be code signed (at least on iOS and MacOS), the bundling of the dylibs is _not_ migrated to reuse `_updateDevFS` (which is used for ordinary assets). Only the 2nd and 3rd invocation of `flutter assemble` from `xcodebuild` has access to the code signing identity. Relevant tests: * test/integration.shard/isolated/native_assets_test.dart - runs `flutter run` with native assets including hot restart and hot reload. TODO: * Undo engine-roll in this PR after engine has rolled in. Issue: * https://github.com/flutter/flutter/issues/154425 Related PRs: * https://dart-review.googlesource.com/c/sdk/+/388161 * https://github.com/flutter/engine/pull/56727 ## 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. <!-- 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 |
||
|
Martin Kustermann
|
01590aa27a
|
Refactor native asset integration into flutter tools (#158932)
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`). |
||
|
Martin Kustermann
|
1636fbd4cb
|
Fix code asset copying logic in native asset code (#158984)
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. |
||
|
Martin Kustermann
|
256359194e
|
Fix duplicate work in native assets release builds (#158980)
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. |
||
|
Martin Kustermann
|
3a83e43ede
|
Make flutter_tools use newest package:{native_assets_builder,native_assets_cli,native_toolchain_c} (#158214)
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). |
||
|
Martin Kustermann
|
abcba1fa7e
|
Reland "[flutter_tools] Cleanup of native asset related code (removes around 50% of the native asset related code) (#155430)" (#155745)
Changes to original CL: The code that issues an error on unsupported
operating system in the dry-run case was missing a case for iOS and
Android
Original CL description
tl;dr Removes 50% (>1650 locs) of native asset related code in
`packages/flutter_tools`
Before this PR the invocation of dart build/link/dry-run was implemented
per OS. This lead to very large code duplication of almost identical,
but slightly different code. It also led to similarly duplicated test
code.
Almost the entire dart build/link/dry-run implementation is identical
across OSes. There's small variations:
- configuration of the build (e.g. android/macos/ios version, ios sdk,
...)
- determining target locations & copying the final shared libraries
This PR unifies the implementation by reducing the code to basically two
main functions:
* `runFlutterSpecificDartBuild` which is responsible for
- obtain flutter configuration
- perform dart build (& link)
- determine target location & install binaries
* `runFlutterSpecificDartDryRunOnPlatforms` which is responsible for a
similar (but not same):
- obtain flutter configuration
- perform dart dry run
- determine target location
these two functions will call out to helpers for the OS specific
functionality:
* `_assetTargetLocationsForOS` for determining the location of the code
assets
* `_copyNativeCodeAssetsForOS` for copying the code assets (and possibly
overriting the install name, etc)
=> Since we get rid of the code duplication across OSes and have only a
single code path for the build/link/dry-run, we can also remove the
duplicated tests that were pretty much identical across OSes.
We also harden the building code by adding asserts, e.g.
* the dry fun functionality should never be used by `flutter test`
* the `build/native_assets/<os>/native_assets.yaml` should only be used
by `flutter test` and the dry-run of `flutter run`
=> We change the tests to also comply with these invariants (so the
tests are not testing things that cannot happen in reality)
We also rename `{,Flutter}NativeAssetsBuildRunner` to disambiguate it
from the `package:native_asset_builder`'s `NativeAssetsBuildRunner`.
|
||
|
auto-submit[bot]
|
4dfa688ec4
|
Reverts "[flutter_tools] Cleanup of native asset related code (removes around 50% of the native asset related code) (#155430)" (#155713)
Reverts: flutter/flutter#155430 Initiated by: eyebrowsoffire Reason for reverting: Postsubmit failures closing the tree. See the following examples: https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_ios%20native_assets_ios/5738/overview https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_arm64_mokey%20native_assets_android/583/overview https://ci.chromium.org/ui/p/flutter/builders/prod/Linux_pixel_7pro%20native_assets_android/4075/overview https://ci.chromium.org/u Original PR Author: mkustermann Reviewed By: {bkonyi, dcharkes} This change reverts the following previous change: tl;dr Removes 50% (>1650 locs) of native asset related code in `packages/flutter_tools` Before this PR the invocation of dart build/link/dry-run was implemented per OS. This lead to very large code duplication of almost identical, but sligthly different code. It also led to similarly duplicated test code. Almost the entire dart build/link/dry-run implementation is identical across OSes. There's small variations: - configuration of the build (e.g. android/macos/ios version, ios sdk, ...) - determining target locations & copying the final shared libraries This PR unifies the implementation by reducing the code to basically two main functions: * `runFlutterSpecificDartBuild` which is responsible for - obtain flutter configuration - perform dart build (& link) - determine target location & install binaries * `runFlutterSpecificDartDryRunOnPlatforms` which is responsible for a similar (but not same): - obtain flutter configuration - perform dart dry run - determine target location these two functions will call out to helpers for the OS specific functionality: * `_assetTargetLocationsForOS` for determining the location of the code assets * `_copyNativeCodeAssetsForOS` for copying the code assets (and possibly overriting the install name, etc) => Since we get rid of the code duplication across OSes and have only a single code path for the build/link/dry-run, we can also remove the duplicated tests that were pretty much identical across OSes. We also harden the building code by adding asserts, e.g. * the dry fun functionality should never be used by `flutter test` * the `build/native_assets/<os>/native_assets.yaml` should only be used by `flutter test` and the dry-run of `flutter run` => We change the tests to also comply with these invariants (so the tests are not testing things that cannot happen in reality) We also rename `{,Flutter}NativeAssetsBuildRunner` to disambiguate it from the `package:native_asset_builder`'s `NativeAssetsBuildRunner`. We also reorganize the main code to make it readable from top-down and make members private where they can be. |
||
|
Martin Kustermann
|
621e7ef951
|
[flutter_tools] Cleanup of native asset related code (removes around 50% of the native asset related code) (#155430)
tl;dr Removes 50% (>1650 locs) of native asset related code in
`packages/flutter_tools`
Before this PR the invocation of dart build/link/dry-run was implemented
per OS. This lead to very large code duplication of almost identical,
but slightly different code. It also led to similarly duplicated test
code.
Almost the entire dart build/link/dry-run implementation is identical
across OSes. There's small variations:
- configuration of the build (e.g. android/macos/ios version, ios sdk, ...)
- determining target locations & copying the final shared libraries
This PR unifies the implementation by reducing the code to basically two
main functions:
* `runFlutterSpecificDartBuild` which is responsible for
- obtain flutter configuration
- perform dart build (& link)
- determine target location & install binaries
* `runFlutterSpecificDartDryRunOnPlatforms` which is responsible for a
similar (but not same):
- obtain flutter configuration
- perform dart dry run
- determine target location
these two functions will call out to helpers for the OS specific
functionality:
* `_assetTargetLocationsForOS` for determining the location of the code
assets
* `_copyNativeCodeAssetsForOS` for copying the code assets (and possibly
overriting the install name, etc)
=> Since we get rid of the code duplication across OSes and have only a
single code path for the build/link/dry-run, we can also remove the
duplicated tests that were pretty much identical across OSes.
We also harden the building code by adding asserts, e.g.
* the dry fun functionality should never be used by `flutter test`
* the `build/native_assets/<os>/native_assets.yaml` should only be used
by `flutter test` and the dry-run of `flutter run`
=> We change the tests to also comply with these invariants (so the
tests are not testing things that cannot happen in reality)
We also rename `{,Flutter}NativeAssetsBuildRunner` to disambiguate it
from the `package:native_asset_builder`'s `NativeAssetsBuildRunner`.
|