-
Notifications
You must be signed in to change notification settings - Fork 6k
[null-safety] updated rules to build sound dill/SDKs for web #19162
Changes from 3 commits
fcd5146
6441329
8a2b7df
a4d7838
c0dc35c
824dd64
ea587fa
e4507cf
271a45b
6676542
23d9524
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 |
|---|---|---|
|
|
@@ -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_strong = | ||
| "$root_out_dir/flutter_web_sdk/kernel/flutter_ddc_sdk_strong.dill" | ||
|
|
||
| web_ui_sources = exec_script("//third_party/dart/tools/list_dart_files.py", | ||
| [ | ||
|
|
@@ -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_strong", | ||
| ":flutter_dartdevc_kernel_sdk", | ||
| ":flutter_dartdevc_kernel_sdk_outline", | ||
| ":flutter_dartdevc_kernel_sdk_outline_strong", | ||
| ":flutter_dartdevc_kernel_sdk_strong", | ||
| ":web_engine_sources", | ||
| ":web_ui_library", | ||
| ":web_ui_sources", | ||
|
|
@@ -244,3 +249,160 @@ 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_strong") { | ||
|
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. nit: not a big deal but we've committed to the term "sound" going forward so I've been trying to use it everywhere we were saying "strong".
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 SGTM, turns out we're still using strong in some places from the Dart 2 migration |
||
| deps = [ | ||
| ":flutter_dartdevc_kernel_sdk_outline_strong", | ||
| "//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-strong/dart_sdk.js", | ||
| "$root_out_dir/flutter_web_sdk/kernel/amd-canvaskit-strong/dart_sdk.js.map", | ||
| ] | ||
|
|
||
| args = [ | ||
| "--enable-experiment=non-nullable", | ||
| "--sound-null-safety", | ||
| "-k", | ||
|
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. nit:
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. k removed
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 see what you did there. |
||
| "--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-strong/dart_sdk.js"), | ||
| "--modules", | ||
| "legacy", | ||
| "-o", | ||
| rebase_path( | ||
| "$root_out_dir/flutter_web_sdk/kernel/legacy-canvaskit-strong/dart_sdk.js"), | ||
| rebase_path(sdk_dill_strong), | ||
| ] | ||
| } | ||
|
|
||
| # Compiles the DDC SDK's JS code for null safety. | ||
| prebuilt_dart_action("flutter_dartdevc_kernel_sdk_strong") { | ||
| deps = [ | ||
| ":flutter_dartdevc_kernel_sdk_outline_strong", | ||
| "//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-strong/dart_sdk.js", | ||
| "$root_out_dir/flutter_web_sdk/kernel/amd-strong/dart_sdk.js.map", | ||
| ] | ||
|
|
||
| args = [ | ||
| "--enable-experiment=non-nullable", | ||
| "--sound-null-safety", | ||
| "-k", | ||
|
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. ditto
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. done |
||
| "--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-strong/dart_sdk.js"), | ||
| "--modules", | ||
| "legacy", | ||
| "-o", | ||
| rebase_path( | ||
| "$root_out_dir/flutter_web_sdk/kernel/legacy-strong/dart_sdk.js"), | ||
| ] | ||
| } | ||
|
|
||
| # Compile the DDC SDK's summary. | ||
| prebuilt_dart_action("flutter_dartdevc_kernel_sdk_outline_strong") { | ||
| 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_strong, | ||
| ] | ||
|
|
||
| script = "//third_party/dart/utils/bazel/kernel_worker.dart" | ||
|
|
||
| args = [ | ||
| "--enable-experiment=non-nullable", | ||
| "--sound-null-safety ", | ||
|
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'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
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 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
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. 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_strong), | ||
| "--source", | ||
| "dart:core", | ||
|
|
||
| # Additional Flutter web dart libraries | ||
| "--source", | ||
| "dart:ui", | ||
| "--source", | ||
| "dart:_engine", | ||
| ] | ||
| } | ||
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.
nit: Rename to mirror the names used in the sdk: I'm in the process of renaming to be more consistent with other tools and avoid confusion between the full and outline dills.
flutter_ddc_outline.dill <-- (weak)
flutter_ddc_outline_sound.dill <-- (strong)
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.
I'm going to leave any renames for later, we'd likely need to upload both though to google3 so all tools could update