Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[null-safety] updated rules to build sound dill/SDKs for web #19162

Closed
jonahwilliams wants to merge 11 commits into from
+157 −2
Closed

[null-safety] updated rules to build sound dill/SDKs for web #19162

Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fcd5146
[null-safety] rules for web
Jun 19, 2020
6441329
Update BUILD.gn
Jun 19, 2020
8a2b7df
fix formatting
Jun 19, 2020
a4d7838
minor renames
Jun 19, 2020
c0dc35c
Merge branch 'master' of github.com:flutter/engine into add_compilati…
Jun 26, 2020
824dd64
update now that _engine is ready
Jun 26, 2020
ea587fa
remove K
Jun 26, 2020
e4507cf
Remove deps
Jun 26, 2020
271a45b
did this get deleted
Jun 26, 2020
6676542
copy paste
Jun 26, 2020
23d9524
Merge branch 'master' of github.com:flutter/engine into add_compilati…
Jun 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 159 additions & 2 deletions web_sdk/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import("//third_party/dart/build/dart/dart_action.gni")
import("//third_party/dart/utils/compile_platform.gni")

sdk_dill = "$root_out_dir/flutter_web_sdk/kernel/flutter_ddc_sdk.dill"
sdk_dill_sound =
"$root_out_dir/flutter_web_sdk/kernel/flutter_ddc_sdk_sound.dill"

