Fix dartc.sh script to use /bin/bash (issue dart:1104)

Fix an error on the buildbots where the dartc shell script was throwing an error.

Also, improve error output from test.dart.

Add verbose output from the dartc bash script, and separators between compilation and execution phase output in the test.dart browser tests.


BUG=dart:1104
TEST=tools/test.dart -cchromium language

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

[Bill Hesse, 2012-01-10 13:09:09]

http://codereview.chromium.org/9158011/diff/1/compiler/scripts/dartc.sh
File compiler/scripts/dartc.sh (right):

http://codereview.chromium.org/9158011/diff/1/compiler/scripts/dartc.sh#newcode1
compiler/scripts/dartc.sh:1: #!/bin/bash
Remove this change?

[ahe, 2012-01-10 13:55:03]

LGTM

@zundel+mmendez: Bill is trying to diagnose a test failure when using test.dart
to test Chromium.

http://codereview.chromium.org/9158011/diff/1/compiler/scripts/dartc.sh
File compiler/scripts/dartc.sh (right):

http://codereview.chromium.org/9158011/diff/1/compiler/scripts/dartc.sh#newcode1
compiler/scripts/dartc.sh:1: #!/bin/bash
On 2012/01/10 13:09:10, Bill Hesse wrote:
> Remove this change?

I was thinking the same.

http://codereview.chromium.org/9158011/diff/1/tools/testing/dart/test_runner....
File tools/testing/dart/test_runner.dart (right):

http://codereview.chromium.org/9158011/diff/1/tools/testing/dart/test_runner....
tools/testing/dart/test_runner.dart:110: '$compilerPath
${Strings.join(compilerArguments, " ")}';
Shouldn't this be:

'$compilerPath "${Strings.join(compilerArguments, " ")}"'

[zundel, 2012-01-10 14:04:52]

lgtm

http://codereview.chromium.org/9158011/diff/1/compiler/scripts/dartc.sh
File compiler/scripts/dartc.sh (right):

http://codereview.chromium.org/9158011/diff/1/compiler/scripts/dartc.sh#newcode5
compiler/scripts/dartc.sh:5: set -e
if this is going to stay in for more than just a few commits, I'd prefer it be
surrounded with a test of a command line flag or environment variable. 
Otherwise, I'm OK with it as a way to quickly diagnose a problem with the build.

http://codereview.chromium.org/9158011/diff/1/tools/testing/dart/test_runner....
File tools/testing/dart/test_runner.dart (right):

http://codereview.chromium.org/9158011/diff/1/tools/testing/dart/test_runner....
tools/testing/dart/test_runner.dart:201: stdout.add('test.dart: Compilation
finished, starting execution\n');
do we really need 2 lines of output?

[Bill Hesse, 2012-01-10 14:13:52]

http://codereview.chromium.org/9158011/diff/1/compiler/scripts/dartc.sh
File compiler/scripts/dartc.sh (right):

http://codereview.chromium.org/9158011/diff/1/compiler/scripts/dartc.sh#newcode1
compiler/scripts/dartc.sh:1: #!/bin/bash
Changing to /bin/bash --posix to agree with the other scripts in this directory.

On 2012/01/10 13:55:03, ahe wrote:
> On 2012/01/10 13:09:10, Bill Hesse wrote:
> > Remove this change?
> 
> I was thinking the same.

Actually, this is the change that fixes the problem.
/bin/sh does not point to bash on the buildbot slaves.
So this change is the fix.

http://codereview.chromium.org/9158011/diff/1/compiler/scripts/dartc.sh#newcode5
compiler/scripts/dartc.sh:5: set -e
On 2012/01/10 14:04:53, zundel wrote:
> if this is going to stay in for more than just a few commits, I'd prefer it be
> surrounded with a test of a command line flag or environment variable. 

We are taking this out.  We fixed it.
> Otherwise, I'm OK with it as a way to quickly diagnose a problem with the
build.

http://codereview.chromium.org/9158011/diff/1/tools/testing/dart/test_runner....
File tools/testing/dart/test_runner.dart (right):

http://codereview.chromium.org/9158011/diff/1/tools/testing/dart/test_runner....
tools/testing/dart/test_runner.dart:110: '$compilerPath
${Strings.join(compilerArguments, " ")}';
No.  Why would it?  This is the human-readable one we print.  And even when
pasting, there shouldn't be quotes around the arguments. On 2012/01/10 13:55:03,
ahe wrote:
> Shouldn't this be:
> 
> '$compilerPath "${Strings.join(compilerArguments, " ")}"'

http://codereview.chromium.org/9158011/diff/1/tools/testing/dart/test_runner....
tools/testing/dart/test_runner.dart:201: stdout.add('test.dart: Compilation
finished, starting execution\n');
These lines are acting as separators in stderr and stdout.  stderr and stdout
are printed separately, not intermingled, in error output, so we need both.

On 2012/01/10 14:04:53, zundel wrote:
> do we really need 2 lines of output?

I think so.  Each of these are lines are really long.

[mmendez, 2012-01-10 14:27:01]

DBC

I'm not really sure what failure specifically the buildbots were seeing, but
I'll trust you that this actually fixes that issue.

http://codereview.chromium.org/9158011/diff/1003/compiler/scripts/dartc.sh
File compiler/scripts/dartc.sh (right):

http://codereview.chromium.org/9158011/diff/1003/compiler/scripts/dartc.sh#ne...
compiler/scripts/dartc.sh:1: #!/bin/bash --posix
I'm not a scripting guru, but why does using bash fix the problem?  Isn't that
less standard?

[ricow1, 2012-01-10 14:34:07]

http://codereview.chromium.org/9158011/diff/1003/compiler/scripts/dartc.sh
File compiler/scripts/dartc.sh (right):

http://codereview.chromium.org/9158011/diff/1003/compiler/scripts/dartc.sh#ne...
compiler/scripts/dartc.sh:1: #!/bin/bash --posix
On 2012/01/10 14:27:01, mmendez wrote:
> I'm not a scripting guru, but why does using bash fix the problem?  Isn't that
> less standard?
Here is the deal: On our normal linux boxes /bin/sh points at /bin/bash
ricow@nobby:~$ lsl /bin/sh
lrwxrwxrwx 1 root root 9 2010-10-01 14:46 /bin/sh -> /bin/bash

On the buildbot, it points at /bin/dash
chrome-bot@vm26-m3:~$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 2010-10-08 16:24 /bin/sh -> dash

Now, dash does not support the variable substitution inside a string, so that
will be an error!
I expect this to be the case for normal Ubuntu users as well