Repository navigation
[flutter_tools] Eliminate globals.dart from flutter_plugins.dart - #194062
Conversation
Eliminates ambient `globals` usage from `packages/flutter_tools/lib/src/build_info.dart` by requiring explicit `Config`, `FileSystem`, `OperatingSystemUtils`, and `Platform` parameters across `getCurrentHostPlatform` and the build directory helper functions. The unreachable unsupported-host fallback in `getCurrentHostPlatform` no longer logs a warning.
Inject dependencies (Analytics, CocoaPods, Config, FeatureFlags, FileSystem, FileSystemUtils, FlutterVersion, Logger, OperatingSystemUtils, Platform, ProcessUtils, SystemClock, TemplateRenderer) explicitly into flutter_plugins.dart functions instead of reading from globals.dart.
There was a problem hiding this comment.
Code Review
This pull request refactors various commands, build targets, and test runners within flutter_tools to explicitly inject TemplateRenderer and Logger dependencies instead of relying on global variables. The review feedback identifies a compile-time issue in TestCompiler where using a private named parameter _templateRenderer in the constructor prevents subclasses, such as FakeTestCompiler, from referencing it. The reviewer suggests making both the constructor parameter and the field public (templateRenderer) to resolve this issue.
# Conflicts: # packages/flutter_tools/lib/src/linux/application_package.dart # packages/flutter_tools/lib/src/test/runner.dart
…ugins # Conflicts: # packages/flutter_tools/bin/fuchsia_tester.dart
# Conflicts: # packages/flutter_tools/lib/src/build_info.dart
Update `FlutterApplicationPackageFactory`, `AndroidApk.fromAndroidProject`, `LinuxApp.fromLinuxProject`, `BuildableLinuxApp`, and `XcodeProjectInterpreter` to accept `required ToolContext toolContext` rather than individual context parameters. Update `BuildSwiftPackage` and `BuildSwiftPackageUtils` to consume `toolContext`, using `@override ToolContext get toolContext => super.toolContext!;` in `BuildSwiftPackage` and removing redundant context parameters. Refactor `getLinuxBuildDirectory` to take `required ToolContext toolContext` and register `ToolContext` in `context_runner.dart` fallbacks and `globals.dart`. Update affected tests in `build_info_test.dart`, `application_package_test.dart`, `xcodeproj_test.dart`, `android_gradle_builder_test.dart`, and `build_swift_package_test.dart` to construct and supply `FakeToolContext`.
Refactor `_writeFlutterPluginsList`, `refreshPluginsList`, and `injectPlugins` in `packages/flutter_tools/lib/src/flutter_plugins.dart` to receive `ToolContext` instead of individual context services. Extract dependencies in methods via pattern destructuring with explicit types. Remove redundant `_toolContext` field in `TestCommand` in favor of `super.toolContext` and override `toolContext` getter. Register `ToolContext` in `packages/flutter_tools/lib/src/context_runner.dart` fallbacks and add `globals.toolContext` in `packages/flutter_tools/lib/src/globals.dart`.
…Device Remove `globals.toolContext` and the `ToolContext` fallback registration in `context_runner.dart` in favor of `_createFallbackToolContext()`. Define `toolContext` on `Device` and `PollingDeviceDiscovery`, and pass `ToolContext` through `LinuxDevices`, `LinuxDevice`, and `buildLinux`.
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
autosubmit label was removed for flutter/flutter/194062, because Pull request flutter/flutter/194062 is not in a mergeable state. |
…ugins # Conflicts: # packages/flutter_tools/lib/src/ios/mac.dart # packages/flutter_tools/lib/src/macos/build_macos.dart # packages/flutter_tools/lib/src/macos/cocoapod_utils.dart # packages/flutter_tools/test/general.shard/macos/cocoapod_utils_test.dart
Reland flutter#193776, reverted by flutter#195635. Match CanvasKit requests by URI path segments before constructing filesystem paths, preserving containment checks and POSIX/Windows regression coverage. Pass MustacheTemplateRenderer to FlutterWebPlatform.start in the regression tests to satisfy the required templateRenderer parameter added by flutter#194062.
…flutter#195770) <!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> Relands flutter#193776, reverted by flutter#195635. On Windows, converting a CanvasKit request URI to a filesystem path before matching `canvaskit/` causes valid asset requests to return HTTP 404. Match URI path segments first, then construct and validate the filesystem path. Restore regression coverage for POSIX and Windows paths, including JavaScript/Wasm assets, response bodies and headers, and invalid paths. The regression tests now pass `MustacheTemplateRenderer` to `FlutterWebPlatform.start`, addressing the required `templateRenderer` parameter introduced by flutter#194062 that caused the original tree breakage. Fixes flutter#193505. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [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 in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [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. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/CONTRIBUTING.ai.md [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 [flutter/website]: https://github.com/flutter/website [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
Eliminates ambient
globals.dartusage frompackages/flutter_tools/lib/src/flutter_plugins.dartby injecting dependencies viaToolContextinto plugin resolution, symlink creation, and platform registrant generation helpers.Part of #47161
Pre-launch Checklist
///).