[null-safety] updated rules to build sound dill/SDKs for web #19162
Conversation
web_sdk/BUILD.gn
Outdated
| 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" |
There was a problem hiding this comment.
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.
I'm going to leave any renames for later, we'd likely need to upload both though to google3 so all tools could update
web_sdk/BUILD.gn
Outdated
| args = [ | ||
| "--enable-experiment=non-nullable", | ||
| "--sound-null-safety", | ||
| "-k", |
There was a problem hiding this comment.
nit: -k is no longer needed and is ignored.
There was a problem hiding this comment.
👆 I see what you did there.
web_sdk/BUILD.gn
Outdated
| } | ||
|
|
||
| # Compiles the DDC CanvasKit SDK's JS code for null safety. | ||
| prebuilt_dart_action("flutter_dartdevc_canvaskit_kernel_sdk_strong") { |
There was a problem hiding this comment.
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".
There was a problem hiding this comment.
That SGTM, turns out we're still using strong in some places from the Dart 2 migration
web_sdk/BUILD.gn
Outdated
| args = [ | ||
| "--enable-experiment=non-nullable", | ||
| "--sound-null-safety", | ||
| "-k", |
|
|
||
| args = [ | ||
| "--enable-experiment=non-nullable", | ||
| "--sound-null-safety ", |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
everything seems to work OK
jonahwilliams
changed the title
| ":flutter_dartdevc_canvaskit_kernel_sdk", | ||
| ":flutter_dartdevc_canvaskit_kernel_sdk_sound", |
There was a problem hiding this comment.
Does the canvaskit version need outlines?
web_sdk/BUILD.gn
Outdated
| # 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", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
It seems like I can remove the dep though, since it isn't used
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
IIRC the source files are used directly, not the dills
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
web_sdk/BUILD.gn
Outdated
| # 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", |
…on_for_sound_null_safety
Re-attempt at #19162 which seems to be stuck on a bad framework revision
Description
Generates sound null safety code to resolve flutter/flutter#59792 .
This is currently blocked by theThis is ready!dart:_enginemigration to null safety.FYI @nshahan