search

georust / geos

Rust bindings for GEOS
https://docs.rs/geos/
MIT License
122 stars 41 forks source link

Guidance for updating geos-sys? #111

Closed brendan-ward closed 2 years ago

brendan-ward commented 2 years ago

I'd like to use some of the new features coming in GEOS 3.11, but the process for updating the raw C to Rust bindings isn't clear. Apologies if there is documentation or an issue or PR that demonstrates this; I couldn't find anything.

It looks like the sys/geos-src/source points at a fairly recent commit of GEOS source tree, and can be updated via git submodules. Great!

However, it looks like the sys/check_missing/geos_c.h is an old copy of capi/geos_c.h.in from the GEOS source tree (includes template parameters for version numbers, etc), rather than the geos_c.h produced during a build of GEOS. It seems like sys/check_missing/check_missing.py should instead use the capi/geos_c.h.in from the GEOS source tree instead of an old copy, or that when a new version of GEOS is bound, that the compiled geos_c.h file (including version numbers) is added here instead.

It looks like sys/check_missing/check_missing.py is used to identify functions available in geos_c.h that haven't yet been bound into sys/src/functions.rs. It isn't clear how entries are added there. Is this done manually? And then version-guarded using #[cfg(feature = "v<GEOS version that enables API>")]? (and associated feature added to sys/Cargo.toml). I'm assuming there is a good reason for doing so rather than using bindgen like in georust/gdal to pregenerate bindings for specific versions of GEOS.

Sorry if these have obvious answers; I'm pretty new to Rust so these weren't obvious to me.

GuillaumeGomez commented 2 years ago

capi was added afterward for static linking. The .in isn't supposed to be used as is I think, so if you want to add bindings for a newer GEOS version, the best is to simply copy the new geos_c.h into the check folder and add what's missing.

It looks like sys/check_missing/check_missing.py is used to identify functions available in geos_c.h that haven't yet been bound into sys/src/functions.rs. It isn't clear how entries are added there. Is this done manually? And then version-guarded using #[cfg(feature = "v<GEOS version that enables API>")]? (and associated feature added to sys/Cargo.toml). I'm assuming there is a good reason for doing so rather than using bindgen like in georust/gdal to pregenerate bindings for specific versions of GEOS.

It is bound manually. bindgen brings some issues on its own. For example, some types won't have the same Rust equivalent depending which target you're on. Also (please correct me if I'm wrong), it doesn't use libc by default, which isn't great.

It was definitely not obvious, I'll send a PR to update the sys documentation to make it clear.

brendan-ward commented 2 years ago

Thank you for adding more documentation so quickly!