Upstream //build/ios/coverage.gypi

Upstream //build/ios/coverage.gypi

Add OWNERS for build/ios.
Use Xcode clang compiler for coverage configuration.

BUG=450379
Committed: https://crrev.com/07a8e92fb99ff03230122c2341e2d3b017216757
Cr-Commit-Position: refs/heads/master@{#312416}

[Eugene But (OOO till 7-30), 2015-01-16 21:27:31]

eugenebut@chromium.org changed reviewers:
+ stuartmorgan@chromium.org

[stuartmorgan, 2015-01-16 21:38:42]

lgtm

https://codereview.chromium.org/853293002/diff/1/base/test/ios/OWNERS
File base/test/ios/OWNERS (right):

https://codereview.chromium.org/853293002/diff/1/base/test/ios/OWNERS#newcode2
base/test/ios/OWNERS:2: 
Please add rohitrao@chromium.org as well.

https://codereview.chromium.org/853293002/diff/1/base/test/ios/coverage.gypi
File base/test/ios/coverage.gypi (right):

https://codereview.chromium.org/853293002/diff/1/base/test/ios/coverage.gypi#...
base/test/ios/coverage.gypi:11: [ 'OS=="ios" and enable_coverage', {
No space after [

Also, why the OS check in an iOS gyp file?

[Eugene But (OOO till 7-30), 2015-01-16 22:04:06]

eugenebut@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Eugene But (OOO till 7-30), 2015-01-16 22:04:07]

https://codereview.chromium.org/853293002/diff/1/base/test/ios/OWNERS
File base/test/ios/OWNERS (right):

https://codereview.chromium.org/853293002/diff/1/base/test/ios/OWNERS#newcode2
base/test/ios/OWNERS:2: 
On 2015/01/16 21:38:42, stuartmorgan wrote:
> Please add mailto:rohitrao@chromium.org as well.

Done.

https://codereview.chromium.org/853293002/diff/1/base/test/ios/coverage.gypi
File base/test/ios/coverage.gypi (right):

https://codereview.chromium.org/853293002/diff/1/base/test/ios/coverage.gypi#...
base/test/ios/coverage.gypi:11: [ 'OS=="ios" and enable_coverage', {
On 2015/01/16 21:38:42, stuartmorgan wrote:
> No space after [
> 
> Also, why the OS check in an iOS gyp file?
Removed space. This file fill be unconditionally included into other gyp files.

[Paweł Hajdan Jr., 2015-01-19 12:44:09]

Why is this under src/base/test as opposed to e.g. src/build?

[Eugene But (OOO till 7-30), 2015-01-20 04:34:44]

On 2015/01/19 12:44:09, Paweł Hajdan Jr. wrote:
> Why is this under src/base/test as opposed to e.g. src/build?
coverage.gypi is configuration for code coverage, it's test-only thing. Why do
you think it should be in src/build?

[Paweł Hajdan Jr., 2015-01-20 13:19:50]

On 2015/01/20 04:34:44, eugenebut wrote:
> On 2015/01/19 12:44:09, Paweł Hajdan Jr. wrote:
> > Why is this under src/base/test as opposed to e.g. src/build?
> coverage.gypi is configuration for code coverage, it's test-only thing. Why do
> you think it should be in src/build?

While files in base/test focus mostly on C++ testing utilities (and I'm owner of
that directory, so at least the intention is true), coverage.gypi seems to
control mostly compile-time flags and settings, and so to me it really would fit
better under src/build.

Is there some specific reason why you picked src/base/test, or would strongly
prefer it over src/build?

[Eugene But (OOO till 7-30), 2015-01-20 17:40:11]

>Is there some specific reason why you picked src/base/test, or would strongly
prefer it over src/build?
Placing coverage.gypi to build/ios makes perfect sense for me. Moved to
build/ios. Thank you for review!

[Eugene But (OOO till 7-30), 2015-01-20 17:40:26]

eugenebut@chromium.org changed reviewers:
+ thakis@chromium.org
- phajdan.jr@chromium.org

[Nico, 2015-01-20 19:04:26]

https://codereview.chromium.org/853293002/diff/40001/build/ios/coverage.gypi
File build/ios/coverage.gypi (right):

https://codereview.chromium.org/853293002/diff/40001/build/ios/coverage.gypi#...
build/ios/coverage.gypi:26: # linked with libclang_rt.profile_ios.
This looks all kinds of wrong.

1.) If gyp's xcode emulation in the ninja generator doesn't work like xcode, we
should fix that
2.) this flag isn't added by xcode, it's added by clang – if -fprofile-arcs is
passed to the linker. So rather than doing this hack, you should probably just
add -fprofile-arcs to OTHER_LDFLAGS.
3.) Does anyone look at this data? People add coverage information to chrome
every other year, then nobody looks at the data, the coverage build file stuff
ends up being a pain, someone else removes it, and it's a bunch of busywork each
time. Is this actually in use?

[stuartmorgan, 2015-01-20 19:26:28]

On 2015/01/20 19:04:26, Nico wrote:
> 3.) Does anyone look at this data? People add coverage information to chrome
> every other year, then nobody looks at the data, the coverage build file stuff
> ends up being a pain, someone else removes it, and it's a bunch of busywork
each
> time. Is this actually in use?

We're working on building out incremental coverage reports, with the goal of
having it do patch-level reporting for iOS (possibly even in the review tool),
rather than having a bot that makes a static page somewhere that we don't go to,
to see if that works out better than the failed attempts in the past.

[Eugene But (OOO till 7-30), 2015-01-21 00:18:40]

PTAL

https://codereview.chromium.org/853293002/diff/40001/build/ios/coverage.gypi
File build/ios/coverage.gypi (right):

https://codereview.chromium.org/853293002/diff/40001/build/ios/coverage.gypi#...
build/ios/coverage.gypi:26: # linked with libclang_rt.profile_ios.
On 2015/01/20 19:04:25, Nico wrote:
> This looks all kinds of wrong.
> 
> 1.) If gyp's xcode emulation in the ninja generator doesn't work like xcode,
we
> should fix that
Filed a bug: crbug.com/450379

> 2.) this flag isn't added by xcode, it's added by clang – if -fprofile-arcs is
> passed to the linker. So rather than doing this hack, you should probably just
> add -fprofile-arcs to OTHER_LDFLAGS.
Done. Requires clang_xcode to be 1.

