Skip to content

win, build: add node.lib file into headers package

Rodrigo Muino Tomonari requested to merge github/fork/gibfahn/headers-fix into master
Checklist
  • tests and code linting passes
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

build (win)

Description of change

This PR makes two changes.

  • It changes the headers function in tools/install.py so that it includes AIX's out/Release/node.exp and Windows Release/node.lib if those files exist.
  • It adds a vcbuild.bat tar-headers option to vcbuild.bat that does the same as make tar-headers.
Issues/Comments
  • AFAIK, the primary purpose of the headers package is to facilitate building native modules, which for windows requires a node.lib, and for AIX requires a node.exp.
  • It would be possible to include the node.lib and node.exp files when building on Linux (or OSX) by copying the node.lib file into Release/node.lib before running make tar-headers.
  • As I understand - e.g. from https://github.com/nodejs/node/issues/4932#issuecomment-205483725 - building native modules on windows can also require things from Release\lib (e.g. openssl.lib). I suspect it would be impractical to include these in the headers package due to their size:
6.1M    cares.lib
13M     gtest.lib
116K    http_parser.lib
1.8M    icudata.lib
29M     icui18n.lib
8.0K    icustubdata.lib
54M     icutools.lib
16M     icuucx.lib
5.4M    libuv.lib
71M     openssl.lib
740K    zlib.lib
  • At the moment everything is put into the node-7.0.0/include/node directory, it might make sense to put the .exp and .lib files in a different directory, e.g. node-7.0.0/lib.
  • A limitation of this approach is that only one node.lib would be included (x64 or x86, depending which version of node had been built).
  • I'm not sure whether the tar-headers process is documented anywhere, if it is I should probably update the documentation.

Related: https://github.com/nodejs/node/pull/6274#issuecomment-213707169

Merge request reports

Loading