Fix dependency issues for etw_manifest.gyp.

Fix dependency issues for etw_manifest.gyp.

BUG=

Committed: https://crrev.com/870867909e7ea656d43405e396c12e088c9e787f
Cr-Commit-Position: refs/heads/master@{#327578}

[Georges Khalil, 2015-04-27 19:16:37]

georgesak@chromium.org changed reviewers:
+ thestig@chromium.org

[Georges Khalil, 2015-04-27 19:16:38]

PTAL.

Still having compiling errors from time to time, which indicates missing
dependencies. I think that these are the last one that were missing.

Example of failure:
http://build.chromium.org/p/chromium/builders/Win/builds/31260/steps/compile/...

[Lei Zhang, 2015-04-27 23:37:04]

I'm not sure this is the right fix. With an empty (as in no build artifacts) out
directory, one should be able to do:

ninja -C out\Debug
obj\base\trace_event\base_win64.trace_event_etw_export_win.obj

Can you change the build dependencies so that command succeeds?

[Lei Zhang, 2015-04-27 23:43:36]

You can also build the win64 target and see if fail the same way.

[Lei Zhang, 2015-04-27 23:43:50]

On 2015/04/27 23:43:36, Lei Zhang wrote:
> You can also build the win64 target and see if fail the same way.

err, base_win64.

[Lei Zhang, 2015-04-27 23:49:00]

So the right answer is in etw_manifest.gyp, where you need to make the
etw_mnanifest target a hard dependency. e.g. add:

'hard_dependency': 1,

on the line above the variables line. I would also then clobber the out
directory, generate the .ninja files with GN, and make sure the base_win64
target builds correctly that way too.

[Lei Zhang, 2015-04-27 23:58:29]

On 2015/04/27 23:49:00, Lei Zhang wrote:
> So the right answer is in etw_manifest.gyp, where you need to make the
> etw_mnanifest target a hard dependency. e.g. add:
> 
> 'hard_dependency': 1,
> 
> on the line above the variables line. I would also then clobber the out
> directory, generate the .ninja files with GN, and make sure the base_win64
> target builds correctly that way too.

And actually the GN dependencies look ok.

[Georges Khalil, 2015-04-28 17:29:40]

On 2015/04/27 23:58:29, Lei Zhang wrote:
> On 2015/04/27 23:49:00, Lei Zhang wrote:
> > So the right answer is in etw_manifest.gyp, where you need to make the
> > etw_mnanifest target a hard dependency. e.g. add:
> > 
> > 'hard_dependency': 1,
> > 
> > on the line above the variables line. I would also then clobber the out
> > directory, generate the .ninja files with GN, and make sure the base_win64
> > target builds correctly that way too.
> 
> And actually the GN dependencies look ok.

Thanks for looking into it. The problem for me is that I'm not able to repro
this
issue locally. I tried deleting the out directory, running gclient runhooks then
only compiling base_win64.trace_event_etw_export_win.obj. It succeeds and I can
verify that the generated files are there. And this is without any modifications
to gyp files (ie. without applying this CL).

What could I be missing?

[Lei Zhang, 2015-04-28 18:10:14]

On 2015/04/28 17:29:40, Georges Khalil (OOO sick) wrote:
> Thanks for looking into it. The problem for me is that I'm not able to repro
> this
> issue locally. I tried deleting the out directory, running gclient runhooks
then
> only compiling base_win64.trace_event_etw_export_win.obj. It succeeds and I
can
> verify that the generated files are there. And this is without any
modifications
> to gyp files (ie. without applying this CL).
> 
> What could I be missing?

If you run ninja -j1 -v, you can see what commands it's running. For me, it
certainly does not generate the .h file without setting hard_dependency. I
simply ran gclient runhooks without any GYP_DEFINES set.

[Georges Khalil, 2015-04-28 19:27:09]

On 2015/04/28 18:10:14, Lei Zhang wrote:
> On 2015/04/28 17:29:40, Georges Khalil (OOO sick) wrote:
> > Thanks for looking into it. The problem for me is that I'm not able to repro
> > this
> > issue locally. I tried deleting the out directory, running gclient runhooks
> then
> > only compiling base_win64.trace_event_etw_export_win.obj. It succeeds and I
> can
> > verify that the generated files are there. And this is without any
> modifications
> > to gyp files (ie. without applying this CL).
> > 
> > What could I be missing?
> 
> If you run ninja -j1 -v, you can see what commands it's running. For me, it
> certainly does not generate the .h file without setting hard_dependency. I
> simply ran gclient runhooks without any GYP_DEFINES set.

Thanks, I'm able to repro now. It does compile with hard_dependency=1. I
also found out that it compiles if I change the target type to 'none' instead of
'static_library', without the need for hard_dependency. Would that also be a
solution? I can't find clear documentation on those.

[Lei Zhang, 2015-04-28 20:35:47]

On 2015/04/28 19:27:09, Georges Khalil (OOO sick) wrote:
> On 2015/04/28 18:10:14, Lei Zhang wrote:
> > On 2015/04/28 17:29:40, Georges Khalil (OOO sick) wrote:
> > > Thanks for looking into it. The problem for me is that I'm not able to
repro
> > > this
> > > issue locally. I tried deleting the out directory, running gclient
runhooks
> > then
> > > only compiling base_win64.trace_event_etw_export_win.obj. It succeeds and
I
> > can
> > > verify that the generated files are there. And this is without any
> > modifications
> > > to gyp files (ie. without applying this CL).
> > > 
> > > What could I be missing?
> > 
> > If you run ninja -j1 -v, you can see what commands it's running. For me, it
> > certainly does not generate the .h file without setting hard_dependency. I
> > simply ran gclient runhooks without any GYP_DEFINES set.
> 
> Thanks, I'm able to repro now. It does compile with hard_dependency=1. I
> also found out that it compiles if I change the target type to 'none' instead
of
> 'static_library', without the need for hard_dependency. Would that also be a
> solution? I can't find clear documentation on those.

chrome_events_win.man is the file that feelds the message_compiler rule, right?
I suppose you can change the type to none. See
https://code.google.com/p/gyp/wiki/GypLanguageSpecification

[Lei Zhang, 2015-04-28 20:39:23]

On 2015/04/28 20:35:47, Lei Zhang wrote:
> chrome_events_win.man is the file that feelds the message_compiler rule,
right?
> I suppose you can change the type to none. See
> https://code.google.com/p/gyp/wiki/GypLanguageSpecification

*feeds*

[Georges Khalil, 2015-04-29 13:09:55]

On 2015/04/28 20:39:23, Lei Zhang wrote:
> On 2015/04/28 20:35:47, Lei Zhang wrote:
> > chrome_events_win.man is the file that feelds the message_compiler rule,
> right?
> > I suppose you can change the type to none. See
> > https://code.google.com/p/gyp/wiki/GypLanguageSpecification
> 
> *feeds*

So to finalize, what's preferable? Changing type to none or keeping
static_library and adding hard_dependency? Both seem to fix the issue
locally for me. I'll update the CL to reflect what you think is best.

[Lei Zhang, 2015-04-29 19:50:21]

On 2015/04/29 13:09:55, Georges Khalil (OOO sick) wrote:
> On 2015/04/28 20:39:23, Lei Zhang wrote:
> > On 2015/04/28 20:35:47, Lei Zhang wrote:
> > > chrome_events_win.man is the file that feelds the message_compiler rule,
> > right?
> > > I suppose you can change the type to none. See
> > > https://code.google.com/p/gyp/wiki/GypLanguageSpecification
> > 
> > *feeds*
> 
> So to finalize, what's preferable? Changing type to none or keeping
> static_library and adding hard_dependency? Both seem to fix the issue
> locally for me. I'll update the CL to reflect what you think is best.

Some places do both. I don't think that'll hurt.

[Georges Khalil, 2015-04-29 20:09:41]

Patchset #2 (id:20001) has been deleted

[Georges Khalil, 2015-04-29 20:14:20]

PTAL.

[Lei Zhang, 2015-04-29 20:45:15]

lgtm

belts and suspenders FTW

[Georges Khalil, 2015-04-29 20:47:09]

The CQ bit was checked by georgesak@chromium.org

[commit-bot: I haz the power, 2015-04-29 20:50:23]

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

[commit-bot: I haz the power, 2015-04-29 21:46:33]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-04-29 21:47:30]

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