cc-rs icon indicating copy to clipboard operation
cc-rs copied to clipboard

feat: support build and link dynamic library

Open westhide opened this issue 9 months ago • 6 comments

Close shared_flag and static_flag don't work

westhide avatar Mar 30 '25 06:03 westhide

In general, I'm a bit inclined to think that -shared is an anti-feature of cc-rs, since:

  • You never want this in a build.rs script, since then you need to distribute the shared library.
  • Outside of build.rs it may be valuable, but it also introduces a big cost, since now cc-rs has to know about linker flags as well (see e.g. https://github.com/rust-lang/cc-rs/pull/1424 and https://github.com/rust-lang/cc-rs/issues/772). I'd be more in favour of deprecating shared_flag, and instead recommending a linker-rs crate or similar.

madsmtm avatar Apr 01 '25 14:04 madsmtm

In general, I'm a bit inclined to think that -shared is an anti-feature of cc-rs, since:

  • You never want this in a build.rs script, since then you need to distribute the shared library.
  • Outside of build.rs it may be valuable, but it also introduces a big cost, since now cc-rs has to know about linker flags as well (see e.g. Fix wasm32-unknown-unknown by passing -c #1424). I'd be more in favour of deprecating shared_flag, and instead recommending a linker-rs crate or similar.

Should I close this PR and use another crate to support dynamic linking? The purpose of submitting this commit was to implement a C/C++ UDF with Rust's compute engine. It's fine for me to just call a bash command in build.rs.

westhide avatar Apr 04 '25 06:04 westhide

The purpose of submitting this commit was to implement a C/C++ UDF with Rust's compute engine. It's fine for me to just call a bash command in build.rs.

Hmm, I guess I don't understand why that has to be dynamically linked?

Should I close this PR and use another crate to support dynamic linking?

We haven't actually decided to deprecate .shared_flag yet, so I'm still unsure what our recommendation should be.

madsmtm avatar Apr 04 '25 10:04 madsmtm

I think this is mostly an API issue, since it doesn't add a lot of code we just want the right API

NobodyXu avatar Apr 04 '25 12:04 NobodyXu

When reading this as a user, it is not really clear how this differs from shared_flag.

I had the same confusion before I forked cc-rs and read the code. For most users' expectations, shared_flag should behave like dynamic linking. However, currently, shared_flag only adds the -shared flag while still linking statically. Since I didn’t want to introduce a breaking change, I added the link_shared_flag function to properly support dynamic linking.

Hmm, I guess I don't understand why that has to be dynamically linked?

In our use case, we have C/C++ UDF code that changes frequently. Static linking isn’t ideal because users would need to download and update the entire binary every time the UDF code changes, which can be time-consuming. With dynamic linking, as long as we keep the Rust FFI interface stable, users only need to:

  • Update the latest UDF .so file,
  • Reset the LD_LIBRARY_PATH environment variable.

This approach significantly reduces update overhead.

westhide avatar Apr 04 '25 12:04 westhide

Since I didn’t want to introduce a breaking change, I added the link_shared_flag function to properly support dynamic linking.

I don't think this should really be seen as a breaking change, since .shared_flag(true) is broken today anyhow.

Alternatively, we deprecate both .shared_flag and .static_flag, and introduce .shared or .dynamic.

In our use case, we have C/C++ UDF code that changes frequently. Static linking isn’t ideal because users would need to download and update the entire binary every time the UDF code changes, which can be time-consuming. With dynamic linking, as long as we keep the Rust FFI interface stable, users only need to:

  • Update the latest UDF .so file,

  • Reset the LD_LIBRARY_PATH environment variable.

This approach significantly reduces update overhead.

Thanks for the context. If we go forwards with this PR, would you mind adding something to this effect to the method documentation, as a use-case example?

madsmtm avatar Apr 04 '25 13:04 madsmtm