Build the Editor from GYP/build.py.

Build the Editor from GYP/build.py.

The Editor is only rebuilt if it changes, not if the SDK changes.
Committed: https://code.google.com/p/dart/source/detail?r=19289

[kustermann, 2013-01-31 14:35:53]

DBC

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
File dart/editor/build/generate_sources.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:1: 'touch_file'#!/usr/bin/env python
What is this 'touch_file' doing there?

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file.py
File dart/editor/build/touch_file.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file....
dart/editor/build/touch_file.py:15: open(file_name, 'w').close()
The name "touch_file.py" is a bit misleading, since the "touch" UNIX command
doesn't modify the content of files whereas this code truncates the file if
there is some content in it.

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py
File dart/tools/create_editor.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py#n...
dart/tools/create_editor.py:12: # Usage: ./tools/build_editor.py
I think this should be "./tools/create_editor.py"?!

[ricow1, 2013-01-31 14:42:11]

LGTM

https://codereview.chromium.org/12090092/diff/1/dart/dart.gyp
File dart/dart.gyp (right):

https://codereview.chromium.org/12090092/diff/1/dart/dart.gyp#newcode127
dart/dart.gyp:127: # uses as input.
Should we state here the reason why we don't want to do that? i..e, maybe add:
This would, however, make us rebuild the editor whenever any dart2js/vm/etc
changed (for which there is no reason, as long as the editor did not change
recompiling it will not change how it uses dart2js or other tools)

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
File dart/editor/build/generate_sources.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:1: 'touch_file'#!/usr/bin/env python
remove 'touch_file'

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:36: return StringIO.StringIO()
Is it really worth it to create a memory file here, we need to read in the old
file to compare anyway?

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:53: for d in remove_me:
what does this do? remove .svn directories?

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:131: for pkg, sources in
_split_sources(pkg_dir, '.java'):
add a blank line before this, it is hard to read otherwise (at least here due to
no code formatting)

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file.py
File dart/editor/build/touch_file.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file....
dart/editor/build/touch_file.py:9: # for file_name in args[1:]:
commented out code:

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py
File dart/tools/create_editor.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py#n...
dart/tools/create_editor.py:119: # TODO(devoncarew): Currently we scrape the
output path to determine the
this seems outdated, we can actually pass int he mode and arch

[devoncarew, 2013-01-31 22:42:17]

lgtm

Thanks much for doing this Peter!

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file.py
File dart/editor/build/touch_file.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file....
dart/editor/build/touch_file.py:15: open(file_name, 'w').close()
On 2013/01/31 14:35:53, kustermann wrote:
> The name "touch_file.py" is a bit misleading, since the "touch" UNIX command
> doesn't modify the content of files whereas this code truncates the file if
> there is some content in it.

Agreed - perhaps truncate_file.py?

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py
File dart/tools/create_editor.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py#n...
dart/tools/create_editor.py:9: # TODO(devoncarew): currently this script is not
callable from tools/build.py
I think we can remove this TODO now.

[ahe, 2013-02-04 11:12:05]

Thank you for your comments, Devon, Rico, and Martin. I have uploaded a new
patch set which should address your concerns.

I'd really like if someone could submit this CL for me. My experience tells me
that it takes a few tries to get GYP changes right, and I'm not sure I have the
time to deal with that before M3.

https://codereview.chromium.org/12090092/diff/1/dart/dart.gyp
File dart/dart.gyp (right):

https://codereview.chromium.org/12090092/diff/1/dart/dart.gyp#newcode127
dart/dart.gyp:127: # uses as input.
On 2013/01/31 14:42:11, ricow1 wrote:
> Should we state here the reason why we don't want to do that? i..e, maybe add:
> This would, however, make us rebuild the editor whenever any dart2js/vm/etc
> changed (for which there is no reason, as long as the editor did not change
> recompiling it will not change how it uses dart2js or other tools)

Done.

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
File dart/editor/build/generate_sources.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:1: 'touch_file'#!/usr/bin/env python
On 2013/01/31 14:35:53, kustermann wrote:
> What is this 'touch_file' doing there?

Done.

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:1: 'touch_file'#!/usr/bin/env python
On 2013/01/31 14:42:11, ricow1 wrote:
> remove 'touch_file'

Done.

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:36: return StringIO.StringIO()
On 2013/01/31 14:42:11, ricow1 wrote:
> Is it really worth it to create a memory file here, we need to read in the old
> file to compare anyway?

I don't understand what you see the alternative is. I think this approach is
better than creating a temporary file.

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:53: for d in remove_me:
On 2013/01/31 14:42:11, ricow1 wrote:
> what does this do? remove .svn directories?

It filters out all directories that start with a dot, for example, .svn.

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/generate_so...
dart/editor/build/generate_sources.py:131: for pkg, sources in
_split_sources(pkg_dir, '.java'):
On 2013/01/31 14:42:11, ricow1 wrote:
> add a blank line before this, it is hard to read otherwise (at least here due
to
> no code formatting)

Done.

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file.py
File dart/editor/build/touch_file.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file....
dart/editor/build/touch_file.py:9: # for file_name in args[1:]:
On 2013/01/31 14:42:11, ricow1 wrote:
> commented out code:

Done.

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file....
dart/editor/build/touch_file.py:15: open(file_name, 'w').close()
On 2013/01/31 14:35:53, kustermann wrote:
> The name "touch_file.py" is a bit misleading, since the "touch" UNIX command
> doesn't modify the content of files whereas this code truncates the file if
> there is some content in it.

Done.

https://codereview.chromium.org/12090092/diff/1/dart/editor/build/touch_file....
dart/editor/build/touch_file.py:15: open(file_name, 'w').close()
On 2013/01/31 22:42:17, devoncarew wrote:
> On 2013/01/31 14:35:53, kustermann wrote:
> > The name "touch_file.py" is a bit misleading, since the "touch" UNIX command
> > doesn't modify the content of files whereas this code truncates the file if
> > there is some content in it.
> 
> Agreed - perhaps truncate_file.py?

Done.

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py
File dart/tools/create_editor.py (right):

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py#n...
dart/tools/create_editor.py:9: # TODO(devoncarew): currently this script is not
callable from tools/build.py
On 2013/01/31 22:42:17, devoncarew wrote:
> I think we can remove this TODO now.

Done.

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py#n...
dart/tools/create_editor.py:12: # Usage: ./tools/build_editor.py
On 2013/01/31 14:35:53, kustermann wrote:
> I think this should be "./tools/create_editor.py"?!

Done.

https://codereview.chromium.org/12090092/diff/1/dart/tools/create_editor.py#n...
dart/tools/create_editor.py:119: # TODO(devoncarew): Currently we scrape the
output path to determine the
On 2013/01/31 14:42:11, ricow1 wrote:
> this seems outdated, we can actually pass int he mode and arch

I'll leave cleanup to others. I do not know how this is used on the editor build
bot.

[ricow1, 2013-03-01 10:02:46]

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