GN: Don't compile battor_agent, imagediff, bitmaptools on Android

GN: Don't compile battor_agent, imagediff, bitmaptools on Android

Came across this because battor_agent doesn't link without
--gc-sections.

I don't think there's a use for running these tools on-device.

BUG=159847
Committed: https://crrev.com/97dd57df5affed43a82441b987c4504b72ac64c5
Cr-Commit-Position: refs/heads/master@{#367609}

[agrieve, 2015-12-23 19:53:57]

agrieve@chromium.org changed reviewers:
+ charliea@chromium.org, dpranke@chromium.org, reillyg@chromium.org

[agrieve, 2015-12-23 19:53:57]

reillyg@chromium.org: Please review changes in //device

dpranke@chromium.org: Please review changes in //BUILD.gn

charliea@chromium.org: Please review changes in //tools

[Dirk Pranke, 2015-12-23 21:19:33]

https://codereview.chromium.org/1545963003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1545963003/diff/1/BUILD.gn#newcode229
BUILD.gn:229: "//tools/telemetry:bitmaptools($host_toolchain)",
I don't think this change is right. 

You definitely still need to build imagediff and bitmaptools when
target_os==android, you just need to build the host versions (as it says).

I suspect that's true for battor_agent as well, so may the right thing to do is
to change that to have ($host_toolchain)?

[agrieve, 2015-12-28 19:46:07]

https://codereview.chromium.org/1545963003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1545963003/diff/1/BUILD.gn#newcode229
BUILD.gn:229: "//tools/telemetry:bitmaptools($host_toolchain)",
On 2015/12/23 21:19:32, Dirk Pranke wrote:
> I don't think this change is right. 
> 
> You definitely still need to build imagediff and bitmaptools when
> target_os==android, you just need to build the host versions (as it says).
> 
> I suspect that's true for battor_agent as well, so may the right thing to do
is
> to change that to have ($host_toolchain)?

Hmm, guess my brain wasn't working correctly :P. I've put back imagediff and
bitmaptools. I tried marking battor_agent as $host_toolchain, but that actually
fails to gn gen due to inconsistencies in how use_gconf is set. Rather than
fixing that, I took it as a sign that battor_agent isn't needed for is_android
and left it in this separate block.

[charliea (OOO until 10-5), 2016-01-04 15:38:37]

The basic idea looks good to me for //tools/battor_agent, but I have to admit
that I'm not familiar with what asserts do in gn files. I assume that they
assert the conditions whenever any target is built in the file. Is that right?

Also, do any corresponding gyp changes need to be made?

https://codereview.chromium.org/1545963003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1545963003/diff/1/BUILD.gn#newcode229
BUILD.gn:229: "//tools/telemetry:bitmaptools($host_toolchain)",
On 2015/12/28 19:46:07, agrieve wrote:
> On 2015/12/23 21:19:32, Dirk Pranke wrote:
> > I don't think this change is right. 
> > 
> > You definitely still need to build imagediff and bitmaptools when
> > target_os==android, you just need to build the host versions (as it says).
> > 
> > I suspect that's true for battor_agent as well, so may the right thing to do
> is
> > to change that to have ($host_toolchain)?
> 
> Hmm, guess my brain wasn't working correctly :P. I've put back imagediff and
> bitmaptools. I tried marking battor_agent as $host_toolchain, but that
actually
> fails to gn gen due to inconsistencies in how use_gconf is set. Rather than
> fixing that, I took it as a sign that battor_agent isn't needed for is_android
> and left it in this separate block.

Yep - you're right in that battor agent is only needed for desktop builds. The
agent is a tracing tool that communicates from a desktop to a connected device
that's measuring power consumption.

[agrieve, 2016-01-04 16:33:09]

On 2016/01/04 15:38:37, charliea wrote:
> The basic idea looks good to me for //tools/battor_agent, but I have to admit
> that I'm not familiar with what asserts do in gn files. I assume that they
> assert the conditions whenever any target is built in the file. Is that right?

Mostly - asserts are evaluated during "gn gen" time rather than build time. In
the case of a top-level assert in a .gn file, it is evaluated the first time
that any target within that BUILD.gn file is referenced from another where there
is a path back to //BUILD.gn.

> 
> Also, do any corresponding gyp changes need to be made?

The motivation for this is https://codereview.chromium.org/1541163002/. Since
--gc-sections is already disabled by default for debug in GYP, I don't think any
GYP changes are necessary.

> 
> https://codereview.chromium.org/1545963003/diff/1/BUILD.gn
> File BUILD.gn (right):
> 
> https://codereview.chromium.org/1545963003/diff/1/BUILD.gn#newcode229
> BUILD.gn:229: "//tools/telemetry:bitmaptools($host_toolchain)",
> On 2015/12/28 19:46:07, agrieve wrote:
> > On 2015/12/23 21:19:32, Dirk Pranke wrote:
> > > I don't think this change is right. 
> > > 
> > > You definitely still need to build imagediff and bitmaptools when
> > > target_os==android, you just need to build the host versions (as it says).
> > > 
> > > I suspect that's true for battor_agent as well, so may the right thing to
do
> > is
> > > to change that to have ($host_toolchain)?
> > 
> > Hmm, guess my brain wasn't working correctly :P. I've put back imagediff and
> > bitmaptools. I tried marking battor_agent as $host_toolchain, but that
> actually
> > fails to gn gen due to inconsistencies in how use_gconf is set. Rather than
> > fixing that, I took it as a sign that battor_agent isn't needed for
is_android
> > and left it in this separate block.
> 
> Yep - you're right in that battor agent is only needed for desktop builds. The
> agent is a tracing tool that communicates from a desktop to a connected device
> that's measuring power consumption.

[charliea (OOO until 10-5), 2016-01-05 17:14:19]

Makes sense to me. lgtm for tools/battor_agent

[Reilly Grant (use Gerrit), 2016-01-05 17:18:49]

device lgtm

[brettw, 2016-01-05 18:04:52]

brettw@chromium.org changed reviewers:
+ brettw@chromium.org

[brettw, 2016-01-05 18:04:52]

lgtm

[agrieve, 2016-01-05 18:35:02]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-01-05 18:36:59]

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

[commit-bot: I haz the power, 2016-01-05 19:26:35]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-05 19:27:19]

Description was changed from

==========
GN: Don't compile battor_agent, imagediff, bitmaptools on Android

Came across this because battor_agent doesn't link without
--gc-sections.

I don't think there's a use for running these tools on-device.

BUG=159847
==========

to

==========
GN: Don't compile battor_agent, imagediff, bitmaptools on Android

Came across this because battor_agent doesn't link without
--gc-sections.

I don't think there's a use for running these tools on-device.

BUG=159847

Committed: https://crrev.com/97dd57df5affed43a82441b987c4504b72ac64c5
Cr-Commit-Position: refs/heads/master@{#367609}
==========

[commit-bot: I haz the power, 2016-01-05 19:27:19]

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