Added `cros_pin` CrOS pin-bump tool.

Added `cros_pin` CrOS pin-bump tool.

BUG=chromium:543290
TEST=local
  - Ran `update` command successfully.
  - Ran `add-release` command successfully.

Committed: https://chromium.googlesource.com/infra/infra/+/74367e15a1678cedc21f17b954fe85f5c983bb8b

[dnj, 2015-10-15 18:48:42]

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

[dnj, 2015-10-15 18:48:43]

PTAL

This is a `cit`-gettable tool that automatically generates and submits Chromite
pin bumps.

It enables update Chromite pins in one line, e.g.:
cit cros_pin update master

It also enables release waterfall reconfiguration in two commands:
cit cros_pin add-release release-R48-9999.B
cit restart chromeos_release

It depends on:

Public:
- https://codereview.chromium.org/1402253002/
- https://codereview.chromium.org/1393893004/

Internal:
- https://chromereviews.googleplex.com/272847013/
- https://chromereviews.googleplex.com/270347013/

[dnj, 2015-10-15 18:49:17]

dnj@chromium.org changed reviewers:
+ hinoka@chromium.org

[dnj, 2015-10-15 18:49:18]

+hinoka@, since you wrote `cit restart` :)

[dnj, 2015-10-15 19:25:59]

Patchset #2 (id:20001) has been deleted

[ghost stip (do not use), 2015-10-15 20:02:55]

there's a lot of code here. I would have appreciated a smaller review in chunks
which builds up the tool

in general lgtm

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
File infra/tools/cros_pin/checkout.py (right):

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
infra/tools/cros_pin/checkout.py:15: class Checkout(object):
can you make this a context manager?

just add __enter__() and __exit__() as in
https://docs.python.org/2/reference/datamodel.html#object.__enter__

then you can do

with Checkout('chillpath') as checkout:
  do_stuff(checkout.subpath('yolo'))

and it will get deleted automatically when you exit the context manager

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
File infra/tools/cros_pin/cros_pin.py (right):

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:88: def checkout_for_args(args):
why not just make Checkout() a contextmanager itself? just add __enter__ and
__exit__

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
File infra/tools/cros_pin/execute.py (right):

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
infra/tools/cros_pin/execute.py:7: import threading
remove unused import

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
infra/tools/cros_pin/execute.py:12: def call(cmd, cwd=None, dry_run=False):
this whole function seems overkill. this is just to execute stuff and print the
output on stderr?

[dnj, 2015-10-15 21:06:41]

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
File infra/tools/cros_pin/checkout.py (right):

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
infra/tools/cros_pin/checkout.py:15: class Checkout(object):
On 2015/10/15 20:02:54, stip wrote:
> can you make this a context manager?
> 
> just add __enter__() and __exit__() as in
> https://docs.python.org/2/reference/datamodel.html#object.__enter__
> 
> then you can do
> 
> with Checkout('chillpath') as checkout:
>   do_stuff(checkout.subpath('yolo'))
> 
> and it will get deleted automatically when you exit the context manager

Yeah! I kind of have something going like that in `checkout_for_args`, but I'll
move it closer to checkout class.

with Checkout.use(...) as c:
  ...

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
File infra/tools/cros_pin/cros_pin.py (right):

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:88: def checkout_for_args(args):
On 2015/10/15 20:02:54, stip wrote:
> why not just make Checkout() a contextmanager itself? just add __enter__ and
> __exit__

I want to have code initialize it, and I don't like doing non-member
initialization in a constructor, but I'll do this in a classmethod.

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
File infra/tools/cros_pin/execute.py (right):

https://chromiumcodereview.appspot.com/1403313002/diff/40001/infra/tools/cros...
infra/tools/cros_pin/execute.py:12: def call(cmd, cwd=None, dry_run=False):
On 2015/10/15 20:02:54, stip wrote:
> this whole function seems overkill. this is just to execute stuff and print
the
> output on stderr?

Well, execute stuff and:
1) Print output to STDERR in realtime, and
2) Capture the output.

Combining (1) and (2) need a function.

[hinoka, 2015-10-21 19:49:40]

in general lgtm (since its an isolated tool, risk of breaking things is low),
but I find this code overly-boilerplate ish, hard to follow, and a bit of an
overkill for a small tool.  Maybe if this was supposed to grow to be the size of
gclient.py the abstraction is justified, but imo I think it's over engineered
for its size :).  I also don't quite understand the use case.

Can you add tests for pinfile.py?

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
File infra/tools/cros_pin/cros_pin.py (right):

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:36:
parser.add_argument('--chromite-repository', default=CHROMITE_REPOSITORY,
What other ones can we query?

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:46: parser.add_argument('-c', '--commit',
What about doing this the other way? have a --no-commit, dest=commit,
action=store_false.  And then also query for a "confirm (y/n)? " from the user,
so that it matches the semantics of cit restart

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:70: subp = subparsers.add_parser('add-release',
When does one use this tool?

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:72: subp.add_argument('--stable-count',
metavar='COUNT',
Under what circumstances does this need to change?

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:81: help="The commit hash for the branch. If
empty, use the branch's "
Should this ever not be ToT?

[dnj, 2015-10-22 21:48:41]

I don't think it's terribly over-engineered. Maybe a little. It does two
relatively complex things and some independent things (such as checkouts), and
it's broken along those boundaries. The structure definitely makes isolated
testing possible.

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
File infra/tools/cros_pin/cros_pin.py (right):

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:36:
parser.add_argument('--chromite-repository', default=CHROMITE_REPOSITORY,
On 2015/10/21 19:49:40, hinoka wrote:
> What other ones can we query?

Some other CrOS-derivative projects are using CrOS Chromite-style
configurations.

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:46: parser.add_argument('-c', '--commit',
On 2015/10/21 19:49:40, hinoka wrote:
> What about doing this the other way? have a --no-commit, dest=commit,
> action=store_false.  And then also query for a "confirm (y/n)? " from the
user,
> so that it matches the semantics of cit restart

Done.

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:70: subp = subparsers.add_parser('add-release',
On 2015/10/21 19:49:40, hinoka wrote:
> When does one use this tool?

Adding a new CrOS release branch to the CrOS release waterfall.

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:72: subp.add_argument('--stable-count',
metavar='COUNT',
On 2015/10/21 19:49:40, hinoka wrote:
> Under what circumstances does this need to change?

It's up to the TPMs, but generally it's only changed once.

https://chromiumcodereview.appspot.com/1403313002/diff/60001/infra/tools/cros...
infra/tools/cros_pin/cros_pin.py:81: help="The commit hash for the branch. If
empty, use the branch's "
On 2015/10/21 19:49:40, hinoka wrote:
> Should this ever not be ToT?

Up to the TPMs, but probably will be ToT.

[dnj (Google), 2015-10-23 00:09:21]

The CQ bit was checked by dnj@google.com

[dnj (Google), 2015-10-23 00:09:21]

The patchset sent to the CQ was uploaded after l-g-t-m from stip@chromium.org,
hinoka@chromium.org
Link to the patchset: https://codereview.chromium.org/1403313002/#ps80001
(title: "Added pinfile.py tests, cleanup.")

[commit-bot: I haz the power, 2015-10-23 00:09:34]

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

[commit-bot: I haz the power, 2015-10-23 00:11:51]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/infra/infra/+/74367e15a1678cedc21f17b954fe8...