Add CIPD support for bot assets

Add CIPD support for bot assets

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2085473002

Committed: https://skia.googlesource.com/skia/+/f9bd9da14a668008b3cff9ef69ebf50962e4fd44

[borenet2, 2016-06-20 12:53:03]

Description was changed from

==========
More WIP


[WIP] Add CIPD support for assets

BUG=skia:
==========

to

==========
More WIP


[WIP] Add CIPD support for assets

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2085473002
==========

[borenet, 2016-06-20 14:12:17]

Description was changed from

==========
More WIP


[WIP] Add CIPD support for assets

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2085473002
==========

to

==========
Add CIPD support for bot assets

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2085473002
==========

[borenet, 2016-06-20 19:30:05]

borenet@google.com changed reviewers:
+ borenet@google.com, kjlubick@google.com, rmistry@google.com

[borenet, 2016-06-20 19:30:06]

https://codereview.chromium.org/2085473002/diff/40001/infra/bots/assets/asset...
File infra/bots/assets/asset_utils.py (right):

https://codereview.chromium.org/2085473002/diff/40001/infra/bots/assets/asset...
infra/bots/assets/asset_utils.py:139: # TODO(borenet): How do we delete
packages?
At the moment, the CIPD CLI has no command for deleting packages.  I am told
that this might exist soon.  We only really need this for tests.

https://codereview.chromium.org/2085473002/diff/40001/infra/bots/assets/asset...
File infra/bots/assets/asset_utils_test.py (right):

https://codereview.chromium.org/2085473002/diff/40001/infra/bots/assets/asset...
infra/bots/assets/asset_utils_test.py:131: class CIPDStoreTest(StoreTest):
These tests do not currently pass, for the following reasons:

- CIPD ignores empty directories like those included in the test (which were
included specifically to test whether empty directories are included...)
- CIPD sets removes write permission bit on files in packages.
- CIPD symlinks the files by default (there's a switch for this, but I don't
know if we care).
- CIPD adds a ".cipd" directory in the dest dir.
- We don't have a way to delete packages, so test_versions fails due to leftover
packages.

I have verified that the scripts still work end to end.

[rmistry, 2016-06-22 12:41:41]

lgtm

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
File infra/bots/assets/asset_utils.py (right):

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
infra/bots/assets/asset_utils.py:36: TAG_VERSION_TMPL = '%s%%s' %
TAG_VERSION_PREFIX
Nit: Could you group some of these constants to make them easier to read.
Something like:

ASSETS_DIR =
SKIA_DIR =

TAG_PROJECT_SKIA =
TAG_VERSION_PREFIX =
TAG_VERSION_TMPL

GS_STUFF1 =
GS_STUFF2 =
GS_.. =

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
infra/bots/assets/asset_utils.py:107: pass
Raise an exception here? seems like this should not happen and if it does then
we want to know about it.

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
infra/bots/assets/asset_utils.py:225: def _prompt(prompt):
Inline? Used in one place and taking up 5 lines.

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
infra/bots/assets/asset_utils.py:288: @classmethod
Why make this a class method? name and store can be specified in the
constructor.

[borenet, 2016-06-28 11:23:19]

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
File infra/bots/assets/asset_utils.py (right):

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
infra/bots/assets/asset_utils.py:36: TAG_VERSION_TMPL = '%s%%s' %
TAG_VERSION_PREFIX
On 2016/06/22 12:41:40, rmistry wrote:
> Nit: Could you group some of these constants to make them easier to read.
> Something like:
> 
> ASSETS_DIR =
> SKIA_DIR =
> 
> TAG_PROJECT_SKIA =
> TAG_VERSION_PREFIX =
> TAG_VERSION_TMPL
> 
> GS_STUFF1 =
> GS_STUFF2 =
> GS_.. =
> 

Done.

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
infra/bots/assets/asset_utils.py:107: pass
On 2016/06/22 12:41:40, rmistry wrote:
> Raise an exception here? seems like this should not happen and if it does then
> we want to know about it.

Done.

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
infra/bots/assets/asset_utils.py:225: def _prompt(prompt):
On 2016/06/22 12:41:40, rmistry wrote:
> Inline? Used in one place and taking up 5 lines.

It needs to be in its own function so that the tests can mock it out.

https://codereview.chromium.org/2085473002/diff/80001/infra/bots/assets/asset...
infra/bots/assets/asset_utils.py:288: @classmethod
On 2016/06/22 12:41:41, rmistry wrote:
> Why make this a class method? name and store can be specified in the
> constructor.

This is a helper function for adding an asset, whereas the constructor assumes
that the asset already exists in the repo.

[borenet, 2016-06-28 11:23:24]

The CQ bit was checked by borenet@google.com

[borenet, 2016-06-28 11:23:24]

The patchset sent to the CQ was uploaded after l-g-t-m from rmistry@google.com
Link to the patchset: https://codereview.chromium.org/2085473002/#ps100001
(title: "Update CIPD to include new pkg-delete command")

[commit-bot: I haz the power, 2016-06-28 11:23:33]

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-06-28 11:41:56]

Description was changed from

==========
Add CIPD support for bot assets

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2085473002
==========

to

==========
Add CIPD support for bot assets

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2085473002

Committed:
https://skia.googlesource.com/skia/+/f9bd9da14a668008b3cff9ef69ebf50962e4fd44
==========

[commit-bot: I haz the power, 2016-06-28 11:41:57]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/f9bd9da14a668008b3cff9ef69ebf50962e4fd44