WebRTC: Add recipe to build iOS API Framework

WebRTC: Add recipe to build iOS API Framework

BUG=webrtc:5737
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299806

[kjellander_chromium, 2016-04-05 21:05:19]

Patchset #1 (id:1) has been deleted

[kjellander_chromium, 2016-04-05 21:06:14]

kjellander@chromium.org changed reviewers:
+ iannucci@chromium.org, tkchin@chromium.org

[kjellander_chromium, 2016-04-05 21:06:14]

iannucci: recipe stuff
tkchin: cocoapod script execution (check the JSON expectation files for details
on what's will happen on the bot).

[tkchin_webrtc, 2016-04-05 21:15:50]

tkchin@webrtc.org changed reviewers:
+ tkchin@webrtc.org

[tkchin_webrtc, 2016-04-05 21:15:51]

https://codereview.chromium.org/1865653002/diff/20001/scripts/slave/recipes/w...
File scripts/slave/recipes/webrtc/ios_cocoapod.expected/build_failure.json
(right):

https://codereview.chromium.org/1865653002/diff/20001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_cocoapod.expected/build_failure.json:33: "iOS
CocoaPod Builder",
nit: this technically builds the WebRTC API framework, not the CocoaPod.

https://codereview.chromium.org/1865653002/diff/20001/scripts/slave/recipes/w...
File scripts/slave/recipes/webrtc/ios_cocoapod.expected/build_ok.json (right):

https://codereview.chromium.org/1865653002/diff/20001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_cocoapod.expected/build_ok.json:111:
"[SLAVE_BUILD]/webrtc_ios_cocoapod.zip",
is it okay that we will be storing ~20MB uncompressed per checkin?

[kjellander_chromium, 2016-04-06 06:52:22]

Description was changed from

==========
WebRTC: Add recipe to build iOS CocoaPod

BUG=
==========

to

==========
WebRTC: Add recipe to build iOS CocoaPod

BUG=webrtc:5737
==========

[kjellander_chromium, 2016-04-06 15:54:05]

Description was changed from

==========
WebRTC: Add recipe to build iOS CocoaPod

BUG=webrtc:5737
==========

to

==========
WebRTC: Add recipe to build iOS API Framework

BUG=webrtc:5737
==========

[kjellander_chromium, 2016-04-06 16:04:55]

The CQ bit was checked by kjellander@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-06 16:05:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865653002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865653002/60001

[kjellander_chromium, 2016-04-06 16:08:51]

PTAL

https://codereview.chromium.org/1865653002/diff/20001/scripts/slave/recipes/w...
File scripts/slave/recipes/webrtc/ios_cocoapod.expected/build_failure.json
(right):

https://codereview.chromium.org/1865653002/diff/20001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_cocoapod.expected/build_failure.json:33: "iOS
CocoaPod Builder",
On 2016/04/05 21:15:51, tkchin_webrtc wrote:
> nit: this technically builds the WebRTC API framework, not the CocoaPod.

I suspected I had got this wrong :) Updated.

https://codereview.chromium.org/1865653002/diff/20001/scripts/slave/recipes/w...
File scripts/slave/recipes/webrtc/ios_cocoapod.expected/build_ok.json (right):

https://codereview.chromium.org/1865653002/diff/20001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_cocoapod.expected/build_ok.json:111:
"[SLAVE_BUILD]/webrtc_ios_cocoapod.zip",
On 2016/04/05 21:15:51, tkchin_webrtc wrote:
> is it okay that we will be storing ~20MB uncompressed per checkin?

I'm quite sure, since many Chromium bots store about a gigabyte per build :-o

https://codereview.chromium.org/1865653002/diff/40001/scripts/slave/recipes/w...
File scripts/slave/recipes/webrtc/ios_api_framework.py (right):

https://codereview.chromium.org/1865653002/diff/40001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:27: for out_dir in
["out_ios_arm",
This part is new in PS#2 (the rename makes the diff fail).

https://codereview.chromium.org/1865653002/diff/40001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:46:
'ios_api_framework/webrtc_ios_api_framework.zip',
I changed cocoapod -> api_framwork everywhere.

[commit-bot: I haz the power, 2016-04-06 16:08:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-06 16:08:55]

Dry run: This issue passed the CQ dry run.

[tkchin_webrtc, 2016-04-06 17:03:21]

On 2016/04/06 16:08:55, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

lgtm

[kjellander_chromium, 2016-04-08 12:04:50]

kjellander@chromium.org changed reviewers:
+ machenbach@chromium.org

[kjellander_chromium, 2016-04-08 12:04:50]

+machenbach to offload iannucci

[Michael Achenbach, 2016-04-08 12:17:50]

LG - one important question below:

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
File scripts/slave/recipes/webrtc/ios_api_framework.py (right):

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:33: api.file.rmtree('clobber
%s' % out_dir, api.path['checkout'].join(out_dir))
I'm ok with this, but it would be nicer if we had a script that took a bunch of
dirs on the cmd line to only have one step.

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:37: api.step('build',
['/bin/bash', build_script])
I think you could drop the '/bin/bash' - the script might need to have
executable perms.

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:46:
'ios_api_framework/webrtc_ios_api_framework.zip',
You continuously overwrite the same file on GS. Is that intended? Or should it
have e.g. the git hash as part of the name?

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:56: slavename='slavename')
I think slavename has already some default when using "generic". You could
probably drop it.

[kjellander_chromium, 2016-04-08 14:27:09]

Comments addressed, thanks!

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
File scripts/slave/recipes/webrtc/ios_api_framework.py (right):

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:33: api.file.rmtree('clobber
%s' % out_dir, api.path['checkout'].join(out_dir))
On 2016/04/08 12:17:49, Michael Achenbach wrote:
> I'm ok with this, but it would be nicer if we had a script that took a bunch
of
> dirs on the cmd line to only have one step.

Yeah, but I didn't find a supported way to do that. I'm fine with these multiple
steps for now.

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:37: api.step('build',
['/bin/bash', build_script])
On 2016/04/08 12:17:49, Michael Achenbach wrote:
> I think you could drop the '/bin/bash' - the script might need to have
> executable perms.

Good point, that's cleaner (and yes: it has +x).

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:46:
'ios_api_framework/webrtc_ios_api_framework.zip',
On 2016/04/08 12:17:50, Michael Achenbach wrote:
> You continuously overwrite the same file on GS. Is that intended? Or should it
> have e.g. the git hash as part of the name?

Good catch, didn't put much thought into this obviously. Moved back to using one
of the generating functions instead.

https://codereview.chromium.org/1865653002/diff/60001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/ios_api_framework.py:56: slavename='slavename')
On 2016/04/08 12:17:49, Michael Achenbach wrote:
> I think slavename has already some default when using "generic". You could
> probably drop it.

Done.

[kjellander_chromium, 2016-04-08 14:27:13]

The CQ bit was checked by kjellander@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-08 14:27:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865653002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865653002/80001

[Michael Achenbach, 2016-04-08 14:35:55]

lgtm

[kjellander_chromium, 2016-04-08 14:43:52]

The CQ bit was unchecked by kjellander@chromium.org

[kjellander_chromium, 2016-04-08 18:15:00]

The CQ bit was checked by kjellander@chromium.org

[kjellander_chromium, 2016-04-08 18:15:01]

The patchset sent to the CQ was uploaded after l-g-t-m from tkchin@webrtc.org
Link to the patchset: https://codereview.chromium.org/1865653002/#ps80001
(title: "Review comments addressed")

[commit-bot: I haz the power, 2016-04-08 18:15:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865653002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865653002/80001

[commit-bot: I haz the power, 2016-04-08 18:18:27]

Description was changed from

==========
WebRTC: Add recipe to build iOS API Framework

BUG=webrtc:5737
==========

to

==========
WebRTC: Add recipe to build iOS API Framework

BUG=webrtc:5737

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299806
==========

[commit-bot: I haz the power, 2016-04-08 18:18:28]

Committed patchset #4 (id:80001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=299806