web_ui_sources = exec_script("//third_party/dart/tools/list_dart_files.py",
[
Expand All @@ -29,8 +31,11 @@ web_engine_sources += [ "//flutter/lib/web_ui/lib/src/engine.dart" ]
group("web_sdk") {
deps = [
":flutter_dartdevc_canvaskit_kernel_sdk",
":flutter_dartdevc_canvaskit_kernel_sdk_sound",
Comment on lines 33 to +34
Copy link
Contributor

@nshahan nshahan Jun 26, 2020

Choose a reason for hiding this comment

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

Does the canvaskit version need outlines?

":flutter_dartdevc_kernel_sdk",
":flutter_dartdevc_kernel_sdk_outline",
":flutter_dartdevc_kernel_sdk_outline_sound",
":flutter_dartdevc_kernel_sdk_sound",
":web_engine_sources",
":web_ui_library",
":web_ui_sources",
Expand Down Expand Up @@ -164,7 +169,6 @@ prebuilt_dart_action("flutter_dartdevc_kernel_sdk") {

args = [
"--enable-experiment=non-nullable",
"-k",
"--compile-sdk",
"dart:core",

Expand Down Expand Up @@ -216,7 +220,6 @@ prebuilt_dart_action("flutter_dartdevc_canvaskit_kernel_sdk") {

args = [
"--enable-experiment=non-nullable",
"-k",
"--compile-sdk",
"dart:core",

Expand Down Expand Up @@ -248,3 +251,157 @@ prebuilt_dart_action("flutter_dartdevc_canvaskit_kernel_sdk") {
"$root_out_dir/flutter_web_sdk/kernel/legacy-canvaskit/dart_sdk.js"),
]
}

# Compiles the DDC CanvasKit SDK's JS code for null safety.
prebuilt_dart_action("flutter_dartdevc_canvaskit_kernel_sdk_sound") {
deps = [
":flutter_dartdevc_kernel_sdk_outline_sound",
Copy link
Contributor

@nshahan nshahan Jun 26, 2020

Choose a reason for hiding this comment

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

What are the outline dep for? I don't see it being passed as an arg.

In the SDK I use the full .dill to compile the js from .dill now.

If you do need it, should this outline be the canvaskit version just to be safe? I suppose we want the API to be the same but is it a guaranty?

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jun 26, 2020

Choose a reason for hiding this comment

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

The outlines are fed to the frontend_server for the hot reload cycle. We don't use them for building the precompiled SDK.

Their APIs should be the same since the only different is a define within the implementation.

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jun 26, 2020

Choose a reason for hiding this comment

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

It seems like I can remove the dep though, since it isn't used

Copy link
Contributor

@nshahan nshahan Jun 26, 2020

Choose a reason for hiding this comment

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

What is the actual input given to DDC to compile the sdk? I don't see where we say use the sound vs unsound dill to compile the js.

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jun 26, 2020

Choose a reason for hiding this comment

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

IIRC the source files are used directly, not the dills

Copy link
Contributor

@nshahan nshahan Jun 26, 2020

Choose a reason for hiding this comment

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

Thanks. I should say that part of my question is rooted in the investigation I'm doing to try and figure out how DDC is being used to compile the sdk in all the different environments. Flutter web is the one I understand the least at the moment.

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jun 27, 2020

Choose a reason for hiding this comment

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

Happy to discuss in person more if it helps. The compilation needs should be fairly similar to whatever the dart SDK is doing, and the current choice of scripts/args is more than likely historical rather than intentional.

"//third_party/dart:create_sdk",
"//third_party/dart/pkg:pkg_files_stamp",
"//third_party/dart/utils/dartdevc:dartdevc_files_stamp",
"//third_party/dart/utils/dartdevc:dartdevc_sdk_patch_stamp",
]

inputs = [ "sdk_rewriter.dart" ] + web_ui_sources + web_engine_sources

packages = "//third_party/dart/.packages"

script = "//third_party/dart/pkg/dev_compiler/bin/dartdevc.dart"

outputs = [
"$root_out_dir/flutter_web_sdk/kernel/amd-canvaskit-sound/dart_sdk.js",
"$root_out_dir/flutter_web_sdk/kernel/amd-canvaskit-sound/dart_sdk.js.map",
]

args = [
"--enable-experiment=non-nullable",
"--sound-null-safety",
"--compile-sdk",
"dart:core",

# Additional Flutter web dart libraries
"dart:ui",
"dart:_engine",
"--no-summarize",
"--packages",
"file:///" + rebase_path("//third_party/dart/.packages"),
"--multi-root-scheme",
"org-dartlang-sdk",
"--multi-root",
"file:///" + rebase_path("$root_out_dir"),
"--multi-root-output-path",
rebase_path("$root_out_dir/"),
"--libraries-file",
"org-dartlang-sdk:///flutter_web_sdk/libraries.json",
"--inline-source-map",
"-DFLUTTER_WEB_USE_SKIA=true",
"--modules",
"amd",
"-o",
rebase_path(
"$root_out_dir/flutter_web_sdk/kernel/amd-canvaskit-sound/dart_sdk.js"),
"--modules",
"legacy",
"-o",
rebase_path(
"$root_out_dir/flutter_web_sdk/kernel/legacy-canvaskit-sound/dart_sdk.js"),
]
}

# Compiles the DDC SDK's JS code for null safety.
prebuilt_dart_action("flutter_dartdevc_kernel_sdk_sound") {
deps = [
":flutter_dartdevc_kernel_sdk_outline_sound",
Copy link
Contributor

@nshahan nshahan Jun 26, 2020

Choose a reason for hiding this comment

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

Same questions here.

"//third_party/dart:create_sdk",
"//third_party/dart/pkg:pkg_files_stamp",
"//third_party/dart/utils/dartdevc:dartdevc_files_stamp",
"//third_party/dart/utils/dartdevc:dartdevc_sdk_patch_stamp",
]

inputs = [ "sdk_rewriter.dart" ] + web_ui_sources + web_engine_sources

packages = "//third_party/dart/.packages"

script = "//third_party/dart/pkg/dev_compiler/bin/dartdevc.dart"

outputs = [
"$root_out_dir/flutter_web_sdk/kernel/amd-sound/dart_sdk.js",
"$root_out_dir/flutter_web_sdk/kernel/amd-sound/dart_sdk.js.map",
]

args = [
"--enable-experiment=non-nullable",
"--sound-null-safety",
"--compile-sdk",
"dart:core",

# Additional Flutter web dart libraries
"dart:ui",
"dart:_engine",
"--no-summarize",
"--packages",
"file:///" + rebase_path("//third_party/dart/.packages"),
"--multi-root-scheme",
"org-dartlang-sdk",
"--multi-root",
"file:///" + rebase_path("$root_out_dir"),
"--multi-root-output-path",
rebase_path("$root_out_dir/"),
"--libraries-file",
"org-dartlang-sdk:///flutter_web_sdk/libraries.json",
"--inline-source-map",
"--modules",
"amd",
"-o",
rebase_path("$root_out_dir/flutter_web_sdk/kernel/amd-sound/dart_sdk.js"),
"--modules",
"legacy",
"-o",
rebase_path(
"$root_out_dir/flutter_web_sdk/kernel/legacy-sound/dart_sdk.js"),
]
}

# Compile the DDC SDK's summary.
prebuilt_dart_action("flutter_dartdevc_kernel_sdk_outline_sound") {
deps = [
"//third_party/dart:create_sdk",
"//third_party/dart/pkg:pkg_files_stamp",
"//third_party/dart/utils/dartdevc:dartdevc_files_stamp",
"//third_party/dart/utils/dartdevc:dartdevc_sdk_patch_stamp",
]

inputs = [ "sdk_rewriter.dart" ] + web_ui_sources + web_engine_sources

outputs = [
sdk_dill_sound,
]

script = "//third_party/dart/utils/bazel/kernel_worker.dart"

args = [
"--enable-experiment=non-nullable",
"--sound-null-safety ",
Copy link
Contributor

@nshahan nshahan Jun 19, 2020

Choose a reason for hiding this comment

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

I'm not familiar with the kernel_worker.dart script and I'm not using it when I build the .dill files in the sdk build target. Does it already support the --sound-null-safety flag? I didn't see it in the args in a quick search.

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jun 19, 2020

Choose a reason for hiding this comment

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

I thought It worked locally but I might have missed some flags. At any rate, everything still fails when compiling, I'll double check what dart_sdk is doing and update though

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jun 26, 2020

Choose a reason for hiding this comment

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

everything seems to work OK

"--summary-only",
"--target",
"ddc",
"--packages-file",
"file:///" + rebase_path("//third_party/dart/.packages"),
"--multi-root-scheme",
"org-dartlang-sdk",
"--multi-root",
"file:///" + rebase_path("$root_out_dir"),
"--libraries-file",
"org-dartlang-sdk:///flutter_web_sdk/libraries.json",
"--output",
rebase_path(sdk_dill_sound),
"--source",
"dart:core",

# Additional Flutter web dart libraries
"--source",
"dart:ui",
"--source",
"dart:_engine",
]
}