Shoehorn dart2js into the SDK.

Shoehorn dart2js into the SDK.

Committed: https://code.google.com/p/dart/source/detail?r=7230

[ahe, 2012-04-30 15:01:33]

Bill: please take a look a the changes to test.dart.

Dan: Copying too many files, I know. I think this could make sense for now, as
we all seem to agree that the SDK layout should be changed.

[Bill Hesse, 2012-04-30 15:21:32]

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
File dart/tools/testing/dart/test_options.dart (right):

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_options.dart:253: 'Use the SDK version',
I would make the descriptive test longer.  At least mention dart2js.

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
File dart/tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:1238: prefix = 'dart-sdk/bin/';
Shouldn't the --use_sdk option apply to frog and to vm as well?  There are
copies of the vm and the frog compiler.  Isn't it the right thing to do to
enable the new option for all appropriate targets?

[kasperl, 2012-05-01 05:51:07]

LGTM, but I'm not sure about the .bat file suffix issue on Windows.

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk.py
File dart/tools/create_sdk.py (right):

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk....
dart/tools/create_sdk.py:83: def CopyDart2Js(build_dir, sdk_root):
Maybe add a comment that explains what you're copying. It looks like you're
(temporarily?) copying too much to make it easier to deal with?

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk....
dart/tools/create_sdk.py:95: dart2js = os.path.join(sdk_root, 'bin', 'dart2js')
Shouldn't this be different on Windows? Don't you need to copy the .bat file
instead there? Adding a TODO would probably be fine unless this breaks our
testing on Windows.

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
File dart/tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:1253: String suffix =
executableSuffix(configuration['compiler']);
Consider replacing this method with a check that configuration['compiler'] isn't
'none' and just return the result of calling executableName(configuration)? ...
or just use executableName(configuration) for all three cases in the switch and
get rid of computing the suffix (preserves the error message better)?

[dgrove, 2012-05-01 06:54:58]

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk.py
File dart/tools/create_sdk.py (right):

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk....
dart/tools/create_sdk.py:94: ignore=ignore_patterns('.svn'))
This approach generally looks orthogonal to the way things are arranged in the
SDK today.  Even if we're going to clean up the SDK over time, it would be
better to have these fit in with the SDK as it stands to (e.g.,
lib/core/dart2js/...)

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk....
dart/tools/create_sdk.py:94: ignore=ignore_patterns('.svn'))
What are you doing about the html and isolate libraries?

[ahe, 2012-05-02 10:32:17]

PTAL

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk.py
File dart/tools/create_sdk.py (right):

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk....
dart/tools/create_sdk.py:83: def CopyDart2Js(build_dir, sdk_root):
On 2012/05/01 05:51:07, kasperl wrote:
> Maybe add a comment that explains what you're copying. It looks like you're
> (temporarily?) copying too much to make it easier to deal with?

Done.

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk....
dart/tools/create_sdk.py:94: ignore=ignore_patterns('.svn'))
On 2012/05/01 06:54:58, dgrove wrote:
> What are you doing about the html and isolate libraries?

I'm copying dart/lib to PRODUCT_DIR/dart-sdk/lib/dart2js/lib.

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk....
dart/tools/create_sdk.py:94: ignore=ignore_patterns('.svn'))
On 2012/05/01 06:54:58, dgrove wrote:
> This approach generally looks orthogonal to the way things are arranged in the
> SDK today.  Even if we're going to clean up the SDK over time, it would be
> better to have these fit in with the SDK as it stands to (e.g.,
> lib/core/dart2js/...) 

I thought the plan was to shuffle directories around in our repository so we can
use the same layout in both SDK and source repository. I'm not aware of which
direction we're going in, but I strongly prefer to not have to temporarily move
files around and rewrite URIs.

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/create_sdk....
dart/tools/create_sdk.py:95: dart2js = os.path.join(sdk_root, 'bin', 'dart2js')
On 2012/05/01 05:51:07, kasperl wrote:
> Shouldn't this be different on Windows? Don't you need to copy the .bat file
> instead there? Adding a TODO would probably be fine unless this breaks our
> testing on Windows.

I think I'll break Windows if I don't create the BAT file as well.

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
File dart/tools/testing/dart/test_options.dart (right):

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_options.dart:253: 'Use the SDK version',
On 2012/04/30 15:21:32, Bill Hesse wrote:
> I would make the descriptive test longer.  At least mention dart2js.

Done.

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
File dart/tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:1238: prefix = 'dart-sdk/bin/';
On 2012/04/30 15:21:32, Bill Hesse wrote:
> Shouldn't the --use_sdk option apply to frog and to vm as well?  There are
> copies of the vm and the frog compiler.  Isn't it the right thing to do to
> enable the new option for all appropriate targets?

Frog is EOL and the frog/bin vs dart-sdk/bin is a further complication.

dartc is not in the SDK.

So the only remaining thing is the VM. I don't know if they plan to use this
option or not, but I would prefer not changing anything that is not tested (as I
do with the dart2js build bot).

https://chromiumcodereview.appspot.com/10266011/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:1253: String suffix =
executableSuffix(configuration['compiler']);
On 2012/05/01 05:51:07, kasperl wrote:
> Consider replacing this method with a check that configuration['compiler']
isn't
> 'none' and just return the result of calling executableName(configuration)?
...
> or just use executableName(configuration) for all three cases in the switch
and
> get rid of computing the suffix (preserves the error message better)?

I didn't touch this code. I could spend a lot of time "cleaning up" this code,
but I'm not sure Bill and Mads would necessary agree with what I think is
cleaner. So I prefer to not get into that.

[kasperl, 2012-05-02 11:20:45]

Still LGTM.

https://chromiumcodereview.appspot.com/10266011/diff/7001/dart/tools/create_s...
File dart/tools/create_sdk.py (right):

https://chromiumcodereview.appspot.com/10266011/diff/7001/dart/tools/create_s...
dart/tools/create_sdk.py:88: layout matches the repository's which frees us from
rewriting
the repository's -> the layout of the part of the repository we're dealing with
here

[dgrove, 2012-05-02 11:23:28]

lgtm