🌈 Android: Run lint using a cache in the output directory (fix-up)

Android: Run lint using a cache in the output directory (fix-up)

Previous commit forgot to set --cache-dir for GN.

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

[agrieve, 2016-03-22 18:53:47]

Description was changed from

==========
Android: Run lint using a cache in the output directory (fix-up)

Previous commit forgot to set --cache-dir for GN.

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

to

==========
Android: Run lint using a cache in the output directory (fix-up)

Previous commit forgot to set --cache-dir for GN.

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

[agrieve, 2016-03-22 18:53:47]

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

[agrieve, 2016-03-22 18:54:24]

The CQ bit was checked by agrieve@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-22 18:54:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1824093002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1824093002/1

[agrieve, 2016-03-22 19:15:44]

On 2016/03/22 18:54:48, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1824093002/1
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1824093002/1

[jbudorick, 2016-03-22 19:27:22]

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

https://codereview.chromium.org/1824093002/diff/1/build/android/gyp/lint.py#n...
build/android/gyp/lint.py:138: stderr_filter = lambda l: '' if '_JAVA_OPTIONS'
in l else l
Thanks. I wound up seeing this locally after landing.

https://codereview.chromium.org/1824093002/diff/1/build/config/android/intern...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1824093002/diff/1/build/config/android/intern...
build/config/android/internal_rules.gni:73: "--depfile",
I had issues with the depfile here previously causing lint to be run even if no
inputs changed. Does that happen with this addition?

https://codereview.chromium.org/1824093002/diff/1/build/config/android/intern...
build/config/android/internal_rules.gni:86: if (_create_cache) {
nit: What about merging the two if (_create_cache) blocks?

[agrieve, 2016-03-22 19:57:18]

https://codereview.chromium.org/1824093002/diff/1/build/config/android/intern...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1824093002/diff/1/build/config/android/intern...
build/config/android/internal_rules.gni:73: "--depfile",
On 2016/03/22 19:27:22, jbudorick wrote:
> I had issues with the depfile here previously causing lint to be run even if
no
> inputs changed. Does that happen with this addition?

I haven't noticed that it does. I verified the depfile has only .py files in it,
so it shouldn't.

lint.py also uses md5check, so it will do it's own double-checking that it needs
to be run.

https://codereview.chromium.org/1824093002/diff/1/build/config/android/intern...
build/config/android/internal_rules.gni:86: if (_create_cache) {
On 2016/03/22 19:27:22, jbudorick wrote:
> nit: What about merging the two if (_create_cache) blocks?

Done.

[jbudorick, 2016-03-22 20:36:26]

lgtm

https://codereview.chromium.org/1824093002/diff/1/build/config/android/intern...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1824093002/diff/1/build/config/android/intern...
build/config/android/internal_rules.gni:73: "--depfile",
On 2016/03/22 19:57:18, agrieve wrote:
> On 2016/03/22 19:27:22, jbudorick wrote:
> > I had issues with the depfile here previously causing lint to be run even if
> no
> > inputs changed. Does that happen with this addition?
> 
> I haven't noticed that it does. I verified the depfile has only .py files in
it,
> so it shouldn't.
> 
> lint.py also uses md5check, so it will do it's own double-checking that it
needs
> to be run.

Hm, maybe I had it misconfigured, then.

[agrieve, 2016-03-22 20:39:42]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-03-22 20:40:02]

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

[commit-bot: I haz the power, 2016-03-22 22:03:51]

Description was changed from

==========
Android: Run lint using a cache in the output directory (fix-up)

Previous commit forgot to set --cache-dir for GN.

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

to

==========
Android: Run lint using a cache in the output directory (fix-up)

Previous commit forgot to set --cache-dir for GN.

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

[commit-bot: I haz the power, 2016-03-22 22:03:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-22 22:05:26]

Description was changed from

==========
Android: Run lint using a cache in the output directory (fix-up)

Previous commit forgot to set --cache-dir for GN.

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

to

==========
Android: Run lint using a cache in the output directory (fix-up)

Previous commit forgot to set --cache-dir for GN.

BUG=583661

Committed: https://crrev.com/99748af41ad073389f3f38274a024a830585609c
Cr-Commit-Position: refs/heads/master@{#382699}
==========

[commit-bot: I haz the power, 2016-03-22 22:05:27]

Patchset 2 (id:??) landed as
https://crrev.com/99748af41ad073389f3f38274a024a830585609c
Cr-Commit-Position: refs/heads/master@{#382699}

[aberent, 2016-03-23 12:55:24]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1829683002/ by aberent@chromium.org.

The reason for reverting is: Breaks on Android 64bit unpublished builders

BUG=597251.

[jbudorick, 2016-03-23 13:03:15]

On 2016/03/23 12:55:24, aberent wrote:
> A revert of this CL (patchset #2 id:20001) has been created in
> https://codereview.chromium.org/1829683002/ by mailto:aberent@chromium.org.
> 
> The reason for reverting is: Breaks on Android 64bit unpublished builders

:(

Not sure I agree with this one.

> 
> BUG=597251.

[jbudorick, 2016-03-23 13:13:13]

On 2016/03/23 13:03:15, jbudorick wrote:
> On 2016/03/23 12:55:24, aberent wrote:
> > A revert of this CL (patchset #2 id:20001) has been created in
> > https://codereview.chromium.org/1829683002/ by mailto:aberent@chromium.org.
> > 
> > The reason for reverting is: Breaks on Android 64bit unpublished builders
> 
> :(
> 
> Not sure I agree with this one.

hm, actually, after looking the failure, I take this back. Reverting/relanding
was probably the best choice.

> 
> > 
> > BUG=597251.