AltF02 / x11-rs

Rust bindings for X11 libraries
https://docs.rs/x11
MIT License
204 stars 66 forks source link

Support libXrandr 1.4.2 #35

Closed jwilm closed 8 years ago

jwilm commented 8 years ago

LTS distros such as ubuntu 14.04 don't have an easy way to run libXrandr 1.5 on their system. Linking x11-dl's xrandr module fails on such systems. I'm not sure of a great solution to support both versions. One option would be feature flags, but that gets cumbersome as the number of releases increases. Custom attributes could be used on nightly (eg #[xrandr(1.5)]), but that's not a very general solution. A third option would be splitting this into several repos which could track their libXwhatever directly. Admittedly, none of these solutions are great. In the mean time, I'm using a fork which drops monitor support to work with 1.4.2.

ghost commented 8 years ago

I think the best solution is either a cargo feature for version 1.5 (which is enabled by default to prevent from breaking semver) or a separate struct which omits the new functions. I've been wanting to update x11-dl to version 3 by generating code from the C headers, but I still have some planning to do. I'm a little worried about the compile time and generated code size of x11-dl, and I'm trying to find a solution to this as well as some other problems in the meantime.

jwilm commented 8 years ago

Version 2.4 was never published to crates.io; seems like there shouldn't be any semver issue. I'm inclined to not have default features as much as possible having watched hyper/iron struggle with openssl as a default feature. Any intermediate dependencies which don't opt-out of default features force the defaults to be enabled downstream.

The separate struct is an interesting idea. It seems that gl-rs has used that idea extensively to the point that individual functions can be loaded without triggering any other dlopens.

ghost commented 8 years ago

It would be nice and ergonomic to lazily load functions as they're called from static function wrappers like you can do with gl-rs. I'm definitely considering this for version 3. I would like a way to query whether a function is available to handle cases where some are missing without having a thread panic in order to find out. This would probably make it unnecessary to even have the static bindings anymore.

jwilm commented 8 years ago

There's some discussion about version based feature flags on the rust user forum with regards to ffi bindings. ilogiq suggested using a built script to set feature flags based on the available binary. Sounds like a reasonable idea here.

ghost commented 8 years ago

Since this hasn't been done in x11-rs before, would you prefer the recently added functions to be behind a feature gate or in a separate struct?

It appears that these are the functions that have been added between the version in Ubuntu 14.04 and the current version in Arch:

jwilm commented 8 years ago

I think the separate struct makes the most sense. The program can then be compiled with support for all of them, and then we can fail at runtime on systems that don't have the newer symbols.

ghost commented 8 years ago

I've opened a pull request #39, but I'd like your thoughts on it before I merge it. It's a little hacky, but it will only be a temporary solution until x11-rs v3.

Edit: The new struct is x11_dl::xrandr::Xrandr_2_2_0. The version number is taken from the binary version rather than the library version.

ghost commented 8 years ago

ba6ae5c

ghost commented 8 years ago

@jwilm

Before I go through with v3, I'd like to wait until untagged unions are available in stable Rust. But I thought of another solution to this problem that may be cleaner for v2 and wouldn't require a separate struct for Xrandr 1.4.2. A cargo feature, probably called lazy, could be added to x11-dl. Enabling this feature would enable lazy initialization of library functions, so they're not loaded until they're called. Unused functions would never be loaded, and this would help backwards compatibility for all X11 libraries without much added complexity. In order to keep the crate compatible with versions that have xrandr::Xrandr_2_2_0, this can be changed to an alias of xrandr::Xrandr. What are your thoughts on this? Do you know of any potential problems this would cause?

In addition, the library structs can have a method along the lines of xrandr.check_function("XRRAllocateMonitor") to see if a function is available without panicking.

Edit: One potential edge case that may cause a problem is if anyone is taking the library functions by reference. Changing them to lazy functions would change them from extern function pointers to struct methods, which would only cause problems in some weird hypothetical case.

rightfold commented 6 years ago

Has this issue been resolved? I am still getting the error "undefined symbol: XRRAllocateMonitor" when creating an X11 event loop using libxrandr 1.4.2.