KWARC / rust-libxml

Rust wrapper for libxml2
https://crates.io/crates/libxml
MIT License
73 stars 38 forks source link

Move bindgen bindings into separate libxml-sys crate #76

Open lweberk opened 4 years ago

lweberk commented 4 years ago

So downstream crates that enhance the rust-libxml wrapper link against the same bindings instead of parallel ones, and eventually enable libxml2 vendoring.

Would benefit siblings like xmlsec greatly, since it allows it to link against the C library through the same bindings.

What are your thoughts about this?

dginev commented 4 years ago

Ties into #73 of which I was recently made aware of. I have a general take that I am quite happy to merge PRs with such upgrades, but that I likely won't have the time to properly develop and test on several architectures, which is what one needs to do to make sure the multiple bindings work as advertised.

If you think the complexity of shipping multiple bindings warrants a standalone crate, I would be tempted to believe that. Can still be hosted in this repository, though if someone feels strongly about maintaining the bindings completely separately from the rust-libxml repository, I am open to that as well.

Strongly in agreement that supporting multiple architectures should happen, just not in a position to contribute it myself in the immediate future.

lweberk commented 4 years ago

Our main angle is making dependence on libxml easier for xmlsec, by sharing the underlying API and thus linking through the same interface. That decouples xmlsec's version from the libxml as far as the low level stuff goes, and leaves only the pure Rust stuff to handle. The former would result in linkage errors, the latter in compilation errors. The second option is way safer and easier to handle.

That said, –at first glance– it would also be beneficial for #73 .

As for implementation, let me look around and learn a little from other crates that do this (there are plenty), specially the ones that do library vendoring across platforms. Regardless of what we manage to come up with at our end, we would also not go as far as to start implementing cross-platform support. Its not our particular itch and we are pressed on resources as it is.

lweberk commented 4 years ago

Is there any source of the headers you are currently including for the bindings generation?

lweberk commented 4 years ago

Nevermind... missed it since it is still a PR

dginev commented 4 years ago

Right, sorry about that - the PR contains the recipe for running bindgen, but simply running the code there isn't complete, as there is also some cleanup needed to restore the repository back to a state where no "helpers" exist in the auto-generated bindings.rs file.

Since we only ever ran bindgen very early in the wrapper lifecycle, we have been somewhat careless on that front. Hopefully it's just a minor pain of a cleanup.