qnighy / libwebp-sys2-rs

Rust raw interface to libwebp
BSD 3-Clause "New" or "Revised" License
21 stars 9 forks source link

Switching build.rs dependency management to `system-deps` #4

Open MathieuDuponchelle opened 3 years ago

MathieuDuponchelle commented 3 years ago

Hey, thanks for the crate!

I'm implementing a GStreamer wrapper around it: https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/485 and the maintainer mentioned that the crate unconditionally building its bundled version for msvc / apple / .. was not ideal. He recommended switching to https://docs.rs/system-deps/3.1.0/system_deps/, which also supports falling back to a bundled build, is declarative, and supposedly does The Right Thing(tm) :)

qnighy commented 3 years ago

Thank you for the feedback!

Random thoughts:

I agree that there's room for improvement in build.rs so PRs are welcome.

sdroege commented 3 years ago

I didn't know system-deps. It looks good, but metadeps (the original version?) looks more popular for the time being.

metadeps is unmaintained though, that's why system-deps came into existence :)

Most of system-deps's dependants seem to be from gstreamer-related projects. I'm not 100% sure if there isn't a pitfall if used in non-gstreamer-related projects.

It's mostly used by GStreamer/GNOME-related crates at this point because we were fed up with how every crate had to cargo cult its own build.rs instead of having a declarative way of specifying dependencies that behaves the same between all crates. At this point, it's also used by various crates of the rust-av project and elsewhere, but unfortunately not more widely yet. It's currently a huge problem for distributing crates that they all behave differently with regards to C libraries.

The main problem with it at this point might be that it (for now) only supports pkg-config, as that's the only cross-platform way for finding C libraries. Support for other mechanisms would be nice to add though but ideally would happen by people actually using those to make sure it's done properly.

sdroege commented 3 years ago

The main problem with it at this point might be that it (for now) only supports pkg-config

I should probably add that it also supports building the C library as part of the whole thing instead. See https://github.com/rust-av/dav1d-rs for example.

@gdesmott might have more to add :)