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

Use a dynamic library instead #7

Closed
tomaka opened this issue May 1, 2015 · 17 comments
Closed

Use a dynamic library instead #7

tomaka opened this issue May 1, 2015 · 17 comments

Comments

@tomaka
Copy link

tomaka commented May 1, 2015

Glutin is soon getting support for Wayland, and as a such there is a small problem: Wayland users don't necessarly have X11 installed.

For this reason, I wanted to do what the SDL already does: dlopen the xlib library at runtime and load the symbols from it, instead of just linking with an extern block.

I have already been experimenting with a library named shared_library that would make this process easier, but I wanted to know if you would agree with this change first. If you do, I'll make the changes and open a PR.

(the same question applies for osmesa, but opening two separate issues would be overkill)

@ghost
Copy link

ghost commented May 2, 2015

I like the idea of compiling a single binary that works with multiple system configurations! I'm not sure how I feel about this being the default behavior, but it would be easy to implement in the form of a feature. Maybe after I play around with it a bit, I'll change my mind and just make it the default anyway. Would it be sufficient to just search /usr/lib and /usr/local/lib for the needed libraries?

@ghost
Copy link

ghost commented May 2, 2015

We can use dylib until std::dynamic_lib is stable.

@tomaka
Copy link
Author

tomaka commented May 2, 2015

I was thinking of using the shared_library library I wrote.
It provides a macro named shared_library! which is similar to export {} blocks and makes the transition easier (it would also make it easier to revert the changes if wanted).
Example here: https://github.com/tomaka/glutin/blob/master/src/api/caca/ffi.rs

@ghost
Copy link

ghost commented May 2, 2015

I can get started on this as soon as I get home.

@tomaka
Copy link
Author

tomaka commented May 2, 2015

I thought about working on it myself, but if you do it that would be awesome!

Note that you probably need to add an attribute like #![recursion_limit="200"], or you will reach the recursion limit.

@ghost
Copy link

ghost commented May 4, 2015

I've run into a problem. The shared_library macro doesn't seem to like variadic C functions like XCreateIC. I would comment them out until finding a workaround, but IIRC, glutin uses this function in particular.

@tomaka
Copy link
Author

tomaka commented May 4, 2015

Arg. I'm aware of this limitation, but I thought that X11 didn't have any variadic function.

@ghost
Copy link

ghost commented May 6, 2015

I may have found a solution, but it's not quite as pretty as I had hoped. I wrote a macro with two patterns: one that takes the usual list of functions, and one that takes a list of functions followed by variadic: and a list of variadic functions, omitting the ellipses in the pattern to make the compiler happy.

  pub fn XWriteBitmapFile (_7: *mut Display, _6: *const c_char, _5: c_ulong, _4: c_uint, _3: c_uint, _2: c_int, _1: c_int) -> c_int,
  pub fn XXorRegion (_3: Region, _2: Region, _1: Region) -> c_int,
variadic:
  pub fn XCreateIC (_1: XIM) -> XIC,
  pub fn XCreateOC (_1: XOM) -> XFontSet,

I'm still working out a bug that's preventing it from compiling, but I should have a test working soon. Should I apply this to shared_library and submit a pull request?

@tomaka
Copy link
Author

tomaka commented May 6, 2015

Should I apply this to shared_library and submit a pull request?

That would be nice!

@ghost
Copy link

ghost commented May 6, 2015

x11-rs now supports dynamic linking with a temporary macro when the dynamic feature is enabled. This temporary macro will be updated once I can wrap my head around the recursiveness of shared_library!.

@ghost
Copy link

ghost commented May 6, 2015

Example usage:

use x11::xlib::Xlib;

fn do_something_but_not_anything_useful_just_as_an_example () {
  let xlib = Xlib::open().unwrap();
  let display = (xlib.XOpenDisplay)(null());
  (xlib.XCloseDisplay)(display);
}

@tomaka
Copy link
Author

tomaka commented May 7, 2015

Note that this is a bad usage of Cargo features.
Cargo features are not flags. If a code depends on x11 without dynamic, then adds a dependency on glutin, then x11 will get compiled with the dynamic feature and break the code.

@ghost
Copy link

ghost commented May 7, 2015

Fixed so extern blocks are not removed. The compiler only tries to link the functions if they are actually called, so this shouldn't cause problems. If this turns out to be wrong, another possible solution is to split the dynamic interface into another crate (but still in to same repo and from mostly the same sources).

@tomaka
Copy link
Author

tomaka commented May 7, 2015

The compiler only tries to link the functions if they are actually called, so this shouldn't cause problems.

Are you sure that it doesn't pass -lx11 to the linker?

@ghost
Copy link

ghost commented May 7, 2015

The build script uses pkg-config, so it might still pass -lX11 if x11-rs was compiled on a system where pkg-config detected Xlib. Because of this, it might not work to have the old API and the dynamic API coexist in one crate unless I'm forgetting another option. I can't remove the old API because it appears that it will soon be used by servo. If you're okay with it, I'll add another crate for the dynamic API. They'll exist in the same repo and use the same sources for bindings, but different sources for internals (such as the x11_link macro).

@ghost
Copy link

ghost commented May 7, 2015

I have the split working in a separate branch (dl_split). If you're okay with the changes, I'll submit a pull request to glutin. The only usage change would be using crate x11_dl instead of x11.

@tomaka
Copy link
Author

tomaka commented May 7, 2015

Sure, as long as you publish it on crates.io

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

1 participant