Repository navigation
[flutter_tools] Inject dependencies into Darwin build targets, native assets, and Xcode build settings - #194052
Conversation
… assets, and Xcode build settings Eliminates ambient `globals.dart` imports and usages from `build_system/targets/darwin.dart`, `isolated/native_assets/windows/native_assets.dart`, `isolated/native_assets/macos/native_assets_host.dart`, `isolated/native_assets/native_assets.dart`, `isolated/native_assets/test/native_assets.dart`, `ios/xcode_build_settings.dart`, and `macos/cocoapod_utils.dart`, injecting explicit dependencies via constructor and method parameters. Note: `printXcodeError`, `printXcodeWarning`, and `printXcodeNote` in `build_system/targets/darwin.dart` now write through `Logger.printError` (restoring `hadErrorOutput` for warnings and notes so `--fatal-warnings` is not triggered), which adds the `[ +N ms]` prefix when running under `-v`. Part of flutter#36484
There was a problem hiding this comment.
Code Review
This pull request refactors various components in flutter_tools to inject dependencies like Logger, Config, FileSystem, Platform, ProcessManager, and CocoaPods instead of relying on the global globals object. Feedback on these changes identifies a potential regression in darwin.dart, where replacing direct stderr writes with logger.printError for Xcode logs can introduce verbose timing prefixes (e.g., [ +N ms]) that break Xcode's log parser and prevent warnings or errors from appearing in the Xcode UI.
Update Darwin build commands and native assets test helpers to adhere to dependency injection conventions: - Use `super.toolContext` and override `toolContext` in `BuildFrameworkCommand` and `BuildSwiftPackage` without redundant fields. - Inject `ToolContext` into `BuildSwiftPackageUtils`, `processPodsIfNeeded`, `updateGeneratedXcodeProperties`, and native assets test helpers instead of individual context parameters. - Register `ToolContext` in `runInContext` fallbacks. - Clean up unused imports in affected tests.
Refactor cCompilerConfigWindows and IOSDeploy to accept ToolContext instead of 4 or more individual ToolContext properties, and update callers in WindowsAssetTarget, XCDevice, and unit tests.
Restores GlobalToolContext in context_runner.dart instead of eagerly snapshotting globals, passes ToolContext to IOSDeviceSupport and IOSCoreDeviceLauncher, and passes toolContext.logger in buildXcodeProject.
Passes super(logger: toolContext.logger) and destructures ToolContext in the FlutterDeviceManager constructor body instead of accepting redundant individual ToolContext parameters.
Exposes DelegatingLogger.delegate and unwraps DelegatingLogger in _printXcodeLog so VerboseLogger does not prefix Xcode error, warning, or note lines.
`DelegatingLogger` previously declared a private `_delegate` field alongside a trivial `delegate` getter. Replace `_delegate` and its getter with a public `final Logger delegate;` field directly and update internal references in `DelegatingLogger` and `asLogger`.
…ative-assets # Conflicts: # packages/flutter_tools/lib/src/context/tool_dependencies.dart # packages/flutter_tools/lib/src/desktop_device.dart # packages/flutter_tools/lib/src/flutter_device_manager.dart # packages/flutter_tools/lib/src/isolated/native_assets/windows/native_assets.dart # packages/flutter_tools/lib/src/macos/macos_device.dart # packages/flutter_tools/test/general.shard/desktop_device_test.dart # packages/flutter_tools/test/general.shard/macos/macos_device_test.dart
…ative-assets # Conflicts: # packages/flutter_tools/lib/src/commands/build.dart # packages/flutter_tools/lib/src/test/flutter_tester_device.dart # packages/flutter_tools/test/general.shard/flutter_tester_device_test.dart
…ative-assets # Conflicts: # packages/flutter_tools/lib/src/commands/build.dart # packages/flutter_tools/lib/src/commands/build_swift_package.dart # packages/flutter_tools/lib/src/context/tool_dependencies.dart # packages/flutter_tools/lib/src/context_runner.dart # packages/flutter_tools/lib/src/ios/core_devices.dart # packages/flutter_tools/lib/src/ios/devices.dart # packages/flutter_tools/lib/src/ios/simulators.dart # packages/flutter_tools/lib/src/ios/xcode_build_settings.dart # packages/flutter_tools/lib/src/isolated/native_assets/test/native_assets.dart # packages/flutter_tools/lib/src/macos/xcdevice.dart # packages/flutter_tools/lib/src/runner/flutter_command.dart # packages/flutter_tools/lib/src/tester/flutter_tester.dart # packages/flutter_tools/test/commands.shard/hermetic/build_swift_package_test.dart # packages/flutter_tools/test/general.shard/flutter_tester_device_test.dart # packages/flutter_tools/test/general.shard/ios/core_devices_test.dart # packages/flutter_tools/test/general.shard/ios/ios_device_start_nonprebuilt_test.dart # packages/flutter_tools/test/general.shard/ios/simulators_test.dart # packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart # packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart # packages/flutter_tools/test/general.shard/macos/xcode_test.dart # packages/flutter_tools/test/general.shard/project_test.dart
There was a problem hiding this comment.
Code Review
This pull request refactors dependency injection across flutter_tools by explicitly passing ToolContext to various targets, commands, and helper classes, reducing reliance on global variables. Feedback suggests utilizing the injected systemClock dependency in native_assets.dart instead of calling DateTime.now() directly to improve testability.
…ironmentToolContext Reverts exposing DelegatingLogger.delegate in logger.dart and removes the synthetic _EnvironmentToolContext adapter in build_system/targets/native_assets.dart by passing explicit dependencies to FlutterNativeAssetsBuildRunnerImpl.
|
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. |
…ative-assets # Conflicts: # packages/flutter_tools/lib/src/ios/core_devices.dart # packages/flutter_tools/lib/src/ios/device_support.dart # packages/flutter_tools/lib/src/ios/devices.dart # packages/flutter_tools/lib/src/ios/ios_deploy.dart # packages/flutter_tools/lib/src/ios/simulators.dart # packages/flutter_tools/lib/src/macos/xcdevice.dart # packages/flutter_tools/test/general.shard/ios/core_devices_test.dart # packages/flutter_tools/test/general.shard/ios/device_support_test.dart # packages/flutter_tools/test/general.shard/ios/devices_test.dart # packages/flutter_tools/test/general.shard/ios/ios_deploy_test.dart # packages/flutter_tools/test/general.shard/ios/ios_device_install_test.dart # packages/flutter_tools/test/general.shard/ios/ios_device_project_test.dart # packages/flutter_tools/test/general.shard/ios/ios_device_start_nonprebuilt_test.dart # packages/flutter_tools/test/general.shard/ios/ios_device_start_prebuilt_test.dart # packages/flutter_tools/test/general.shard/ios/lldb_test.dart # packages/flutter_tools/test/general.shard/ios/simulators_test.dart
flutter/flutter@d4c8577...0c2473c 2026-10-28 [email protected] Roll Packages from a8549ae to 13d38e0 (9 revisions) (flutter/flutter#195488) 2026-10-28 [email protected] Use int.oneBitCount in PersistentHashMap and scope 53-bit caps to JS (flutter/flutter#195005) 2026-10-28 [email protected] Roll pub packages (flutter/flutter#195483) 2026-10-28 [email protected] [iPhone Duo] Use the correct UIScreen to report the display size and update when fold/unfold (flutter/flutter#195199) 2026-10-28 [email protected] [fuchsia] Fix portable_ui_test.h includes for newer Fuchsia SDKs (flutter/flutter#195420) 2026-10-28 [email protected] Change default pipeline options to match production draw calls (flutter/flutter#194597) 2026-10-28 [email protected] Roll Fuchsia Test Scripts from 2T18KC7-LDQtmQiLP... to xl1LWBuppzu-BnOzV... (flutter/flutter#195450) 2026-10-27 [email protected] [flutter_tools] Inject dependencies into Darwin build targets, native assets, and Xcode build settings (flutter/flutter#194052) 2026-10-27 [email protected] Revert: Reland square corners fix (flutter/flutter#195436) 2026-10-27 [email protected] [flutter_tools] Add DeviceService app launch and flutter run support for tool extensions (flutter/flutter#194589) 2026-10-27 [email protected] [engine] Honor targetWidth/targetHeight for animated images (flutter/flutter#194615) 2026-10-27 [email protected] [Android][a11y] Set visibleToUser=false for hidden semantics nodes (flutter/flutter#194411) 2026-10-27 [email protected] Fix repo deletion test flake (flutter/flutter#195349) 2026-10-27 [email protected] Roll Packages from 23a05b6 to a8549ae (10 revisions) (flutter/flutter#195434) 2026-10-27 [email protected] [web] Capture pointers to the Flutter view so drags survive platform view iframes (flutter/flutter#195187) 2026-10-27 [email protected] [web] Add opt-in raw samples, measured sample count and warm-up phase hook to the web benchmark recorder (flutter/flutter#195118) 2026-10-27 [email protected] Link migration guide in deprecation messages (flutter/flutter#195361) 2026-10-27 [email protected] Pin basic_material_app_macos__compile to macOS 26 (flutter/flutter#195429) 2026-10-27 [email protected] Add benchmark for warmed pipeline usage (flutter/flutter#195001) 2026-10-27 [email protected] [flutter_tools] Add device log streaming support to DeviceService and ExtensionBackedDevice (flutter/flutter#194964) 2026-10-27 [email protected] Roll Dart SDK from 1d16c04dc346 to 65fc979759ca (4 revisions) (flutter/flutter#195383) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…merge After merging master (including flutter#194052), processPodsIfNeeded in buildXcodeProject required TemplateRenderer and CocoaPods parameters and introduced a duplicate logger variable alongside globals references. Remove the duplicate logger variable, pass the existing cocoaPods parameter, and thread TemplateRenderer into buildXcodeProject and its callers (IOSDevice, BuildIOSCommand, BuildIOSArchiveCommand, IOSSimulatorUtils, and IOSSimulator).
… assets, and Xcode build settings (flutter#194052) Eliminates ambient `globals.dart` usage from `build_system/targets/darwin.dart`, `isolated/native_assets/`, `ios/xcode_build_settings.dart`, and `macos/cocoapod_utils.dart` by injecting dependencies via `ToolContext` and explicit parameters. Part of flutter#47161 ## 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] All existing and new tests are passing.
…nd Xcode project" (flutter#195552) Relands flutter#193652, which was reverted in flutter#195535 due to a semantic merge conflict with flutter#194052: `runMacOSPodInstall` in `build_macos.dart` referenced `globals.cocoaPods` and `globals.templateRenderer` after the `globals.dart` import was removed. This reland fixes the conflict by passing the injected `cocoaPods` and threading `required TemplateRenderer templateRenderer` into `buildMacOS`, `BuildMacosCommand`, `MacOSDevice`, `MacOSDevices`, and `FlutterDeviceManager`. Part of flutter#47161 ## 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
Eliminates ambient
globals.dartusage frombuild_system/targets/darwin.dart,isolated/native_assets/,ios/xcode_build_settings.dart, andmacos/cocoapod_utils.dartby injecting dependencies viaToolContextand explicit parameters.Part of #47161
Pre-launch Checklist
///).