Introduce dart_host_toolchain arg, remove '--checked' option from patch_sdk.py

Introduce dart_host_toolchain argument for generate_patch_sdk.gni. This allows for Flutter build to make sure that host_toolchain used for patched_sdk generation word size matches target platform.

Flutter Dart in product configuration doesn't allow --checked option. So remove '--checked' from invocation.

R=rmacnak@google.com

BUG:https://github.com/flutter/flutter/issues/10841
Committed: https://github.com/dart-lang/sdk/commit/8cd713bed3bc739e6008dd101acca659b90b5ef9

[aam, 2017-06-19 21:42:05]

Description was changed from

==========
Make generate_patched_sdk uses current_toolchain.

Flutter patched_sdk generation needs to override
host_toolchain when targeting Android: use ia32 instead of x64.

Also, Flutter release Dart doesn't recognize/support --checked option.

BUG:https://github.com/flutter/flutter/issues/10841
==========

to

==========
Make generate_patched_sdk use current_toolchain.

Flutter patched_sdk generation needs to override
host_toolchain when targeting Android: use x86 instead of x64.

Also, Flutter release Dart doesn't recognize/support --checked option.

BUG:https://github.com/flutter/flutter/issues/10841
==========

[aam, 2017-06-19 21:42:05]

aam@google.com changed reviewers:
+ rmacnak@google.com

[aam, 2017-06-20 03:44:02]

