Adapt to new rustc-cdylib-link-arg parameter

[lrascao]

After rust-lang/rust/pull/36574 has been merged the latest Rust
version 1.49 breaks the linking of dynamic libs in MacOS. These
changes ensure that the extra link args are applied only at the
dynamic lib generation linking step.

[lrascao]

This depends on rusterlium/erlang-cargo#9, once that's merged and published to hex this one can updated

[filmor]

Do you have an example of the breakage? The PR you are mentioning was merged over 5 years ago. I would really like to prevent the necessity of a build.rs if in any way possible, and I don't really see how that differs from passing RUSTFLAGS in the environment.

[lrascao]

ah right, i've probably linked the wrong one, can't seem to find it now..
this is the error i observed after bumping rust to 1.49 on a nif test project

8d [:~rustler_test] master+* 2s 1 ± rebar3 compile
===> Analyzing applications...
===> Compiling cargo
===> Compiling rebar3_cargo
===> Analyzing applications...
===> Compiling cargo
===> Compiling rebar3_cargo
===> Verifying dependencies...
   Compiling proc-macro2 v1.0.10
   Compiling unicode-xid v0.2.0
   Compiling syn v1.0.18
   Compiling void v1.0.2
   Compiling rustler_sys v2.1.0 (https://github.com/rusterlium/rustler?branch=master#d1764588)
   Compiling unicode-segmentation v1.6.0
   Compiling rustler v0.22.0-rc.0 (https://github.com/rusterlium/rustler?branch=master#d1764588)
   Compiling lazy_static v1.4.0
   Compiling queues v1.1.0
   Compiling unreachable v1.0.0
   Compiling heck v0.3.1
   Compiling quote v1.0.4
   Compiling rustler_codegen v0.22.0-rc.0 (https://github.com/rusterlium/rustler?branch=master#d1764588)
   Compiling rustler_test v0.1.0 (/Users/luis/Projects/rust/rustler_test/native)
dyld: lazy symbol binding failed: Symbol not found: __ZN5alloc3vec12Vec$LT$T$GT$3new17hb4372ec879f1f33bE
  Referenced from: /Users/luis/Projects/rust/rustler_test/_build/default/lib/rustler_test/target/debug/deps/librustler_codegen-7ce2b29f8f8a34c9.dylib
  Expected in: flat namespace

dyld: Symbol not found: __ZN5alloc3vec12Vec$LT$T$GT$3new17hb4372ec879f1f33bE
    Building [=====================================================>   ] 21/22: rustler_test                                                                                                                                                                                                                                                                  error: build failed
===> Uncaught error in rebar_core. Run with DIAGNOSTIC=1 to see stacktrace or consult rebar3.crashdump
===> When submitting a bug report, please include the output of `rebar3 report "your command"`

[belltoy]

@lrascao Would you update this MR?

[lrascao]

👍🏻 will do

[davisp]

@lrascao @belltoy The build.rs approach can be avoided by using the .cargo/config.toml approach that @belltoy suggested here: rusterlium/erlang-cargo#9 (comment)

Also of note, the Symbol not found: __ZN5alloc3vec12Vec$LT$T$GT$3new17hb4372ec879f1f33bE error still happens when using the -flat_namespace -undefined suppress arguments. Switching the erlang-cargo app to include rusterlium/erlang-cargo#9 and specifying -undefined dynamic_lookup via .cargo/config.toml is all that I need. Although I can't quite figure out why that is. I can manually get things to link properly specifying the arguments to cargo directly via cargo rustc. My only hunch is that there's a slight difference on how those arguments are passed when compiling dependencies since the undefined Vec::new symbol is showing up in rustler_codegen.

[davisp]

That is to say, I can get things linked correctly by specifying -flat_namespace -undefined suppress if I pass them via cargo rustc --codegen link-arg=....

I also noticed the switch from dylib to cdylib in this PR. I didn't need that change with -undefined dynamic_lookup so that could also be part of the difference.

[davisp]

If anyone cares, here's my minimal working example that works on macOS without issue:

https://github.com/davisp/rust-nif-examples

Once rebar3_cargo is updated I'll remove the hacky forked version.

[belltoy]

Thank you @davisp for pointing out these.

Actually, there are no too many differences between dylib and cdylib for Erlang NIF lib linkage and usage, but during the intermediate link stages for cargo and rustc. So in theory, we can use both dylib and cdylib to build a NIF .so.

On macOS, we need to tell the rust compiler the additional link args, via one of three methods:

• The build.rs file in the root of crate directory
• The .cargo/config.toml config file or the corresponding command line environments
• Directly tell rustc in command line via cargo rustc -- -C link-args=...

The differences of all these three methods are:

• build.rs may have too invasive or too heavy for the NIF crate
• The command line env or additional args are too trivial for typing
• The TOML config file inside a hidden dot directory is the ideal ergonomics choice

I think the choice should let the NIF author make.

But as @davisp mentioned, Symbol not found error still happens when using the flat_namespace args. That's because a bug of link-arg propagates transitively in cargo I think.

If use TOML config or RUSTFLAGS env variables to specify the link args, these args will pass to all dependencies build phases, which will cause failure at the final target NIF crate linking if use flat namespace option.

Notice that the println!("cargo:rustc-cdylib-link-arg=..."); in build.rs is only works for cdylib crate type.

The Cargo Book

The rustc-cdylib-link-arg instruction tells Cargo to pass the -C link-arg=FLAG option to the compiler, but only when building a cdylib library target. Its usage is highly platform specific. It is useful to set the shared library version or the runtime-path.

So if you use this, you should set the crate type to cdylib, which is the reason why the crate type changes in the test SUITE in this PR.

The best choice for now in my opinion is to use the new cdylib crate type, and choose dynamic_lookup on macOS, put this config in the .cargo/config.toml file, for more convenience in the daily dev env or CI env. Use additional CLI args for rustc if you want to use the flat namespace link.

[belltoy]

How is going?

[lrascao]

sorry, not enough bandwidth to pick this up, feel free to take over if it's urgent to you