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

Remove cdylib crate-type now that cargo rustc accepts --crate-type #932

Closed
jrose-signal opened this issue Feb 14, 2023 · 1 comment · Fixed by #933
Closed

Remove cdylib crate-type now that cargo rustc accepts --crate-type #932

jrose-signal opened this issue Feb 14, 2023 · 1 comment · Fixed by #933

Comments

@jrose-signal
Copy link
Contributor

sysinfo's Cargo.toml builds as both an rlib and cdylib according to the crate-types in its Cargo.toml. However, when used as a dependency by another Rust target, the cdylib is completely unnecessary; in the best case it adds time to the build, and in the worst case it can actually get in the way of the desired build (if the root build product is a staticlib and the current target isn't set up with a dynamic linker).

This has come up before (#246), but now that Cargo supports overriding crate-type, sysinfo has the option to declare itself as only building an rlib by default, and adding --crate-type cdylib to the instructions for integrating with C instead (or rather the Makefile). Furthermore, it suggests that C integration could use static or dynamic linking, by using --crate-type staticlib instead.

The caveat is that cargo rustc --crate-type was added in Rust 1.61, and this crate's MSRV is 1.59. But since the cdylib will only ever be used by someone building sysinfo directly, I think it's reasonable to bump that particular use case's MSRV-in-practice while still treating the crate's base MSRV as 1.59.

@GuillaumeGomez
Copy link
Owner

I agree. Want to send a PR to remove both crate-type and add instructions about it?

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 a pull request may close this issue.

2 participants