GeezerVison

A summary for the change:

• In default build process, the static lib is built first and used to build the executable.
• For ./configure --enable-static or .\vcbuild.bat static, they do the same thing as default build process
• For ./configure --shared or .\vcbuild.bat dll, the shared lib is built first and the executable links to the shared lib.

For cctest, since it depends some internal APIs, it can only build from obj files in all of the cases.

I hit two Windows issues mainly when creating this PR, including:

• building shared lib and executable with the same filename causes the filename collision.
• when using the static lib to build the executable, even using /WHOLEARCHIVE to include
  the static lib into the executable, the functions that are not used by node_main.cc are optimized out

For first one, there are two ways to solve it (maybe more):

• using different filename for shared lib and executable (I used this in the PR)
• output to different sub folder

For the second one, I found two approaches,

• use /INCLUDE option in linker. You put function names in /INCLUDE option and use comma as separator for multiple functions. Linker finds the obj files for each of the function name you specified and include the whole objs into the executable.
• build the executable from obj files but not static lib.

Since our purpose is to used the static lib to build the executable and verify it. I used /INCLUDE in my PR. And /INCLUDE could also be coded inside the node_main.cc as #pragma. For now, I do it in the node.gyp

@digitalinfinity can you take a look as many of the challenges were around windows.

My 2 cents given the choices for the name of the exe and the dll I think we want to:

1. keep the naming as node.dll so we don't break existing users
2. do the rename to node.exe from node-win.exe (as the PR currently does) so that tests that spawn node.js still work

I do wonder if there is any dependency on node.exe when the addons built or if they just use the node.pdb and node.lib files (ie addons would build/run fine without node.exe)

CI run to validate tests pass across platforms in base case (particularly interested in AIX were there was some extra steps previously) https://ci.nodejs.org/job/node-test-pull-request/12040/

@mhdawson It's good to run CI for verifying. I do see the failures. let me check those failures

If you need access to AIX to test out changes just let me know

Yes, I need to access AIX to verify the change

(cc @joaocgreis @bzoz since I'm technically on vacation 😄)

@yhwang forgive me, I'm missing a bit of context here so I'll ask a somewhat basic question- in your case where you're building the exe out of the static lib, what is the problem if functions not used by the exe are optimized out? Or is the problem that the exe fails to link?

@digitalinfinity I guess you were asking about Windows specifically, right? Yes, unused functions are optimized out, the N-API. Because N-API is used by user addon. In node_main.cc, N-API is not used. That's why I added /INCLUDE napi_module_register and the whole node_api.o is included into the exe. It would be good to run a dumpbin /exports Release\node.exe to see if there is any other function is optimized out. However, I locally run the .\vcbuild.bat test. seems the N-API is the only one. But comparing the output of dumpbin will be better. I am fixing the AIX issue now. I will circle back to this later.

And this issue only exists in Windows, in other platforms, the force_load or archive_whole options include the whole static lib into the executable.

Updated my change to address AIX failure. May I have another CI to verify the AIX build?

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

updated the change to fix fips related failures. Please kick off another CI. Thanks

Another CI run: https://ci.nodejs.org/job/node-test-pull-request/12067/console

@yhwang that's very odd- the NAPI methods are marked as __declspec(dllexport) so whole-program-optimization shouldn't optimize it away. The other question is, is it common for native module authors to link against the node exe on Windows rather than the node static lib? If not, as long as the lib has the methods that we want, we should be fine, but it wouldn't surprise me if there were modules that linked directly against the exe.

Actually, I asked around and noticed this: https://blogs.msdn.microsoft.com/oldnewthing/20140321-00/?p=1433

So @yhwang it looks like the behavior you're seeing is by design and if we want those functions to actually get exported, the solution is to probably use a .DEF file

is it common for native module authors to link against the node exe on Windows rather than the node static lib?

For link, I guess people may choose shared lib. But I don’t know how many pepole use node.exe with node.dll. and agree with you that people may link to the exe for native modules.

I fixed AIX failure and found we use .exp in AIX. However, the way to generate the exp is not perfect yet. maybe we could refine the script and use it for AIX and Windows. I could create another PR for it later.

I am trying to find the failure in windows with VS 2017 now. The weird thing is I am using VS 2017 and I can build the node without any issue. Trying to recreate it first.

For smartos, @cjihrig can I access the smartos env in CI? Then I can try to fix the failure. Thanks.

for the windows failures, I can recreated it after I updated my VS2017. and the reason is one static project can't have more then one .res file. maybe merge the node_etw and node_perfctr into one... let me try it. If anyone has suggestion for this, please let me know.

For smartos, @cjihrig can I access the smartos env in CI? Then I can try to fix the failure. Thanks.

@yhwang, I don't have access to any of the CI machines, but someone from the build group might be able to help. cc: @mhdawson

For Windows, I updated change and the .rc files are compiled and included in shared lib and executable. For static lib case, users need to add .rc files to their projects by themselves.

May I have a new CI to verify the Windows build? If everything goes well, then only smartos left.

P.S. this change broke ninja build on Windows (due to use of presumed location of *.lib files in a <(PRODUCT_DIR)\\lib directory, which ninja does not do).
I'm investigating ways to work around this.

I opened nodejs/build#1288 so that once we get this working again we can keep it from regressing unintentionaly.

@refack I can help to solve the issue. But I don't know how to use ninja to build node in windows. do you have any instruction?

BTW, currently, the master branch has an error when using .\vcbuild.bat dll. The problem is the cctest can't link to the shared lib in Windows, since some symbols that cctest need are not exported. I am trying to fix that.