rust-lang / pkg-config-rs

Build library for invoking pkg-config for Rust
https://docs.rs/pkg-config
Apache License 2.0
173 stars 79 forks source link

Unable to correctly identify a type of Windows static library name, such as `foo.lib` #146

Open spritetong opened 1 year ago

spritetong commented 1 year ago

I have made a patch to fix it.

fn is_static_available(name: &str, system_roots: &[PathBuf], dirs: &[PathBuf]) -> bool {
    let libname = format!("lib{}.a", name);
    let libname_win = if cfg!(windows) {
        format!("{}.lib", name)
    } else {
        Default::default()
    };

    dirs.iter().any(|dir| {
        !system_roots.iter().any(|sys| dir.starts_with(sys))
            && (dir.join(&libname).exists()
                || (!libname_win.is_empty() && dir.join(&libname_win).exists()))
    })
}
diff --git a/src/lib.rs b/src/lib.rs
index 2c690fa..a5df218 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -832,9 +832,16 @@ fn envify(name: &str) -> String {
 /// System libraries should only be linked dynamically
 fn is_static_available(name: &str, system_roots: &[PathBuf], dirs: &[PathBuf]) -> bool {
     let libname = format!("lib{}.a", name);
+    let libname_win = if cfg!(windows) {
+        format!("{}.lib", name)
+    } else {
+        Default::default()
+    };

     dirs.iter().any(|dir| {
-        !system_roots.iter().any(|sys| dir.starts_with(sys)) && dir.join(&libname).exists()
+        !system_roots.iter().any(|sys| dir.starts_with(sys))
+            && (dir.join(&libname).exists()
+                || (!libname_win.is_empty() && dir.join(&libname_win).exists()))
     })
 }
sdroege commented 1 year ago

Unfortunately it's not that easy. .lib files are also DLL import libraries.

To know whether it's one or another you actually need to parse the file.

spritetong commented 1 year ago

Yes, there's no simple way to automatically tell a .lib file is an archive or a DLL import library without parsing its content. But the problem is, if I'm sure this is a static library, Config::probe() always treats it as a shared library.

If I call, pkg_config::Config::new().statik(true).probe("foo").unwrap(); then if statik && is_static_available(val, &system_roots, &dirs) { failed.

Another solution is to add a special option to Config that forces it to link the static library, like

pkg_config::Config::new().force_static(true).probe("foo").unwrap();

ChrisDenton commented 1 year ago

Hmm... on Windows both static .lib files and DLL import libraries are really static libraries. You can even mix the two in the same .lib. So I'm uncertain why the distinction matters here? Does something fail in some way?

spritetong commented 1 year ago

Hmm..., I mean, for a static library named foo.lib, is_static_available() always returns false, even if statik(true) is set. There's no way to force is_static_available() to return true.

ChrisDenton commented 1 year ago

Right but if we look at the code that uses it, there doesn't seem to much difference:

https://github.com/rust-lang/pkg-config-rs/blob/d039d32155fc3afec9867aa66c29747cbd6f95e5/src/lib.rs#L718-L726

The only difference between the two branches is =static is added in one case and in the other it's left up to rustc to decide.

spritetong commented 1 year ago

For a static library, if it runs to the branch else { ... } , MSVC linker will fail.

ChrisDenton commented 1 year ago

I'm confused as to why that would be as rustc treats them the same way (see here and here):

    fn link_dylib(&mut self, lib: &str, verbatim: bool, _as_needed: bool) {
        self.cmd.arg(format!("{}{}", lib, if verbatim { "" } else { ".lib" }));
    }
    fn link_staticlib(&mut self, lib: &str, verbatim: bool) {
        self.cmd.arg(format!("{}{}", lib, if verbatim { "" } else { ".lib" }));
    }

I think it would help to have a minimal example to test this. It could be a problem in rust or cargo itself.