Skip to content

[tool] Migrate BuildMacOSFrameworkCommand to modular dependency injection - #192456

Closed
bkonyi wants to merge 55 commits into
flutter:masterfrom
bkonyi:di/15e-macos-framework-command
Closed

bkonyi wants to merge 55 commits into
flutter:masterfrom
bkonyi:di/15e-macos-framework-command

Conversation

@bkonyi

@bkonyi bkonyi commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Part 15e of the modular dependency injection migration.

Migrates BuildMacOSFrameworkCommand (flutter build macos-framework) to modular dependency injection with AppleContext and ToolContext, eliminating direct dependencies on globals.dart.

Stacked on #192252
Compare: bkonyi/flutter@di/15d-darwin-framework-commands...di/15e-macos-framework-command

Part of #47161

Migrates `_toolContext` and `_appleContext` field extractions to Dart 3 object pattern destructuring in `BuildIOSArchiveCommand.runCommand`.
Migrates context field extractions to Dart 3 object pattern destructuring in `build_ios_framework.dart` and `build_macos_framework.dart`.
…lar context objects

Refactors XcodeCodeSigningSettings to provide context constructors
(XcodeCodeSigningSettings({required PlistParser plistParser, required ToolContext toolContext})
and XcodeCodeSigningSettings.fromContexts({required AppleContext appleContext, required ToolContext toolContext}))
instead of requiring callers to unpack 8+ individual arguments. Adds
DarwinAddToAppCodesigning.fromContexts and refactors BuildCommand to instantiate
codesigning from context objects and pass directly to subcommands.
…dependency injection

Migrates BuildIOSCommand and BuildIOSArchiveCommand in build_ios.dart
to receive AppleContext, BuildSystem, and ToolContext via constructor
dependency injection, eliminating ambient globals reads. Updates build.dart
to pass the injected contexts and uses XcodeCodeSigningSettings.fromContexts.
Migrates BuildMacosCommand in build_macos.dart to receive
BuildSystem and ToolContext via constructor dependency injection,
eliminating ambient globals reads. Updates build.dart to pass the
injected contexts.
…d to modular dependency injection

Migrates BuildIOSFrameworkCommand and BuildMacOSFrameworkCommand to receive
AppleContext and ToolContext via constructor dependency injection, eliminating
ambient globals reads. Updates build.dart to pass the injected contexts.
…ipa tests

Remove ambient `context.get<*>` fallbacks from `createBuildCommand` in both `build_ios_test.dart` and `build_ipa_test.dart`, defaulting directly to local test doubles. Bridge `ToolContext` and `AppleContext` dependencies into `contextOverrides` in `TestFlutterCommandRunner` during `runCommand` so child and unmigrated downstream commands resolve fakes without test-level `overrides`.
Converts `build_darwin_framework_test.dart` from `testUsingContext` to `testWithoutContext`:
- Avoids ambient `context.get<*>` fallbacks in test factories, routing test doubles directly through `FakeToolContext` and `FakeAppleContext`.
- Supplies `Artifacts.test()` to `FakeToolContext` for native asset manifest checks.
- Guards `processPodsIfNeeded` under `if (boolArg('plugins'))` in `BuildIOSFrameworkCommand` and `BuildMacOSFrameworkCommand`.
- Short-circuits Swift Package Manager checks in `processPodsIfNeeded` when `forceCocoaPodsOnly` is set.
- Bridges `ToolContext` and `AppleContext` test doubles into `TestFlutterCommandRunner` context overrides.
@bkonyi
bkonyi requested review from a team as code owners September 9, 2026 00:04
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Sep 9, 2026
@github-actions github-actions Bot added tool Affects the "flutter" command-line tool. See also t: labels. platform-macos Building on or for macOS specifically a: desktop Running on desktop team-ios Owned by iOS platform team team-macos Owned by the macOS platform team labels Sep 9, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors several build commands, including BuildIOSCommand, BuildIOSFrameworkCommand, and BuildMacOSFrameworkCommand, to accept and use AppleContext and ToolContext directly instead of relying on global variables, which allows tests to run hermetically without global context overrides. The review feedback suggests using Dart 3 pattern destructuring to extract dependencies from toolContext consistently across commands, destructuring logger in BuildIOSArchiveCommand.runCommand, and adding the required documentation to the public appleContext getter in BuildFrameworkCommand to comply with the Repository Style Guide.

Comment on lines +491 to +496
final ToolContext(
:FileSystem fs,
:OperatingSystemUtils os,
:ProcessUtils processUtils,
:Terminal terminal,
) = _toolContext;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logger variable is used extensively in runCommand but is not destructured from _toolContext like other dependencies. To ensure consistency and avoid relying on implicit getters or global fallbacks, please destructure logger from _toolContext.

Suggested change
final ToolContext(
:FileSystem fs,
:OperatingSystemUtils os,
:ProcessUtils processUtils,
:Terminal terminal,
) = _toolContext;
final ToolContext(
:FileSystem fs,
:Logger logger,
:OperatingSystemUtils os,
:ProcessUtils processUtils,
:Terminal terminal,
) = _toolContext;

Comment on lines +64 to +66
final FileSystem fs = toolContext.fs;
final Logger logger = toolContext.logger;
final ProcessManager processManager = toolContext.processManager;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with other commands in this PR, consider using Dart 3 pattern destructuring to extract dependencies from toolContext instead of individual variable assignments.

Suggested change
final FileSystem fs = toolContext.fs;
final Logger logger = toolContext.logger;
final ProcessManager processManager = toolContext.processManager;
final ToolContext(
:FileSystem fs,
:Logger logger,
:ProcessManager processManager,
) = toolContext;

final BuildSystem _buildSystem;
final ToolContext _toolContext;

AppleContext get appleContext => _appleContext;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The public getter appleContext is missing documentation. According to the Repository Style Guide (line 62), all public members should have documentation.

Suggested change
AppleContext get appleContext => _appleContext;
/// The Apple-specific context dependencies.
AppleContext get appleContext => _appleContext;
References
  1. All public members should have documentation. (link)


@override
Future<FlutterCommandResult> runCommand() async {
final ProcessManager processManager = toolContext.processManager;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with other commands in this PR, consider using Dart 3 pattern destructuring to extract dependencies from toolContext instead of individual variable assignments.

Suggested change
final ProcessManager processManager = toolContext.processManager;
final ToolContext(
:FileSystem fs,
:Logger logger,
:ProcessManager processManager,
) = toolContext;

@bkonyi

bkonyi commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Closing as consolidated into #192252.

@bkonyi bkonyi closed this Sep 9, 2026
pull Bot pushed a commit to Budda0ne/flutter that referenced this pull request Sep 11, 2026
…ion (flutter#192252)

## Summary

Part 15d of the modular dependency injection migration.

Migrates Darwin framework build commands (`BuildIOSFrameworkCommand` and
`BuildMacOSFrameworkCommand`) to modular dependency injection with
`AppleContext` and `ToolContext`, requiring non-null context parameters
and eliminating direct dependencies and fallbacks on `globals.dart` and
AppContext.

Consolidates PR flutter#192456 into this PR.

Targeting `master` directly.
Part of flutter#47161
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop CICD Run CI/CD platform-macos Building on or for macOS specifically team-ios Owned by iOS platform team team-macos Owned by the macOS platform team tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant