eclipse / paho.mqtt.rust

paho.mqtt.rust
Other
511 stars 102 forks source link

Optimize `openssl` linkage #200

Open gaoqiangz opened 1 year ago

gaoqiangz commented 1 year ago

Sorry for the code formating... Here is the change list:

We should explicit import openssl_sys crate to make build.rs works.

#[cfg(feature = "ssl")]
mod __make_openssl_linkage_work {
    #[allow(unused_imports)]
    use openssl_sys::*;
}

Delete these lines. https://github.com/eclipse/paho.mqtt.rust/blob/be43ef8f2ec25bda2a37850c0ef5227971049e38/paho-mqtt-sys/build.rs#L309-L337

Change openssl_root_dir https://github.com/eclipse/paho.mqtt.rust/blob/be43ef8f2ec25bda2a37850c0ef5227971049e38/paho-mqtt-sys/build.rs#L242-L248

to

    fn openssl_root_dir() -> Option<String> {
        use std::ffi::OsString;

        fn env_inner(name: &str) -> Option<OsString> {
            let var = env::var_os(name);
            println!("cargo:rerun-if-env-changed={}", name);

            match var {
                Some(ref v) => println!("{} = {}", name, v.to_string_lossy()),
                None => println!("{} unset", name),
            }

            var
        }
        fn env(name: &str) -> Option<OsString> {
            let prefix = env::var("TARGET").unwrap().to_uppercase().replace('-', "_");
            let prefixed = format!("{}_{}", prefix, name);
            env_inner(&prefixed).or_else(|| env_inner(name))
        }

        env("OPENSSL_INCLUDE_DIR").and_then(|path| {
            Path::new(&path)
                .parent()
                .map(|path| path.display().to_string())
        })
    }

See also https://github.com/sfackler/rust-openssl/blob/979f982e067bd310a7aff1cfa5f711ed7bcf0d95/openssl-sys/build/main.rs#L49.

fpagliughi commented 1 year ago

Hey. Thanks for the PR! Unfortunately, since this is an Eclipse project, I won't be able to merge it unless you sign an Eclipse ECA. See here: https://www.eclipse.org/legal/ECA.php

But also...

Please describe the specific problem which you are attempting to fix. On which platform(s).

Also:

I know the build still needs work, but so far, every attempt to update it has broken at least on of these platforms.

gaoqiangz commented 1 year ago

Sorry.....I have been busy recently.I haven't tested on Linux/MacOS yet. This PR solves the issue of openssl linking by delegating to openssl-sys to avoid static link error (see: https://github.com/eclipse/paho.mqtt.rust/issues/158, https://github.com/eclipse/paho.mqtt.rust/issues/134). I have tested on Windows MSVC it works fine. Once I have time, I will test it on other platforms, maybe you can help me? :-)