Allow linking to libxkbcommon at compile time

[milkey-mouse]

I am packaging a version of alacritty that runs on Wayland (without Xwayland) for Guix, a distro which, like NixOS, stores all shared libraries in a weird place. The dynamic linker doesn't check this path by default, so sometimes packagers are forced to modify the library search path after building, but a better way is to link to the library at compile time.

This commit introduces a "dlopen" feature that is enabled by default that preserves the existing behavior of not linking to libxkbcommon except for trying to dlopen it at runtime. If the feature is disabled, cargo will link to the system's libxkbcommon.so at compile time.

To do this, I used the generic macros in dlib which either dlopen the library in a lazy_static struct like the current behavior, or invoke the "statically dynamically-linked" functions if the dlopen feature is turned off.

[milkey-mouse]

Sorry about the force-pushes. CI was having some trouble and I haven't used Ubuntu in a while.

[elinorbgr]

Thanks for the PR, and sorry for the delay.

If my memory serves me well, I initially made xkbcommon dlopen-only because I was worried about weird interactions with wayland-client, which uses dlib behind the hood as well. It is entierely possible that this is no longer a concern, given the numerous refactors that occured since.

To make everything clean, I think first, the dlopen feature of SCTK should imply wayland-client/dlopen as well. But also, we should ensure that everything goes well if a project depends on both SCTK and wayland-client, but ony activates the dlopen feature of wayland-client. Could you check this?

[milkey-mouse]

To make everything clean, I think first, the dlopen feature of SCTK should imply wayland-client/dlopen as well.

Done.

But also, we should ensure that everything goes well if a project depends on both SCTK and wayland-client, but ony activates the dlopen feature of wayland-client.

I think this is actually impossible without changing dlib. As you probably know Cargo features are additive and it'll only build one copy of dlib with the union of all features enabled in dependent crates. So if wayland-client/dlopen is enabled, dlib/dlopen is enabled, but smithay-client-toolkit has no way of knowing whether that feature is enabled by some other package.

I do have an idea how to fix this, but it'd require changes to dlib: in the next major version, remove the dlopen feature from dlib entirely and move #[cfg(feature = "dlopen")] inside the generic external_library! macro, something like this:

#[macro_export]
macro_rules! ffi_dispatch(
    ($handle: ident, $func: ident, $($arg: expr),*) => (
        {
            // TODO: #[cfg] on statements will only work on rust 1.42 and up:
            // https://github.com/rust-lang/rust/pull/69201/
            #[cfg(feature = "dlopen")]
            let ret = ($handle.$func)($($arg),*);
            #[cfg(not(feature = "dlopen"))]
            let ret = $func($($arg),*);

            ret
        }
    )
);

#[macro_export]
macro_rules! ffi_dispatch_static(
    ($handle: ident, $name: ident) => (
        {
            #[cfg(feature = "dlopen")]
            let ret = $handle.$name;
            #[cfg(not(feature = "dlopen"))]
            let ret = &$name;

            ret
        }
    )
);

#[macro_export]
macro_rules! external_library(
    ($structname: ident, $link: expr,
        $(statics: $($sname: ident: $stype: ty),+,)|*
        $(functions: $(fn $fname: ident($($farg: ty),*) -> $fret:ty),+,)|*
        $(varargs: $(fn $vname: ident($($vargs: ty),+) -> $vret: ty),+,)|*
    ) => (
        #[cfg(feature = "dlopen")]
        dlopen_external_library!(
            $structname, $(statics: $($sname: $stype),+,)|*
            $(functions: $(fn $fname($($farg),*) -> $fret),+,)|*
            $(varargs: $(fn $vname($($vargs),+) -> $vret),+,)|*
        );

        #[cfg(not(feature = "dlopen"))]
        link_external_library!(
            $link, $(statics: $($sname: $stype),+,)|*
            $(functions: $(fn $fname($($farg),*) -> $fret),+,)|*
            $(varargs: $(fn $vname($($vargs),+) -> $vret),+,)|*
        );
    );
);

When I patch dlib like this and build examples with wayland-client/dlopen enabled and my-example/dlopen disabled, it works. However, this requires each package depending on dlib to have its own dlopen feature, which might be too much of a hack. I'm not sure how else to make this possible though.

[elinorbgr]

Actually, this is an interesting idea. The #[cfg(...)] statements inside the macro resolve using the cargo features of the crate the macro was expanded into, I didn't realize that. That would allow fine-control over which libraries are dlopen-ed, and which are linked.

It would even be possible to allow customization of the feature name, to let the crate control everything. Much better than relying on the good propagation of cargo features.

[milkey-mouse]

OK, I'll make a PR for dlib. Would you be OK with increasing the minimum supported Rust version to 1.43 for the next dlib release? It would be nice not to have to work around the lack of support for #[cfg] directly on statements.

[milkey-mouse]

It would even be possible to allow customization of the feature name, to let the crate control everything. Much better than relying on the good propagation of cargo features.

I was about to add this, but I realized you can already do this easily in a dependent crate (in fact, this was how I was originally going to fix the SCTK/wayland-client dlopen problem before I thought of changing the macro) like this:

// in your own crate, depending on dlib:
#[cfg(feature = "dlopen-mylib")]
dlopen_external_library!(MyLib);
#[cfg(not(feature = "dlopen-mylib"))]
link_external_library!("mylib");
// you no longer care whether dlib's dlopen feature is enabled

However then to access something from that library in a generic way you have to repeat the #[cfg()] statement every time.

[elinorbgr]

Looks good, thanks!