[Android] Run lint using a cache in the output directory.

[Android] Run lint using a cache in the output directory.

BUG=583661
Committed: https://crrev.com/c2b563ca5489aaf7579a4b9f389533b7af336187
Cr-Commit-Position: refs/heads/master@{#382306}

[jbudorick, 2016-03-18 21:33:39]

jbudorick@chromium.org changed reviewers:
+ agrieve@chromium.org

[jbudorick, 2016-03-18 21:33:40]

ptal

[agrieve, 2016-03-19 00:12:54]

looks great! just nits. Fingers crossed this makes it less breaky! lgtm

https://codereview.chromium.org/1815563005/diff/40001/build/android/BUILD.gn
File build/android/BUILD.gn (right):

https://codereview.chromium.org/1815563005/diff/40001/build/android/BUILD.gn#...
build/android/BUILD.gn:14: cache_dir = "${root_out_dir}/lint_cache"
nit: can we call it "android_lint_cache"?

https://codereview.chromium.org/1815563005/diff/40001/build/android/BUILD.gn#...
build/android/BUILD.gn:16: manifest_file = "//build/android/AndroidManifest.xml"
nit: add _ prefix to these two and cache_dir so they don't look like they belong
to action()

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

https://codereview.chromium.org/1815563005/diff/40001/build/android/gyp/lint....
build/android/gyp/lint.py:137: build_utils.CheckOutput(cmd, cwd=_SRC_ROOT,
env=env or None)
I'm seeing a message "Picked up _JAVA_OPTIONS: ..." when I try this locally.

Might need to add:
stderr_filter=lambda l: '' if '_JAVA_OPTIONS' in l else l

https://codereview.chromium.org/1815563005/diff/40001/build/config/android/co...
File build/config/android/config.gni (right):

https://codereview.chromium.org/1815563005/diff/40001/build/config/android/co...
build/config/android/config.gni:153: android_sdk_build_tools_version =
android_sdk_build_tools_version
no-op?

https://codereview.chromium.org/1815563005/diff/40001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1815563005/diff/40001/build/config/android/in...
build/config/android/internal_rules.gni:57: if (!defined(deps)) {
nit: you can save a couple lines if you put "deps = []" before
foward_variables_from.

[jbudorick, 2016-03-19 00:57:43]

https://codereview.chromium.org/1815563005/diff/40001/build/android/BUILD.gn
File build/android/BUILD.gn (right):

https://codereview.chromium.org/1815563005/diff/40001/build/android/BUILD.gn#...
build/android/BUILD.gn:14: cache_dir = "${root_out_dir}/lint_cache"
On 2016/03/19 00:12:53, agrieve wrote:
> nit: can we call it "android_lint_cache"?

Done.

https://codereview.chromium.org/1815563005/diff/40001/build/android/BUILD.gn#...
build/android/BUILD.gn:16: manifest_file = "//build/android/AndroidManifest.xml"
On 2016/03/19 00:12:53, agrieve wrote:
> nit: add _ prefix to these two and cache_dir so they don't look like they
belong
> to action()

Done.

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

https://codereview.chromium.org/1815563005/diff/40001/build/android/gyp/lint....
build/android/gyp/lint.py:137: build_utils.CheckOutput(cmd, cwd=_SRC_ROOT,
env=env or None)
On 2016/03/19 00:12:53, agrieve wrote:
> I'm seeing a message "Picked up _JAVA_OPTIONS: ..." when I try this locally.
> 
> Might need to add:
> stderr_filter=lambda l: '' if '_JAVA_OPTIONS' in l else l

Interesting, I don't see that in either gyp or gn. How is your build configured?

https://codereview.chromium.org/1815563005/diff/40001/build/config/android/co...
File build/config/android/config.gni (right):

https://codereview.chromium.org/1815563005/diff/40001/build/config/android/co...
build/config/android/config.gni:153: android_sdk_build_tools_version =
android_sdk_build_tools_version
On 2016/03/19 00:12:54, agrieve wrote:
> no-op?

hm, yeah. Probably a relic from an wip version?

https://codereview.chromium.org/1815563005/diff/40001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1815563005/diff/40001/build/config/android/in...
build/config/android/internal_rules.gni:57: if (!defined(deps)) {
On 2016/03/19 00:12:54, agrieve wrote:
> nit: you can save a couple lines if you put "deps = []" before
> foward_variables_from.

Done.

[agrieve, 2016-03-19 01:02:44]

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

https://codereview.chromium.org/1815563005/diff/40001/build/android/gyp/lint....
build/android/gyp/lint.py:137: build_utils.CheckOutput(cmd, cwd=_SRC_ROOT,
env=env or None)
On 2016/03/19 00:57:42, jbudorick wrote:
> On 2016/03/19 00:12:53, agrieve wrote:
> > I'm seeing a message "Picked up _JAVA_OPTIONS: ..." when I try this locally.
> > 
> > Might need to add:
> > stderr_filter=lambda l: '' if '_JAVA_OPTIONS' in l else l
> 
> Interesting, I don't see that in either gyp or gn. How is your build
configured?

Weird - seems to be a common issue:
http://stackoverflow.com/questions/11683715/suppressing-the-picked-up-java-op...

I'm just doing:
rm -r ./gen/base/base_java__lint
_JAVA_OPTIONS=-Duser.home=$PWD/lint_cache ninja base_java

[agrieve, 2016-03-19 01:03:14]

On 2016/03/19 01:02:44, agrieve wrote:
>
https://codereview.chromium.org/1815563005/diff/40001/build/android/gyp/lint.py
> File build/android/gyp/lint.py (right):
> 
>
https://codereview.chromium.org/1815563005/diff/40001/build/android/gyp/lint....
> build/android/gyp/lint.py:137: build_utils.CheckOutput(cmd, cwd=_SRC_ROOT,
> env=env or None)
> On 2016/03/19 00:57:42, jbudorick wrote:
> > On 2016/03/19 00:12:53, agrieve wrote:
> > > I'm seeing a message "Picked up _JAVA_OPTIONS: ..." when I try this
locally.
> > > 
> > > Might need to add:
> > > stderr_filter=lambda l: '' if '_JAVA_OPTIONS' in l else l
> > 
> > Interesting, I don't see that in either gyp or gn. How is your build
> configured?
> 
> Weird - seems to be a common issue:
>
http://stackoverflow.com/questions/11683715/suppressing-the-picked-up-java-op...
> 
> I'm just doing:
> rm -r ./gen/base/base_java__lint
> _JAVA_OPTIONS=-Duser.home=$PWD/lint_cache ninja base_java

Although note that this is super minor and can be fixed up later if it's not
showing for you but shows for others.

[jbudorick, 2016-03-19 01:04:19]

On 2016/03/19 01:03:14, agrieve wrote:
> On 2016/03/19 01:02:44, agrieve wrote:
> >
>
https://codereview.chromium.org/1815563005/diff/40001/build/android/gyp/lint.py
> > File build/android/gyp/lint.py (right):
> > 
> >
>
https://codereview.chromium.org/1815563005/diff/40001/build/android/gyp/lint....
> > build/android/gyp/lint.py:137: build_utils.CheckOutput(cmd, cwd=_SRC_ROOT,
> > env=env or None)
> > On 2016/03/19 00:57:42, jbudorick wrote:
> > > On 2016/03/19 00:12:53, agrieve wrote:
> > > > I'm seeing a message "Picked up _JAVA_OPTIONS: ..." when I try this
> locally.
> > > > 
> > > > Might need to add:
> > > > stderr_filter=lambda l: '' if '_JAVA_OPTIONS' in l else l
> > > 
> > > Interesting, I don't see that in either gyp or gn. How is your build
> > configured?
> > 
> > Weird - seems to be a common issue:
> >
>
http://stackoverflow.com/questions/11683715/suppressing-the-picked-up-java-op...
> > 
> > I'm just doing:
> > rm -r ./gen/base/base_java__lint
> > _JAVA_OPTIONS=-Duser.home=$PWD/lint_cache ninja base_java
> 
> Although note that this is super minor and can be fixed up later if it's not
> showing for you but shows for others.

?

The patch handles setting the _JAVA_OPTIONS in its call to lint...

[jbudorick, 2016-03-19 01:04:51]

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

https://codereview.chromium.org/1815563005/diff/60001/build/android/gyp/lint....
build/android/gyp/lint.py:134: env['_JAVA_OPTIONS'] = '-Duser.home=%s' %
_RelativizePath(cache_dir)
  (i.e. here)

so you shouldn't have to do so manually.

[jbudorick, 2016-03-21 14:21:35]

The CQ bit was checked by jbudorick@chromium.org

[jbudorick, 2016-03-21 14:21:36]

The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/1815563005/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-03-21 14:21:56]

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

[commit-bot: I haz the power, 2016-03-21 15:45:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-21 15:45:59]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[jbudorick, 2016-03-21 15:46:26]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2016-03-21 15:46:38]

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

[commit-bot: I haz the power, 2016-03-21 16:43:49]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-21 16:44:43]

Description was changed from

==========
[Android] Run lint using a cache in the output directory.

BUG=583661
==========

to

==========
[Android] Run lint using a cache in the output directory.

BUG=583661

Committed: https://crrev.com/c2b563ca5489aaf7579a4b9f389533b7af336187
Cr-Commit-Position: refs/heads/master@{#382306}
==========

[commit-bot: I haz the power, 2016-03-21 16:44:44]

Patchset 5 (id:??) landed as
https://crrev.com/c2b563ca5489aaf7579a4b9f389533b7af336187
Cr-Commit-Position: refs/heads/master@{#382306}