[tracing] Move GN/gyp build files under trace_event

[tracing] Move GN/gyp build files under trace_event

Move all trace_event related build files in their own
gyp / gn target under base/trace_event.
This should allow easier maintenance of tracing code.

BUG=

Committed: https://crrev.com/7c942e47606da9e81d9c8621d7fd01b6171b99a7
Cr-Commit-Position: refs/heads/master@{#321430}

[Primiano Tucci (use gerrit), 2015-03-17 19:35:54]

primiano@chromium.org changed reviewers:
+ dsinclair@chromium.org, nduca@chromium.org

[Primiano Tucci (use gerrit), 2015-03-17 19:35:54]

Nat, Dan: in a recent CL danakj@ suggested to move our build files under
trace_event so that we can avoid OWNERS dance every time we add/remove files.
Can you TAL, before I send this out to base OWNERS and brettw (for GN)?

[dsinclair, 2015-03-17 20:00:54]

Looks like there are a bunch of unhappy bots.

https://codereview.chromium.org/1014923002/diff/1/base/base.gyp
File base/base.gyp (right):

https://codereview.chromium.org/1014923002/diff/1/base/base.gyp#newcode683
base/base.gyp:683: '<@(trace_event_test_sources)',
Does this need an includes: section like the gypi?

https://codereview.chromium.org/1014923002/diff/1/base/trace_event/BUILD.gn
File base/trace_event/BUILD.gn (right):

https://codereview.chromium.org/1014923002/diff/1/base/trace_event/BUILD.gn#n...
base/trace_event/BUILD.gn:77: "//base/test:test_support",
Does this target build with just these deps? Seems like it should depend on
"//base" as well since we use things like base::Value?

[Primiano Tucci (use gerrit), 2015-03-17 20:29:12]

> Looks like there are a bunch of unhappy bots.
Most bots went red in PS1 because the coffeine level is decreasing at this time
of the day and I forgot to commit one extra line locally before uploading.

PS2 looks much better ;-)

https://codereview.chromium.org/1014923002/diff/1/base/base.gyp
File base/base.gyp (right):

https://codereview.chromium.org/1014923002/diff/1/base/base.gyp#newcode683
base/base.gyp:683: '<@(trace_event_test_sources)',
On 2015/03/17 20:00:53, dsinclair wrote:
> Does this need an includes: section like the gypi?

The include is inherited through base.gypi, i.e.

base.gyp -> base.gypi -> trace_event.gypi

If we move the include to base.gypi it becomes awkward because then we'd
copy/paste the include into base_nacl.gyp.
Also, from a logic viewpoint is base.gypi that depends on trace_event.gypi (and
there seem to be other precedents int he codebase of gypi files including other
gypi files)

https://codereview.chromium.org/1014923002/diff/1/base/trace_event/BUILD.gn
File base/trace_event/BUILD.gn (right):

https://codereview.chromium.org/1014923002/diff/1/base/trace_event/BUILD.gn#n...
base/trace_event/BUILD.gn:77: "//base/test:test_support",
On 2015/03/17 20:00:53, dsinclair wrote:
> Does this target build with just these deps?
Well, bots seem happy at least.

> Seems like it should depend on
> "//base" as well since we use things like base::Value?

Here's something I don't quite know. This is a source set which is part of base,
do we have to make the reverse (base) dependency explicit? 
I think this is a question for brettw@

[dsinclair, 2015-03-17 20:32:17]

lgtm from the trace-viewer, no real knowledge of gyp/gn, side.

[Primiano Tucci (use gerrit), 2015-03-17 20:35:32]

primiano@chromium.org changed reviewers:
+ brettw@chromium.org, danakj@chromium.org

[Primiano Tucci (use gerrit), 2015-03-17 20:35:32]

Brett / Dana:  would you please take a look to these gyp/gn changes?

Brett: see Dan's comment on the dependency form //base/trace_event_tests back to
//base. Not sure what is the right pattern there.

Thanks.

[danakj, 2015-03-17 20:52:12]

On 2015/03/17 20:35:32, Primiano Tucci wrote:
> Brett / Dana:  would you please take a look to these gyp/gn changes?
> 
> Brett: see Dan's comment on the dependency form //base/trace_event_tests back
to
> //base. Not sure what is the right pattern there.
> 
> Thanks.

Not sure either.. will let Brett comment first.

[brettw, 2015-03-19 18:09:29]

lgtm

https://codereview.chromium.org/1014923002/diff/20001/base/trace_event/BUILD.gn
File base/trace_event/BUILD.gn (right):

https://codereview.chromium.org/1014923002/diff/20001/base/trace_event/BUILD....
base/trace_event/BUILD.gn:75: ]
Can you be sure to not regress gn check with this? I think you will need to add
this new target to the allow_circular_includes_from in //base:base.

To test, do
  gn check out/foo "//base/*"
and don't worry about the few errors that aren't related to this change. We're
working on enabling this all the time but there are some blockers.

[Primiano Tucci (use gerrit), 2015-03-19 19:37:47]

Gn check seem 100% happy on both //base/* and //base/trace_event/*.

Dana, does the rest look good to you?

https://codereview.chromium.org/1014923002/diff/20001/base/trace_event/BUILD.gn
File base/trace_event/BUILD.gn (right):

https://codereview.chromium.org/1014923002/diff/20001/base/trace_event/BUILD....
base/trace_event/BUILD.gn:75: ]
On 2015/03/19 18:09:29, brettw wrote:
> Can you be sure to not regress gn check with this? I think you will need to
add
> this new target to the allow_circular_includes_from in //base:base.
> 
> To test, do
>   gn check out/foo "//base/*"
> and don't worry about the few errors that aren't related to this change. We're
> working on enabling this all the time but there are some blockers.

$ gn check out "//base/*"
Header dependency check OK

Seems fine.

[Primiano Tucci (use gerrit), 2015-03-19 20:01:07]

The CQ bit was checked by primiano@chromium.org

[Primiano Tucci (use gerrit), 2015-03-19 20:01:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsinclair@chromium.org, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1014923002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2015-03-19 20:01:26]

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

[danakj, 2015-03-19 20:02:15]

Yep. Looks like you got enough owners now :)

On Thu, Mar 19, 2015 at 12:37 PM, <primiano@chromium.org> wrote:

> Gn check seem 100% happy on both //base/* and //base/trace_event/*.
>
> Dana, does the rest look good to you?
>
>
> https://codereview.chromium.org/1014923002/diff/20001/
> base/trace_event/BUILD.gn
> File base/trace_event/BUILD.gn (right):
>
> https://codereview.chromium.org/1014923002/diff/20001/
> base/trace_event/BUILD.gn#newcode75
> base/trace_event/BUILD.gn:75: ]
> On 2015/03/19 18:09:29, brettw wrote:
>
>> Can you be sure to not regress gn check with this? I think you will
>>
> need to add
>
>> this new target to the allow_circular_includes_from in //base:base.
>>
>
>  To test, do
>>    gn check out/foo "//base/*"
>> and don't worry about the few errors that aren't related to this
>>
> change. We're
>
>> working on enabling this all the time but there are some blockers.
>>
>
> $ gn check out "//base/*"
> Header dependency check OK
>
> Seems fine.
>
> https://codereview.chromium.org/1014923002/
>

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

[commit-bot: I haz the power, 2015-03-19 21:20:47]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-19 21:21:49]

Patchset 3 (id:??) landed as
https://crrev.com/7c942e47606da9e81d9c8621d7fd01b6171b99a7
Cr-Commit-Position: refs/heads/master@{#321430}