Add a script that runs cov-build, cov-analyze, and...

Add a script that runs cov-build, cov-analyze, and
cov-commit-defects on a Chromium tree.

R=patrick
BUG=N/A
TEST=N/A

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=15831

[wtc, 2009-05-01 00:14:35]

This is my first Python script.

[Patrick Johnson, 2009-05-04 16:41:21]

Very nice for a first Python script!  I mostly just have style nits.

http://codereview.chromium.org/99264/diff/4/1001
File tools/coverity/coverity.py (right):

http://codereview.chromium.org/99264/diff/4/1001#newcode13
Line 13: %comspec% /c ""C:\Program Files\Microsoft Visual Studio
8\VC\vcvarsall.bat" x86 && python C:\coverity.py"
Make sure comments also don't exceed 80 characters.

http://codereview.chromium.org/99264/diff/4/1001#newcode25
Line 25: CHROMIUM_SOURCE_DIR = 'C:\\chromium.latest'
I think some of these constants be command-line flags instead, particularly the
ones that are paths.  You can set default values for the flags.

http://codereview.chromium.org/99264/diff/4/1001#newcode26
Line 26: CHROMIUM_SOLUTION_FILE = 'src\\chrome\\chrome.sln'  # Relative to
CHROMIUM_SOURCE_DIR.
This line exceeds 80 characters with the comment, as well as some lines below.

http://codereview.chromium.org/99264/diff/4/1001#newcode29
Line 29: COVERITY_INTERMEDIATE_DIR = 'C:\\coverity\\cvbuild\\cr_int.3'
Maybe we should just call it cr_int instead of cr_int.3.

http://codereview.chromium.org/99264/diff/4/1001#newcode31
Line 31: COVERITY_ANALYZE_OPTIONS = '--all --enable-single-virtual
--enable-constraint-fpp --enable-callgraph-metrics --checker-option
PASS_BY_VALUE:size_threshold:16'
Here's the way to wrap a long string like this:

my_variable = ('foo bar '
               'baz')

(The parentheses are important)

http://codereview.chromium.org/99264/diff/4/1001#newcode34
Line 34: LOCK_FILE = 'coverity.lock'  # Relative to CHROMIUM_SOURCE_DIR. 
Contains the pid of this script.
Nit: more than 80 characters with the comment.

http://codereview.chromium.org/99264/diff/4/1001#newcode40
Line 40: subprocess.call(cmd, shell=shell)
We may want to consider checking the return value of subprocess.call (this will
be the return code of the command).

http://codereview.chromium.org/99264/diff/4/1001#newcode47
Line 47: lock_file = os.open(lock_filename,
os.O_CREAT|os.O_EXCL|os.O_TRUNC|os.O_RDWR)
Nit: more than 80 characters here.  Wrap the same way as for C++ code.

http://codereview.chromium.org/99264/diff/4/1001#newcode67
Line 67: cmd = 'rmdir /s /q %s\\%s\\%s' % (CHROMIUM_SOURCE_DIR,
CHROMIUM_SOLUTION_DIR, options.target)
To wrap this line correctly, use this style:

my_variable = '%s%s' % (foo,
                        bar)

http://codereview.chromium.org/99264/diff/4/1001#newcode67
Line 67: cmd = 'rmdir /s /q %s\\%s\\%s' % (CHROMIUM_SOURCE_DIR,
CHROMIUM_SOLUTION_DIR, options.target)
As a note, Python's shutil module has an rmtree function.  But this has
limitations, such as not working for read-only files on Windows.  So maybe it's
better to stick with this method.

Also, if you're interested, see
trunk/tools/buildbot/scripts/common/chromium_utils.py RemoveDirectory for a
fairly elaborate implementation of this function in Python.

[wtc, 2009-05-07 22:28:08]

Patrick, please look at the latest patch set.

I added TODO comments for the issues that I didn't address.

I'd like to check in a version that meets the minimum
requirement, and then improve it.  Thanks.

http://codereview.chromium.org/99264/diff/3004/4002
File tools/coverity/coverity.py (right):

http://codereview.chromium.org/99264/diff/3004/4002#newcode13
Line 13: %comspec% /c ""C:\Program Files\Microsoft Visual Studio
8\VC\vcvarsall.bat" x86 && python C:\coverity.py"
I don't know how to wrap this line in a comment.  Should I use
a backslash at the end of the line?

http://codereview.chromium.org/99264/diff/3004/4002#newcode109
Line 109: cmd = ('%s\\cov-commit-defects.exe --dir %s --datadir %s --product %s
'
Did I wrap this one correctly?  I had to add () around the
format string as well.

[Patrick Johnson, 2009-05-08 04:47:14]

LGTM

http://codereview.chromium.org/99264/diff/3004/4002
File tools/coverity/coverity.py (right):

http://codereview.chromium.org/99264/diff/3004/4002#newcode13
Line 13: %comspec% /c ""C:\Program Files\Microsoft Visual Studio
8\VC\vcvarsall.bat" x86 && python C:\coverity.py"
On 2009/05/07 22:28:08, wtc wrote:
> I don't know how to wrap this line in a comment.  Should I use
> a backslash at the end of the line?

Yes, that's fine.  It seems fine to simply wrap and indent the line without the
backslash too.  It's not too terribly important since it's a comment.  Also,
feel free do it however you would in C++ code.

http://codereview.chromium.org/99264/diff/3004/4002#newcode42
Line 42: COVERITY_ANALYZE_OPTIONS = ('--all --enable-single-virtual '
How about an option to increase the stack usage size threshold?

http://codereview.chromium.org/99264/diff/3004/4002#newcode68
Line 68: lock_file = os.open(lock_filename,
This looks fine to me, but have you verified it works properly?  Just making
sure before we get any unexpected surprises.

http://codereview.chromium.org/99264/diff/3004/4002#newcode97
Line 97: cmd = '%s\\cov-build.exe --dir %s devenv.com %s\\%s /build %s' % (
Maybe make a note that devenv.com is expected to be on the PATH.

http://codereview.chromium.org/99264/diff/3004/4002#newcode109
Line 109: cmd = ('%s\\cov-commit-defects.exe --dir %s --datadir %s --product %s
'
On 2009/05/07 22:28:08, wtc wrote:
> Did I wrap this one correctly?  I had to add () around the
> format string as well.

Looks right to me.

[wtc, 2009-05-12 18:27:26]

I have checked in this script.  Thanks for the review.

http://codereview.chromium.org/99264/diff/3004/4002
File tools/coverity/coverity.py (right):

http://codereview.chromium.org/99264/diff/3004/4002#newcode68
Line 68: lock_file = os.open(lock_filename,
On 2009/05/08 04:47:14, Patrick Johnson wrote:
> This looks fine to me, but have you verified it works properly?

Yes, I have.

http://codereview.chromium.org/99264/diff/3004/4002#newcode97
Line 97: cmd = '%s\\cov-build.exe --dir %s devenv.com %s\\%s /build %s' % (
On 2009/05/08 04:47:14, Patrick Johnson wrote:
> Maybe make a note that devenv.com is expected to be on the PATH.

I added a comment to note that this script should be run
in a Visual Studio Command Prompt.