add basic bootstrap for coverage

add basic bootstrap for coverage

BUG=618709
R=iannucci@chromium.org, phajdan.jr@chromium.org

Committed: https://github.com/luci/recipes-py/commit/a6baded1c8c7a15dd1e4020289fd19d985f58c25

[martiniss, 2016-06-14 22:43:22]

martiniss@chromium.org changed reviewers:
+ estaab@chromium.org, iannucci@chromium.org, phajdan.jr@chromium.org

[martiniss, 2016-06-14 22:43:22]

PTAL

[tandrii(chromium), 2016-06-15 09:57:52]

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

[tandrii(chromium), 2016-06-15 09:57:53]

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/README.md
File bootstrap/README.md (right):

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/README.md#new...
bootstrap/README.md:1: ## bootstrap.py (a.k.a. "I just want a working
recipes-py)
nit: " at the end

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/README.md#new...
bootstrap/README.md:7: This creates a virtualenv called `{repo_root}/ENV` with
all the deps
how about s/{repo_root}/.

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/README.md#new...
bootstrap/README.md:10: For more information, see the infra.git bootstrap
[README](https://chromium.googlesource.com/infra/infra/+/master/bootstrap/README.md)
also, can you add another file in this dir to specify which revision of
infra/infra the scripts were copy-pasted from.

[tandrii(chromium), 2016-06-15 09:58:56]

thanks, this is what I wanted. Let's see what others say.

[Paweł Hajdan Jr., 2016-06-15 10:56:19]

LGTM w/comments . I'm fine with US folks finishing this review.

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/bootstrap.py
File bootstrap/bootstrap.py (right):

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/bootstrap.py#...
bootstrap/bootstrap.py:22: # /path/to/infra
nit: Update this comment (it's recipes-py repo, not infra repo).

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/bootstrap.py#...
bootstrap/bootstrap.py:42: 'infra virtualenv environment. Please move it to the
side and bootstrap ',
nit: Also consider replacing infra here and below.

https://codereview.chromium.org/2061263003/diff/20001/recipes.py
File recipes.py (right):

https://codereview.chromium.org/2061263003/diff/20001/recipes.py#newcode221
recipes.py:221: sysargs = sys.argv
What's the intention of this code?

It doesn't seem to copy original contents.

Please either name it e.g. original_sys_argv and make a copy, or remove it and
use sys.argv.

https://codereview.chromium.org/2061263003/diff/20001/recipes.py#newcode386
recipes.py:386: ['bootstrap/bootstrap.py', '--deps-file', 'bootstrap/deps.pyl',
'ENV'],
nit: Start with sys.executable .

https://codereview.chromium.org/2061263003/diff/20001/recipes.py#newcode389
recipes.py:389: args = [arg for arg in sysargs[1:] if 'use-bootstrap' not in
arg]
nit: This might interfere with other legitimate args which might happen to
contain use-bootstrap (e.g. a build property).

Is there some other way we could use to prevent infinite loop here, e.g. an
environment variable similar to how we do for update_scripts, or a
--no-bootstrap flag that overrides any --use-bootstrap flags?

[martiniss, 2016-06-15 20:36:53]

Responded to comments.

Forgot to vendor in virtualenv, which is DEPS-ed in in infra.git

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/README.md
File bootstrap/README.md (right):

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/README.md#new...
bootstrap/README.md:1: ## bootstrap.py (a.k.a. "I just want a working
recipes-py)
On 2016/06/15 at 09:57:53, tandrii(chromium) wrote:
> nit: " at the end

Done.

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/README.md#new...
bootstrap/README.md:7: This creates a virtualenv called `{repo_root}/ENV` with
all the deps
On 2016/06/15 at 09:57:53, tandrii(chromium) wrote:
> how about s/{repo_root}/.

Done.

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/README.md#new...
bootstrap/README.md:10: For more information, see the infra.git bootstrap
[README](https://chromium.googlesource.com/infra/infra/+/master/bootstrap/README.md)
On 2016/06/15 at 09:57:53, tandrii(chromium) wrote:
> also, can you add another file in this dir to specify which revision of
infra/infra the scripts were copy-pasted from.

Done; README.recipes.

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/bootstrap.py
File bootstrap/bootstrap.py (right):

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/bootstrap.py#...
bootstrap/bootstrap.py:22: # /path/to/infra
On 2016/06/15 at 10:56:19, Paweł Hajdan Jr. wrote:
> nit: Update this comment (it's recipes-py repo, not infra repo).

Done.

https://codereview.chromium.org/2061263003/diff/20001/bootstrap/bootstrap.py#...
bootstrap/bootstrap.py:42: 'infra virtualenv environment. Please move it to the
side and bootstrap ',
On 2016/06/15 at 10:56:19, Paweł Hajdan Jr. wrote:
> nit: Also consider replacing infra here and below.
Done.

https://codereview.chromium.org/2061263003/diff/20001/recipes.py
File recipes.py (right):

https://codereview.chromium.org/2061263003/diff/20001/recipes.py#newcode221
recipes.py:221: sysargs = sys.argv
On 2016/06/15 at 10:56:19, Paweł Hajdan Jr. wrote:
> What's the intention of this code?
> 
> It doesn't seem to copy original contents.
> 
> Please either name it e.g. original_sys_argv and make a copy, or remove it and
use sys.argv.

I moved this to where we mutate sys.argv, and made it make a copy.

https://codereview.chromium.org/2061263003/diff/20001/recipes.py#newcode386
recipes.py:386: ['bootstrap/bootstrap.py', '--deps-file', 'bootstrap/deps.pyl',
'ENV'],
On 2016/06/15 at 10:56:19, Paweł Hajdan Jr. wrote:
> nit: Start with sys.executable .

Done.

https://codereview.chromium.org/2061263003/diff/20001/recipes.py#newcode389
recipes.py:389: args = [arg for arg in sysargs[1:] if 'use-bootstrap' not in
arg]
On 2016/06/15 at 10:56:19, Paweł Hajdan Jr. wrote:
> nit: This might interfere with other legitimate args which might happen to
contain use-bootstrap (e.g. a build property).
> 
> Is there some other way we could use to prevent infinite loop here, e.g. an
environment variable similar to how we do for update_scripts, or a
--no-bootstrap flag that overrides any --use-bootstrap flags?

I used an environment variable, like update scripts.

[iannucci, 2016-06-15 22:01:31]

lgtm, though I think you'll need to `git cl land` due to the whl files.

[martiniss, 2016-06-15 22:18:09]

The CQ bit was checked by martiniss@chromium.org

[martiniss, 2016-06-15 22:18:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
phajdan.jr@chromium.org
Link to the patchset: https://codereview.chromium.org/2061263003/#ps60001
(title: "Fix recipes readme.")

[commit-bot: I haz the power, 2016-06-15 22:18:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2061263003/60001

[martiniss, 2016-06-15 22:26:00]

Description was changed from

==========
add basic bootstrap for coverage

BUG=618709
==========

to

==========
add basic bootstrap for coverage

BUG=618709
R=iannucci@chromium.org, phajdan.jr@chromium.org

Committed:
https://github.com/luci/recipes-py/commit/a6baded1c8c7a15dd1e4020289fd19d985f...
==========

[martiniss, 2016-06-15 22:26:02]

Committed patchset #4 (id:60001) manually as
a6baded1c8c7a15dd1e4020289fd19d985f58c25 (presubmit successful).