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

Fixed crate type #358

Merged
merged 1 commit into from
Mar 1, 2019
Merged

Fixed crate type #358

merged 1 commit into from
Mar 1, 2019

Conversation

katyo
Copy link
Contributor

@katyo katyo commented Sep 19, 2018

The proper crate type should be "cdylib" not "dylib" because "dylib" produces an extra dependency from libstd.so which cannot be resolved at runtime in common case.

With "dylib":

$ ldd native/index.node
        linux-vdso.so.1 (0x00007fff3cd1b000)
        libstd-0cce0e0e34e933aa.so => not found
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007ff4cc07c000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007ff4cbcdd000)
        /lib64/ld-linux-x86-64.so.2 (0x00007ff4cc545000)

The "not found" means the problems with module loading by node:

$ node
> require("./")
Error: libstd-0cce0e0e34e933aa.so: cannot open shared object file: No such file or directory
    at Object.Module._extensions..node (module.js:681:18)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Module.require (module.js:596:17)
    at require (internal/module.js:11:18)

With "cdylib":

$ ldd native/index.node                                                                
        linux-vdso.so.1 (0x00007ffcc1dc0000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f6ddae13000)
        librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f6ddac0b000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f6dda9ee000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f6dda7d7000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f6dda438000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f6ddb25c000)
$ nodejs                                                                               
> require("./")
hello node
{}

@dherman
Copy link
Collaborator

dherman commented Jan 11, 2019

@katyo So if I understand correctly, the cdylib approach statically links the Rust std library into the compiled output, right? So this way if you deploy your Neon app on a machine that doesn't have Rust on it, it won't fail at runtime looking for the shared libstd library?

I think having this as an option makes sense. The big question in my mind is which should be the default. The main tradeoff seems to me to be: transparency vs sharing. The benefit of transparency is that deployment machines don't need to have anything special installed on them. The benefits of sharing are decreased filesystem and memory footprint (especially when there are multiple Neon modules in a single app's dependency graph) and ugpradability (e.g. for security fixes to libstd).

In reality, right now I don't think many people are building libraries with Neon, so the question is not urgent. I'd just like to have a good understanding of the tradeoffs for the long term.

@retep998
Copy link
Contributor

retep998 commented Feb 7, 2019

Whether you build as cdylib or dylib is orthogonal to the issue of how libstd is linked. The fact of the matter is that dylib is intended to be consumed as a rust crate while cdylib is intended to be consumed as normal native dynamic library that other languages can call into. dylib has a lot of bloat via rust metdata and exported symbols that is not needed when you're just trying to call into your dynamic library from JS. Please switch to cdylib and never look back.

@dherman
Copy link
Collaborator

dherman commented Mar 1, 2019

Thanks for the explanation, @retep998! @katyo thank you for the PR!

@dherman dherman merged commit eb7359c into neon-bindings:master Mar 1, 2019
@dherman
Copy link
Collaborator

dherman commented Mar 1, 2019

A followup: I talked with Alex Crichton to learn a little more about cdylib. Alex agreed with @retep998's opinion: cdylib is first and foremost the choice you should use whenever building for linking from a non-Rust language.

Alex also added that he believes that cdylib does static linking by default, and he argued that this is what we want. In particular, related to my claim above:

The benefit of transparency is that deployment machines don't need to have anything special installed on them. The benefits of sharing are decreased filesystem and memory footprint (especially when there are multiple Neon modules in a single app's dependency graph) and ugpradability (e.g. for security fixes to libstd).

Alex argued that none of the benefits of dynamic linking are actually there for the Rust stdlib, for several reasons:

  • There's no distribution mechanism for shipping updated DLLs of the Rust stdlib.
  • The Rust ABI is not stable so you'd have to recompile to work with an upgraded DLL anyway.
  • The size issue is probably not big enough to worry about.

I find this pretty compelling. Just thought I'd note that here for posterity.

splix added a commit to emeraldpay/emerald-vault-node that referenced this pull request Oct 23, 2019
kdy1 added a commit to swc-project/node-swc that referenced this pull request Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants