Add flutter mode to patched_sdk

Add flutter mode to patched_sdk

This is needed to create a platform.dill for flutter.

R=aam@google.com

Committed: https://github.com/dart-lang/sdk/commit/5a63b0e3392ad52da5bda3554490be8c3f5a837f

[Siggi Cherem (dart-lang), 2017-06-08 22:16:03]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2017-06-08 22:18:52]

sigmund@google.com changed reviewers:
+ aam@google.com, ahe@google.com

[Siggi Cherem (dart-lang), 2017-06-08 22:18:53]

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/dill/dill_loader.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/dill/dill_loader.dart:38: assert(builder.library !=
null,
(platched_sdk often fails here when files are not found, this helps debug the
underlying issue.)

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/body_builder.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/body_builder.dart:96: final bool
ignoreMainInGetMainClosure;
Peter - I don't think it is worth moving this into the backendTarget because the
plan is to get rid of it very soon.

https://codereview.chromium.org/2931773003/diff/20001/pkg/kernel/lib/target/f...
File pkg/kernel/lib/target/flutter_fasta.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/pkg/kernel/lib/target/f...
pkg/kernel/lib/target/flutter_fasta.dart:20: class FlutterFastaTarget extends
flutter_target.FlutterTarget {
I saw that Dima added the vm_fasta target, that overrides how transformers are
run. Do we really need the distinction, any reason why we don't replace how
transformations are run in the regular targets?

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart
File tools/patch_sdk.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:191: "vmservice_io": const LibraryInfo(
Just discovered from talking with Siva that vmservice_io and vmservice_sky
should not be in platform.dill. I'll remove that on a separate change though.

[Siggi Cherem (dart-lang), 2017-06-08 22:19:46]

Alex - I tried this with your instructions on the flutter repos and I was able
to build the platform.dill file without crashing. `dart:ui` appears to be
included too. I haven't validated that you can build the app and run it though.

To use this I did:
 - change the engine/src/dart to use our master and patch in this CL
 - added the patches you have in engine/src/flutter
 - also added an extra dependency from flutter_patched_sdk to the vm
patched_sdk:


   generate_patched_sdk(target_name) {                                          
                                           
     mode = "flutter"                                                           
 
-    deps = concatenation_target_names
+    deps = concatenation_target_names + ["//dart/runtime/vm:patched_sdk"]      
 
     input_patches_dir = "$target_gen_dir/patches"                              
 
     patched_sdk_dir = "flutter_patched_sdk"                                    
 
   }

[aam, 2017-06-08 23:30:40]

Awesome! One quick observation - it seems that this patch produces platform.dill
with both dart:ui and builtin libraries. I don't think we want builtin in the
flutter platform.dill.

[ahe, 2017-06-09 09:23:35]

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/dill/dill_loader.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/dill/dill_loader.dart:38: assert(builder.library !=
null,
On 2017/06/08 22:18:53, Siggi Cherem (dart-lang) wrote:
> (platched_sdk often fails here when files are not found, this helps debug the
> underlying issue.)

Please use internalError.

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/body_builder.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/body_builder.dart:96: final bool
ignoreMainInGetMainClosure;
On 2017/06/08 22:18:53, Siggi Cherem (dart-lang) wrote:
> Peter - I don't think it is worth moving this into the backendTarget because
the
> plan is to get rid of it very soon.

Agreed.

https://codereview.chromium.org/2931773003/diff/20001/pkg/kernel/lib/target/f...
File pkg/kernel/lib/target/flutter_fasta.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/pkg/kernel/lib/target/f...
pkg/kernel/lib/target/flutter_fasta.dart:20: class FlutterFastaTarget extends
flutter_target.FlutterTarget {
On 2017/06/08 22:18:53, Siggi Cherem (dart-lang) wrote:
> I saw that Dima added the vm_fasta target, that overrides how transformers are
> run. Do we really need the distinction, any reason why we don't replace how
> transformations are run in the regular targets?

I don't know. I've been wondering about that as well.

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart
File tools/patch_sdk.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:108: if ((forVm || forFlutter) && library.isDart2JsLibrary)
continue;
!forDart2js

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:126: backendTarget: forVm ? 'vm_fasta' : 'flutter_fasta');
I think you'll get a simple conflict here as Dima will land a change that
converts backendTarget to an instance of Target instead of a String.

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:146: //    exists.
I don't understand this.

[Siggi Cherem (dart-lang), 2017-06-14 17:49:01]

Patchset #2 (id:40001) has been deleted

[Siggi Cherem (dart-lang), 2017-06-14 17:54:04]

Patchset #2 (id:60001) has been deleted

[Siggi Cherem (dart-lang), 2017-06-14 17:54:08]

Patchset #2 (id:80001) has been deleted

[Siggi Cherem (dart-lang), 2017-06-14 17:55:37]

PTAL

@aam - good point, I went ahead and removed `dart:_builtin` in the flutter
target. I also removed the builtin transformer entirely - AFAIK it is no longer
needed.

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/dill/dill_loader.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/dill/dill_loader.dart:38: assert(builder.library !=
null,
On 2017/06/09 09:23:35, ahe wrote:
> On 2017/06/08 22:18:53, Siggi Cherem (dart-lang) wrote:
> > (platched_sdk often fails here when files are not found, this helps debug
the
> > underlying issue.)
> 
> Please use internalError.

Done.

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart
File tools/patch_sdk.dart (right):

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:108: if ((forVm || forFlutter) && library.isDart2JsLibrary)
continue;
On 2017/06/09 09:23:35, ahe wrote:
> !forDart2js

Done.

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:126: backendTarget: forVm ? 'vm_fasta' : 'flutter_fasta');
On 2017/06/09 09:23:35, ahe wrote:
> I think you'll get a simple conflict here as Dima will land a change that
> converts backendTarget to an instance of Target instead of a String.

Thanks for the heads up. Tuns out this API is still going through the
options-parsing logic, so his change didn't affect this APIs. We can change that
separately.

https://codereview.chromium.org/2931773003/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:146: //    exists.
On 2017/06/09 09:23:35, ahe wrote:
> I don't understand this.

Just clarified, hope this helps.

[aam, 2017-06-14 20:55:38]

LGTM, thanks!

[Siggi Cherem (dart-lang), 2017-06-15 00:29:43]

Description was changed from

==========
Add flutter mode to patched_sdk

This is needed to create a platform.dill for flutter.
==========

to

==========
Add flutter mode to patched_sdk

This is needed to create a platform.dill for flutter.

R=aam@google.com

Committed:
https://github.com/dart-lang/sdk/commit/5a63b0e3392ad52da5bda3554490be8c3f5a837f
==========

[Siggi Cherem (dart-lang), 2017-06-15 00:29:45]

Committed patchset #2 (id:100001) manually as
5a63b0e3392ad52da5bda3554490be8c3f5a837f (presubmit successful).