Changes to annotated step scripts: dart-editor-installer-* builders will build installer

Changes to annotated step scripts: dart-editor-installer-* builders will build installer

R=ricow@google.com

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

[ricow1, 2013-10-18 11:18:08]

LGTM

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py
File editor/build/build.py (right):

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:391: EDITOR_REGEXP =
(r'^dart-editor(-(?P<installer>(installer)))?(-(?P<system>(win|mac|linux)))?' +
long line

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:432: os_rename = {'win': 'win32', 'mac': 'macos', 'linux':
'linux'}
we have a bunch of dictionaries for renames, we could extract that - don't do
that  in this cl though

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:439: for arch in ['32', '64']:
I would extract this into create_mac_installer()

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:444: if gsu.Copy(gsu_editor_zip, 'dart.zip', False):
dart.zip -> editor.zip (+ you could put it into a variable, you use it twice

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:460: assert os.path.exists(dmg_installer)
No need for this, os.path.isfile will return false if the file does not exist as
well

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:476: "We currently cannot build installers for %s" %
builder_name)
builder_name -> SYSTEM?

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:522: #tell the ant script where to write the sdk zip file
so it can
I know this is not you, but fix the comment #tell -> # Tell
space before be below
+ below

https://codereview.chromium.org/28773003/diff/100001/tools/mac_build_editor_d...
File tools/mac_build_editor_dmg.sh (right):

https://codereview.chromium.org/28773003/diff/100001/tools/mac_build_editor_d...
tools/mac_build_editor_dmg.sh:48: replace_folder_icon() {
could you give a little context here in a comment, what are we doing and why?

https://codereview.chromium.org/28773003/diff/100001/tools/mac_build_editor_d...
tools/mac_build_editor_dmg.sh:103: set position of item "$FOLDER_NAME" to {64,
64}
consider removing tabs

[kustermann, 2013-10-18 14:12:12]

Thanks.

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py
File editor/build/build.py (right):

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:391: EDITOR_REGEXP =
(r'^dart-editor(-(?P<installer>(installer)))?(-(?P<system>(win|mac|linux)))?' +
On 2013/10/18 11:18:08, ricow1 wrote:
> long line

Done.

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:432: os_rename = {'win': 'win32', 'mac': 'macos', 'linux':
'linux'}
On 2013/10/18 11:18:08, ricow1 wrote:
> we have a bunch of dictionaries for renames, we could extract that - don't do
> that  in this cl though

Well, we can remove all this special casing once we no longer use the old bucket
locations.

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:439: for arch in ['32', '64']:
On 2013/10/18 11:18:08, ricow1 wrote:
> I would extract this into create_mac_installer()

Done.

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:444: if gsu.Copy(gsu_editor_zip, 'dart.zip', False):
On 2013/10/18 11:18:08, ricow1 wrote:
> dart.zip -> editor.zip (+ you could put it into a variable, you use it twice

Partly done (I think the additional variable would make the code harder to
read).

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:460: assert os.path.exists(dmg_installer)
On 2013/10/18 11:18:08, ricow1 wrote:
> No need for this, os.path.isfile will return false if the file does not exist
as
> well

Done.

https://codereview.chromium.org/28773003/diff/100001/editor/build/build.py#ne...
editor/build/build.py:476: "We currently cannot build installers for %s" %
builder_name)
On 2013/10/18 11:18:08, ricow1 wrote:
> builder_name -> SYSTEM?

Done.

https://codereview.chromium.org/28773003/diff/100001/tools/mac_build_editor_d...
File tools/mac_build_editor_dmg.sh (right):

https://codereview.chromium.org/28773003/diff/100001/tools/mac_build_editor_d...
tools/mac_build_editor_dmg.sh:48: replace_folder_icon() {
On 2013/10/18 11:18:08, ricow1 wrote:
> could you give a little context here in a comment, what are we doing and why?

Done.

https://codereview.chromium.org/28773003/diff/100001/tools/mac_build_editor_d...
tools/mac_build_editor_dmg.sh:103: set position of item "$FOLDER_NAME" to {64,
64}
On 2013/10/18 11:18:08, ricow1 wrote:
> consider removing tabs

Done.

[kustermann, 2013-10-18 14:13:44]

Committed patchset #5 manually as r28857 (presubmit successful).

[kustermann, 2013-10-18 14:49:54]

@devon: Just FYI. This CL should have 0 impact on the normal editor building.

Since we're not going to use the "./tools/build.py editor" approach anytime
soon, I added this to the 'editor/build/build.py' script. 

We'll setup a builder which will build installers on-demand (i.e. when manually
triggered). We can't automate this because the builder can only create the
installer once the bits have been signed (which is a manual step).

We're announcing this once everything is setup and works.