-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Only bundle assets and plugins from transitive closure of dependencies #160443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Only bundle assets and plugins from transitive closure of dependencies #160443
Conversation
1024789 to
95ce951
Compare
|
I think all local tests are passing now. (perhaps we should refactor some tests, so package_config.json doesn't have to be created uniquely for so many tests....) Not sure what happens in the customer tests, but I think it might be unrelated to this change. |
bkonyi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial quick review. Wil give this another pass later.
|
Gentle ping on this one |
|
@spydon I know you are one of the melos maintainers. I would suggest you to keep track of this pull request. I would say this is a blocker to release the workspaces support to melos. |
Thanks, I'm already subscribed to it. I've created one of the issues above. :) |
andrewkolos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits
packages/flutter_tools/test/general.shard/web/compile_web_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/general.shard/compute_dev_dependencies_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/general.shard/compute_dev_dependencies_test.dart
Outdated
Show resolved
Hide resolved
|
Do you think you could help me out with the customer_testing tests? I cannot understand how my change could cause errors like: But also I don't want to land this with red tests I don't understand... |
|
From what I can tell, the tests depend on the Roboto font being available (src). Before, it was getting bundled from the To verify yourself, clone the super_editor repo and navigate to the failing test (https://github.com/superlistapp/super_editor/blob/dc7013f2f9be145d655e891eb2502baf2d6795a1/super_text_layout/test/super_text_test.dart#L192). To see what assets are getting bundled within the test code, add final assets = (await AssetManifest.loadFromAssetBundle(rootBundle))
.listAssets();
print(assets.join('\n'));When on which I don't see when I have this PR checked out. |
|
I understand now, thanks! The super_text_layout package dev_depends on golden_toolkit which has the font as an asset. This PR does not allow you to use assets from your dev_dependencies. So in general when testing, you might want to use assets from your dev_dependencies, but not when building for deployment I had not really realized that. I see three ways forward:
|
Yeah, I was thinking the same. There are probably edge cases where including a font from a dev dependency would unexpectedly cause a test to pass (i.e. the font was meant to be included from a regular dependency or the |
|
I think including assets from dev dependencies in tests is fine since I assume our main concern here is reducing bundle size in shipped applications. |
|
Thanks for addressing the comments! LGTM! |
My understanding is that we are working towards what I think #79261 proposes, i.e. assets from dev dependencies should be included in debug builds but not release. The changes I made are irrespective to testing I think. cc @matanlurey for input |
matanlurey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the reason this failing is because the feature that @camsim99 and I implemented, which is, to omit plugins specified in dev_dependencies from release mode apps, looks like was broken as a result of the refactoring in this PR.
I can try cloning it locally and debug a bit tomorrow morning if you are still stuck, but as a starting point:
- https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8723951633781029105/+/u/run_test.dart_for_tool_tests_shard_and_subshard_general/stdout "plugins injectPlugins in release mode excludes dev dependencies from MacOS plugin registrant"
- https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8723951633490693953/+/u/run_android_release_builds_exclude_dev_dependencies_test/stdout "android_release_builds_exclude_dev_dependencies_test"
Not directly related: It's nice that this code was refactored to be synchronous and read the file system instead of shelling out to pub deps, but this was a large refactoring and in the future I should be included as a PR reviewer as someone who has to also maintain and understand these changes.
matanlurey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a chance to do a longer review. Still needs more work, and I still think it makes sense to break this up into 2+ PRs.
packages/flutter_tools/test/general.shard/compute_dev_dependencies_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/general.shard/compute_dev_dependencies_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/general.shard/compute_dev_dependencies_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/general.shard/compute_dev_dependencies_test.dart
Outdated
Show resolved
Hide resolved
Roll Flutter from 9a78af5eb067 to 33cdd8ef31dc (60 revisions) flutter/flutter@9a78af5...33cdd8e 2025-05-21 [email protected] Feat: Add persistentFooterDecoration for scaffold (flutter/flutter#167524) 2025-05-21 [email protected] Removed repeated entry in `CHANGELOG.md` (flutter/flutter#165273) 2025-05-21 [email protected] [native assets] Graduate to preview (flutter/flutter#169194) 2025-05-21 [email protected] [Impeller] disable gl ext render to texture on vivante. (flutter/flutter#169153) 2025-05-21 [email protected] fix(widget_inspector): add null check for flex factor property to prevent exception (flutter/flutter#167890) 2025-05-21 [email protected] Unpin leak_tracker. (flutter/flutter#169079) 2025-05-21 [email protected] runtime/dart: fuchsia::io::MODE_TYPE_FILE -> V_TYPE_FILE (flutter/flutter#168952) 2025-05-21 [email protected] Remove `isExplicitPackageDependenciesEnabled: true`, it is the default. (flutter/flutter#169156) 2025-05-21 [email protected] Roll pub packages (flutter/flutter#169181) 2025-05-20 [email protected] Fix the issue with Tooltip (flutter/flutter#168546) 2025-05-20 [email protected] [native assets] Roll dependencies (flutter/flutter#169073) 2025-05-20 [email protected] Add documentation for experimental branches, update artifacts. (flutter/flutter#169109) 2025-05-20 [email protected] [flutter_tool] Remove unused environment flags in JS compiler (flutter/flutter#169097) 2025-05-20 [email protected] Add support for hiding widget subtrees from the widget inspector (flutter/flutter#169007) 2025-05-20 [email protected] Roll Fuchsia GN SDK from jsZSHIOmQAs3URvWU... to _tkqOQZ2qB5CxDe57... (flutter/flutter#169113) 2025-05-20 [email protected] Skip running `Linux fuchsia_test` on non-master channel. (flutter/flutter#169106) 2025-05-19 [email protected] Roll Skia from c97451da059f to 13a299964c9f (61 revisions) (flutter/flutter#169099) 2025-05-19 [email protected] Shared element transition for predictive back (flutter/flutter#154718) 2025-05-19 [email protected] Fix DDC library bundle format test files to correctly pass flags (flutter/flutter#169095) 2025-05-19 [email protected] Clean up redundant new line in the GPUSurfaceGLSkia constructor initializer list (flutter/flutter#169031) 2025-05-19 [email protected] Fix keyboard_hot_restart_ios flakes (flutter/flutter#168518) 2025-05-19 [email protected] fix android studio lint about lambda argument (flutter/flutter#168901) 2025-05-19 [email protected] Fix typo in gpu_surface_gl_impeller.cc (flutter/flutter#168395) 2025-05-19 [email protected] Modernize system executable detection across components (flutter/flutter#169018) 2025-05-19 [email protected] Update documentation for `Size` and `Rect` classes (flutter/flutter#168031) 2025-05-19 [email protected] Update the `RangeSlider` widget to the 2024 Material Design appearance (flutter/flutter#163736) 2025-05-19 [email protected] Roll Packages from 58d4016 to af0b9a9 (5 revisions) (flutter/flutter#169075) 2025-05-19 [email protected] Only bundle assets and plugins from transitive closure of dependencies (flutter/flutter#160443) 2025-05-19 [email protected] Make FlutterGeneratedPluginSwiftPackage an Xcode root package (flutter/flutter#168789) 2025-05-19 [email protected] docs: Update deprecation message for Slider.year2023 (flutter/flutter#169053) 2025-05-18 [email protected] macOS: port ResizeSynchronizer to Swift (flutter/flutter#168959) 2025-05-17 [email protected] Roll Dart SDK from dc323ec0c1a3 to 7c40eba6bf77 (3 revisions) (flutter/flutter#169024) 2025-05-17 [email protected] [tool] Remove unused `reportNullSafety` getter (flutter/flutter#168484) 2025-05-17 [email protected] Add flag to skip bundling extension safe builds in frameworks for DDM (flutter/flutter#168955) 2025-05-16 [email protected] Fixes Navigator calls onPopInvokedWithResult when onPopPage return false (flutter/flutter#168567) 2025-05-16 [email protected] [hcpp/hc] Fix talkback for HC and HCPP Android platform views (flutter/flutter#168939) 2025-05-16 [email protected] [Impeller] separate immutable sampler descriptors. (flutter/flutter#169011) 2025-05-16 [email protected] TextField magnifier stuck on long press cancel (flutter/flutter#167881) 2025-05-16 [email protected] Fix Chip delete button semantic bounds (flutter/flutter#168310) 2025-05-16 [email protected] Roll Fuchsia Linux SDK from Jj-iDG5uPOsFgY2_H... to XtPp9bBW49iDJ0wbA... (flutter/flutter#169009) 2025-05-16 [email protected] [ Widget Preview ] Refactor `@Preview()` detection and code generation (flutter/flutter#168307) 2025-05-16 [email protected] Roll Packages from 2dff621 to 58d4016 (2 revisions) (flutter/flutter#168999) 2025-05-16 [email protected] Remove `unittests` from `windows_host_engine` GN targets. (flutter/flutter#168991) 2025-05-16 [email protected] Fix bug with debugging support code not getting injected on edge devices (flutter/flutter#168073) 2025-05-16 [email protected] Roll Dart SDK from a1db62a5dd14 to dc323ec0c1a3 (4 revisions) (flutter/flutter#168989) 2025-05-16 [email protected] Resolve Cupertino textstyle in MaterialBasedCupertinoThemeData (flutter/flutter#167597) ...
|
@sigurdm I suppose this PR could not land on the current flutter stable release, right? |
…r#9305) Roll Flutter from 9a78af5eb067 to 33cdd8ef31dc (60 revisions) flutter/flutter@9a78af5...33cdd8e 2025-05-21 [email protected] Feat: Add persistentFooterDecoration for scaffold (flutter/flutter#167524) 2025-05-21 [email protected] Removed repeated entry in `CHANGELOG.md` (flutter/flutter#165273) 2025-05-21 [email protected] [native assets] Graduate to preview (flutter/flutter#169194) 2025-05-21 [email protected] [Impeller] disable gl ext render to texture on vivante. (flutter/flutter#169153) 2025-05-21 [email protected] fix(widget_inspector): add null check for flex factor property to prevent exception (flutter/flutter#167890) 2025-05-21 [email protected] Unpin leak_tracker. (flutter/flutter#169079) 2025-05-21 [email protected] runtime/dart: fuchsia::io::MODE_TYPE_FILE -> V_TYPE_FILE (flutter/flutter#168952) 2025-05-21 [email protected] Remove `isExplicitPackageDependenciesEnabled: true`, it is the default. (flutter/flutter#169156) 2025-05-21 [email protected] Roll pub packages (flutter/flutter#169181) 2025-05-20 [email protected] Fix the issue with Tooltip (flutter/flutter#168546) 2025-05-20 [email protected] [native assets] Roll dependencies (flutter/flutter#169073) 2025-05-20 [email protected] Add documentation for experimental branches, update artifacts. (flutter/flutter#169109) 2025-05-20 [email protected] [flutter_tool] Remove unused environment flags in JS compiler (flutter/flutter#169097) 2025-05-20 [email protected] Add support for hiding widget subtrees from the widget inspector (flutter/flutter#169007) 2025-05-20 [email protected] Roll Fuchsia GN SDK from jsZSHIOmQAs3URvWU... to _tkqOQZ2qB5CxDe57... (flutter/flutter#169113) 2025-05-20 [email protected] Skip running `Linux fuchsia_test` on non-master channel. (flutter/flutter#169106) 2025-05-19 [email protected] Roll Skia from c97451da059f to 13a299964c9f (61 revisions) (flutter/flutter#169099) 2025-05-19 [email protected] Shared element transition for predictive back (flutter/flutter#154718) 2025-05-19 [email protected] Fix DDC library bundle format test files to correctly pass flags (flutter/flutter#169095) 2025-05-19 [email protected] Clean up redundant new line in the GPUSurfaceGLSkia constructor initializer list (flutter/flutter#169031) 2025-05-19 [email protected] Fix keyboard_hot_restart_ios flakes (flutter/flutter#168518) 2025-05-19 [email protected] fix android studio lint about lambda argument (flutter/flutter#168901) 2025-05-19 [email protected] Fix typo in gpu_surface_gl_impeller.cc (flutter/flutter#168395) 2025-05-19 [email protected] Modernize system executable detection across components (flutter/flutter#169018) 2025-05-19 [email protected] Update documentation for `Size` and `Rect` classes (flutter/flutter#168031) 2025-05-19 [email protected] Update the `RangeSlider` widget to the 2024 Material Design appearance (flutter/flutter#163736) 2025-05-19 [email protected] Roll Packages from 58d4016 to af0b9a9 (5 revisions) (flutter/flutter#169075) 2025-05-19 [email protected] Only bundle assets and plugins from transitive closure of dependencies (flutter/flutter#160443) 2025-05-19 [email protected] Make FlutterGeneratedPluginSwiftPackage an Xcode root package (flutter/flutter#168789) 2025-05-19 [email protected] docs: Update deprecation message for Slider.year2023 (flutter/flutter#169053) 2025-05-18 [email protected] macOS: port ResizeSynchronizer to Swift (flutter/flutter#168959) 2025-05-17 [email protected] Roll Dart SDK from dc323ec0c1a3 to 7c40eba6bf77 (3 revisions) (flutter/flutter#169024) 2025-05-17 [email protected] [tool] Remove unused `reportNullSafety` getter (flutter/flutter#168484) 2025-05-17 [email protected] Add flag to skip bundling extension safe builds in frameworks for DDM (flutter/flutter#168955) 2025-05-16 [email protected] Fixes Navigator calls onPopInvokedWithResult when onPopPage return false (flutter/flutter#168567) 2025-05-16 [email protected] [hcpp/hc] Fix talkback for HC and HCPP Android platform views (flutter/flutter#168939) 2025-05-16 [email protected] [Impeller] separate immutable sampler descriptors. (flutter/flutter#169011) 2025-05-16 [email protected] TextField magnifier stuck on long press cancel (flutter/flutter#167881) 2025-05-16 [email protected] Fix Chip delete button semantic bounds (flutter/flutter#168310) 2025-05-16 [email protected] Roll Fuchsia Linux SDK from Jj-iDG5uPOsFgY2_H... to XtPp9bBW49iDJ0wbA... (flutter/flutter#169009) 2025-05-16 [email protected] [ Widget Preview ] Refactor `@Preview()` detection and code generation (flutter/flutter#168307) 2025-05-16 [email protected] Roll Packages from 2dff621 to 58d4016 (2 revisions) (flutter/flutter#168999) 2025-05-16 [email protected] Remove `unittests` from `windows_host_engine` GN targets. (flutter/flutter#168991) 2025-05-16 [email protected] Fix bug with debugging support code not getting injected on edge devices (flutter/flutter#168073) 2025-05-16 [email protected] Roll Dart SDK from a1db62a5dd14 to dc323ec0c1a3 (4 revisions) (flutter/flutter#168989) 2025-05-16 [email protected] Resolve Cupertino textstyle in MaterialBasedCupertinoThemeData (flutter/flutter#167597) ...
…r#9305) Roll Flutter from 9a78af5eb067 to 33cdd8ef31dc (60 revisions) flutter/flutter@9a78af5...33cdd8e 2025-05-21 [email protected] Feat: Add persistentFooterDecoration for scaffold (flutter/flutter#167524) 2025-05-21 [email protected] Removed repeated entry in `CHANGELOG.md` (flutter/flutter#165273) 2025-05-21 [email protected] [native assets] Graduate to preview (flutter/flutter#169194) 2025-05-21 [email protected] [Impeller] disable gl ext render to texture on vivante. (flutter/flutter#169153) 2025-05-21 [email protected] fix(widget_inspector): add null check for flex factor property to prevent exception (flutter/flutter#167890) 2025-05-21 [email protected] Unpin leak_tracker. (flutter/flutter#169079) 2025-05-21 [email protected] runtime/dart: fuchsia::io::MODE_TYPE_FILE -> V_TYPE_FILE (flutter/flutter#168952) 2025-05-21 [email protected] Remove `isExplicitPackageDependenciesEnabled: true`, it is the default. (flutter/flutter#169156) 2025-05-21 [email protected] Roll pub packages (flutter/flutter#169181) 2025-05-20 [email protected] Fix the issue with Tooltip (flutter/flutter#168546) 2025-05-20 [email protected] [native assets] Roll dependencies (flutter/flutter#169073) 2025-05-20 [email protected] Add documentation for experimental branches, update artifacts. (flutter/flutter#169109) 2025-05-20 [email protected] [flutter_tool] Remove unused environment flags in JS compiler (flutter/flutter#169097) 2025-05-20 [email protected] Add support for hiding widget subtrees from the widget inspector (flutter/flutter#169007) 2025-05-20 [email protected] Roll Fuchsia GN SDK from jsZSHIOmQAs3URvWU... to _tkqOQZ2qB5CxDe57... (flutter/flutter#169113) 2025-05-20 [email protected] Skip running `Linux fuchsia_test` on non-master channel. (flutter/flutter#169106) 2025-05-19 [email protected] Roll Skia from c97451da059f to 13a299964c9f (61 revisions) (flutter/flutter#169099) 2025-05-19 [email protected] Shared element transition for predictive back (flutter/flutter#154718) 2025-05-19 [email protected] Fix DDC library bundle format test files to correctly pass flags (flutter/flutter#169095) 2025-05-19 [email protected] Clean up redundant new line in the GPUSurfaceGLSkia constructor initializer list (flutter/flutter#169031) 2025-05-19 [email protected] Fix keyboard_hot_restart_ios flakes (flutter/flutter#168518) 2025-05-19 [email protected] fix android studio lint about lambda argument (flutter/flutter#168901) 2025-05-19 [email protected] Fix typo in gpu_surface_gl_impeller.cc (flutter/flutter#168395) 2025-05-19 [email protected] Modernize system executable detection across components (flutter/flutter#169018) 2025-05-19 [email protected] Update documentation for `Size` and `Rect` classes (flutter/flutter#168031) 2025-05-19 [email protected] Update the `RangeSlider` widget to the 2024 Material Design appearance (flutter/flutter#163736) 2025-05-19 [email protected] Roll Packages from 58d4016 to af0b9a9 (5 revisions) (flutter/flutter#169075) 2025-05-19 [email protected] Only bundle assets and plugins from transitive closure of dependencies (flutter/flutter#160443) 2025-05-19 [email protected] Make FlutterGeneratedPluginSwiftPackage an Xcode root package (flutter/flutter#168789) 2025-05-19 [email protected] docs: Update deprecation message for Slider.year2023 (flutter/flutter#169053) 2025-05-18 [email protected] macOS: port ResizeSynchronizer to Swift (flutter/flutter#168959) 2025-05-17 [email protected] Roll Dart SDK from dc323ec0c1a3 to 7c40eba6bf77 (3 revisions) (flutter/flutter#169024) 2025-05-17 [email protected] [tool] Remove unused `reportNullSafety` getter (flutter/flutter#168484) 2025-05-17 [email protected] Add flag to skip bundling extension safe builds in frameworks for DDM (flutter/flutter#168955) 2025-05-16 [email protected] Fixes Navigator calls onPopInvokedWithResult when onPopPage return false (flutter/flutter#168567) 2025-05-16 [email protected] [hcpp/hc] Fix talkback for HC and HCPP Android platform views (flutter/flutter#168939) 2025-05-16 [email protected] [Impeller] separate immutable sampler descriptors. (flutter/flutter#169011) 2025-05-16 [email protected] TextField magnifier stuck on long press cancel (flutter/flutter#167881) 2025-05-16 [email protected] Fix Chip delete button semantic bounds (flutter/flutter#168310) 2025-05-16 [email protected] Roll Fuchsia Linux SDK from Jj-iDG5uPOsFgY2_H... to XtPp9bBW49iDJ0wbA... (flutter/flutter#169009) 2025-05-16 [email protected] [ Widget Preview ] Refactor `@Preview()` detection and code generation (flutter/flutter#168307) 2025-05-16 [email protected] Roll Packages from 2dff621 to 58d4016 (2 revisions) (flutter/flutter#168999) 2025-05-16 [email protected] Remove `unittests` from `windows_host_engine` GN targets. (flutter/flutter#168991) 2025-05-16 [email protected] Fix bug with debugging support code not getting injected on edge devices (flutter/flutter#168073) 2025-05-16 [email protected] Roll Dart SDK from a1db62a5dd14 to dc323ec0c1a3 (4 revisions) (flutter/flutter#168989) 2025-05-16 [email protected] Resolve Cupertino textstyle in MaterialBasedCupertinoThemeData (flutter/flutter#167597) ...
|
When will it be possible to have this fix on stable |
Since it's in beta now I would guess that it will land in stable around the end of August. |
Fixes dart-lang/pub#4486
Fixes dart-lang/pub#4473
Fixes #79261
Fixes #160142
Fixes #155242
Use the new
.dart_tool/package_graph.jsonfile to compute the dependency graph. DeterminingBundles assets from dev_dependencies when running
flutter test, but not otherwise.