dartfmt SDK executables.

dartfmt SDK executables.

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

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

[pquitslund, 2013-12-10 19:22:40]

As per our discussion, this breaks the SDK bin bits into it's own CL.  Once
we're happy with how it works we can land another CL with the required build
changes.

Martin: I think I got it working from with the SDK dir.  Do let me know if I got
things tangled up.

Thanks!

[danrubel, 2013-12-10 19:24:24]

lgtm

[kustermann, 2013-12-10 20:30:46]

lgtm assuming you just copied existing files in sdk/bin/* and made it work for
dartfmt

You don't have to address the comments, I think these scripts should all be
cleaned up (as I mentioned in the other CL, I don't like the duplication in the
sdk/bin/* shell scripts)

https://codereview.chromium.org/98043005/diff/1/sdk/bin/dartfmt
File sdk/bin/dartfmt (right):

https://codereview.chromium.org/98043005/diff/1/sdk/bin/dartfmt#newcode39
sdk/bin/dartfmt:39: if [[ `uname` == 'Darwin' ]];
i think this should be
  if [ "$(uname)" = "Darwin" ]; then
    ...
  else
    ...
  fi

Don't know why a '[[' ']]' is used here.

https://codereview.chromium.org/98043005/diff/1/sdk/bin/dartfmt#newcode50
sdk/bin/dartfmt:50: exec "$DART" "--package-root=$PKG_DIR" "$DARTFMT" "$@"
I think you can get rid of most of this, by finding the dart executable the same
way as "sdk/bin/dart2js" does. (i.e. there is a sdk/bin/dart shell script with
that logic already).

[dgrove, 2013-12-10 20:39:55]

On 2013/12/10 20:30:46, kustermann wrote:
> lgtm assuming you just copied existing files in sdk/bin/* and made it work for
> dartfmt
> 
> You don't have to address the comments, I think these scripts should all be
> cleaned up (as I mentioned in the other CL, I don't like the duplication in
the
> sdk/bin/* shell scripts)
> 
> https://codereview.chromium.org/98043005/diff/1/sdk/bin/dartfmt
> File sdk/bin/dartfmt (right):
> 
> https://codereview.chromium.org/98043005/diff/1/sdk/bin/dartfmt#newcode39
> sdk/bin/dartfmt:39: if [[ `uname` == 'Darwin' ]];
> i think this should be
>   if [ "$(uname)" = "Darwin" ]; then
>     ...
>   else
>     ...
>   fi
> 
> Don't know why a '[[' ']]' is used here.
> 
> https://codereview.chromium.org/98043005/diff/1/sdk/bin/dartfmt#newcode50
> sdk/bin/dartfmt:50: exec "$DART" "--package-root=$PKG_DIR" "$DARTFMT" "$@"
> I think you can get rid of most of this, by finding the dart executable the
same
> way as "sdk/bin/dart2js" does. (i.e. there is a sdk/bin/dart shell script with
> that logic already).

There have been some proposals to replace the .sh/.bat scripts with Dart
scripts.
I think we should do that refactoring/replacement outside this CL.

[kustermann, 2013-12-10 20:45:20]

On 2013/12/10 20:39:55, dgrove wrote:
> On 2013/12/10 20:30:46, kustermann wrote:
> > lgtm assuming you just copied existing files in sdk/bin/* and made it work
for
> > dartfmt
> > 
> > You don't have to address the comments, I think these scripts should all be
> > cleaned up (as I mentioned in the other CL, I don't like the duplication in
> the
> > sdk/bin/* shell scripts)
> > 
> > https://codereview.chromium.org/98043005/diff/1/sdk/bin/dartfmt
> > File sdk/bin/dartfmt (right):
> > 
> > https://codereview.chromium.org/98043005/diff/1/sdk/bin/dartfmt#newcode39
> > sdk/bin/dartfmt:39: if [[ `uname` == 'Darwin' ]];
> > i think this should be
> >   if [ "$(uname)" = "Darwin" ]; then
> >     ...
> >   else
> >     ...
> >   fi
> > 
> > Don't know why a '[[' ']]' is used here.
> > 
> > https://codereview.chromium.org/98043005/diff/1/sdk/bin/dartfmt#newcode50
> > sdk/bin/dartfmt:50: exec "$DART" "--package-root=$PKG_DIR" "$DARTFMT" "$@"
> > I think you can get rid of most of this, by finding the dart executable the
> same
> > way as "sdk/bin/dart2js" does. (i.e. there is a sdk/bin/dart shell script
with
> > that logic already).
> 
> There have been some proposals to replace the .sh/.bat scripts with Dart
> scripts.
> I think we should do that refactoring/replacement outside this CL.

Agreed. As I said, all of the scripts should be cleaned up (and some are still
broken if the sdk contains parenthesis on windows AFAIK ...)

It's not so easy to replace them with dart code, but that's another story.

[pquitslund, 2014-01-27 23:10:39]

Committed patchset #2 manually as r32040 (presubmit successful).