handle file paths to libraries

[Be-ing]

Typically pkgconfig files specify cflags for linking with -L and -l, however, pkgconfig files can also specify paths to library files. For example, building Qt5 statically on macOS generates the following pkgconfig file. Notice the absolute path to libqtpcre.a in Libs.private:

prefix=/Users/be/qt5-installed
exec_prefix=${prefix}
libdir=${prefix}/lib
includedir=${prefix}/include

host_bins=${prefix}/bin
qt_config=debug_and_release release debug build_all c++11 c++14 c++17 c++1z concurrent dbus no-pkg-config reduce_exports release_tools static stl

Name: Qt5 Core
Description: Qt Core module
Version: 5.15.5
Libs: -L${libdir} -lQt5Core
Libs.private: -framework DiskArbitration -framework IOKit -lm -framework AppKit -framework Security -framework ApplicationServices -framework CoreServices -framework CoreFoundation -framework Foundation -lz /Users/be/sw/qt-everywhere-src-5.15.5/qtbase/lib/libqtpcre2.a
Cflags: -DQT_CORE_LIB -I${includedir}/QtCore -I${includedir}

Building Qt5 statically on macOS with vcpkg generates this pkgconfig file which has a handful of file paths for libraries:

prefix=${pcfiledir}/../..

exec_prefix=${prefix}
libdir=${prefix}/lib
includedir=${prefix}/include/qt5

host_bins=${prefix}/tools/qt5/bin
qt_config=release c++11 c++14 c++17 c++1z concurrent dbus no-pkg-config reduce_exports static stl properties animation textcodec big_codecs codecs itemmodel proxymodel concatenatetablesproxymodel textdate datestring doubleconversion filesystemiterator filesystemwatcher gestures identityproxymodel library mimetype process statemachine regularexpression settings sharedmemory sortfilterproxymodel stringlistmodel systemsemaphore temporaryfile translation transposeproxymodel xmlstream xmlstreamreader xmlstreamwriter

Name: Qt5 Core
Description: Qt Core module
Version: 5.15.3

Libs: -L"${libdir}" -lQt5Core  -L"${prefix}/lib" -L"${prefix}/lib/manual-link" -framework DiskArbitration -framework IOKit -lm -framework AppKit -framework Security -framework ApplicationServices -framework CoreServices -framework CoreFoundation -framework Foundation ${prefix}/lib/libz.a -ldouble-conversion ${prefix}/lib/libicui18n.a ${prefix}/lib/libicutu.a ${prefix}/lib/libicuuc.a ${prefix}/lib/libicuio.a ${prefix}/lib/libicudata.a ${prefix}/lib/libpcre2-16.a -lzstd  ${prefix}/lib/libbz2.a ${prefix}/lib/libpng16.a ${prefix}/lib/libicui18n.a ${prefix}/lib/libicutu.a ${prefix}/lib/libicuuc.a ${prefix}/lib/libicuio.a ${prefix}/lib/libicudata.a ${prefix}/lib/libzstd.a
Cflags: -DQT_CORE_LIB -I"${includedir}/QtCore" -I"${includedir}"

[Be-ing]

I stumbled on this trying to statically link Qt built from vcpkg: https://github.com/KDAB/cxx-qt/pull/174

[Be-ing]

I'm not sure how to handle the build failure with Rust 1.30.0. Any suggestions how to keep this MSRV? Or raise the MSRV?

[sdroege]

I think we should just update the MSRV. It's not sustainable to stay at an older version if the whole ecosystem moved on already.

[nirbheek]

The file matching is incorrectly strict.

On windows if you're using MSVC or MinGW or Clang, you can link to: libfoo.a (static library built by Autotools with gcc or Meson with all compilers) or foo.lib (import library or static library with visual studio project files or cmake).

If you are using Clang or GCC you can also link to libfoo.dll.a (import library generated by GCC) or foo.dll (directly to the DLL; not recommended, but possible with GCC and Clang). You can generate foo.lib from libfoo.dll.a if you want your mingw-built libraries to be linkable by MSVC, and people do that.

Cygwin is a whole other beast, with different file naming.

[sdroege]

So maybe we should just search for (lib)?foo.(a|lib|lib.a|dll|dll.a) here and be done with it?

[Be-ing]

(lib)?foo.(a|lib|lib.a|dll|dll.a)

I'm not sure it would be that simple. It depends if rustc-link-lib wants the lib prefix or not and in which conditions.

[sdroege]

I guess this then needs checking what rustc actually expects/does here on Windows.

