feat: support build and link dynamic library #1444

westhide · 2025-03-30T06:56:59Z

Close shared_flag and static_flag don't work

NobodyXu

Thank you!

There are two places where I would like to see some changes

NobodyXu · 2025-03-30T10:35:40Z

src/lib.rs

+        if self.link_shared_flag {
+            let objects = objects.iter().map(|o| o.dst.clone()).collect::<Vec<_>>();
+
+            let mut command = self.try_get_compiler()?.to_command();
+            let cmd = command
+                .args(["-shared", "-o"])
+                .arg(dst.join(dynlib_name))
+                .args(&objects);
+            run(cmd, &self.cargo_output)?;
+        }
+
+        self.assemble(lib_name, &dst.join(static_name), &objects)?;


I think we don't need to run assemble if it's already dynamically linked?

Should we consider support dll on windows?

I think we don't need to run assemble if it's already dynamically linked?

Sure, fixed.

Should we consider support dll on windows?

Can you elaborate on that please?

Windows ddl is complicated, i have not idea how to add a dynamic library test case for it. It's weird as cl.exe require a main function to build a dll library.

With this source file, it will cause a Entry Point Not Found error

#include <windows.h> void msvc() {}

Test pass if add a main function

#include <windows.h> void msvc() {} void main() {}

Thanks for explanation, I'm ok with it not tested, if you have time to add another test for it I would greatly appreciate

tests/test.rs

src/lib.rs

tests/test.rs

NobodyXu

Thanks LGTM!

cc @madsmtm can you review this PR please?

madsmtm · 2025-04-01T14:50:22Z

src/lib.rs

+    /// Configure whether cc should build dynamic library and link with `rustc-link-lib=dylib`
+    ///
+    /// This option defaults to `false`.
+    pub fn link_shared_flag(&mut self, link_shared_flag: bool) -> &mut Build {


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

Stated another way: When would you ever want shared_flag(true) but not link_shared_flag(true)? And vice-versa?

madsmtm · 2025-04-01T14:53:40Z

src/lib.rs

+    /// Configures the output directory where shared library will be located.
+    ///
+    /// This option defaults to `out_dir`.
+    pub fn shared_lib_out_dir<P: AsRef<Path>>(&mut self, out_dir: P) -> &mut Build {
+        self.shared_lib_out_dir = Some(out_dir.as_ref().into());
+        self
+    }


I'm against adding this, I think the user should copy it from out_dir if they want the final shared library to be placed elsewhere.

madsmtm · 2025-04-01T14:58:05Z

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 and Fails to compile with nvcc when passing RUSTFLAGS='-C target-feature=+crt-static' #772). I'd be more in favour of deprecating shared_flag, and instead recommending a linker-rs crate or similar.

westhide · 2025-04-04T06:21:12Z

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.

madsmtm · 2025-04-04T10:41:36Z

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.

NobodyXu · 2025-04-04T12:04:43Z

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

westhide · 2025-04-04T12:46:39Z

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.

madsmtm · 2025-04-04T13:08:16Z

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 · 2025-04-04T13:10:50Z

tests/test.rs

-            .target(&target)
-            .host(&target)
+            .target(target)
+            .host(target)
            .file("foo.c")
            .compile("foo");



Documentation on compile should also be updated to reflect the new renaming rules.

madsmtm · 2025-04-04T13:17:21Z

tests/test.rs

-            .target(&target)
-            .host(&target)
+            .target(target)
+            .host(target)
            .file("foo.c")
            .compile("foo");



If we actually want to support shared libraries, we should consider a .linker and .get_linker similar to .archiver and .get_archiver, as well as maybe allowing overriding the linker with an LD env var (?)

Though if we allow using a raw linker (e.g. invoke rust-lld binary instead of cc), we're getting into hairy territory of needing to know how to invoke the linker if it's not a compiler driver.

(Which again is why I'm really not in favour of it).

westhide added 2 commits March 30, 2025 14:55

feat: support build and link dynamic library

21f2b00

fix: cargo clippy lint

1e8de34

NobodyXu requested changes Mar 30, 2025

View reviewed changes

feat: support build and link dynamic library

f9e4a70

westhide force-pushed the main branch from e60d734 to f9e4a70 Compare March 30, 2025 16:01

NobodyXu requested changes Mar 31, 2025

View reviewed changes

src/lib.rs Show resolved Hide resolved

tests/test.rs Show resolved Hide resolved

westhide force-pushed the main branch 7 times, most recently from f9dead6 to f9e4a70 Compare March 31, 2025 16:15

feat: support build and link dynamic library

a6da499

NobodyXu approved these changes Apr 1, 2025

View reviewed changes

madsmtm requested changes Apr 1, 2025

View reviewed changes

madsmtm reviewed Apr 4, 2025

View reviewed changes

madsmtm mentioned this pull request Apr 4, 2025

shared_flag and static_flag don't work #594

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support build and link dynamic library #1444

feat: support build and link dynamic library #1444

westhide commented Mar 30, 2025

NobodyXu left a comment

NobodyXu Mar 30, 2025

westhide Mar 30, 2025

westhide Mar 30, 2025

NobodyXu Mar 31, 2025

westhide Mar 31, 2025 •

edited

Loading

NobodyXu Apr 1, 2025

NobodyXu left a comment

madsmtm Apr 1, 2025

madsmtm Apr 1, 2025

madsmtm Apr 1, 2025

madsmtm commented Apr 1, 2025 •

edited

Loading

westhide commented Apr 4, 2025 •

edited

Loading

madsmtm commented Apr 4, 2025

NobodyXu commented Apr 4, 2025 •

edited

Loading

westhide commented Apr 4, 2025 •

edited

Loading

madsmtm commented Apr 4, 2025 •

edited

Loading

madsmtm Apr 4, 2025

madsmtm Apr 4, 2025

madsmtm Apr 4, 2025

feat: support build and link dynamic library #1444

Are you sure you want to change the base?

feat: support build and link dynamic library #1444

Conversation

westhide commented Mar 30, 2025

NobodyXu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westhide Mar 31, 2025 • edited Loading

Choose a reason for hiding this comment

With this source file, it will cause a Entry Point Not Found error

Test pass if add a main function

Choose a reason for hiding this comment

NobodyXu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madsmtm commented Apr 1, 2025 • edited Loading

westhide commented Apr 4, 2025 • edited Loading

madsmtm commented Apr 4, 2025

NobodyXu commented Apr 4, 2025 • edited Loading

westhide commented Apr 4, 2025 • edited Loading

madsmtm commented Apr 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westhide Mar 31, 2025 •

edited

Loading

With this source file, it will cause a `Entry Point Not Found` error

Test pass if add a `main` function

madsmtm commented Apr 1, 2025 •

edited

Loading

westhide commented Apr 4, 2025 •

edited

Loading

NobodyXu commented Apr 4, 2025 •

edited

Loading

westhide commented Apr 4, 2025 •

edited

Loading

madsmtm commented Apr 4, 2025 •

edited

Loading