-
Notifications
You must be signed in to change notification settings - Fork 6k
Build Web SDK in wasm build tree #36520
Changes from 5 commits
543b02d
07690ee
2259916
d1a9f0a
4650b9e
1041f93
e4f0c0d
81eff9c
82812b6
f3b6cfd
a9e7f3f
9d1d6ae
8bfb22f
094c678
d81efbd
a0d5345
aeaa119
3d667d2
924697d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import("//flutter/common/config.gni") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two things:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original reason for moving this was that the After some additional changes though, the web sdk build now uses the prebuilt sdk in-place and so does not have a dependency on the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want to allow Dart SDK hackers to test Dart SDK builds with local source changes, so we may end up with this dependency again. If it's better to have it split out into a standalone
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't follow how that's related. The existing way to hack on the Dart SDK in the engine tree is to make changes under
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the That being said, we aren't actually using the dart-sdk target for the web build anymore, we just use prebuilts. I also don't actually see us adding a dependency on the |
||
| import("//third_party/dart/build/dart/copy_tree.gni") | ||
|
|
||
| if (flutter_prebuilt_dart_sdk) { | ||
| copy_trees("_copy_trees") { | ||
| sources = [ | ||
| { | ||
| target = "copy_dart_sdk" | ||
| visibility = [ ":dart_sdk" ] | ||
| source = prebuilt_dart_sdk | ||
| dest = "$root_out_dir/dart-sdk" | ||
| ignore_patterns = "{}" | ||
| }, | ||
| ] | ||
| } | ||
| } | ||
|
|
||
| group("dart_sdk") { | ||
| if (flutter_prebuilt_dart_sdk) { | ||
| public_deps = [ ":copy_dart_sdk" ] | ||
| } else { | ||
| public_deps = [ "//third_party/dart:create_sdk" ] | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:async'; | ||
| import 'dart:io' show Directory, Platform; | ||
| import 'dart:io' show Directory; | ||
|
|
||
| import 'package:args/command_runner.dart'; | ||
| import 'package:path/path.dart' as path; | ||
|
|
@@ -43,14 +43,8 @@ class BuildCommand extends Command<bool> with ArgUtils<bool> { | |
| final FilePath libPath = FilePath.fromWebUi('lib'); | ||
| final List<PipelineStep> steps = <PipelineStep>[ | ||
| GnPipelineStep(), | ||
| NinjaPipelineStep(target: environment.engineBuildDir), | ||
| NinjaPipelineStep(target: environment.wasmReleaseOutDir), | ||
| ]; | ||
| if (buildCanvasKit) { | ||
| steps.addAll(<PipelineStep>[ | ||
| GnPipelineStep(target: 'canvaskit'), | ||
| NinjaPipelineStep(target: environment.wasmReleaseOutDir), | ||
| ]); | ||
| } | ||
| final Pipeline buildPipeline = Pipeline(steps: steps); | ||
| await buildPipeline.run(); | ||
|
|
||
|
|
@@ -73,39 +67,19 @@ class BuildCommand extends Command<bool> with ArgUtils<bool> { | |
| /// Not safe to interrupt as it may leave the `out/` directory in a corrupted | ||
| /// state. GN is pretty quick though, so it's OK to not support interruption. | ||
| class GnPipelineStep extends ProcessStep { | ||
| GnPipelineStep({this.target = 'engine'}) | ||
| : assert(target == 'engine' || target == 'canvaskit'); | ||
|
|
||
| @override | ||
| String get description => 'gn'; | ||
|
|
||
| @override | ||
| bool get isSafeToInterrupt => false; | ||
|
|
||
| /// The target to build with gn. | ||
| /// | ||
| /// Acceptable values: engine, canvaskit | ||
| final String target; | ||
|
|
||
| @override | ||
| Future<ProcessManager> createProcess() { | ||
| print('Running gn for $target...'); | ||
| final List<String> gnArgs = <String>[]; | ||
| if (target == 'engine') { | ||
| gnArgs.addAll(<String>[ | ||
| '--unopt', | ||
| if (Platform.isMacOS) '--xcode-symlinks', | ||
| '--full-dart-sdk', | ||
| if (environment.isMacosArm) '--mac-cpu=arm64', | ||
| ]); | ||
| } else if (target == 'canvaskit') { | ||
| gnArgs.addAll(<String>[ | ||
| '--wasm', | ||
| '--runtime-mode=release', | ||
| ]); | ||
| } else { | ||
| throw StateError('Target was not engine or canvaskit: $target'); | ||
| } | ||
| print('Running gn...'); | ||
| const List<String> gnArgs = <String>[ | ||
| '--wasm', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that this target is going to be used for all of flutter web (not just for wasm), does it make sense to change the name to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there was some extensive discussion about this with the rest of the engine team, I'd like to ask @hterkelsen what their concerns were and why they landed on this. In any case, the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My concern with using
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense. I think I get what you're going for. Let me make some changes and see what I can do. |
||
| '--runtime-mode=release', | ||
| ]; | ||
| return startProcess( | ||
| path.join(environment.flutterDirectory.path, 'tools', 'gn'), | ||
| gnArgs, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file needs a copyright header