Pass path to platform kernel binary to kernel-service.

Pass path to platform kernel binary to kernel-service.

Currently kernel-service derives path to platform binary from path to dart executable. It doesn't always work, so dartk tests fail.

R=asiva@google.com, sigmund@google.com

BUG:http://dartbug.com/30456
Committed: https://github.com/dart-lang/sdk/commit/ba543167ec656ec85aa675ca2d90037a999726a8

[aam, 2017-08-16 19:59:32]

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

[aam, 2017-08-16 19:59:33]

This is slightly different implementation than what was suggested on
http://dartbug.com/30456. Here I pass path to platform.dill instead of path to
sdkRoot. Main reason originally was that runtime/bin/dfe.h DFE class exposes
paths to platform.dill and vmservice.dill only. But it does sound reasonable to
me.

PTAL, thanks!

[Siggi Cherem (dart-lang), 2017-08-16 23:29:19]

lgtm

https://codereview.chromium.org/3001013002/diff/40001/runtime/bin/dfe.cc
File runtime/bin/dfe.cc (right):

https://codereview.chromium.org/3001013002/diff/40001/runtime/bin/dfe.cc#newc...
runtime/bin/dfe.cc:77: Dart_CompileToKernel(url_string,
platform_binary_filename_);
Could you add a TODO here that we should be passing 'outline.dill' instead of
'platform.dill' (same below in the other call to Dart_CompileToKernel)?

[aam, 2017-08-17 02:10:51]

https://codereview.chromium.org/3001013002/diff/40001/runtime/bin/dfe.cc
File runtime/bin/dfe.cc (right):

https://codereview.chromium.org/3001013002/diff/40001/runtime/bin/dfe.cc#newc...
runtime/bin/dfe.cc:77: Dart_CompileToKernel(url_string,
platform_binary_filename_);
On 2017/08/16 23:29:18, Siggi Cherem (dart-lang) wrote:
> Could you add a TODO here that we should be passing 'outline.dill' instead of
> 'platform.dill' (same below in the other call to Dart_CompileToKernel)?
> 

Done.

[siva, 2017-08-17 15:40:56]

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

[siva, 2017-08-17 15:40:57]

lgtm

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/isolate_relo...
File runtime/vm/isolate_reload_test.cc (right):

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/isolate_relo...
runtime/vm/isolate_reload_test.cc:191: "file:///test-app.dart",
why is the '.dart' suffix needed ?

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/unit_test.cc
File runtime/vm/unit_test.cc (right):

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/unit_test.cc...
runtime/vm/unit_test.cc:194: url, NULL /* platform_binary */, sourcefiles_count,
sourcefiles,
Why is platform_binary null here, is it because the unit tests do not depend on
anything in the platform?

I am wondering if we are testing in a different mode than what is used in
regular runs.

[aam, 2017-08-17 16:29:57]

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

[aam, 2017-08-17 16:29:58]

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/isolate_relo...
File runtime/vm/isolate_reload_test.cc (right):

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/isolate_relo...
runtime/vm/isolate_reload_test.cc:191: "file:///test-app.dart",
On 2017/08/17 15:40:57, siva wrote:
> why is the '.dart' suffix needed ?

Reverted. At some point I was fighting with some weird errors about files not
found, but those turned out to be caused by source files not passed correctly.

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/unit_test.cc
File runtime/vm/unit_test.cc (right):

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/unit_test.cc...
runtime/vm/unit_test.cc:194: url, NULL /* platform_binary */, sourcefiles_count,
sourcefiles,
On 2017/08/17 15:40:57, siva wrote:
> Why is platform_binary null here, is it because the unit tests do not depend
on
> anything in the platform?
> 
> I am wondering if we are testing in a different mode than what is used in
> regular runs.

Basically for these tests we don't have to pass path to platform.dill because it
can be found in standard place, deduced by kernel-service relative to the
executable($PATH/run_vm_tests -> $PATH/patched_sdk/platform.dill).
It felt like making this test framework aware of platform.dill was not necessary
at this point.

[aam, 2017-08-17 16:49:53]

Description was changed from

==========
Pass path to platform kernel binary to kernel-service.

Currently kernel-service derives path to platform binary from path to dart
executable. It doesn't always work, so dartk tests fail.

BUG:http://dartbug.com/30456
==========

to

==========
Pass path to platform kernel binary to kernel-service.

Currently kernel-service derives path to platform binary from path to dart
executable. It doesn't always work, so dartk tests fail.

R=asiva@google.com, sigmund@google.com

BUG:http://dartbug.com/30456
Committed:
https://github.com/dart-lang/sdk/commit/ba543167ec656ec85aa675ca2d90037a999726a8
==========

[aam, 2017-08-17 16:49:54]

Committed patchset #5 (id:80001) manually as
ba543167ec656ec85aa675ca2d90037a999726a8 (presubmit successful).

[siva, 2017-08-18 00:40:45]

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/unit_test.cc
File runtime/vm/unit_test.cc (right):

https://codereview.chromium.org/3001013002/diff/60001/runtime/vm/unit_test.cc...
runtime/vm/unit_test.cc:194: url, NULL /* platform_binary */, sourcefiles_count,
sourcefiles,
On 2017/08/17 16:29:58, aam wrote:
> On 2017/08/17 15:40:57, siva wrote:
> > Why is platform_binary null here, is it because the unit tests do not depend
> on
> > anything in the platform?
> > 
> > I am wondering if we are testing in a different mode than what is used in
> > regular runs.
> 
> Basically for these tests we don't have to pass path to platform.dill because
it
> can be found in standard place, deduced by kernel-service relative to the
> executable($PATH/run_vm_tests -> $PATH/patched_sdk/platform.dill).
> It felt like making this test framework aware of platform.dill was not
necessary
> at this point.

Sounds good. I agree not necessary for the unit test framework.