Skip to content

[flutter_tool] Use engine flutter_runner prebuilts#43381

Merged
zanderso merged 2 commits intoflutter:masterfrom
zanderso:use-correct-flutter-runner
Oct 28, 2019
Merged

[flutter_tool] Use engine flutter_runner prebuilts#43381
zanderso merged 2 commits intoflutter:masterfrom
zanderso:use-correct-flutter-runner

Conversation

@zanderso
Copy link
Member

@zanderso zanderso commented Oct 23, 2019

Description

This PR switches Fuchsia targets to use the engine prebuilts instead of the prebuilts bundled with the Fuchsia SDK. The prebuilts bundled with the Fuchsia SDK are to be removed as part of the flutter_runner migration to the engine repo.

Tests

Updated existing tests, and added tests for querying FuchsiaDevice.targetPlatform.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@zanderso zanderso added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Oct 23, 2019
Copy link
Contributor

Choose a reason for hiding this comment

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

that this works blows my mind

Copy link
Contributor

Choose a reason for hiding this comment

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

we should use the aot runner

Copy link
Contributor

Choose a reason for hiding this comment

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

(can be a separate PR of course)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the tool doesn't know how to build aot Fuchsia packages, yet, so that'll probably be a bigger change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we link to an example cmx file here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a cmx file for the stocks example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could this use the existing artifacts.getArtifactPath(...) APIs instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: represent this as an enum/class or create const values to avoid stringly-typed issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

case Artifact.fuchsiaPlatformDill:
case Artifact.fuchsiaPatchedSdk:
case Artifact.fuchsiaFlutterJitRunner:
assert(false, 'Invalid local engine artifact $artifact.');
Copy link
Member Author

Choose a reason for hiding this comment

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

@iskakaushik Do you know where these would be in a local engine build?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, missed this somehow. They would be in the out/fuchsia_x64_.../flutter_aot_runner.far for example.

@zanderso
Copy link
Member Author

ptal

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM with nit

@zanderso zanderso merged commit 0dfabb2 into flutter:master Oct 28, 2019
@zanderso zanderso deleted the use-correct-flutter-runner branch October 28, 2019 16:38
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
* [flutter_tool] Use engine flutter_runner prebuilts

* Update packages/flutter_tools/lib/src/fuchsia/fuchsia_build.dart

Co-Authored-By: Jonah Williams <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants