[native_assets_builder] Separate KernelAssets from Assets
#964
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR HealthBreaking changes ✔️Details
Changelog Entry ✔️Details
Changes to files need to be accounted for in their respective changelogs. Coverage ✔️Details
This check for test coverage is informational (issues shown here will not fail the PR). License Headers ✔️Details
All source files should start with a license header. Unrelated files missing license headers
Package publish validation ✔️Details
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
mosuem
approved these changes
Feb 14, 2024
copybara-service bot
pushed a commit
to dart-lang/sdk
that referenced
this pull request
Feb 14, 2024
Manuall roll for dart-lang/native#964. TEST=pkg/dartdev/test/native_assets/ Change-Id: Ie1e762ed57971db105cc9d6d5fa21e9f99e76453 Cq-Include-Trybots: luci.dart.try:pkg-win-release-try,pkg-win-release-arm64-try,pkg-mac-release-try,pkg-mac-release-arm64-try,pkg-linux-release-try,pkg-linux-release-arm64-try,pkg-linux-debug-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352642 Reviewed-by: Moritz Sümmermann <[email protected]> Commit-Queue: Daco Harkes <[email protected]>
8 tasks
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Feb 14, 2024
Roll of dart-lang/native#964, which separates the `KernelAsset`s (the asset information embedded in the Dart kernel snapshot) from `Asset`s (the assets in the `build.dart` protocol). See the linked issue for why they ought to be different instead of shared. This PR does not change any functionality in Flutter. (Now that #143055 has landed, we can land breaking changes.) For reference, the same roll in the Dart SDK: https://dart-review.googlesource.com/c/sdk/+/352642
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are multiple differences between the
native_assets.yamlthat is embedded in the kernel file and thebuild_output.yaml -> assets.pathfor assets should be inAsset, not inAssetPathfor the file-path the asset has after thebuild.dartrun. When the embedders (Flutter/Dart) embed the native assets mapping then thepaths start representing the path on the system where the Dart/Flutter app is running. This should be embedded in the path-type there.Assets to output OS and Architecture instead of Target.Therefore we should not share the data structure between
Assets andKernelAssets (new name for the entries that are embedded via native_assets.yaml in a kernel file.)This means that
dartdevandflutter_toolswill have to start convertingAssets toKernelAssets when making thenative_assets.yamlfile. Therefore this is a breaking change and will have to be rolled into Dart and flutter/flutter manually.