Please take a look when you have a second.
This is needed to get ball rolling with creation of flutter platform.dill/patch
sdks(per
https://github.com/flutter/engine/compare/master...aam:generate-patched-sdk)

[rmacnak, 2017-06-20 20:40:50]

https://codereview.chromium.org/2944103003/diff/20001/tools/patch_sdk.py
File tools/patch_sdk.py (right):

https://codereview.chromium.org/2944103003/diff/20001/tools/patch_sdk.py#newc...
tools/patch_sdk.py:51: # '--checked' is not recognized by Dart executable built
for Flutter Release
Product mode does not allow checked mode. This should fail in build.py -mproduct
create_sdk as well. I would just remove the flag.

[aam, 2017-06-21 01:04:57]

On 2017/06/20 20:40:50, rmacnak wrote:
> https://codereview.chromium.org/2944103003/diff/20001/tools/patch_sdk.py
> File tools/patch_sdk.py (right):
> 
>
https://codereview.chromium.org/2944103003/diff/20001/tools/patch_sdk.py#newc...
> tools/patch_sdk.py:51: # '--checked' is not recognized by Dart executable
built
> for Flutter Release
> Product mode does not allow checked mode. This should fail in build.py
-mproduct
> create_sdk as well. I would just remove the flag.

Done

[aam, 2017-06-21 14:36:57]

Description was changed from

==========
Make generate_patched_sdk use current_toolchain.

Flutter patched_sdk generation needs to override
host_toolchain when targeting Android: use x86 instead of x64.

Also, Flutter release Dart doesn't recognize/support --checked option.

BUG:https://github.com/flutter/flutter/issues/10841
==========

to

==========
Flutter Dart in product configuration doesn't allow --checked option. So remove
'--checked' from invocation.

BUG:https://github.com/flutter/flutter/issues/10841
==========

[aam, 2017-06-21 18:13:48]

Description was changed from

==========
Flutter Dart in product configuration doesn't allow --checked option. So remove
'--checked' from invocation.

BUG:https://github.com/flutter/flutter/issues/10841
==========

to

==========
Introduce dart_host_toolchain argument for generate_patch_sdk.gni. This allows
for Flutter build to make sure that host_toolchain used for patched_sdk
generation word size matches target platform.

Flutter Dart in product configuration doesn't allow --checked option. So remove
'--checked' from invocation.

BUG:https://github.com/flutter/flutter/issues/10841
==========

[aam, 2017-06-21 18:14:39]

On 2017/06/21 01:04:57, aam wrote:
> On 2017/06/20 20:40:50, rmacnak wrote:
> > https://codereview.chromium.org/2944103003/diff/20001/tools/patch_sdk.py
> > File tools/patch_sdk.py (right):
> > 
> >
>
https://codereview.chromium.org/2944103003/diff/20001/tools/patch_sdk.py#newc...
> > tools/patch_sdk.py:51: # '--checked' is not recognized by Dart executable
> built
> > for Flutter Release
> > Product mode does not allow checked mode. This should fail in build.py
> -mproduct
> > create_sdk as well. I would just remove the flag.
> 
> Done

Another attempt at actually solving host_toolchain problem with
generate_patch_sdk. PTAL.

[rmacnak, 2017-06-21 21:15:34]

https://codereview.chromium.org/2944103003/diff/60001/utils/generate_patch_sd...
File utils/generate_patch_sdk.gni (right):

https://codereview.chromium.org/2944103003/diff/60001/utils/generate_patch_sd...
utils/generate_patch_sdk.gni:10: dart_host_toolchain = host_toolchain
Can we use this for the standalone's invocation of gen_snapshot($host_toolchain)
as well? See also use of host_toolchain in compiled_action.gni

[aam, 2017-06-21 21:39:18]

https://codereview.chromium.org/2944103003/diff/60001/utils/generate_patch_sd...
File utils/generate_patch_sdk.gni (right):

https://codereview.chromium.org/2944103003/diff/60001/utils/generate_patch_sd...
utils/generate_patch_sdk.gni:10: dart_host_toolchain = host_toolchain
On 2017/06/21 21:15:34, rmacnak wrote:
> Can we use this for the standalone's invocation of
gen_snapshot($host_toolchain)
> as well? 

Do you mean invocation in flutter/build/flutter_app.gni? Sure if that's make
sense.

> See also use of host_toolchain in compiled_action.gni

Does that have to be switched to use of dart_host_toolchain too?

[siva, 2017-06-22 01:15:14]

asiva@google.com changed reviewers:
+ asiva@google.com

[siva, 2017-06-22 01:15:14]

https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py
File tools/patch_sdk.py (right):

https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py#newc...
tools/patch_sdk.py:51: subprocess.check_call([options.dart_executable,
dart_file] + args)
Why do we drop the '--checked' option?

[aam, 2017-06-22 04:17:25]

On 2017/06/22 01:15:14, siva wrote:
> https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py
> File tools/patch_sdk.py (right):
> 
>
https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py#newc...
> tools/patch_sdk.py:51: subprocess.check_call([options.dart_executable,
> dart_file] + args)
> Why do we drop the '--checked' option?

dart built as part of Flutter release(product) configuration doesn't recognize
--checked option, so attempt to run patch_sdk script when building flutter
release app fails because it complains about unrecognized option.

[aam, 2017-06-22 04:19:09]

On 2017/06/22 04:17:25, aam wrote:
> On 2017/06/22 01:15:14, siva wrote:
> > https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py
> > File tools/patch_sdk.py (right):
> > 
> >
>
https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py#newc...
> > tools/patch_sdk.py:51: subprocess.check_call([options.dart_executable,
> > dart_file] + args)
> > Why do we drop the '--checked' option?
> 
> dart built as part of Flutter release(product) configuration doesn't recognize
> --checked option, so attempt to run patch_sdk script when building flutter
> release app fails because it complains about unrecognized option.

It's this code that excludes support for --checked option for PRODUCT
configuration:
https://github.com/dart-lang/sdk/blob/master/runtime/vm/isolate.cc#L67

[aam, 2017-06-22 04:19:10]

On 2017/06/22 04:17:25, aam wrote:
> On 2017/06/22 01:15:14, siva wrote:
> > https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py
> > File tools/patch_sdk.py (right):
> > 
> >
>
https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py#newc...
> > tools/patch_sdk.py:51: subprocess.check_call([options.dart_executable,
> > dart_file] + args)
> > Why do we drop the '--checked' option?
> 
> dart built as part of Flutter release(product) configuration doesn't recognize
> --checked option, so attempt to run patch_sdk script when building flutter
> release app fails because it complains about unrecognized option.

It's this code that excludes support for --checked option for PRODUCT
configuration:
https://github.com/dart-lang/sdk/blob/master/runtime/vm/isolate.cc#L67

[aam, 2017-06-22 04:19:11]

On 2017/06/22 04:17:25, aam wrote:
> On 2017/06/22 01:15:14, siva wrote:
> > https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py
> > File tools/patch_sdk.py (right):
> > 
> >
>
https://codereview.chromium.org/2944103003/diff/60001/tools/patch_sdk.py#newc...
> > tools/patch_sdk.py:51: subprocess.check_call([options.dart_executable,
> > dart_file] + args)
> > Why do we drop the '--checked' option?
> 
> dart built as part of Flutter release(product) configuration doesn't recognize
> --checked option, so attempt to run patch_sdk script when building flutter
> release app fails because it complains about unrecognized option.

It's this code that excludes support for --checked option for PRODUCT
configuration:
https://github.com/dart-lang/sdk/blob/master/runtime/vm/isolate.cc#L67

[aam, 2017-06-22 04:34:07]

https://codereview.chromium.org/2944103003/diff/60001/utils/generate_patch_sd...
File utils/generate_patch_sdk.gni (right):

https://codereview.chromium.org/2944103003/diff/60001/utils/generate_patch_sd...
utils/generate_patch_sdk.gni:10: dart_host_toolchain = host_toolchain
On 2017/06/21 21:15:34, rmacnak wrote:
> Can we use this for the standalone's invocation of
gen_snapshot($host_toolchain)
> as well? See also use of host_toolchain in compiled_action.gni

Done.

[rmacnak, 2017-06-22 17:10:18]

lgtm

[zra, 2017-06-22 17:15:47]

zra@google.com changed reviewers:
+ zra@google.com

[zra, 2017-06-22 17:15:47]

Should dart_host_toolchain replace all uses of host_toolchain? It is confusing
to use host_toolchain in some places and dart_host_toolchain in others.

[zra, 2017-06-22 17:16:39]

https://codereview.chromium.org/2944103003/diff/80001/build/dart_host_toolcha...
File build/dart_host_toolchain.gni (right):

https://codereview.chromium.org/2944103003/diff/80001/build/dart_host_toolcha...
build/dart_host_toolchain.gni:6: dart_host_toolchain = host_toolchain
Please add comments here explaining what this option does. In particular, if
dart_host_toolchain should not replace all uses of host_toolchain, this comment
should explain when to use one and not the other.

[aam, 2017-06-22 17:52:09]

On 2017/06/22 17:16:39, zra wrote:
>
https://codereview.chromium.org/2944103003/diff/80001/build/dart_host_toolcha...
> File build/dart_host_toolchain.gni (right):
> 
>
https://codereview.chromium.org/2944103003/diff/80001/build/dart_host_toolcha...
> build/dart_host_toolchain.gni:6: dart_host_toolchain = host_toolchain
> Please add comments here explaining what this option does. In particular, if
> dart_host_toolchain should not replace all uses of host_toolchain, this
comment
> should explain when to use one and not the other.

[aam, 2017-06-22 17:52:26]

On 2017/06/22 17:52:09, aam wrote:
> On 2017/06/22 17:16:39, zra wrote:
> >
>
https://codereview.chromium.org/2944103003/diff/80001/build/dart_host_toolcha...
> > File build/dart_host_toolchain.gni (right):
> > 
> >
>
https://codereview.chromium.org/2944103003/diff/80001/build/dart_host_toolcha...
> > build/dart_host_toolchain.gni:6: dart_host_toolchain = host_toolchain
> > Please add comments here explaining what this option does. In particular, if
> > dart_host_toolchain should not replace all uses of host_toolchain, this
> comment
> > should explain when to use one and not the other.

Done.

[aam, 2017-06-22 17:52:40]

On 2017/06/22 17:15:47, zra wrote:
> Should dart_host_toolchain replace all uses of host_toolchain? It is confusing
> to use host_toolchain in some places and dart_host_toolchain in others.

Yes, that sounds right. I replaced two remaining uses of host_toolchain in
observatory and kernel-service builds with dart_host_toolchain.

[aam, 2017-06-22 19:41:36]

Description was changed from

==========
Introduce dart_host_toolchain argument for generate_patch_sdk.gni. This allows
for Flutter build to make sure that host_toolchain used for patched_sdk
generation word size matches target platform.

Flutter Dart in product configuration doesn't allow --checked option. So remove
'--checked' from invocation.

BUG:https://github.com/flutter/flutter/issues/10841
==========

to

==========
Introduce dart_host_toolchain argument for generate_patch_sdk.gni. This allows
for Flutter build to make sure that host_toolchain used for patched_sdk
generation word size matches target platform.

Flutter Dart in product configuration doesn't allow --checked option. So remove
'--checked' from invocation.

R=rmacnak@google.com

BUG:https://github.com/flutter/flutter/issues/10841
Committed:
https://github.com/dart-lang/sdk/commit/8cd713bed3bc739e6008dd101acca659b90b5ef9
==========

[aam, 2017-06-22 19:41:37]

Committed patchset #7 (id:120001) manually as
8cd713bed3bc739e6008dd101acca659b90b5ef9 (presubmit successful).

[zra, 2017-06-22 19:42:44]

lgtm