urschrei / polylabel-rs

A Rust implementation of the Polylabel algorithm, with FFI.
https://docs.rs/polylabel
Other
54 stars 8 forks source link

Would you consider making cbindgen dependency optional? #20

Closed michaelkirk closed 2 years ago

michaelkirk commented 2 years ago

I have a pure rust project which has no need for the generated bindings.

I'm not familiar enough with cbindgen to know what the conventions are, but I was thinking it might make sense to simply make cbindgen an optional build dependency, and only generate the bindings when the feature is enabled.

Are you amenable to a PR?

For context, what brought me here was I noticed that your (up-to-date!) cbindgen resulted in my project duplicating the dependency. Now, I know I should go hound all the other crates to update their cbindgen, but the only thing better than an up-to-date dependency is no dependency at all!

michaelkirk commented 2 years ago

Ah, actually looking at the code I bit, it seems like you've already started down this road with the headers feature.

I realize the crux is I don't know how (or if it's possible) to actually use something like "optional" build-dependencies... 🤔

e.g. just doing this [target.build-dependencies] cbindgen = { version = "0.22.0", optional = true }

produces output:

error: Package polylabel v2.4.0 (/Users/mkirk/src/georust/polylabel-rs) does not have the feature cbindgen

Presumably because features are only inferred from dependencies not dev-dependencies?

EDIT: This was all due to a local typo. Sorry!

michaelkirk commented 2 years ago

Sheesh, please disregard my previous comment - I had a typo in my Cargo.toml that lead me astray. 🤷

To prove to myself that this was possible, see: https://github.com/urschrei/polylabel-rs/pull/21

urschrei commented 2 years ago

WHAT HAPPENED IN HERE

michaelkirk commented 2 years ago

🙈