-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Put bindgen behind a seperate feature #191
Put bindgen behind a seperate feature #191
Conversation
I don't know whether there's a prevailing best practice when it comes to sys crates, but my assumption has always been that the crate defaults to trying to use system libraries and the bundled library is an option / fallback. There's also a technical question around the bundled bindings: do they work correctly cross-platform? on 32-bit vs 64-bit? |
At least most of the Examples:
Some of them provide the possibility to use bindgen at build time to build your own bindings instead.
That depends on the header as far as I can tell. If it's written in a cross-platform compatible way it does work otherwise there might be issues for 32-bit vs 64bit or for windows vs linux or … Some of the crates above handle that by just providing different bindings for different platforms and conditionally including the right bindings for the target. Other provide the some feature flag to run bindgen at buildtime for that, and others do both. |
Is there any interest in moving this PR forward from the relevant maintainers? |
7446119
to
1d1a3e9
Compare
@urschrei @michaelkirk @lnicola What is required to move this forward? |
In I don't know the ABI compatibility story of GDAL, but I doubt that they are backwards-compatible. |
This commit changes proj-sys to not depend on bindgen by default as that introduces a quite heavy build time dependency (libclang) which might not be there on all systems. Instead bundled bindings for the proj version build by the `bundled_proj` feature are provided.
1d1a3e9
to
70c5fea
Compare
@urschrei @michaelkirk @lnicola This is another gentle ping if there is any interest in moving this forward |
@michaelkirk I'm inclined to accept this: libproj doesn't update often enough for this to cause maintainer overhead of a magnitude that would concern me (we may need to add some docs on how to do it though). WDYT? |
LGTM. but it would be nice to add a line to the README mentioning the version of
I think we want I'd also prefer |
In theory I'm on board. Can you also include instructions and/or a script for the maintainers to run when we want to refresh the bindings?
Nice - I didn't know about this flag.
My estimation is that https://github.com/search?q=%22buildtime_bindgen%22&type=code |
I've pushed e381d58 to add a |
Seems like CI failed due to a container failure: https://github.com/georust/proj/actions/runs/9495284106/job/26167576710?pr=191 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we surface this buildtime_bindgen feature in the proj
crate as well?
@michaelkirk its certainly possible to add that as well if that's wanted by the maintainers. That written as a more general note: It becomes really hard to keep this going due to how the communication from your side. Everytime everything seems to be resolved things stale for longer time and after that someone comes back with new requests. This PR is now open for nearly a half a year. While I can understand that time for maintaining crates is something that is always scarce, I do not think that this change is that large or complicated that it requires that much time to review and decide what to do. Given the experience of my contributions here and for the gdal bindings as well I seriously consider if it might be a better idea for us to just drop the whole thing and have our own more minimal version of that internally. After all it does look to me that you are either not interested in contributions from the outside or that you just do not have any capacity for maintaining left. Neither is a good sign for something to depend on. |
Speaking purely for myself, I can understand your frustration. However, I'm time-constrained (and so is every other maintainer of this repository) and I am not in a position to change that. As you're no doubt aware, the problem is inherent to open source. That having been said, I am not interested in entertaining passive-aggressive "thinking out loud" comments about what you should do. We are not responsible for your pressing issues, and it is unhelpful for you to express your frustration in this way, given its systemic causes. You've proposed a number of fairly large changes to the build system, and we're in the process of accepting them. You are more than welcome to help us to maintain the two libraries if you feel you have time and that you can work with us. If you'd prefer not to, you can either make your own arrangements, or wait for your changes to land. |
To be clear here: My main criticism is not that it takes time. That's ok, although half a year is quite a bit of time. My main criticism is that you seemingly approve changes, but do not merge the and if I ask for any progress weeks late you (as some of the maintainers) come up with yet another thing to discuss or yet another change to make. As someone that maintains several crates large than any of the gdal related crates: The only thing you manage to do with this moving goalpost behaviour is to scare away people that might help you. You as a maintainer team need to find a better solution to that! This is especially frustrating for features like the gdal bundling feature where I reached out quite a bit beforehand to see if you would be interested in that feature and to get some feedback of the requirements. |
My understanding is that most people who use proj in rust are using the |
Cargo unifies crate features across your dependency tree. That means a user can always add a dependency on proj-sys as well and enable the feature there. For sys crates cargo will also ensure that it only uses a single version of that crate. I personally prefer keeping features like that one on the sys crate only as I expect that it will only be used by a rather small fraction of potential users (Mainly distro-maintainers that want to ensure that the bindings match the packaged version?). |
This commit changes proj-sys to not depend on bindgen by default as that introduces a quite heavy build time dependency (libclang) which might not be there on all systems. Instead bundled bindings for the proj version build by the `bundled_proj` feature are provided.
This commit changes proj-sys to not depend on bindgen by default as that introduces a quite heavy build time dependency (libclang) which might not be there on all systems. Instead bundled bindings for the proj version build by the
bundled_proj
feature are provided.