Repository navigation
Link libuv & zlib with --whole-archive #17444
Description
Activity
- addedbuildIssues and PRs related to Node.js builds or CI infrastructure.Issues and PRs related to Node.js builds or CI infrastructure.libuvIssues and PRs related to the libuv dependency or the uv binding.Issues and PRs related to the libuv dependency or the uv binding.zlibIssues and PRs related to the zlib module and its compression dependencies.Issues and PRs related to the zlib module and its compression dependencies.
on Dec 4, 2017 This is definatly an interesting suggestion. Some points that come to mind:
- For libuv this is probably an easier decision...
- Does it affect the binary size?
- node implicitly taking responsibility of those libraries' ABI
- Is this just a convenience or does it protect addons from dll-hell (linking to two incompatible zlib files for example)?
- How does this fit with the N-API roadmap.
Bottom line, I think I'm +1 on this.
P.S. I just saw that the full headers files for both zlib and libuv are in the headers tarball, so this is actually a bugfix.
P.P.S. Relevant also for Windows https://docs.microsoft.com/en-us/cpp/build/reference/wholearchive-include-all-library-object-files where it is probably even more usefull since there is no real
apt-get install zlib-develDoes it affect the binary size?
Probably not all that much – these libs are tiny compared to e.g. V8, and they don’t follow a one-function-per-file pattern.
How does this fit with the N-API roadmap.
Fwiw I raised this issue in response to not seeing the symbols from libuv/libuv#1657 included in the Node binary, which in turn was made with the intention of having libuv be ABI-stable indefinitely.
P.S. I just saw that the full headers files for both zlib and libuv are in the headers tarball, so this is actually a bugfix.
Yeah, exactly. :)
Reacted by Refael Ackermann- Platform: probably not Windows, maybe only Linux
FWIW On AIX we already export these symbols (and more) via
tools/create_expfile.sh.I'm +1 as my assumption is today we just get a semi-random set (those used within Node.js) which does not make sense given we include the full headers.
Btw, I opened this as an issue because I really don’t know enough about GYP to have this figured out myself at this point. I’m convinced it should happen, though.
The obvious approach could be copying what we do for openssl, but that seems like it’s done in a kind of hacky way to begin with.
@yhwang, I know you have been working on the static/shared lib which has required a number of changes to the gyp files and even overcoming issues with symbols not being included. Any chance you could take a look at this issue ?
Sure this is what I learned/know so for:
The obj files that contain the export functions used bynode_zlib.ccanduv.ccare included in our node executable. So I guess most of the functions are in. And yes,--whole-archive,force_loadcould help us to include all functions. In Windows/AIX, we need .exp file to explicitly list the functions. And @richardlau is right, we have that for AIX for now. @digitalinfinity suggested that we should generate .exp for Windows and I agree. We also need to improve the way we generate the .exp.Adding
--whole-archiveandforce_loadis easy in gyp. However, improving the symbol generator for AIX and Windows may need a little more work. Right now, it finds all static archive files and list all symbols inside these static libs. We should explicitly specify what components we want to export and only dump their symbols.I can work on the exp generator after I finish the static/shared lib refine task. Then enable the libuv and zlib would be easy (I guess :-p).
[edit] Forgot to mention that we can also compile node into shared and static lib with current node.gyp.
- for static lib, we only have our node_xxx objs. no other lib
- for shared lib, it would be the same as executable. So those
--whole-archive,force_loadand .exp stuffs also need to apply to shared lib as well.
Update more findings. Actually, for
zlib, all zlib are in the node executable now. themkssldeftarget in thenode.gypgenerates the def file for openssl and zlib. therefore the thing we need to do is add libuv into it.We have mkssldef because openssl and zlib don't export functions inline but libuv does (the
UV_EXTERNmacro.) Nothing more than--whole-archiveshould be needed.8 remaining items
- added a commit that references this issue
on Feb 22, 2018 - added 2 commits that reference this issue
on Feb 26, 2018 - added 2 commits that reference this issue
on May 8, 2018 - added a commit that references this issue
on Jun 29, 2018 - added a commit that references this issue
on Aug 16, 2018 - added a commit that references this issue
on Aug 17, 2018 - added a commit that references this issue
on Sep 6, 2018 - added a commit that references this issue
on May 5, 2024 - added a commit that references this issue
on Jul 27, 2026
Currently, we link V8 and openssl with
-Wl,--whole-archiveinto thenodebinary, but we should probably do the same for libuv and maybe zlib since we want to expose their entire functionality to userland addons./cc @refack @bnoordhuis