lightweight builds archiving for mac and win64

lightweight builds archiving for mac and win64.
Changing perf buildbot to add a step 'repackage
for bisect' to Win64/Mac. 


mac,win,win64 support

BUG=604452
R=dimu

Committed: https://chromium.googlesource.com/chromium/tools/build/+/9c526af4f649b33e3a1351dd9c9bbe6fdb7122da

[miimnk, 2016-08-29 18:26:11]

Description was changed from

==========
lightweight builds archiving for mac and win64


mac,win,win64 support

BUG=604452
R=dimu
==========

to

==========
lightweight builds archiving for mac and win64.
Changing perf buildbot to add a step 'repackage
for bisect' to Win64/Mac. 


mac,win,win64 support

BUG=604452
R=dimu
==========

[miimnk, 2016-08-29 18:26:11]

miimnk@google.com changed reviewers:
+ dtu@chromium.org

[dtu, 2016-08-30 02:40:21]

dtu@chromium.org changed reviewers:
+ stip@chromium.org

[dtu, 2016-08-30 02:40:22]

https://codereview.chromium.org/2279953002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/archive/api.py (right):

https://codereview.chromium.org/2279953002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:97: include_bisect_whitelist=None,
**kwargs):
This is a lot of plumbing/parameters in common code for one particular use case.
Can we push the logic about what lists to use lower down in the call chain?

https://codereview.chromium.org/2279953002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/manual_bisect_files.py (right):

https://codereview.chromium.org/2279953002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/manual_bisect_files.py:15: 'Linux
Builder': [
Prefer to not index by display name. If we could use a more canonical ID that'd
be nicer.
Maybe you can use chromium_utils.IsWindows(), etc. to detect the platform and
index by platform name?

[miimnk, 2016-08-30 21:14:54]

Hi Dave.
Thank you so much for your code review. I fixed the issues you mentioned in your
comments.

https://codereview.chromium.org/2279953002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/archive/api.py (right):

https://codereview.chromium.org/2279953002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:97: include_bisect_whitelist=None,
**kwargs):
On 2016/08/30 02:40:22, dtu wrote:
> This is a lot of plumbing/parameters in common code for one particular use
case.
> Can we push the logic about what lists to use lower down in the call chain?

Yes. Instead, passed only platform name to get these list later in
recipe_modules/archive/api.py

https://codereview.chromium.org/2279953002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/manual_bisect_files.py (right):

https://codereview.chromium.org/2279953002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/manual_bisect_files.py:15: 'Linux
Builder': [
On 2016/08/30 02:40:22, dtu wrote:
> Prefer to not index by display name. If we could use a more canonical ID
that'd
> be nicer.
> Maybe you can use chromium_utils.IsWindows(), etc. to detect the platform and
> index by platform name?

Passed in the platform name from chromium_tests/api.py instead of using
chromium_utils.IsWindows() because using the latter masses up with
recipe_simulation_test. When running recipe_simulation_test, the script assumes
that all the builders are running on the platform of the workstation.

[ghost stip (do not use), 2016-08-30 23:48:53]

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/archive/api.py (right):

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:11: 
nit: remove space

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:129:
CHROME_REQUIRED_FILES[platform])
should we make this .get(platform) so it doesn't bomb out if the platform is
new?

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/zip_build.py
File scripts/slave/zip_build.py (right):

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/zip_build...
scripts/slave/zip_build.py:434: default=False, help='Only includes include file
list and '
nit: 80 chars

[miimnk, 2016-08-31 00:19:05]

Thank you so much for your code review Mike. I fixed the issues you mentioned in
your comments.

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/archive/api.py (right):

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:11: 
On 2016/08/30 23:48:53, stip wrote:
> nit: remove space

Done.

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:129:
CHROME_REQUIRED_FILES[platform])
On 2016/08/30 23:48:53, stip wrote:
> should we make this .get(platform) so it doesn't bomb out if the platform is
> new?

Done.

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/zip_build.py
File scripts/slave/zip_build.py (right):

https://codereview.chromium.org/2279953002/diff/80001/scripts/slave/zip_build...
scripts/slave/zip_build.py:434: default=False, help='Only includes include file
list and '
On 2016/08/30 23:48:53, stip wrote:
> nit: 80 chars

Done.

[ghost stip (do not use), 2016-08-31 16:38:27]

sorry for the delay. lgtm w/ nit

https://codereview.chromium.org/2279953002/diff/100001/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/archive/api.py (right):

https://codereview.chromium.org/2279953002/diff/100001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/archive/api.py:7: import manual_bisect_files
nit: put a space between system imports and local imports

https://codereview.chromium.org/2279953002/diff/100001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/archive/api.py:127: include_bisect_file_list =
(manual_bisect_files.
nit: move the line down and indent so it looks like this
include_bisect_file_list = (
    manual_bisect_files.get_required_files(platform))

https://codereview.chromium.org/2279953002/diff/100001/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/archive/manual_bisect_files.py (right):

https://codereview.chromium.org/2279953002/diff/100001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/archive/manual_bisect_files.py:81: def
get_required_files(platform):
could have just used CHROME_REQUIRED_FILES.get(platform)   :)

https://docs.python.org/2/library/stdtypes.html#dict.get

[chromium-reviews, 2016-08-31 18:19:27]

Thank you so much Mike!
Reverted the changes in manual_bisect_files.py and used .get instead.

Best,

Min

On Wed, Aug 31, 2016 at 9:38 AM, <stip@chromium.org> wrote:

> sorry for the delay. lgtm w/ nit
>
>
> https://codereview.chromium.org/2279953002/diff/100001/
> scripts/slave/recipe_modules/archive/api.py
> File scripts/slave/recipe_modules/archive/api.py (right):
>
> https://codereview.chromium.org/2279953002/diff/100001/
> scripts/slave/recipe_modules/archive/api.py#newcode7
> scripts/slave/recipe_modules/archive/api.py:7: import
> manual_bisect_files
> nit: put a space between system imports and local imports
>
> https://codereview.chromium.org/2279953002/diff/100001/
> scripts/slave/recipe_modules/archive/api.py#newcode127
> scripts/slave/recipe_modules/archive/api.py:127:
> include_bisect_file_list = (manual_bisect_files.
> nit: move the line down and indent so it looks like this
> include_bisect_file_list = (
> manual_bisect_files.get_required_files(platform))
>
> https://codereview.chromium.org/2279953002/diff/100001/
> scripts/slave/recipe_modules/archive/manual_bisect_files.py
> File scripts/slave/recipe_modules/archive/manual_bisect_files.py
> (right):
>
> https://codereview.chromium.org/2279953002/diff/100001/
> scripts/slave/recipe_modules/archive/manual_bisect_files.py#newcode81
> scripts/slave/recipe_modules/archive/manual_bisect_files.py:81: def
> get_required_files(platform):
> could have just used CHROME_REQUIRED_FILES.get(platform) :)
>
> https://docs.python.org/2/library/stdtypes.html#dict.get
>
> https://codereview.chromium.org/2279953002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[miimnk, 2016-08-31 18:32:39]

The CQ bit was checked by miimnk@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-31 18:32:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 18:37:00]

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

[commit-bot: I haz the power, 2016-08-31 18:37:01]

Dry run: This issue passed the CQ dry run.

[Michael Moss, 2016-08-31 18:53:55]

mmoss@chromium.org changed reviewers:
+ mmoss@chromium.org

[Michael Moss, 2016-08-31 18:53:56]

lgtm

[miimnk, 2016-08-31 18:57:02]

The CQ bit was checked by miimnk@google.com

[miimnk, 2016-08-31 18:57:03]

The patchset sent to the CQ was uploaded after l-g-t-m from stip@chromium.org
Link to the patchset: https://codereview.chromium.org/2279953002/#ps140001
(title: "resolved merge conflicts")

[commit-bot: I haz the power, 2016-08-31 18:57:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 19:01:29]

Description was changed from

==========
lightweight builds archiving for mac and win64.
Changing perf buildbot to add a step 'repackage
for bisect' to Win64/Mac. 


mac,win,win64 support

BUG=604452
R=dimu
==========

to

==========
lightweight builds archiving for mac and win64.
Changing perf buildbot to add a step 'repackage
for bisect' to Win64/Mac. 


mac,win,win64 support

BUG=604452
R=dimu

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/9c526af4f649b33e3a13...
==========

[commit-bot: I haz the power, 2016-08-31 19:01:30]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/tools/build/+/9c526af4f649b33e3a13...

[miimnk, 2016-08-31 21:00:13]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/2303433002/ by miimnk@google.com.

The reason for reverting is: Ran into an error. .

[dimu, 2016-08-31 22:38:33]

lgtm