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

LGTM, I suppose, but that 'libraries' list is such a hideous hack.

I'll take another look at this and see how this could be done differently. I'll do that as a separate PR after sorting out the failing test when using --enable-static.

LGTM, I suppose, but that 'libraries' list is such a hideous hack.

All's fair in love and GYP. For MSVS it circumvents the horrible project dependencies, so I don't hate it.

@danbev if you're up to it, for cleanliness you could define the list as a variable:

'variables': {
  ...
  'obj_files': [		
    '<(OBJ_GEN_PATH)<(OBJ_SEPARATOR)node_javascript.<(OBJ_SUFFIX)',		
    '<(OBJ_PATH)<(OBJ_SEPARATOR)node_debug_options.<(OBJ_SUFFIX)',
    ...
  ]
  ...
}
...
     'conditions': [
        ['node_target_type!="static_library"', {
          'libraries': [ '<@(obj_files)' ]
          ...

The test failure I mentioned above was test/parallel/test-process-config.js which compares the configuration in config.gypi with the one compiled. But when using --enable-static no node executable is created and when the test run they are using the previous version (if any) and there is a difference, hence the test failure. I leaning towards this is a non issue and not worth the time coming up with a workaround.

@danbev pushed a suggested workaround for the test. Feel free to force push it out...
Quick (and probably irrelevant) sanity: https://ci.nodejs.org/job/node-test-commit-linuxone/7999/

@danbev pushed a suggested workaround for the test. Feel free to force push it out...
Quick (and probably irrelevant) sanity: https://ci.nodejs.org/job/node-test-commit-linuxone/7999/

Thanks @refack. The workaround I had in mind was to still create the node executable which is not created when building with --enable-static. If node was not built previously there will be an error earlier as there will be no out/Release/node. The idea I had was to perhaps have a conditional target that would build an executable even when --enable-static is used, but that executable would link to the static library libnode.a. But I'm not sure if this is worth the effort.

If node was not built previously there will be an error earlier as there will be no out/Release/node.

Ack. Obviously...

The idea I had was to perhaps have a conditional target that would build an executable even when --enable-static is used, but that executable would link to the static library libnode.a. But I'm not sure if this is worth the effort.

I like that a lot (also for dynamic_library building) but I agree that's out of scope for this PR.
See #14158 I'm trying the get the people of @electron to help with that, but if you have the "spare cycles" to take that on, that would be great. We could setup a CI job that makes sure it doesn't rot.

Landed in be63c26

I suspect this PR of breaking the Windows build with the following error:

LINK : fatal error LNK1181: cannot open input file 'c:\workspace\node-compile-windows\label\win-vs2015\Release\obj\node\gen\node_javascript.o'

Revert CI to verify: https://ci.nodejs.org/job/node-test-commit-windows-fanned/11154/

edit: yep, it's this PR.

I suspect this PR of breaking the Windows build with the following error

I'm looking into it now, thanks