win: Fix missing loadable_module dependency in ULDI mode

win: Fix missing loadable_module dependency in ULDI mode

Also, add test for similar setup in non-ULDI mode for all platforms.

R=thakis@chromium.org, dpranke@chromium.org
BUG=548300
Committed: https://chromium.googlesource.com/external/gyp/+/f2c3cfc3b1df1a9658c6646faa67c670209cf963

[scottmg, 2015-11-03 00:50:27]

Description was changed from

==========
win: Maybe a way to fix missing loadable_module dependency in ULDI mode

BUG=548300
==========

to

==========
win: Maybe a way to fix missing loadable_module dependency in ULDI mode

R=dpranke@chromium.org, thakis@chromium.org
BUG=548300
==========

[scottmg, 2015-11-03 00:50:27]

scottmg@chromium.org changed reviewers:
+ dpranke@chromium.org, thakis@chromium.org

[scottmg, 2015-11-03 00:51:06]

Description was changed from

==========
win: Maybe a way to fix missing loadable_module dependency in ULDI mode

R=dpranke@chromium.org, thakis@chromium.org
BUG=548300
==========

to

==========
win: Fix missing loadable_module dependency in ULDI mode

R=dpranke@chromium.org, thakis@chromium.org
BUG=548300
==========

[Nico, 2015-11-03 01:21:55]

We probably already have a test like this for non-uldi, right? Should that test
just be extended with the uldi case instead of adding a new test?

[Dirk Pranke, 2015-11-03 01:36:23]

This seems to make sense to me, but I mostly defer to Nico.

[scottmg, 2015-11-03 16:03:15]

On 2015/11/03 01:21:55, Nico wrote:
> We probably already have a test like this for non-uldi, right? Should that
test
> just be extended with the uldi case instead of adding a new test?

I don't see one (test/dependencies?) but I might be missing it. I could move
this test to there if you think it makes more sense there.

[Nico, 2015-11-03 16:37:01]

lgtm

Huh, that's surprising to me, but not your bug to fix then. Maybe add a non-uldi
version of this test and make that part run on all platforms; from what I
understand the point of the change is to make uldi targets behave like regular
targets, but due to lack of testing for regular targets the test doesn't
currently prove this. As-is is ok too though.

[scottmg, 2015-11-03 16:51:41]

On 2015/11/03 16:37:01, Nico wrote:
> lgtm
> 
> Huh, that's surprising to me, but not your bug to fix then. Maybe add a
non-uldi
> version of this test and make that part run on all platforms; from what I
> understand the point of the change is to make uldi targets behave like regular
> targets, but due to lack of testing for regular targets the test doesn't
> currently prove this. As-is is ok too though.

OK, added a generic version to test/dependencies.

(Filed https://code.google.com/p/chromium/issues/detail?id=550990 to get win_try
back online.)

[scottmg, 2015-11-03 16:52:01]

Description was changed from

==========
win: Fix missing loadable_module dependency in ULDI mode

R=dpranke@chromium.org, thakis@chromium.org
BUG=548300
==========

to

==========
win: Fix missing loadable_module dependency in ULDI mode

Also, add test for similar setup in non-ULDI mode for all platforms.

R=dpranke@chromium.org, thakis@chromium.org
BUG=548300
==========

[scottmg, 2015-11-03 19:52:47]

On 2015/11/03 16:51:41, scottmg wrote:
> On 2015/11/03 16:37:01, Nico wrote:
> > lgtm
> > 
> > Huh, that's surprising to me, but not your bug to fix then. Maybe add a
> non-uldi
> > version of this test and make that part run on all platforms; from what I
> > understand the point of the change is to make uldi targets behave like
regular
> > targets, but due to lack of testing for regular targets the test doesn't
> > currently prove this. As-is is ok too though.
> 
> OK, added a generic version to test/dependencies.
> 
> (Filed https://code.google.com/p/chromium/issues/detail?id=550990 to get
win_try
> back online.)

Did you want to look again, or happy with this?

[Nico, 2015-11-03 20:39:15]

still lgtm, thanks!

https://codereview.chromium.org/1410213005/diff/140001/test/dependencies/gypt...
File test/dependencies/gyptest-indirect-module-dependency.py (right):

https://codereview.chromium.org/1410213005/diff/140001/test/dependencies/gypt...
test/dependencies/gyptest-indirect-module-dependency.py:14: test =
TestGyp.TestGyp(formats=['msvs', 'ninja'])
should this pass with all generators?

[scottmg, 2015-11-03 20:54:34]

https://codereview.chromium.org/1410213005/diff/140001/test/dependencies/gypt...
File test/dependencies/gyptest-indirect-module-dependency.py (right):

https://codereview.chromium.org/1410213005/diff/140001/test/dependencies/gypt...
test/dependencies/gyptest-indirect-module-dependency.py:14: test =
TestGyp.TestGyp(formats=['msvs', 'ninja'])
On 2015/11/03 20:39:15, Nico wrote:
> should this pass with all generators?

Oops, yes, thanks. Done.

[scottmg, 2015-11-03 21:10:03]

Committed patchset #9 (id:160001) manually as
f2c3cfc3b1df1a9658c6646faa67c670209cf963 (presubmit successful).