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

handle file paths to libraries #134

Merged
merged 3 commits into from
Oct 25, 2022
Merged

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Jul 26, 2022

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

Be-ing commented Jul 26, 2022

I stumbled on this trying to statically link Qt built from vcpkg: KDAB/cxx-qt#174

Cargo.toml Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@sdroege sdroege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me otherwise, thanks!

@sdroege sdroege linked an issue Aug 9, 2022 that may be closed by this pull request
@Be-ing Be-ing force-pushed the library_file_paths branch 2 times, most recently from ee07d94 to 164f47d Compare August 28, 2022 20:00
@Be-ing
Copy link
Contributor Author

Be-ing commented Aug 28, 2022

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

sdroege commented Aug 29, 2022

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.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Be-ing Be-ing force-pushed the library_file_paths branch 2 times, most recently from 2f1ab93 to a04c543 Compare August 29, 2022 07:19
src/lib.rs Outdated Show resolved Hide resolved
@nirbheek
Copy link

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

sdroege commented Aug 29, 2022

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

@Be-ing
Copy link
Contributor Author

Be-ing commented Aug 29, 2022

(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
Copy link
Collaborator

sdroege commented Aug 29, 2022

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

@nirbheek
Copy link

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

sdroege commented Aug 29, 2022

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

@sdroege
Copy link
Collaborator

sdroege commented Oct 7, 2022

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

@Be-ing
Copy link
Contributor Author

Be-ing commented Oct 11, 2022

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

Be-ing commented Oct 13, 2022

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. 😞

@Be-ing Be-ing force-pushed the library_file_paths branch 2 times, most recently from 2c1fd9f to 5001232 Compare October 13, 2022 04:56
@Be-ing
Copy link
Contributor Author

Be-ing commented Oct 13, 2022

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

@sdroege
Copy link
Collaborator

sdroege commented Oct 13, 2022

Ah that looks nicely simple, thanks!

src/lib.rs Outdated Show resolved Hide resolved
@Be-ing Be-ing force-pushed the library_file_paths branch 2 times, most recently from 4dfa744 to e208b2e Compare October 22, 2022 07:04
src/lib.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@thomcc
Copy link
Member

thomcc commented Oct 22, 2022

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 Be-ing force-pushed the library_file_paths branch 2 times, most recently from 73b6b76 to d1d7272 Compare October 22, 2022 15:09
src/lib.rs Show resolved Hide resolved
@Be-ing Be-ing force-pushed the library_file_paths branch 2 times, most recently from d6422df to bcccda0 Compare October 22, 2022 15:53
@Be-ing
Copy link
Contributor Author

Be-ing commented Oct 22, 2022

I think this is finally ready to merge.

@thomcc
Copy link
Member

thomcc commented Oct 22, 2022

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

@sdroege
Copy link
Collaborator

sdroege commented Oct 22, 2022

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

Copy link
Member

@thomcc thomcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I have one trivial nit (which is mostly nice to have and it could land without it).

Thanks for the great comments too, that made it much more straightforward to review.

src/lib.rs Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
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}"
No longer needed as of Rust 2018
@thomcc thomcc merged commit d5e10b7 into rust-lang:master Oct 25, 2022
@Be-ing
Copy link
Contributor Author

Be-ing commented Oct 25, 2022

Thanks for helping make this robust!

@Be-ing
Copy link
Contributor Author

Be-ing commented Oct 25, 2022

Could you publish a new release?

@sdroege
Copy link
Collaborator

sdroege commented Oct 26, 2022

Could you publish a new release?

Sure, done :)

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.

Incorrect parse result for raw archive path
4 participants