Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 2 commits into from

Conversation

gibfahn
Copy link
Member

@gibfahn gibfahn commented May 5, 2016

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 Windows: addons cannot use the bundled OpenSSL #4932 (comment) - 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: #6274 (comment)

@mscdex mscdex added windows Issues and PRs related to the Windows platform. build Issues and PRs related to build files or the CI. labels May 5, 2016
tools/install.py will now include the AIX node.exp and the Windows
node.lib file if they are present.
Adds option to vcbuild.bat to build header package.
@silverwind
Copy link
Contributor

@nodejs/platform-windows @nodejs/build

@orangemocha
Copy link
Contributor

Thanks for the contribution @gibm . I think Node can use some improvement in this area.

How do you envision the header package being consumed? Are you thinking to extend node-gyp to point to it? Fwiw, I think node-gyp can consume headers/libs from a local build by using the --node-dir option.

One thing that has been discussed is adding the headers/libs to the installer so that node-gyp can find them locally rather than having to download them.

7z a %TARNAME%-headers.tar %TARNAME%
rmdir /s /q %TARNAME%
7z a %TARNAME%-headers.tar.gz %TARNAME%-headers.tar
del %TARNAME%-headers.tar
Copy link
Member

@joaocgreis joaocgreis May 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A zip and 7z would be better (similar to the gz and xz on unix). We did some work to find the best flags for 7z: https://github.com/janeasystems/node/blob/36b525a78119a1fc3af9a6934762b4cee0f78a5c/vcbuild.bat#L243-L251

@gibfahn
Copy link
Member Author

gibfahn commented May 16, 2016

@orangemocha Thanks for the response, I'm currently doing a bit more research into how it all works, and I'll get back to you when I'm more clear.

@gibfahn
Copy link
Member Author

gibfahn commented May 18, 2016

As I understand it the state of headers/node-gyp is as follows:

Unix:

The node-v6.1.0-linux-x64.tar.*z files contain the headers in $NODE/include/node/ , these work for 32 and 64 bit builds. If you install node from source, it will do the same thing as extracting the tar and adding $NODE to the path (i.e. headers in the same place).

If you install the mac pkg presumably the $NODE/include/node/ folder is there as well?

If you don't have the $NODE/include/node/ folder for whatever reason, node-gyp will download node-v6.1.0-headers.tar.gz from nodejs.org, and extract this to ~/.node-gyp/6.1.0/, so your headers are in ~/node-gyp/6.1.0/include/node/. I'm not sure node-gyp actually does this, but maybe it should? Possibly related: nodejs/node-gyp#492.

Windows:

Windows requires both the .h files (what's currently in node-v6.1.0-headers.tar.gz) and a 32 or 64 bit node.lib file, e.g C:\Users\me\.node-gyp\6.1.0\Release\node.lib. There is currently no way of telling which is 64 bit.

The node-v6.1.0-win-x64.7z/zip files proposed in #5995 don't currently include include\node\*.h or a node.lib.

AFAIK by default node-gyp will download the header tarball (on *nix/mac), and also the node.lib file (on windows), or in theory the node.exp file (on AIX) when it is used to build native modules.

@orangemocha Is there a situation in which you would want a header package for a version of node that you didn't have installed? For example can you use 64 bit node to compile 32 bit native modules? Judging by this you can, in which case it would probably be useful to provide both.

Possible changes/fixes

My initial idea would be to do some/all of these. If they make sense, I can add them to this PR.

  • rename the node.lib files to something equivalent to the other platforms (e.g. node-v6.1.0-x64.lib). We should at least include the bitness for the .lib files. If this isn't possible, then maybe put them in /x86/ or /x64/ subdirectories?
  • include the headers package in the msi installer (windows), as suggested in windows: install header files required for compiled addons #859 last year.
  • include the headers package in the pkg installer (mac) if it's not there already
  • make the headers package include the 32bit and 64bit node.lib files. I'd suggest making the makefile target include them if they are in the right place, and not otherwise.
  • change node-gyp to use the installed headers if the version of Node was correct.
Installer/Tarball Sizes
node-v6.1.0-headers.tar.gz                         05-May-2016 22:05            0.5 MB
node-v6.1.0-headers.tar.xz                         05-May-2016 22:05            0.3 MB
node.lib                                           05-May-2016 22:20            0.6 MB
node.lib                                           05-May-2016 22:13            0.6 MB

# Compressed lib files
node.lib.tar.gz                                    05-May-2016 22:20            0.08 MB
node.lib.tar.xz                                    05-May-2016 22:13            0.05 MB
node.lib.zip                                       05-May-2016 22:20            0.08 MB
node.lib.7z                                        05-May-2016 22:13            0.05 MB

node-v6.1.0-darwin-x64.tar.gz                      05-May-2016 21:58            11 MB
node-v6.1.0-darwin-x64.tar.xz                      05-May-2016 21:59             8 MB
node-v6.1.0-linux-ppc64le.tar.gz                   05-May-2016 21:37            12 MB
node-v6.1.0-linux-ppc64le.tar.xz                   05-May-2016 21:37             8 MB
node-v6.1.0-linux-x64.tar.gz                       05-May-2016 21:42            13 MB
node-v6.1.0-linux-x64.tar.xz                       05-May-2016 21:42             9 MB
node-v6.1.0-linux-x86.tar.gz                       05-May-2016 21:41            13 MB
node-v6.1.0-linux-x86.tar.xz                       05-May-2016 21:41             8 MB
node-v6.1.0-x64.msi                                05-May-2016 22:20            11 MB
node-v6.1.0-x86.msi                                05-May-2016 22:13            10 MB
node-v6.1.0.pkg                                    05-May-2016 22:00            14 MB
node-v6.1.0.tar.gz                                 05-May-2016 22:01            20 MB
node-v6.1.0.tar.xz                                 05-May-2016 22:02            12 MB
node.exe                                           05-May-2016 22:20            15 MB
node.exe                                           05-May-2016 22:13            13 MB

Let me know if I've misunderstood anything!

@bnoordhuis bnoordhuis added the stalled Issues and PRs that are stalled. label Aug 22, 2016
@bnoordhuis
Copy link
Member

Adding the 'stalled' label. Please remove again if that is mistaken.

@ncook-hxgn
Copy link

@gibfahn what happened, did something like this ever appear in the node build for windows?
Currently trying to put together a nuget package of Node.lib, Node.dll/.exe, and whichever header files are relevant. I'd like the nuget package to cater to x86 and x64 builds if possible..

I'm having the worst luck with the vcbuild script..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. stalled Issues and PRs that are stalled. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants