Make patch_sdk step invoke fasta's compile_platform to generate patched_sdk/platform.dill.

Make patch_sdk step invoke fasta's compile_platform to generate patched_sdk/platform.dill.

This is where Kernel isolate expects to find it currently.

This is a preparation for flipping DFE_USE_FASTA to true.

R=ahe@google.com, kustermann@google.com

Committed: https://github.com/dart-lang/sdk/commit/80c3247f99651a15bbdee93d3af950bceacb00bf

[Vyacheslav Egorov (Google), 2017-02-23 15:20:45]

vegorov@google.com changed reviewers:
+ ahe@google.com, kustermann@google.com

[Vyacheslav Egorov (Google), 2017-02-23 15:20:46]

Please take a look.

[ahe, 2017-02-23 15:27:10]

lgtm

I'm not sure why you needed to implement --packages for compile-platform.

[kustermann, 2017-02-23 15:59:39]

LGTM

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

https://codereview.chromium.org/2710943006/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:22: if (argv.length != 5 || (!argv.isEmpty && argv.first !=
'vm')) {
We have shortcut operators: Just use

if (argv.length != 5 || argv.first != 'vm') {
}

https://codereview.chromium.org/2710943006/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:25: print('MODE must be one of ddc or vm.');
Remove "ddc or".

https://codereview.chromium.org/2710943006/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:33: 'obj', 'gen', 'patched_sdk'));
final?

https://codereview.chromium.org/2710943006/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:45: var packagesFile = argv[4];
final?

https://codereview.chromium.org/2710943006/diff/20001/tools/patch_sdk.dart#ne...
tools/patch_sdk.dart:197: compile_platform.main(<String>[
await compile_platform.main();

[Vyacheslav Egorov (Google), 2017-02-23 16:12:49]

Description was changed from

==========
Make patch_sdk step invoke fasta's compile_platform to generate
patched_sdk/platform.dill.

This is where Kernel isolate expects to find it currently.

This is a preparation for flipping DFE_USE_FASTA to true.
==========

to

==========
Make patch_sdk step invoke fasta's compile_platform to generate
patched_sdk/platform.dill.

This is where Kernel isolate expects to find it currently.

This is a preparation for flipping DFE_USE_FASTA to true.

R=ahe@google.com, kustermann@google.com

Committed:
https://github.com/dart-lang/sdk/commit/80c3247f99651a15bbdee93d3af950bceacb00bf
==========

[Vyacheslav Egorov (Google), 2017-02-23 16:12:50]

Committed patchset #2 (id:20001) manually as
80c3247f99651a15bbdee93d3af950bceacb00bf (presubmit successful).

[kustermann, 2017-03-02 12:14:14]

https://codereview.chromium.org/2710943006/diff/20001/runtime/vm/BUILD.gn
File runtime/vm/BUILD.gn (right):

https://codereview.chromium.org/2710943006/diff/20001/runtime/vm/BUILD.gn#new...
runtime/vm/BUILD.gn:416: kernel_sources =
exec_script("../../tools/list_dart_files.py",
These exec_script things have just broken the buildbot

The underlying issue that when one of the files returned here gets renamed or
removed, GN doesn't know it needs to rerun (and generate new ninja files)

It's described here:

https://chromium.googlesource.com/chromium/src/tools/gn/+/HEAD/docs/reference.md

Script input gotchas

  It may be tempting to write a script that enumerates all files in a directory
  as inputs. Don't do this! Even if you specify all the files in the inputs or
  sources in the GN target (or worse, enumerate the files in an exec_script
  call when running GN, which will be slow), the dependencies will be broken.

  The problem happens if a file is ever removed because the inputs are not
  listed on the command line to the script. Because the script hasn't changed
  and all inputs are up to date, the script will not re-run and you will get a
  stale build. Instead, either list all inputs on the command line to the
  script, or if there are many, create a separate list file that the script
  reads. As long as this file is listed in the inputs, the build will detect
  when it has changed in any way and the action will re-run.

[ahe, 2017-03-02 12:23:49]

Thank you, Martin.

This is a todo in my inbox: "Are we actually depending on fasta to recompile
platform.dill."

I should have asked about this earlier. I'm not yet familiar with how
dependencies are specified, would you be able to help us solve this?

On 2017/03/02 12:14:14, kustermann wrote:
> It's described here:
> 
>
https://chromium.googlesource.com/chromium/src/tools/gn/+/HEAD/docs/reference.md

Direct link:

https://chromium.googlesource.com/chromium/src/tools/gn/+/HEAD/docs/reference...

[kustermann, 2017-03-02 12:40:48]

Talked with Slava offline. He'll see if the "depfile" feature of GN is enough
for us.

This issue is general, not kernel specific. For example

$ tools/build.py -mrelease -ax64 create_sdk
$ rm pkg/compiler/lib/src/dart2js.dart
$ tools/build.py --mode=release --arch=x64 create_sdk
ninja -C out/ReleaseX64 create_sdk
ninja: Entering directory `out/ReleaseX64'
ninja: error: '../../pkg/compiler/lib/src/dart2js.dart', needed by
'obj/ABS_PATH/...pkg/make_third_party_pkg_files_0_stamp.inputdeps.stamp',
missing and no known rule to make it
BUILD FAILED

[ahe, 2017-03-02 12:43:27]

Martin, how does this break the build bots? I thought they ran gclient runhooks
which would unconditionally regenerate ninja files.

[kustermann, 2017-03-02 12:59:55]

This is a question for zachary/bill. I don't know why the runhooks didn't solve
it for the bot.