[Android] Add lint as a gyp action.

[Android] Add lint as a gyp action.

- Run lint on all java/apk targets using a dummy AndroidManifest.xml
- Add build/android/lint/suppress.py for suppressing generated
  errors
- Enable lint on FYI builders as a first step

BUG=None
R=cjhopman@chromium.org, newt@chromium.org
TBR=navabi@chromium.org

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

[frankf, 2013-11-26 01:01:39]

cjhopman -> Everything
yfriedman/newt -> FYI

Some points:

- This is a cleaner alternative to https://codereview.chromium.org/72273003/
- Total time added for the clobber build is about 1 minute locally, and 5
minutes on the bot.
- One downside is that you might need to run suppress.py multiple times
- Right now all warnings are treated as errors, we might need to lax this later

[cjhopman, 2013-11-26 23:44:15]

Do you have an idea of how long this takes for a single java library? Maybe how
long it takes for chrome_java?

https://codereview.chromium.org/86313004/diff/50001/build/android/lint/suppre...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/50001/build/android/lint/suppre...
build/android/lint/suppress.py:36: '  build process by running:\n'
Maybe mention that this string should be edited in this file rather than editing
the comment in the generated file.

Also add a mention that the file is (or may be) generated.

https://codereview.chromium.org/86313004/diff/50001/build/android/lint/suppre...
build/android/lint/suppress.py:41: 
I think these three functions would be simpler if you tracked individual
suppressions and global suppressions separately:

issues_dict = defaultdict(set)
disabled_issues = set()

where disabled_issues are those with severity==ignore.

An extra benefit is that it would then be natural to write all the global ones
at the top to make them more prominent (though I don't know if that's allowed or
would even be better than having them sorted).

https://codereview.chromium.org/86313004/diff/50001/build/android/lint/suppre...
build/android/lint/suppress.py:49: print 'Warning: %s is suppresed globally.' %
issue_id
s/suppresed/suppressed/

[frankf, 2013-11-27 03:28:44]

ptal

It takes 3s for chrome_java. I've left debug printing to see the timing on the
bots (Look for "Lint took"), this will be removed before committing.

https://codereview.chromium.org/86313004/diff/50001/build/android/lint/suppre...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/50001/build/android/lint/suppre...
build/android/lint/suppress.py:36: '  build process by running:\n'
On 2013/11/26 23:44:15, cjhopman wrote:
> Maybe mention that this string should be edited in this file rather than
editing
> the comment in the generated file.
> 
> Also add a mention that the file is (or may be) generated.

Done.

https://codereview.chromium.org/86313004/diff/50001/build/android/lint/suppre...
build/android/lint/suppress.py:41: 
Actually, I've changed this to use a namedtuple to store the severity in case we
want to differentiate between warning/error in the future (Currently we pass
-Werror). I've also simplified it a little.

On 2013/11/26 23:44:15, cjhopman wrote:
> I think these three functions would be simpler if you tracked individual
> suppressions and global suppressions separately:
> 
> issues_dict = defaultdict(set)
> disabled_issues = set()
> 
> where disabled_issues are those with severity==ignore.
> 
> An extra benefit is that it would then be natural to write all the global ones
> at the top to make them more prominent (though I don't know if that's allowed
or
> would even be better than having them sorted).

https://codereview.chromium.org/86313004/diff/50001/build/android/lint/suppre...
build/android/lint/suppress.py:49: print 'Warning: %s is suppresed globally.' %
issue_id
On 2013/11/26 23:44:15, cjhopman wrote:
> s/suppresed/suppressed/

Done.

[frankf, 2013-12-02 18:20:08]

ping

[newt (away), 2013-12-03 00:54:16]

Reviewed the python files (but not gyp)

Frank, can you explain the rationale for making this a build step instead of
separate script? I think I missed that discussion/explanation. The benefit I see
is that we know exactly what Java files need to be linted, instead of just
looking for "classes" files in the out dir. Just wondering.

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py
File build/android/gyp/lint.py (right):

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:66: sys.exit(rc)
I'd probably return "rc" instead of calling sys.exit() here. BTW, is lint's
return code non-zero whenever it detects a new lint error, or only when lint
fails to run (e.g. because of invalid command line arguments)?

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:75: parser.add_option('--result-path', help='Path to
xml file')
"Path to xml file"?

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:80: parser.add_option('--enable', action='store_true',
I would use gyp to conditionally enable or disable running lint, not an
"--enable" command line flag

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:85: assert (options.lint_path and options.config_path
and options.result_path and
you can use build_utils.CheckOptions() to make this a little more user friendly:

    build_utils.CheckOptions(options, parser, required=['lint_path', ...])

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:94: if options.stamp:
should the stamp only be touched if lint detects no new errors?

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:102: sys.exit(main(sys.argv))
main() doesn't return anything, so this line isn't useful. Perhaps main *should*
return a value?

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/Androi...
File build/android/lint/AndroidManifest.xml (right):

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/Androi...
build/android/lint/AndroidManifest.xml:1: <?xml version="1.0" encoding="utf-8"?>
Can you use build/android/AndroidManifest.xml instead of introducing a new one?

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
build/android/lint/suppress.py:100: for dirpath, _, files in
os.walk(constants.GetOutDirectory()):
Can we just pass in the path to lint_result.xml instead of walking the out
directory? This seems a bit too magical to me (i.e. difficult for developers to
understand what's happening without reading the source) and could lead to weird
results if there's a stale lint_result.xml in the out directory from another
Java target that failed to build a while back.

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
build/android/lint/suppress.py:109: parser = optparse.OptionParser()
duplicate line

[frankf, 2013-12-03 01:13:03]

The issue is stale class files for incremental builds. It's particularly a
concern on trybots, where someone else's interim patch could leave class files
affecting lint. I believe this is also an issue with findbugs. With current
approach, you can think of lint as an extension of javac.

I'll address your other comments in a future upload.

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
build/android/lint/suppress.py:100: for dirpath, _, files in
os.walk(constants.GetOutDirectory()):
Yes, there is a trade-off here. With your suggestion, the develop needs to look
up the exact path from the log. On the other hand, you could have stale
lint_result.xml as you suggest (but it's erring on the safe side). Another con
with current solution is figuring out the out directory path which is always
error prone. I'm open to this suggestion.

On 2013/12/03 00:54:16, newt wrote:
> Can we just pass in the path to lint_result.xml instead of walking the out
> directory? This seems a bit too magical to me (i.e. difficult for developers
to
> understand what's happening without reading the source) and could lead to
weird
> results if there's a stale lint_result.xml in the out directory from another
> Java target that failed to build a while back.

[newt (away), 2013-12-03 02:00:59]

Another issue to think about: how can developers see the full lint output? If I
understand correctly, they'll just see the summary or warning name (e.g. NewApi)
when they try to build, but they won't get the full prose explanation of what
that warning means. Without the full explanation, most of the warnings aren't
actionable. Android posts a list of explanations of lint checks
(http://tools.android.com/tips/lint-checks), but the list isn't currently
up-to-date: InlinedApi is missing, for example.

One solution to this is to change suppress.py to actually run lint instead of
passing in lint_result.xml. If we do this, then if lint.py fails, it should
print out the suppress.py command to run, with all the needed flags specifying
src dirs, class dirs, etc. Then developers can copy and paste the command. In
this case, it might make sense to unify lint.py and suppress.py into a single
script.

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
build/android/lint/suppress.py:100: for dirpath, _, files in
os.walk(constants.GetOutDirectory()):
On 2013/12/03 01:13:03, frankf wrote:
> Yes, there is a trade-off here. With your suggestion, the develop needs to
look
> up the exact path from the log. On the other hand, you could have stale
> lint_result.xml as you suggest (but it's erring on the safe side). Another con
> with current solution is figuring out the out directory path which is always
> error prone. I'm open to this suggestion.
> 
> On 2013/12/03 00:54:16, newt wrote:
> > Can we just pass in the path to lint_result.xml instead of walking the out
> > directory? This seems a bit too magical to me (i.e. difficult for developers
> to
> > understand what's happening without reading the source) and could lead to
> weird
> > results if there's a stale lint_result.xml in the out directory from another
> > Java target that failed to build a while back.
> 

I'd suggest that if lint.py fails, it print out the exact suppress.py command to
run. E.g.:

    > lint.py

    Lint failed with 7 warnings:
      Main.java NewApi: blah blah

    To suppress these warnings, run
      suppress.py --lint-result-path out/Debug/blah/blah/lint_result.xml

[frankf, 2013-12-03 02:07:52]

On 2013/12/03 02:00:59, newt wrote:
> Another issue to think about: how can developers see the full lint output? If
I
> understand correctly, they'll just see the summary or warning name (e.g.
NewApi)
> when they try to build, but they won't get the full prose explanation of what
> that warning means. Without the full explanation, most of the warnings aren't
> actionable. Android posts a list of explanations of lint checks
> (http://tools.android.com/tips/lint-checks), but the list isn't currently
> up-to-date: InlinedApi is missing, for example.
> 
> One solution to this is to change suppress.py to actually run lint instead of
> passing in lint_result.xml. If we do this, then if lint.py fails, it should
> print out the suppress.py command to run, with all the needed flags specifying
> src dirs, class dirs, etc. Then developers can copy and paste the command. In
> this case, it might make sense to unify lint.py and suppress.py into a single
> script.
> 
>
https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
> File build/android/lint/suppress.py (right):
> 
>
https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
> build/android/lint/suppress.py:100: for dirpath, _, files in
> os.walk(constants.GetOutDirectory()):
> On 2013/12/03 01:13:03, frankf wrote:
> > Yes, there is a trade-off here. With your suggestion, the develop needs to
> look
> > up the exact path from the log. On the other hand, you could have stale
> > lint_result.xml as you suggest (but it's erring on the safe side). Another
con
> > with current solution is figuring out the out directory path which is always
> > error prone. I'm open to this suggestion.
> > 
> > On 2013/12/03 00:54:16, newt wrote:
> > > Can we just pass in the path to lint_result.xml instead of walking the out
> > > directory? This seems a bit too magical to me (i.e. difficult for
developers
> > to
> > > understand what's happening without reading the source) and could lead to
> > weird
> > > results if there's a stale lint_result.xml in the out directory from
another
> > > Java target that failed to build a while back.
> > 
> 
> I'd suggest that if lint.py fails, it print out the exact suppress.py command
to
> run. E.g.:
> 
>     > lint.py
> 
>     Lint failed with 7 warnings:
>       Main.java NewApi: blah blah
> 
>     To suppress these warnings, run
>       suppress.py --lint-result-path out/Debug/blah/blah/lint_result.xml

The explanation is part of lint_result.xml. Example:

https://x20web.corp.google.com/~frankf/lint_result.xml

We can output this but it's pretty long. We can also point
the developer to read the xml file. E.g.


  Lint failed with 7 warnings:
      Main.java NewApi: blah blah
    
    For full explanation refer to out/Debug/blah/blah/lint_result.xml

    To suppress these warnings, run
      suppress.py --lint-result-path out/Debug/blah/blah/lint_result.xml

[frankf, 2013-12-04 23:29:30]

ptal.

Patch 4 includes these additional changes:

- Add 'never_lint' gyp variable to exclude downstream targets. We'll use the
existing dowstream script to run lint which doesn't use gyp
- Allow PRODUCT_DIR token in suppression file instead of hardcoding the path
- Cleans up output when there are lint issues

In patch 5, I removed some suppression so reviewers can see an example error
message.

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py
File build/android/gyp/lint.py (right):

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:66: sys.exit(rc)
On 2013/12/03 00:54:16, newt wrote:
> I'd probably return "rc" instead of calling sys.exit() here. BTW, is lint's
> return code non-zero whenever it detects a new lint error, or only when lint
> fails to run (e.g. because of invalid command line arguments)?

Done. You only return rc of zero if there are no lint issues (or lint is
disabled) and no exceptions are raised.

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:75: parser.add_option('--result-path', help='Path to
xml file')
On 2013/12/03 00:54:16, newt wrote:
> "Path to xml file"?

Rephrased

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:80: parser.add_option('--enable', action='store_true',
On 2013/12/03 00:54:16, newt wrote:
> I would use gyp to conditionally enable or disable running lint, not an
> "--enable" command line flag

We want to trivially touch the stamp if lint is not enabled. We could do this
directly in gyp, but this is a pattern used elsewhere in build/android/gyp as
well. I guess the motivation is that it keeps logic in gyp simpler.

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:85: assert (options.lint_path and options.config_path
and options.result_path and
Done. Although having required be an optional parameter kind of beats the
purpose.

On 2013/12/03 00:54:16, newt wrote:
> you can use build_utils.CheckOptions() to make this a little more user
friendly:
> 
>     build_utils.CheckOptions(options, parser, required=['lint_path', ...])

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:94: if options.stamp:
On 2013/12/03 00:54:16, newt wrote:
> should the stamp only be touched if lint detects no new errors?

Nope, previously we exited in line 66, with new patch we explicitly check the
rc.

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/Androi...
File build/android/lint/AndroidManifest.xml (right):

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/Androi...
build/android/lint/AndroidManifest.xml:1: <?xml version="1.0" encoding="utf-8"?>
On 2013/12/03 00:54:16, newt wrote:
> Can you use build/android/AndroidManifest.xml instead of introducing a new
one?

Done, assuming that adding uses-sdk there is inconsequential per the comment
there.

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/90001/build/android/lint/suppre...
build/android/lint/suppress.py:109: parser = optparse.OptionParser()
On 2013/12/03 00:54:16, newt wrote:
> duplicate line

Done.

[newt (away), 2013-12-05 23:39:33]

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py
File build/android/gyp/lint.py (right):

https://codereview.chromium.org/86313004/diff/90001/build/android/gyp/lint.py...
build/android/gyp/lint.py:85: assert (options.lint_path and options.config_path
and options.result_path and
On 2013/12/04 23:29:31, frankf wrote:
> Done. Although having required be an optional parameter kind of beats the
> purpose.

The irony ;)

> 
> On 2013/12/03 00:54:16, newt wrote:
> > you can use build_utils.CheckOptions() to make this a little more user
> friendly:
> > 
> >     build_utils.CheckOptions(options, parser, required=['lint_path', ...])
>

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.py
File build/android/gyp/lint.py (right):

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.p...
build/android/gyp/lint.py:53: num_issues = 0
just an idea: instead of counting num_issues manually:

    issues = dom.getElementsByTagName('issue')
    for issue in issues:
      ...
    return len(issues)

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.p...
build/android/gyp/lint.py:87: child = subprocess.Popen(cmd,
stdout=subprocess.PIPE, stderr=subprocess.PIPE,
I have a CL out for review to change build_utils.CheckCallDie() to throw an
exception instead of calling sys.exit(). If that lands, you can catch the
exception (build_utils.CalledProcessError) and re-raise it or do other things as
needed:

    try:
      # Use the new "CheckCallDie" aka "CheckOutput"
      build_utils.CheckOutput(cmd, cwd=cwd)
    except build_utils.CalledProcessError as e:
      if not os.path.exists(result_path):
        raise
      else:
        # Print out list of lint errors, then
        return 1

    return 0

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.p...
build/android/gyp/lint.py:141: parser.add_option('--enable',
action='store_true',
Huh, where else is this used?

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.p...
build/android/gyp/lint.py:166: # TODO(frankf): Remove this before committing.
don't forget...

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
build/android/lint/suppress.py:40: '  Note: PRODUCT_DIR will be substituted at
run-time.\n'
substituted with what? I'd add an example here, e.g. "out/Debug"

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
build/android/lint/suppress.py:109: parser.print_usage()
parser.error('Must provide RESULT-FILE') will show the usage, the error, and
exit for you.

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
File build/android/lint/suppressions.xml (right):

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
build/android/lint/suppressions.xml:31: <ignore
path="PRODUCT_DIR/gen/guava_javalib/classes/com/google/common/util/concurrent/RateLimiter.class"/>
why are we running lint on guava and protobuf files?

[cjhopman, 2013-12-06 02:50:58]

https://codereview.chromium.org/86313004/diff/130001/build/android/gyp/lint.py
File build/android/gyp/lint.py (right):

https://codereview.chromium.org/86313004/diff/130001/build/android/gyp/lint.p...
build/android/gyp/lint.py:137: parser.add_option('--src-root', help='Path to
top-level src dir.')
I think we usually just get this from __file__

https://codereview.chromium.org/86313004/diff/130001/build/android/lint/suppr...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/130001/build/android/lint/suppr...
build/android/lint/suppress.py:24: _RESULT_FILE_NAME = 'lint_result.xml'
Isn't used. Also, the path is available as an argument.

https://codereview.chromium.org/86313004/diff/130001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/86313004/diff/130001/build/java_apk.gypi#newc...
build/java_apk.gypi:587: '<(lint_stamp)',
Why does this depend on the lint step? Can't we just keep it depending on the
compile step?

[frankf, 2013-12-06 22:17:35]

Thanks cjhopman/newt. All comments are addressed. Patch 6 includes newt's
refactoring, I'll rebase once that lands. ptal.

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.py
File build/android/gyp/lint.py (right):

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.p...
build/android/gyp/lint.py:53: num_issues = 0
On 2013/12/05 23:39:33, newt wrote:
> just an idea: instead of counting num_issues manually:
> 
>     issues = dom.getElementsByTagName('issue')
>     for issue in issues:
>       ...
>     return len(issues)

Done.

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.p...
build/android/gyp/lint.py:87: child = subprocess.Popen(cmd,
stdout=subprocess.PIPE, stderr=subprocess.PIPE,
Nice refactoring. Done. I'll rebase after your CL lands.

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.p...
build/android/gyp/lint.py:141: parser.add_option('--enable',
action='store_true',
See lint_action.gypi

On 2013/12/05 23:39:33, newt wrote:
> Huh, where else is this used?

https://codereview.chromium.org/86313004/diff/110001/build/android/gyp/lint.p...
build/android/gyp/lint.py:166: # TODO(frankf): Remove this before committing.
On 2013/12/05 23:39:33, newt wrote:
> don't forget...
Removed debugging logs.

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
build/android/lint/suppress.py:40: '  Note: PRODUCT_DIR will be substituted at
run-time.\n'
On 2013/12/05 23:39:33, newt wrote:
> substituted with what? I'd add an example here, e.g. "out/Debug"

Done.

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
build/android/lint/suppress.py:109: parser.print_usage()
On 2013/12/05 23:39:33, newt wrote:
> parser.error('Must provide RESULT-FILE') will show the usage, the error, and
> exit for you.

Done.

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
File build/android/lint/suppressions.xml (right):

https://codereview.chromium.org/86313004/diff/110001/build/android/lint/suppr...
build/android/lint/suppressions.xml:31: <ignore
path="PRODUCT_DIR/gen/guava_javalib/classes/com/google/common/util/concurrent/RateLimiter.class"/>
Cruft from before. Removed.

On 2013/12/05 23:39:33, newt wrote:
> why are we running lint on guava and protobuf files?

https://codereview.chromium.org/86313004/diff/130001/build/android/gyp/lint.py
File build/android/gyp/lint.py (right):

https://codereview.chromium.org/86313004/diff/130001/build/android/gyp/lint.p...
build/android/gyp/lint.py:137: parser.add_option('--src-root', help='Path to
top-level src dir.')
On 2013/12/06 02:50:59, cjhopman wrote:
> I think we usually just get this from __file__

Done.

https://codereview.chromium.org/86313004/diff/130001/build/android/lint/suppr...
File build/android/lint/suppress.py (right):

https://codereview.chromium.org/86313004/diff/130001/build/android/lint/suppr...
build/android/lint/suppress.py:24: _RESULT_FILE_NAME = 'lint_result.xml'
On 2013/12/06 02:50:59, cjhopman wrote:
> Isn't used. Also, the path is available as an argument.

Done.

https://codereview.chromium.org/86313004/diff/130001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/86313004/diff/130001/build/java_apk.gypi#newc...
build/java_apk.gypi:587: '<(lint_stamp)',
On 2013/12/06 02:50:59, cjhopman wrote:
> Why does this depend on the lint step? Can't we just keep it depending on the
> compile step?

Ah, good catch.

[newt (away), 2013-12-06 23:59:24]

All the python stuff LGTM. I did not look at gyp or buildbot code.

Note: I just landed my CL to change CheckCallDie -> CheckOutput.

https://codereview.chromium.org/86313004/diff/150001/build/android/gyp/lint.py
File build/android/gyp/lint.py (right):

https://codereview.chromium.org/86313004/diff/150001/build/android/gyp/lint.p...
build/android/gyp/lint.py:89: print_stdout=False, print_stderr=False)
don't need "print_stdout=False" since that's the default. Does lint regularly
print stuff to stderr that we want to suppress? If not, I'd remove
"print_stderr=False" too.

[frankf, 2013-12-07 01:51:00]

Just waiting for Chris.

https://codereview.chromium.org/86313004/diff/150001/build/android/gyp/lint.py
File build/android/gyp/lint.py (right):

https://codereview.chromium.org/86313004/diff/150001/build/android/gyp/lint.p...
build/android/gyp/lint.py:89: print_stdout=False, print_stderr=False)
On 2013/12/06 23:59:24, newt wrote:
> don't need "print_stdout=False" since that's the default. Does lint regularly
> print stuff to stderr that we want to suppress? If not, I'd remove
> "print_stderr=False" too.

Done.

[frankf, 2013-12-10 19:29:59]

On 2013/12/07 01:51:00, frankf wrote:
> Just waiting for Chris.
> 
> https://codereview.chromium.org/86313004/diff/150001/build/android/gyp/lint.py
> File build/android/gyp/lint.py (right):
> 
>
https://codereview.chromium.org/86313004/diff/150001/build/android/gyp/lint.p...
> build/android/gyp/lint.py:89: print_stdout=False, print_stderr=False)
> On 2013/12/06 23:59:24, newt wrote:
> > don't need "print_stdout=False" since that's the default. Does lint
regularly
> > print stuff to stderr that we want to suppress? If not, I'd remove
> > "print_stderr=False" too.
> 
> Done.

Friendly ping

[cjhopman, 2013-12-10 20:49:29]

lgtm

[frankf, 2013-12-11 03:02:19]

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