georust / proj

Rust bindings for the latest stable release of PROJ
https://docs.rs/proj
Apache License 2.0
146 stars 46 forks source link

include pre-compiled headers, bindgen only as optional feature #44

Open michaelkirk opened 4 years ago

michaelkirk commented 4 years ago

bindgen accounts for the vast majority of the remaining deps for proj. Since they are build_dependencies they don't contribute to the size of the output binary, but it does take time to compile.

It seems like a popular approach is to bake in the prebuilt bindings, but then allow the user a way to manually generate via a feature flag.

e.g. rustsqlite bundled bindings: https://github.com/rusqlite/rusqlite/tree/master/libsqlite3-sys/bindgen-bindings

zstd

gdal bundled bindings: https://github.com/georust/gdal/tree/master/gdal-sys/prebuilt-bindings

So achieving this would entail at least:

  1. baking in bindings for the supported versions
  2. including the proper one in build.rs
  3. adding a bindgen feature maintains the current behavior.
  4. ensuring there is tooling/documentation for maintainers to easily add new pre-baked bindings as new proj versions come out.

Would you be interested in merging such a thing?

urschrei commented 4 years ago

Yes! The only concern I have is around cross-platform / cross architecture interaction. Are we sure that bindgen generates the same bindings for e.g. PJ_DIRECTION on x86_64 macOS as it does for e.g. aarch64 linux (or even just x86_64 linux) – we would need to establish that first.

urschrei commented 4 years ago

I did some digging at the time, and I think there's no issue around generated types differing btw.

michaelkirk commented 4 years ago

Nice! Yeah, it seems common enough practice. I'll probably generate the headers on ubuntu though (maybe on the ci-docker container).

It'd be nice to script, or at least document, the process.

It's next on my list after #41