[Nico, 2015-01-21 00:26:57]

BUG=450379 in the cl description. with that, lgtm

Thanks!

[Eugene But (OOO till 7-30), 2015-01-21 00:32:14]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2015-01-21 00:33:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/853293002/60001

[Nico, 2015-01-21 00:57:48]

Actually! I think instead of these two flags, you can use -coverage, which is a
bit more readable. Maybe you can land a follow-up with that?

[Eugene But (OOO till 7-30), 2015-01-21 00:58:47]

The CQ bit was unchecked by eugenebut@chromium.org

[Nico, 2015-01-21 01:06:18]

Actually actually, this was broken in clang until clang r184666 from Jun
2013 – so there are probably still xcodes out there that don't have that
yet. So nevermind.

On Tue, Jan 20, 2015 at 4:57 PM, <thakis@chromium.org> wrote:

> Actually! I think instead of these two flags, you can use -coverage, which
> is a
> bit more readable. Maybe you can land a follow-up with that?
>
> https://codereview.chromium.org/853293002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Eugene But (OOO till 7-30), 2015-01-21 01:06:58]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2015-01-21 01:14:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/853293002/60001

[commit-bot: I haz the power, 2015-01-21 02:44:23]

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

[commit-bot: I haz the power, 2015-01-21 02:44:24]

Try jobs failed on following builders:
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/11...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[Eugene But (OOO till 7-30), 2015-01-21 16:46:26]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2015-01-21 16:47:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/853293002/60001

[commit-bot: I haz the power, 2015-01-21 16:48:06]

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

[commit-bot: I haz the power, 2015-01-21 16:48:07]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ng...)

[Eugene But (OOO till 7-30), 2015-01-21 17:10:01]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2015-01-21 17:11:22]

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

[commit-bot: I haz the power, 2015-01-21 18:29:43]

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

[commit-bot: I haz the power, 2015-01-21 18:29:44]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Eugene But (OOO till 7-30), 2015-01-21 18:30:40]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2015-01-21 18:32:22]

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

[commit-bot: I haz the power, 2015-01-21 18:33:20]

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

[commit-bot: I haz the power, 2015-01-21 18:33:22]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Eugene But (OOO till 7-30), 2015-01-21 19:32:35]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2015-01-21 19:32:57]

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

[commit-bot: I haz the power, 2015-01-21 20:21:41]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-01-21 20:22:55]

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