[nirbheek]

I suspect the linker used by rustc (lld or link or whatever) will accept whatever library you want to link to, if you use a full path.

[sdroege]

The problem is that rustc wants libraries separately as library-path / library-name AFAIU

[sdroege]

How should we proceed here? Are you planning to check what rustc is doing @Be-ing ?

[Be-ing]

I'll dig into the rustc source code to get a clearer idea of how it works, unless someone beats me to that.

[Be-ing]

Reading the rustc source code, I found that there is an undocumented verbatim modifier to the -l option specifically for this use case. Unfortunately, it is still unstable. :disappointed:

[Be-ing]

A simpler solution: pass the file path to the linker with rustc-link-arg instead of rustc-link-lib.

[sdroege]

Ah that looks nicely simple, thanks!

[Be-ing]

A simpler solution: pass the file path to the linker with rustc-link-arg instead of rustc-link-lib.

Unfortunately this won't work reliably because cargo doesn't transitively pass rustc-link-arg down to reverse dependencies.

I had a discussion with the developer working on the unstable +verbatim option for rustc-link-lib and he said that actually what we'd want is println!("cargo:rustc-link-lib=link-arg={lib_name}"). The -l link-arg option to rustc (passed from cargo via cargo:rustc-link-lib) will be very similar to the existing -C link-arg option (passed from cargo via cargo:rustc-link-arg), however the important difference for the purposes of this PR is that Cargo transitively passes cargo:rustc-link-lib to reverse dependencies, but not cargo:rustc-link-arg (though I'm not certain that's intended).

For now, this is all still unstable, and I want to get this PR merged soon, so I think we'll have to go back to hacking around the platform-specific prefixes/suffixes that cargo:rustc-link-lib adds.

[sdroege]

That's disappointing but yes, let's do the complicated way then for now and hope this gets stabilized at some point.

though I'm not certain that's intended

Can you create an issue against cargo for that specific part?

[Be-ing]

Internally rustc gives each target a LinkerFlavor which is converted to a Linker by rustc_codgen_ssa::back::linker::get_linker. The windows_gnu targets use LinkerFlavor::Gcc (including the new gnullvm targets).

impl Linker for GccLinker's link_dylib and link_staticlib methods pass the CLI arguments from rustc to the linker without prepending/appending anything.

impl Linker for MsvcLinker's link_dylib and link_staticlib methods append .lib to the CLI argument passed to rustc

GNU ld's documentation for Windows says:

when ld is called with the argument ‘-lxxx’ it will attempt to find, in the first directory of its search path,

libxxx.dll.a xxx.dll.a libxxx.a xxx.lib libxxx.lib cygxxx.dll (*) libxxx.dll xxx.dll

before moving on to the next directory in the search path.

Considering the linker will prepend lib if necessary, it's fine to strip that prefix from the filename if it's present. So I think this regex would work: (lib)?(.*).(a|lib|dll|dll.a)

This is different from the msvc targets, so we do need separate regexes for #[cfg(not(all(windows, target_env = "gnu")))] and #[cfg(not(all(windows, target_env = "msvc")))].

On windows if you're using MSVC or MinGW or Clang, you can link to: libfoo.a (static library built by Autotools with gcc or Meson with all compilers) or foo.lib (import library or static library with visual studio project files or cmake).

Even if MSVC is able to link to .a files, rustc doesn't support that currently because it always appends .lib.

[Be-ing]

Oof, using #[cfg] attributes for this is wrong because that applies to the configuration of rustc when it's compiling the build script, not when running the build script, so I think it wouldn't work for cross compiling. I think the correct way to implement this platform-specific behavior is using the TARGET environment variable.

[thomcc]

I think the correct way to implement this platform-specific behavior is using the TARGET environment variable.

It depends on the case. Each cfg comes through as an environment variable CARGO_CFG_{name}: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts. (Note that for multi-valued cfgs like target_feature or target_family this is a comma-separated list).

[Be-ing]

I think this is finally ready to merge.

[thomcc]

I'll try and do a more thorough review on Monday or Tuesday (presumably by then https://github.com/rust-lang/team/pull/861#issuecomment-1286736563 will be done and I'll be able to actually land it if it's good). Initial impressions are fairly positive.

[sdroege]

I'll go over this tomorrow but I'll wait then for @thomcc's review too early next week :)

[Be-ing]

Thanks for helping make this robust!

[Be-ing]

Could you publish a new release?

[sdroege]

Could you publish a new release?

Sure, done :)