Upgrade to coverage 3.7.1 and have it auto-build itself on first use.

Upgrade to coverage 3.7.1 and have it auto-build itself on first use.

R=vadimsh@chromium.org, agable@chromium.org, dpranke@chromium.org
BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261861

[iannucci, 2014-04-04 18:29:32]

On 2014/04/04 18:26:47, iannucci wrote:

+vadim

[Dirk Pranke, 2014-04-04 18:30:20]

Can we not just upgrade and use the one in depot_tools instead? That's what
Blink does ...

[iannucci, 2014-04-04 18:34:48]

On 2014/04/04 18:30:20, Dirk Pranke wrote:
> Can we not just upgrade and use the one in depot_tools instead? That's what
> Blink does ...

I don't understand how depot_tools factors into this? We aren't doing a
cross-repo library import, and the 'coverage' CLI tool doesn't correctly handle
multithreaded processes?

[iannucci, 2014-04-04 18:35:07]

On 2014/04/04 18:34:48, iannucci wrote:
> On 2014/04/04 18:30:20, Dirk Pranke wrote:
> > Can we not just upgrade and use the one in depot_tools instead? That's what
> > Blink does ...
> 
> I don't understand how depot_tools factors into this? We aren't doing a
> cross-repo library import, and the 'coverage' CLI tool doesn't correctly
handle
> multithreaded processes?

(and even if it did, we need finer-grained control over the reporting)

[Dirk Pranke, 2014-04-04 18:40:08]

On 2014/04/04 18:35:07, iannucci wrote:
> On 2014/04/04 18:34:48, iannucci wrote:
> > On 2014/04/04 18:30:20, Dirk Pranke wrote:
> > > Can we not just upgrade and use the one in depot_tools instead? That's
what
> > > Blink does ...
> > 
> > I don't understand how depot_tools factors into this? We aren't doing a
> > cross-repo library import, and the 'coverage' CLI tool doesn't correctly
> handle
> > multithreaded processes?
> 
> (and even if it did, we need finer-grained control over the reporting)

I am suggesting that you should use the cross-repo library import.

[iannucci, 2014-04-04 18:40:58]

Confirmed working on OS X and linux. Windows will work as long as `python
setup.py bdist_egg` would work.

[iannucci, 2014-04-04 18:41:37]

On 2014/04/04 18:40:08, Dirk Pranke wrote:
> On 2014/04/04 18:35:07, iannucci wrote:
> > On 2014/04/04 18:34:48, iannucci wrote:
> > > On 2014/04/04 18:30:20, Dirk Pranke wrote:
> > > > Can we not just upgrade and use the one in depot_tools instead? That's
> what
> > > > Blink does ...
> > > 
> > > I don't understand how depot_tools factors into this? We aren't doing a
> > > cross-repo library import, and the 'coverage' CLI tool doesn't correctly
> > handle
> > > multithreaded processes?
> > 
> > (and even if it did, we need finer-grained control over the reporting)
> 
> I am suggesting that you should use the cross-repo library import.

!!!!!!!!!

We're desperately trying to get away from that....

[iannucci, 2014-04-04 18:42:08]

On 2014/04/04 18:41:37, iannucci wrote:
> On 2014/04/04 18:40:08, Dirk Pranke wrote:
> > On 2014/04/04 18:35:07, iannucci wrote:
> > > On 2014/04/04 18:34:48, iannucci wrote:
> > > > On 2014/04/04 18:30:20, Dirk Pranke wrote:
> > > > > Can we not just upgrade and use the one in depot_tools instead? That's
> > what
> > > > > Blink does ...
> > > > 
> > > > I don't understand how depot_tools factors into this? We aren't doing a
> > > > cross-repo library import, and the 'coverage' CLI tool doesn't correctly
> > > handle
> > > > multithreaded processes?
> > > 
> > > (and even if it did, we need finer-grained control over the reporting)
> > 
> > I am suggesting that you should use the cross-repo library import.
> 
> !!!!!!!!!
> 
> We're desperately trying to get away from that....

caveat: if build pinned depot_tools in DEPS, then that would be fine, but it
doesn't.

[agable, 2014-04-04 19:02:05]

Thinking forward: either land this in depot_tools or land in in
infra.git/third_party too.

[iannucci, 2014-04-04 19:09:46]

On 2014/04/04 19:02:05, agable wrote:
> Thinking forward: either land this in depot_tools or land in in
> infra.git/third_party too.

I'd be happy to land this in depot_tools as well, though it won't really help
people who just 'import coverage' since they'd need to know to add the egg to
their path. I can fix up the 3-4 test cases in depot_tools which use coverage to
use this, but other users would be on their own.

[Vadim Sh., 2014-04-04 19:21:58]

For what it's worth, I think we shouldn't do cross repo imports (unless DEPS are
pinned), and there's not sense in adding it to infra.git/third_party right now,
since it's not yet very alive (and we may eventually build virtualenv-based
mechanism there for development-time dependencies, that doesn't need "let's
build everything during import time" hacks).

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
File scripts/slave/unittests/test_env.py (right):

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:46: cov_egg = os.path.join(cov_dir, 'dist',
'coverage-3.7.1-py%s-%s.egg' % (
That's what bdist_egg produces? It there a way to FORCE it to produce this file,
instead of guessing it?

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:54: p = subprocess.Popen([sys.executable,
'setup.py', 'bdist_egg'], cwd=cov_dir,
It may be worth to add some instructions how to build it on windows. OR, allow
system-wide coverage 3.7.1 to be used on windows and refer to
https://pypi.python.org/packages/2.7/c/coverage/coverage-3.7.1.win32-py2.7.ex...
(or whatever, from PyPI: https://pypi.python.org/pypi/coverage/3.7.1)

That may be easier than requiring working MSVC environment.

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:63: sys.path.insert(0, cov_egg)
Call 'ensure_coverage_importable' again, to ensure it is importable now? With a
flag to disable auto build...

[Vadim Sh., 2014-04-04 19:32:20]

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
File scripts/slave/unittests/test_env.py (right):

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:54: p = subprocess.Popen([sys.executable,
'setup.py', 'bdist_egg'], cwd=cov_dir,
On 2014/04/04 19:21:59, Vadim Sh. wrote:
> It may be worth to add some instructions how to build it on windows. OR, allow
> system-wide coverage 3.7.1 to be used on windows and refer to
>
https://pypi.python.org/packages/2.7/c/coverage/coverage-3.7.1.win32-py2.7.ex...
> (or whatever, from PyPI: https://pypi.python.org/pypi/coverage/3.7.1)
> 
> That may be easier than requiring working MSVC environment.

Oh, it's already possible to import it from system location. Then just add a
pointer to PyPI page? To let people know that failure on windows is
half-expected and there's a workaround.

[iannucci, 2014-04-04 19:34:34]

PTAL

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
File scripts/slave/unittests/test_env.py (right):

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:46: cov_egg = os.path.join(cov_dir, 'dist',
'coverage-3.7.1-py%s-%s.egg' % (
On 2014/04/04 19:21:59, Vadim Sh. wrote:
> That's what bdist_egg produces? It there a way to FORCE it to produce this
file,
> instead of guessing it?

This is taken from setuptools' code. I can force get_build_platform, but not
get_python_version. Setuptools is going to import these exact same functions
anyway.

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:54: p = subprocess.Popen([sys.executable,
'setup.py', 'bdist_egg'], cwd=cov_dir,
On 2014/04/04 19:21:59, Vadim Sh. wrote:
> It may be worth to add some instructions how to build it on windows. OR, allow
> system-wide coverage 3.7.1 to be used on windows and refer to
>
https://pypi.python.org/packages/2.7/c/coverage/coverage-3.7.1.win32-py2.7.ex...
> (or whatever, from PyPI: https://pypi.python.org/pypi/coverage/3.7.1)
> 
> That may be easier than requiring working MSVC environment.

Yeah I wasn't sure how to support windows, but if you look at the first import
block at the top, if there is a correctly-configured system-wide python, it'll
take that.

I'll add a message on windows which links to 'how to install python-coverage on
winders'

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:63: sys.path.insert(0, cov_egg)
On 2014/04/04 19:21:59, Vadim Sh. wrote:
> Call 'ensure_coverage_importable' again, to ensure it is importable now? With
a
> flag to disable auto build...

? It's called below... it will either pass (and coverage will be importable) or
it will sys.exit.

[Vadim Sh., 2014-04-04 19:47:07]

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
File scripts/slave/unittests/test_env.py (right):

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:54: p = subprocess.Popen([sys.executable,
'setup.py', 'bdist_egg'], cwd=cov_dir,
On 2014/04/04 19:34:35, iannucci wrote:
> On 2014/04/04 19:21:59, Vadim Sh. wrote:
> > It may be worth to add some instructions how to build it on windows. OR,
allow
> > system-wide coverage 3.7.1 to be used on windows and refer to
> >
>
https://pypi.python.org/packages/2.7/c/coverage/coverage-3.7.1.win32-py2.7.ex...
> > (or whatever, from PyPI: https://pypi.python.org/pypi/coverage/3.7.1)
> > 
> > That may be easier than requiring working MSVC environment.
> 
> Yeah I wasn't sure how to support windows, but if you look at the first import
> block at the top, if there is a correctly-configured system-wide python, it'll
> take that.
> 
> I'll add a message on windows which links to 'how to install python-coverage
on
> winders'
> 
Also, bdist_egg needs python-dev and setuptools, right? They are not installed
by default...

https://codereview.chromium.org/225633007/diff/20001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:63: sys.path.insert(0, cov_egg)
On 2014/04/04 19:34:35, iannucci wrote:
> On 2014/04/04 19:21:59, Vadim Sh. wrote:
> > Call 'ensure_coverage_importable' again, to ensure it is importable now?
With
> a
> > flag to disable auto build...
> 
> ? It's called below... it will either pass (and coverage will be importable)
or
> it will sys.exit.

I mean second time, after egg is build, to assert it is indeed used. Well, fine.

https://codereview.chromium.org/225633007/diff/40001/scripts/slave/unittests/...
File scripts/slave/unittests/test_env.py (right):

https://codereview.chromium.org/225633007/diff/40001/scripts/slave/unittests/...
scripts/slave/unittests/test_env.py:65: this executable from the internet:
:D

Maybe just link to PyPI page?

[Vadim Sh., 2014-04-04 19:56:29]

lgtm, assuming you add some helpful message about what to do if 'bdist_egg' step
doesn't work.

For example, on Windows it's "please download from blah-blah", on Linux "please
ensure python-dev and <all other system dependencies it needs> are installed",
etc. 

Failure here should not look like completely unexpected, imho.

[iannucci, 2014-04-04 20:19:53]

ok ptal

[iannucci, 2014-04-04 20:56:24]

Committed patchset #8 manually as r261861 (presubmit successful).