Skip to content

Link libuv & zlib with --whole-archive #17444

Closed
Closed
Link libuv & zlib with --whole-archive#17444
Labels
buildIssues and PRs related to Node.js builds or CI infrastructure.libuvIssues and PRs related to the libuv dependency or the uv binding.zlibIssues and PRs related to the zlib module and its compression dependencies.

Description

@addaleax
addaleax
opened on Dec 4, 2017
  • Version: all
  • Platform: probably not Windows, maybe only Linux
  • Subsystem: deps/libuv

Currently, we link V8 and openssl with -Wl,--whole-archive into the node binary, 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

Activity

  1. addaleax
    added
    buildIssues and PRs related to Node.js builds or CI infrastructure.
    libuvIssues and PRs related to the libuv dependency or the uv binding.
    zlibIssues and PRs related to the zlib module and its compression dependencies.
    on Dec 4, 2017

  2. refack commented on Dec 4, 2017

    @refack
    refack
    on Dec 4, 2017
    Contributor

    This is definatly an interesting suggestion. Some points that come to mind:

    1. For libuv this is probably an easier decision...
    2. Does it affect the binary size?
    3. node implicitly taking responsibility of those libraries' ABI
    4. Is this just a convenience or does it protect addons from dll-hell (linking to two incompatible zlib files for example)?
    5. How does this fit with the N-API roadmap.

    Bottom line, I think I'm +1 on this.

  3. refack commented on Dec 4, 2017

    @refack
    refack
    on Dec 4, 2017
    Contributor

    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-devel

  4. addaleax commented on Dec 5, 2017

    @addaleax
    addaleax
    on Dec 5, 2017
    MemberAuthor

    Does 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. :)

  5. richardlau commented on Dec 5, 2017

    @richardlau
    richardlau
    on Dec 5, 2017
    Member
    • Platform: probably not Windows, maybe only Linux

    FWIW On AIX we already export these symbols (and more) via tools/create_expfile.sh.

  6. mhdawson commented on Dec 12, 2017

    @mhdawson
    mhdawson
    on Dec 12, 2017
    Member

    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.

  7. addaleax commented on Dec 12, 2017

    @addaleax
    addaleax
    on Dec 12, 2017
    MemberAuthor

    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.

  8. mhdawson commented on Dec 13, 2017

    @mhdawson
    mhdawson
    on Dec 13, 2017
    Member

    @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 ?

  9. yhwang commented on Dec 14, 2017

    @yhwang
    yhwang
    on Dec 14, 2017
    Member

    Sure this is what I learned/know so for:
    The obj files that contain the export functions used by node_zlib.cc and uv.cc are included in our node executable. So I guess most of the functions are in. And yes, --whole-archive, force_load could 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-archive and force_load is 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_load and .exp stuffs also need to apply to shared lib as well.

  10. yhwang commented on Dec 17, 2017

    @yhwang
    yhwang
    on Dec 17, 2017
    Member

    Update more findings. Actually, for zlib, all zlib are in the node executable now. the mkssldef target in the node.gyp generates the def file for openssl and zlib. therefore the thing we need to do is add libuv into it.

  11. bnoordhuis commented on Dec 17, 2017

    @bnoordhuis
    bnoordhuis
    on Dec 17, 2017
    Member

    We have mkssldef because openssl and zlib don't export functions inline but libuv does (the UV_EXTERN macro.) Nothing more than --whole-archive should be needed.

  12. 8 remaining items

  13. yhwang
    added a commit that references this issue on Feb 22, 2018

    build: include the libuv and zlib into node

    4aef7df
  14. addaleax
    added 2 commits that reference this issue on Feb 26, 2018

    build: include the libuv and zlib into node

    8beecb2

    build: include the libuv and zlib into node

    f7e1402
  15. MayaLekova
    added 2 commits that reference this issue on May 8, 2018

    build: include the libuv and zlib into node

    52b229b

    doc: update list of re-exported symbols

    ea080ea
  16. addaleax
    added a commit that references this issue on Jun 29, 2018

    build: include the libuv and zlib into node

    4f21b66
  17. rvagg
    added a commit that references this issue on Aug 16, 2018

    build: include the libuv and zlib into node

    0bef960
  18. jasnell
    added a commit that references this issue on Aug 17, 2018

    doc: update list of re-exported symbols

    aaa6fe5
  19. MylesBorins
    added a commit that references this issue on Sep 6, 2018

    doc: update list of re-exported symbols

    c2978ac
  20. abhishekumar-tyagi
    added a commit that references this issue on May 5, 2024

    doc: update list of re-exported symbols

    afe8c56
  21. sjarmak
    added a commit that references this issue on Jul 27, 2026

    build: include the libuv and zlib into node

    d161625
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    buildIssues and PRs related to Node.js builds or CI infrastructure.libuvIssues and PRs related to the libuv dependency or the uv binding.zlibIssues and PRs related to the zlib module and its compression dependencies.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions
      Sponsor
      SponsoredKunjungi sekarang
      Promo