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

Optimize openssl linkage #200

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

gaoqiangz
Copy link

No description provided.

@gaoqiangz gaoqiangz mentioned this pull request Mar 14, 2023
@gaoqiangz
Copy link
Author

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

  • lib.rs

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::*;
}
  • build.rs

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

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:

  • Did you test this update on all the major platforms? Linux, Windows (MSVC and mingw), and Mac?
  • Did you test without the 'ssl' feature - when no OpenSSL library is installed?

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

@gaoqiangz
Copy link
Author

gaoqiangz commented Mar 30, 2023

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: #158, #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? :-)

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

Successfully merging this pull request may close these issues.

2 participants