mirror of
https://github.com/flutter/flutter.git
synced 2026-02-20 02:29:02 +08:00
a6118612ac
Fixes: https://github.com/flutter/flutter/issues/124970 Part of https://github.com/flutter/flutter/issues/47161 Before this change, there were two places we overrode the `Artifacts` in a Zone: 1. if/when we parse local-engine CLI options:1cf3907407/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart (L281)2. an additional override for fuchsia platform dill (no longer used, deleted in this PR):1cf3907407/packages/flutter_tools/lib/src/commands/attach.dart (L274)Note 1 above creates a new instance of `Artifacts.getLocalEngine()`. In this flow, there exist two instances of `Artifacts`: 1. The default fallback instance of `CachedArtifacts` (which gets all artifacts from flutter/bin/cache), instantiated in context_runner.dart:1cf3907407/packages/flutter_tools/lib/src/context_runner.dart (L137)2. An instance of `CachedLocalEngineArtifacts` created in the command runner once the CLI options have been parsed:1cf3907407/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart (L281)The regression happened when we direct injected the Artifacts 1 from above BEFORE we parsed the local-engine flag, and then used this in the second zone override, and then when creating the `FlutterDevice` there are multiple calls to `globals.artifacts` returned it when it should have returned Artifacts 2:1cf3907407/packages/flutter_tools/lib/src/resident_runner.dart (L80)Device.artifactOverrides was originally introduced in https://github.com/flutter/flutter/pull/32071, but is no longer used, so I deleted it. I also removed direct injection of `Artifacts` to the attach sub-command, because that class now no longer references artifacts. I believe the ideal true fix for this would be to: 1. Migrate all leaf calls to `globals.artifacts` to use direct injection (in this case, the offending invocations were in [`FlutterDevice.create()`](1cf3907407/packages/flutter_tools/lib/src/resident_runner.dart (L80-L218)), but I'm not sure that something else would not have broken later) 2. Ensure we are always direct injecting the desired instance of `Artifacts`--that is, if the user desires local engine artifacts, that we are passing an instance of `CachedLocalEngineArtifacts`. a. Alternatively, and probably simpler, teach `CachedArtifacts` to know about the local engine. This would mean parsing the global CLI options BEFORE we ever construct any instance of `Artifacts`. As an overall recommendation for implementing https://github.com/flutter/flutter/issues/47161, in the overall tree of tool function calls, we should probably migrate the leaves first (that is, migrate the sub-commands last). We should also audit and reconsider any usage of `runZoned()` or `context.run()` for the purpose overriding zoneValues.