search

Rust-SDL2 / rust-sdl2

SDL2 bindings for Rust
MIT License
2.71k stars 466 forks source link

The sdl2-sys crate is annoying to use #404

Open emberian opened 9 years ago

emberian commented 9 years ago

It has all of these modules and seems to not be generated by bindgen. The "rust best practice" right now is to have the -sys crate have very little structure and be generated from headers automatically.

AngryLawyer commented 9 years ago

Yeah, -sys existed before bindgen worked for Rust. Feel free to create a pull request to improve this, if everyone else agrees it needs to be sorted.

nukep commented 9 years ago

:+1: for automation.

There's functionality missing from -sys, particularly from newer SDL 2.0.x releases. There could even be a chance that some of the definitions are flat out wrong, seeing that they're hand-written. It'd be nice to have this sorted out properly.

My only concern is that bindgen depends on clang. Would this mean that everyone who downloads rust-sdl2 from crates.io needs clang on their system, or would it be "pre-generated" on crates in some way?

emberian commented 9 years ago

Generally, you run bindgen locally and commit that, but that obviously has issues with platform differences and doesn't scale very well.

On Wed, May 20, 2015, at 00:20, Dan Spencer wrote:

:+1: for automation.

There's functionality missing from -sys, particularly from newer SDL 2.0.x releases. There could even be a chance that some of the definitions are flat out wrong, seeing that they're hand-written. It'd be nice to have this sorted out properly.

My only concern is that bindgen depends on clang. Would this mean that everyone who downloads rust-sdl2 from crates.io needs clang on their system, or would it be "pre-generated" on crates in some way?

— Reply to this email directly or view it on GitHub[1].

/cmr

Links:

  1. https://github.com/AngryLawyer/rust-sdl2/issues/404#issuecomment-103750294
flukejones commented 7 years ago

Any status update on this? I might look in to it soon.

Cobrand commented 7 years ago

@Luke-Nukem If you want to take a look in to it, that's fine, but please be aware that it will probably break a lot of crates in the way, and the amount of work required is quite high.

What you will need to do is use bindgen or servo's bindgen.

You will need to delete almost everything from sdl2-sys, generate new headers with bindgen, and then adapt the whole sdl2 crates as well as its siblings (sdl2-ttf, sdl2-image, sdl2-mixer, sdl2-gfx, sdl2-net; even though sdl2-net is not quite required since a lot of crates make networking easier compared to sdl2-net, so I guess you can abandon this crate). Once you adapt everything from these crates so it compiles with your new sdl2-sys, you will need to run all the tests, select a few crates / applications to test that use sdl2, sdl2-image, sdl2-gfx,... and then check that the API is not too broken. If it's only one or two lines to change it's fine, but don't make everyone rewrite all its code that uses sdl2.

As they've talked about this here, you might want to use automation with bindgen, meaning that instead of generating once for all the headers, they will be generated everytime someone tries to compile sdl2-sys.

If you've never done this, I highly recommend you pick a short library with a short API, and try using bindgen with this, and make sure it works as well.

Know that this step (generating from bindgen) is a stepping stone to 1.0.0, so the code review will be very thorough and you must make it work on everything already available (OS X, Linux 32, 64, Win32, Win64, *BSD, ...).

If you still want to do this, then go ahead, we're looking for your PR !

flukejones commented 7 years ago

Thanks for the feedback @Cobrand, I'm still relatively inexperienced with many things, so those are some good insights you've provided. I'll have a crack at it soon and see what I learn from a first attempt.