Adds world to the link path; bad neighbor

[lilith]

As openssl-sys adds the world to the link path, it prevents linking against any libs that have a different version in /usr/lib

cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu
cargo:rustc-link-lib=ssl
cargo:rustc-link-lib=crypto

It appears that link path order is randomized by cargo, so this will cause nicely randomized link failures if other native crates are in use (or silently cause the wrong/unpatched version of a lib to be linked instead).

This can be fixed by producing the following output instead:

cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu/libssl*
cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu/libcrypto*
cargo:rustc-link-lib=ssl
cargo:rustc-link-lib=crypto

[sfackler]

We pull this information directly from pkg-config. This seems like an upstream issue?

[lilith]

What would you expect pkg-config to provide? It's the correct folder.

[sfackler]

I guess I'm a bit confused about what's being proposed. This crate uses the pkg-config-rs crate which just calls down to pkg-config. Is there a C or Rust project that postprocesses the output of pkg-config in the way you're proposing that I can look at for reference?

[lilith]

What method on pkg-config is in use? I find build.rs a bit hard to reconcile with the output I'm seeing.

[lilith]

Snap! pkg_config::find_library writes to STDOUT. That was highly unexpected.

[lilith]

As far as I can tell, pkg-config is behaving as expected:

PKG_CONFIG_ALLOW_SYSTEM_LIBS=1 pkg-config --cflags --libs openssl
 -L/usr/lib/x86_64-linux-gnu -lssl -lcrypto  

The output is correct; and we're telling it it's okay to include system lib folders. Have you seen pkg-config provide -L flags more specific than a folder? What does this command give you?

[sfackler]

I don't think I've seen it though I couldn't say that nothing is that specific.

[lilith]

This produces the desired output on my system. Instead of letting pkg_config control STDOUT, we replicate the code paths for a dynamic link. Is static linking of libssl a thing?

        if !target.contains("windows") {

            let mut conf = ::pkg_config::Config::new();
            // Disable writing to STDOUT from pkg_config directly; we need to 
            // take a less clobbering approach
            conf.cargo_metadata(false);
            //Probe for openssl
            if let Ok(info) = conf.probe("openssl") {
                //We don't need to consider frameworks -F

                let ref pkg_libs = info.libs;
                let ref pkg_link_paths = info.link_paths;
                // handle (-l) and (-L)
                for libname in pkg_libs {
                    // We asked pkg-config to include system lib folders, 
                    // So it's up to use to prevent clobbering
                    // rust link ordering is non-deterministic
                    for path in pkg_link_paths{
                        println!("cargo:rustc-link-search=native={}/lib{}*", path.to_str().unwrap(), libname);
                    }

                    //We aren't supporting static linking here
                    //but assuming this is OK
                    println!("cargo:rustc-link-lib={}", libname);

                }                   

                // handle (-I), avoid empty include paths as they are not supported by GCC
                if info.include_paths.len() > 0 {
                    let paths = env::join_paths(info.include_paths).unwrap();
                    println!("cargo:include={}", paths.to_str().unwrap());
                }
                return;
            }

Produces:

cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu/libssl*
cargo:rustc-link-lib=ssl
cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu/libcrypto*
cargo:rustc-link-lib=crypto

[sfackler]

Static linkage is a very common scenario, particularly for OpenSSL.

This kind of logic seems like it should be handled in pkg-config-rs itself since this seems like not an issue unique to this library?

[lilith]

Do you think it can be generalized? Can we always know the library names to generate more specific paths?

[sfackler]

I don't see any library names hardcoded in that snippet?

I am still confused as to why this is necessary. What other projects do this kind of thing? Is this a policy change that needs to be made over the entire Rust ecosystem?

[sfackler]

cc @alexcrichton

[alexcrichton]

To me this is both simultaneously correct and incorrect behavior. It's correct that pkg-config prints out a search path because the lib is actually located there. It's incorrect because it's already baked in the linker and wasn't necessary (and then allows picking up the wrong libs).

I'm not really sure what the best solution is here for everyone, but for OpenSSL specifically it's probably worth having very specific logic as it's just so common. I wouldn't mind adding a pkg-config to suppress /usr/lib linkage requests, @sfackler what do you think?

[sfackler]

@alexcrichton why is PKG_CONFIG_ALLOW_SYSTEM_LIBS set? According to the man page is has the effect of doing exactly this - adding /usr/lib to the -L paths.