Don't exclude sources for 'none' targets

Don't exclude sources for 'none' targets

MSVS doesn't compile files for 'none' targets, so there is no need to "exclude"
them.

BUG=
R=scottmg@chromium.org

Committed: https://code.google.com/p/gyp/source/detail?r=1920

[Shezan Baig (Bloomberg), 2014-05-13 17:07:28]

Hey Scott,
Pretty simple change, but it removes one extra level of unnecessary folders in
msvs.
Thanks!

[scottmg, 2014-05-13 18:06:34]

lgtm

[Shezan Baig (Bloomberg), 2014-05-14 13:17:43]

Committed patchset #1 manually as r1920 (presubmit successful).

[Nico, 2014-05-19 18:13:53]

(for generator-specific things, consider prepending the first line of the
description with the generator name, i.e. "msvs: Don't exc…")

[Romain Pokrzywka, 2014-05-28 01:12:38]

On 2014/05/13 17:07:28, Shezan Baig (Bloomberg) wrote:
> Hey Scott,
> Pretty simple change, but it removes one extra level of unnecessary folders in
> msvs.
> Thanks!

Hi Shezan,

that change makes Blink's core_bindings_generated target fail for VS
(https://chromium.googlesource.com/chromium/blink/+/3b4063534ca0a98b16746882e5...),
because the target is of type 'none' and the inputs have a '.idl' extension
which is detected as MIDL files by the msvs generator. Normally those files
would be excluded from the VS build, so only used for the custom action, but
following your change they're not excluded anymore, so VS tries to build those
files using the MIDL compiler and fails.

It looks like only .c/.cpp files are not triggered for 'none' projects. MIDL and
Resource files are still built if not excluded.

I've reverted the change locally for now, but if you have a chance to upload a
new patch to fix this issue I'd appreciate it.

Thanks!
Romain

[Shezan Baig (Bloomberg), 2014-05-28 13:23:27]

On 2014/05/28 01:12:38, Romain Pokrzywka wrote:
> Hi Shezan,
> 
> that change makes Blink's core_bindings_generated target fail for VS
>
(https://chromium.googlesource.com/chromium/blink/+/3b4063534ca0a98b16746882e5...),
> because the target is of type 'none' and the inputs have a '.idl' extension
> which is detected as MIDL files by the msvs generator. Normally those files
> would be excluded from the VS build, so only used for the custom action, but
> following your change they're not excluded anymore, so VS tries to build those
> files using the MIDL compiler and fails.
> 
> It looks like only .c/.cpp files are not triggered for 'none' projects. MIDL
and
> Resource files are still built if not excluded.
> 
> I've reverted the change locally for now, but if you have a chance to upload a
> new patch to fix this issue I'd appreciate it.
> 
> Thanks!
> Romain


Hmm, interesting.  I intended this to be only a "cosmetic" change... I didn't
think it would affect Visual Studio's behavior.  If the behavior is changing,
then I think it would be best to revert this, since the "cosmetic" benefit
doesn't seem worth the difference in behavior.  I'll post a CL to revert it
soon.

[Shezan Baig (Bloomberg), 2014-05-28 14:00:43]

Looking at this a bit more, it seems like the msvs generator already has logic
to specifically exclude idl files (see '_IdlFilesHandledNonNatively'), but it
only looks at rules, not actions.  Perhaps it should look at actions as well.

So I could either post a CL to revert this, or post a CL to make
'_IdlFilesHandledNonNatively' also return idl files used in actions.  In either
case, I'll make a test for this because it seems like this currently isn't being
tested at all.

[Romain Pokrzywka, 2014-05-28 18:22:31]

On 2014/05/28 14:00:43, Shezan Baig (Bloomberg) wrote:
> Looking at this a bit more, it seems like the msvs generator already has logic
> to specifically exclude idl files (see '_IdlFilesHandledNonNatively'), but it
> only looks at rules, not actions.  Perhaps it should look at actions as well.
> 
> So I could either post a CL to revert this, or post a CL to make
> '_IdlFilesHandledNonNatively' also return idl files used in actions.  In
either
> case, I'll make a test for this because it seems like this currently isn't
being
> tested at all.

Adding the fix inside _IdlFilesHandledNonNatively could work, but I'm afraid
that would make things quite complicated, given all the different use cases
(action only, rule only, action+rule).
I would favor a simpler fix, basically just take the if block out into its own
function, and add the extra check for c/cpp sources there.
That said there's a few other places where action inputs are excluded/included
in the generator code (e.g. _FilterActionsFromExcluded), so maybe there's some
cleanup and/or simplification to be done at the same time.

I agree that adding tests is the right way to go before making more changes. I
can help with that. For the rest there's no rush on my side so feel free to look
at this at you convenience.

Thanks,
Romain