Move the dart2js snapshot into bin directory of sdk root.

Move the dart2js snapshot into bin directory of sdk root.

This is the first step in eliminating lib/_internal.
Committed: https://code.google.com/p/dart/source/detail?r=21247

[ahe, 2013-04-09 14:37:21]

Let's talk about what I'm saying below.  Not sure I'm clear.

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py
File tools/create_sdk.py (right):

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py#newcode123
tools/create_sdk.py:123: internal_build_dir)
Did you test that this works?  The relative references in dart2js will be off
when you do this.

I'd rather set the BUILD_ID using a wrapper script around dart2js.  If the
compiler sources are not included in the SDK, they don't need to be patched.  I
think this can simplify this step greatly, and allow us to use the snapshot we
build in dart/utils/compiler/compiler.gyp.

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py#newcode240
tools/create_sdk.py:240: # Remove this once we can add it as part of the dart2js
snapshot.
The Editor will still need it.

[kustermann, 2013-04-09 15:02:28]

https://codereview.chromium.org/13945008/diff/1/sdk/bin/dart2js
File sdk/bin/dart2js (right):

https://codereview.chromium.org/13945008/diff/1/sdk/bin/dart2js#newcode28
sdk/bin/dart2js:28: 
You should change that path in test_suite.dart:477 as well, otherwise you
basically disable the 'dart2js compilation skipping feature'.

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py
File tools/create_sdk.py (right):

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py#newcode110
tools/create_sdk.py:110: subprocess.call([os.path.join(build_dir, 'dart'),
Does this work on windows as well (or do we need to use 'dart.exe' there?

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py#newcode112
tools/create_sdk.py:112: snapshot_name,
You could move this a line up.

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py#newcode140
tools/create_sdk.py:140: # Pull in all of the gpyi files which will be munged
into the sdk.
gpyi -> gypi

[ricow1, 2013-04-09 15:26:58]

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py
File tools/create_sdk.py (right):

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py#newcode110
tools/create_sdk.py:110: subprocess.call([os.path.join(build_dir, 'dart'),
On 2013/04/09 15:02:28, kustermann wrote:
> Does this work on windows as well (or do we need to use 'dart.exe' there?
This is using this existing code on with other arguments

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py#newcode123
tools/create_sdk.py:123: internal_build_dir)
On 2013/04/09 14:37:21, ahe wrote:
> Did you test that this works?  The relative references in dart2js will be off
> when you do this.
> 
> I'd rather set the BUILD_ID using a wrapper script around dart2js.  If the
> compiler sources are not included in the SDK, they don't need to be patched. 
I
> think this can simplify this step greatly, and allow us to use the snapshot we
> build in dart/utils/compiler/compiler.gyp.

As discussed offline I will change this to a wrapper script, regarding if it
works, remember that this internal_build_dir is _only_ used for creating the
snapshot, the internal references will be relative to the path I give in the
shell script

https://codereview.chromium.org/13945008/diff/1/tools/create_sdk.py#newcode140
tools/create_sdk.py:140: # Pull in all of the gpyi files which will be munged
into the sdk.
On 2013/04/09 15:02:28, kustermann wrote:
> gpyi -> gypi

Done.

[ricow1, 2013-04-10 12:02:20]

Changed this to use a wrapper script (auto generated) that we then create a
snapshot from.
I currently only added dart2js, I will add dartdoc as well when we know all of
this is working.
Also added a few todos to not complicate this cl to much

https://codereview.chromium.org/13945008/diff/7001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/13945008/diff/7001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:13: const String
BUILD_ID = 'NEED_TO_PASS_IN';
Peter: How do you want this passed down?
All the way as parameters from the dart2js script?

[ricow1, 2013-04-10 13:10:09]

Fixed build id argument passing in the compiler, PTAL

[ahe, 2013-04-10 14:50:08]

LGTM!

Thank you for doing this!

We should have a sync version of Process.run.

https://codereview.chromium.org/13945008/diff/12003/sdk/bin/dart2js
File sdk/bin/dart2js (right):

https://codereview.chromium.org/13945008/diff/12003/sdk/bin/dart2js#newcode63
sdk/bin/dart2js:63: exec "$DART" "${EXTRA_VM_OPTIONS[@]}" "$SNAPSHOT" "dart2js"
"${EXTRA_OPTIONS[@]}" "$@"
Long line.

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compiler.dart:412: String buildId:
"build number could not be determined",
Could this be this.buildId?

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compiler.dart:427: buildId = buildId,
this.buildId

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/dart2js.dart (right):

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/dart2js.dart:33: 
Extra line.

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/dart2js.dart:329: 
Extra space.

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/dart2js.dart:520: 
Extra line.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
File utils/compiler/create_snapshot.dart (right):

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:8: * Takes the following arguments:
This documentation comment applies to the following method.  I suggest you move
it to the main method.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:24: 
Extra line.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:49: 
Extra line.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:52: File file = new File(path);
Indentation.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:57: 
Extra line.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:67: return;
Remove.

[ricow1, 2013-04-11 05:45:12]

Thanks for the comments

https://codereview.chromium.org/13945008/diff/12003/sdk/bin/dart2js
File sdk/bin/dart2js (right):

https://codereview.chromium.org/13945008/diff/12003/sdk/bin/dart2js#newcode63
sdk/bin/dart2js:63: exec "$DART" "${EXTRA_VM_OPTIONS[@]}" "$SNAPSHOT" "dart2js"
"${EXTRA_OPTIONS[@]}" "$@"
On 2013/04/10 14:50:08, ahe wrote:
> Long line.

Done.

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compiler.dart:412: String buildId:
"build number could not be determined",
On 2013/04/10 14:50:08, ahe wrote:
> Could this be this.buildId?
This is a named optional parameter, like the rest

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compiler.dart:427: buildId = buildId,
On 2013/04/10 14:50:08, ahe wrote:
> this.buildId
again, I just followed the existing convention, but changed

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/dart2js.dart (right):

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/dart2js.dart:329: 
On 2013/04/10 14:50:08, ahe wrote:
> Extra space.

Done.

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/dart2js.dart:520: 
On 2013/04/10 14:50:08, ahe wrote:
> Extra line.

Done.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
File utils/compiler/create_snapshot.dart (right):

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:24: 
On 2013/04/10 14:50:08, ahe wrote:
> Extra line.
That really depends if you like 1 or 2 lines between methods, but since I don't
really care -> removed :-)

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:49: 
On 2013/04/10 14:50:08, ahe wrote:
> Extra line.

Done.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:52: File file = new File(path);
On 2013/04/10 14:50:08, ahe wrote:
> Indentation.

Done.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:57: 
On 2013/04/10 14:50:08, ahe wrote:
> Extra line.

Done.

https://codereview.chromium.org/13945008/diff/12003/utils/compiler/create_sna...
utils/compiler/create_snapshot.dart:67: return;
On 2013/04/10 14:50:08, ahe wrote:
> Remove.

Done.

[ricow1, 2013-04-11 05:47:36]

Committed patchset #6 manually as r21247 (presubmit successful).

[ahe, 2013-04-11 06:03:46]

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compiler.dart:412: String buildId:
"build number could not be determined",
On 2013/04/11 05:45:12, ricow1 wrote:
> On 2013/04/10 14:50:08, ahe wrote:
> > Could this be this.buildId?
> This is a named optional parameter, like the rest

I'm not sure you understood my question. I checked the spec, it should be
possible to replace this line by:

this.buildId: "...",

https://codereview.chromium.org/13945008/diff/12003/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compiler.dart:427: buildId = buildId,
On 2013/04/11 05:45:12, ricow1 wrote:
> On 2013/04/10 14:50:08, ahe wrote:
> > this.buildId
> again, I just followed the existing convention, but changed

I need to check the history of these assignments, I find it highly confusing
that they don't start with this. But if the above suggestion works, this line
can be removed.