Skip to content
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

Support libXrandr 1.4.2 #35

Closed
jwilm opened this issue Feb 28, 2016 · 10 comments
Closed

Support libXrandr 1.4.2 #35

jwilm opened this issue Feb 28, 2016 · 10 comments

Comments

@jwilm
Copy link

jwilm commented Feb 28, 2016

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 libX_whatever_ 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
Copy link

ghost commented Feb 29, 2016

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
Copy link
Author

jwilm commented Feb 29, 2016

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
Copy link

ghost commented Mar 1, 2016

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
Copy link
Author

jwilm commented Mar 9, 2016

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
Copy link

ghost commented Mar 16, 2016

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:

  • XRRAllocateMonitor
  • XRRDeleteMonitor
  • XRRFreeMonitors
  • XRRGetMonitors
  • XRRSetMonitor

@jwilm
Copy link
Author

jwilm commented Mar 16, 2016

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
Copy link

ghost commented Mar 16, 2016

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
Copy link

ghost commented Mar 16, 2016

ba6ae5c

@ghost ghost closed this as completed Mar 16, 2016
@ghost
Copy link

ghost commented Jun 4, 2016

@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.

@no-longer-on-githu-b
Copy link

no-longer-on-githu-b commented Oct 22, 2017

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.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants