win,build: Always generate .rc and .h files

[joaocgreis]

Pull Request check-list

• Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
  this change (including linting)?
• Is the commit message formatted according to [CONTRIBUTING.md][0]?

Affected core subsystem(s)

Windows, build.

Description of change

Since Visual Studio 2012, the Windows SDK is included in all Visual Studio Editions. We can now assume the Windows SDK is installed, hence the intermediate files generated from manifest should not be part of the source tree.

This also fixes incorrect detection of ctrpp.exe, that should be in the path.

cc @nodejs/platform-windows

[joaocgreis]

CI: https://ci.nodejs.org/job/node-test-pull-request/1899/

Also tested locally with Visual Studio Express 2013 for Windows Desktop.

[joaocgreis]

Ref: nodejs/node-v0.x-archive#4450 (comment)

[orangemocha]

LGTM

[rvagg]

How different do the generated files look to what we have there now? we're not losing anything in doing this are we?

Also, could you try and get "etw" (or similar) into the commit msg, otherwise it's an inaccurate summary.

[rvagg]

^ in fact you could probably exchange "win" in the subsystem for "etw" in the summary msg

[joaocgreis]

The generated files (VS2015 on W10) are exactly the same as the ones now committed, except for node_perfctr_provider.h. Here's the diff:

diff master/node_perfctr_provider.h tools/msvs/genfiles/node_perfctr_provider.h
19c19
< EXTERN_C DECLSPEC_SELECTANY GUID NodeCounterProviderGuid = { 0x1e2e15d7, 0x3760, 0x470e, 0x86, 0x99, 0xb9, 0xdb, 0x52, 0x48, 0xed, 0xd5 };
---
> EXTERN_C DECLSPEC_SELECTANY GUID NodeCounterProviderGuid = { 0x793c9b44, 0x3d6b, 0x4f57, 0xb5, 0xd7, 0x4f, 0xf8, 0xa, 0xdc, 0xf9, 0xa2 };
39c39
<     { { 0x3a22a8ec, 0x297c, 0x48ac, 0xab, 0x15, 0x33, 0xec, 0x93, 0x3, 0x3f, 0xd8 }, { 0x1e2e15d7, 0x3760, 0x470e, 0x86, 0x99, 0xb9, 0xdb, 0x52, 0x48, 0xed, 0xd5 }, 10, PERF_COUNTERSET_MULTI_AGGREGATE },
---
>     { { 0x3a22a8ec, 0x297c, 0x48ac, 0xab, 0x15, 0x33, 0xec, 0x93, 0x3, 0x3f, 0xd8 }, { 0x793c9b44, 0x3d6b, 0x4f57, 0xb5, 0xd7, 0x4f, 0xf8, 0xa, 0xdc, 0xf9, 0xa2 }, 10, PERF_COUNTERSET_MULTI_AGGREGATE },

This reflects the change in the provider GUID introduced in 7887e11 , that did not update the generated files. Hence, in master, the manifest has the new GUID and the generated files have the old one.

However, performance counters do work and can be viewed from the Performance Monitor. I did not find a solid reference for this, but the only use I found for the Provider GUID is during installation when the installer registers the provider. To publish the events in runtime, the Counter Set GUID is used instead (tested by changing it in genfiles).

I also tested upgrading.

With the current evidence, I think we should land this as it is. The Provider GUID in the executable will change, but it does not seem to be used anywhere, and it is currently wrong.

@nodejs/platform-windows , @rvagg , any comments or objections?

[joaocgreis]

I will replace win by etw in the commit message.

[joaocgreis]

Updated and rebased, will land in about 24h if there are no objections.

[MylesBorins]

@joaocgreis should this be backported to v4?

[joaocgreis]

@thealphanerd I don't think so. On one hand this is a bug fix, but on the other there is a GUID change. To the best of my knowledge it is harmless, but I can't claim to be completely sure. The advantage of this is to ease development of ETW, so I don't think it's even needed.

[MylesBorins]

thanks @joaocgreis