search

RustAudio / rust-lv2

A safe, fast, and modular framework to create LV2 plugins, written in Rust
Apache License 2.0
171 stars 23 forks source link

Pregenerated sys #68

Closed JOOpdenhoevel closed 4 years ago

JOOpdenhoevel commented 4 years ago

I've made some modifications to the works of @YruamaLairba's #64. In the meantime, some of the changes were also made to #64, but there are still some that can not be found there.

The most important change is the way different bindings are placed in the src folder: Now, there are different bindings for different targets, which are chosen using conditional compilation. This also means that the "style" of target support is different: We set up the project in a way that certain targets work. Others might work too, but they aren't tested and therefore not supported. I've managed to integrate the following targets in the following bindings:

The only missing Tier 1 targets are x86_64-pc-windows-gnu and i686-pc-windows-gnu since I wasn't able to set up Rust and Clang for them. I'm guessing that they will be either unique or the same as the unix bindings and would, therefore, suggest to restructure the src folder accordingly.

There is still some documentation to be done and it would also be good to do more integration testing and automation, but in the long run, this might be the most maintainable path to go.

I've also done some minor things:

YruamaLairba commented 4 years ago

Seems we progressed concurrently :/ I have some questions:

JOOpdenhoevel commented 4 years ago

does ordering header really allow to produce same bindings order across platform ? Does it really allow to compare with a diff?

Yep, that's the intention and that's how it works. Why didn't you just try it? :wink:

are unix x86_64 and i686 bindings really different ?

Yes, for some reason they are: In one instance, struct alignment is notated differently and in another one, a libc type is interpreted differently. However, I wonder how thinks look for other BSDs! :sweat_smile:

is there differences other than enum sign between windows and unix ?

Again, a libc type is also interpreted differently, but from my point of view the different enum types are enough reason to use different bindings for Unix and Windows.

for the different platform, did you use `--target' option of clang or you did it on a dedicated host ?

I've used a Linux host (My workstation), a Windows host (Also my workstation) and a macOS host (The Github Actions Runner) to generate the bindings. All of them are and were x86_64 machines, and I've also used them to generate the x86-versions of their native targets.

Some proper deployment pipeline would be the most clean solution, but I haven't managed that for all targets.

YruamaLairba commented 4 years ago

i looked to your job, compared bindings and did some tests:

JOOpdenhoevel commented 4 years ago

having the possibility to compare with [vim]diff is awesome!

I'm glad you like it :grin:

funny fact, a binding generated from my x86_64 linux have some subtle irrelevant difference from yours. The most noticeable is some unused type in yours that don't appears in mine.

I'm guessing it's because we might use different versions of glibc and clang. The used software should definitely be documented too!

why there is an "alignment hint" on union ? doesn't #[repr(C)] already do this job ?

IDK, maybe they're playing safe? :confused: They probably know better than we do!

for va_list, the underlying structure is unspecified by C so i think we should instruct bindgen to make it opaque. EDIT: tryed it, don't solve anything, it's even worst.

Yes, that whole va_list thing feels wrong. I did a bit of research and found out that the whole logging feature relies on variadic, printf-style functions, which aren't stable yet. Therefore, we could consider to ignore the whole logging specification for now, which would also resolve these problems. I know, this is kind of lighthearted and the logging feature is quite useful, but the whole variadic function semantic isn't even documented yet!

even if we need to wait to have the "x86_64-pc-windows-gnu" bindings, and even it's doesn't matter, I think "msvc" and "gnu or unix" are more accurate condition for selecting the right bindings than "windows" or "unix".

I've made this distinction because there are the unix and windows fields available for conditional compilated, which fit for now. Please wait for the GNU-Windows bindings before making any judgements!

Quite a bit of work has been poured into this PR now and it contains most of the tools to generate bindings as well as bindings for the most important development platforms. Therefore, it might be good to pull it know and then work on documentation, proper binding deployment and validation, in parallel. I would love to use your binding validation tools, but I don't like cross-PR merging and would therefore need to pull this one first. Would that be okay for you?

YruamaLairba commented 4 years ago

@Janonard did you see my pull request on your personnal repo ? It's needed for cross-compiling bindings generation.

For union alignment, i checked the reference, i confirm, #[repr(C)] already do the job. The hint may here for non-trivial case.

For the logger spec, calling an extern C variadic is stable, so we can implement something. But i'm ok to remove logger from sys bindings and eventually have a separate bindings for it.

For conditionnal, to use it with "msvc" or "gnu", it look like:

#[cfg_attr(target_env = "gnu", path = "unix/mod.rs")]

I tested binddgen for windows-gnu targets, i686 and x86_64 are identical, but doesn't look exactly like msvc or linux target.

More generally, i was able to generate several bindings using cross compiling. I'm not 100% sure of the result, but the only difference is va_list definition: it's always a pointer to something, but this something often is different.

JOOpdenhoevel commented 4 years ago

More generally, i was able to generate several bindings using cross compiling. I'm not 100% sure of the result, but the only difference is va_list definition: it's always a pointer to something, but this something often is different.

Yes, and I don't know how we could create some proper library support for that. However, we can definitely use the simpler printf function and therefore have some logging support, after all. All things considered, I now think that we should leave the logging spec in and consider bindings as different if their va_list is different.

It's really bothering me that we have such a hard time properly validating the bindings. Clang can generate different bindings for different platforms, sure, but it always uses the headers that are installed locally and therefore, a binding is only really valid if it was generated by the targeted system.

Supporting a target is a big statement and I don't want to be held responsible if a target is broken which I assumed as working. Therefore, this problem got me into playing safe: I will only support targets I actually have access to and that work for me. Currently, this is only Linux and I am working on getting MSVC and GNU Windows to work. This, however, means that I need to drop the macOS support, since I don't own a Mac. I know, this breaks my requirement that the replacement should support all Tier 1 targets, but I didn't knew of these validation problems when I made it. I hope you understand my decision.

This means that I've now switched the Unix bindings to Linux only, added an experimental feature for, well, experimental targets, and gated the windows targets behind it. I don't make a difference between msvc and gnu yet since this difference isn't officially known yet and this is going to stay that way until we know more.

I'm also going to merge this pull request now so we can do other work with it.