linker: Allow MSVC to use import libraries following the Meson/MinGW convention #123436
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @compiler-errors (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
|
Sorry for the offtpic but I'm curious whether there weren't compatibility issues regarding Meson decision. That said, making dylibs consistent with ribs and static libs sounds like a good idea. |
|
typo:
This is not accurate, we wrote the above-linked Meson FAQ because it's a common misconception. I'll repeat and expand the relevant bits here for the benefit of everyone reading:
In summation: there is no reason to treat MinGW-built and MSVC-built static libraries as incompatible. In fact, at GStreamer we have been linking them together for 5 years with no adverse effects, since everything is using UCRT. FWIW, as of recently, CMake also accepts An important quality-of-life change for users there is to ignore static libraries presented by Strawberry Perl's pkg-config[1][2] (which is unconditionally added to Similar to CMake and Meson, this needs to be handled in pkg-config-rs, and I have filed an issue about it. |
@nirbheek I was thinking of the particular case of import libraries (libfoo.dll.a) for which I already experienced such a clash elsewhere: https://aomedia-review.googlesource.com/c/aom/+/173603 |
|
r? @wesleywiser sorry, not the best person to review this |
|
@nirbheek at MSYS2 we've had many reports of broken builds when using static libraries made with MSVC in mingw-w64 based toolchains and we always referred to what I think was original MinGW (not mingw-w64) webpage or some mailing list message that said DLLs are the only supported interface between MSVC and MinGW. Some of the issues on top of my head:
File format is not the problem here but since
Unfortunately there are still many vendors providing MSVCRT toolchain as the default one, especially when cross-compiling.
OFC, we've been telling it at MSYS2 for years, it's also mentioned at MSYS2 webpage.
This is refreshing to hear things are getting better but I still worry those are either too simple libraries to exhibit incompatibility issues or was not tested deeply enough.
Oh, I didn't know about it yet. Regarding Meson's FAQ
Both ld.bfd and LLD search for
Meson is free to support it (and workaround eventual issues) but this is not officially supported by GNU nor LLVM mingw-w64 based toolchains. |
|
I have a general concern about turning In the first case, the search directories are determined by the linker and may include some system directories, specific to a distribution directory layout, or controlled by some default linker config, or something else outside of rustc's purview.
What system directories we can lose when replacing linker search with rustc search on windows-(msvc,gnu) targetes? |
|
If we just fully switched to rustc-based library search everywhere, then I suspect we'd fail to find libc on like half of Unix-like systems. |
petrochenkov
added
S-waiting-on-author
|
@mati865 Thanks for the detailed explanation of MinGW and MSVC compatibility. My point was primarily that the two are not "incompatible" as was stated, but that they are indeed compatible in many cases, and people have been using it for years. You've given many more cases, such as C++ static libraries, where there is incompatibility even with static libraries produced by the same toolchain, which IMO supports the argument that this is not the linker's job. It's up to the user and the build system.
We build 100 projects, most of which used to be built with Autotools/MinGW and have been slowly ported over to Meson.
The point is that |
It gets worse, as I understand it, Rust is also converting absolute paths into |
This comment was marked as resolved.
This comment was marked as resolved.
amyspark
force-pushed
the
allow-msvc-to-use-meson-and-mingw-import-libraries
branch
from
33da95a to
df739a4
Compare
@petrochenkov If I understand you correctly, by Rebase pushed to address bors's detection too. |
This comment has been minimized.
This comment has been minimized.
amyspark
force-pushed
the
allow-msvc-to-use-meson-and-mingw-import-libraries
branch
from
df739a4 to
8cdf1ee
Compare
This comment has been minimized.
This comment has been minimized.
amyspark
force-pushed
the
allow-msvc-to-use-meson-and-mingw-import-libraries
branch
2 times, most recently
from
c44e94f to
4d3b7e8
Compare
This comment has been minimized.
This comment has been minimized.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
Doesn't look like an error in the PR, was that a race condition? |
|
That looks like a problem we've been having with CI lately. GitHub are looking into it. @bors retry |
bors
added
S-waiting-on-bors
…-mingw-import-libraries, r=petrochenkov linker: Allow MSVC to use import libraries following the Meson/MinGW convention Hi all, This PR implements support for `MsvcLinker` to use import libraries following Meson and the MinGW toolchain's naming convention. Meson [follows the `libfoo.dll.a` naming convention](https://mesonbuild.com/FAQ.html#why-does-building-my-project-with-msvc-output-static-libraries-called-libfooa) to disambiguate between static and import libraries. This support already existed for static libraries (see rust-lang#100101), but not for dynamic libraries. The latter case was added by duplicating the logic in `native_libs::find_native_static_library`, but a separate case was added in `link_dylib_by_name` for the Windows CRT libraries which must be handled by the linker itself. See for prerequisites rust-lang#129366, rust-lang#126094, and rust-lang#128370. All feedback is appreciated! Fixes rust-lang#122455 cc `@sdroege` `@nirbheek`
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
@bors retry |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#123436 (linker: Allow MSVC to use import libraries following the Meson/MinGW convention) - rust-lang#130410 (Don't ICE when generating `Fn` shim for async closure with borrowck error) - rust-lang#130412 (Don't ICE when RPITIT captures more method args than trait definition) - rust-lang#130436 (Ignore reduce-fadd-unordered on SGX platform) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#123436 - amyspark:allow-msvc-to-use-meson-and-mingw-import-libraries, r=petrochenkov linker: Allow MSVC to use import libraries following the Meson/MinGW convention Hi all, This PR implements support for `MsvcLinker` to use import libraries following Meson and the MinGW toolchain's naming convention. Meson [follows the `libfoo.dll.a` naming convention](https://mesonbuild.com/FAQ.html#why-does-building-my-project-with-msvc-output-static-libraries-called-libfooa) to disambiguate between static and import libraries. This support already existed for static libraries (see rust-lang#100101), but not for dynamic libraries. The latter case was added by duplicating the logic in `native_libs::find_native_static_library`, but a separate case was added in `link_dylib_by_name` for the Windows CRT libraries which must be handled by the linker itself. See for prerequisites rust-lang#129366, rust-lang#126094, and rust-lang#128370. All feedback is appreciated! Fixes rust-lang#122455 cc `@sdroege` `@nirbheek`
metadata: Ignore sysroot when doing the manual native lib search in rustc This is the opposite alternative to rust-lang#138170 and another way to make native library search consistent between rustc and linker. This way the directory list searched by rustc is still a prefix of the directory list considered by linker, but it's a shorter prefix than in rust-lang#138170. We can include the sysroot directories into rustc's search again later if the issues with rust-lang#138170 are resolved, it will be a backward compatible change. This may break some code doing weird things on unstable rustc, or tier 2-3 targets, like bundling `libunwind.a` or sanitizers into something. Note that this doesn't affect shipped `libc.a`, because it lives in `self-contained` directories in sysroot, and `self-contained` sysroot is already not included into the rustc's search. All libunwind and sanitizer libs should be moved to `self-contained` sysroot too eventually. With the consistent search directory list between rustc and linker we can make rustc own the native library search (at least for static libs) and use linker search only as a fallback (like in rust-lang#123436). This will allow addressing issues like rust-lang#132394 once and for all on all targets. r? `@bjorn3`
metadata: Ignore sysroot when doing the manual native lib search in rustc This is the opposite alternative to rust-lang#138170 and another way to make native library search consistent between rustc and linker. This way the directory list searched by rustc is still a prefix of the directory list considered by linker, but it's a shorter prefix than in rust-lang#138170. We can include the sysroot directories into rustc's search again later if the issues with rust-lang#138170 are resolved, it will be a backward compatible change. This may break some code doing weird things on unstable rustc, or tier 2-3 targets, like bundling `libunwind.a` or sanitizers into something. Note that this doesn't affect shipped `libc.a`, because it lives in `self-contained` directories in sysroot, and `self-contained` sysroot is already not included into the rustc's search. All libunwind and sanitizer libs should be moved to `self-contained` sysroot too eventually. With the consistent search directory list between rustc and linker we can make rustc own the native library search (at least for static libs) and use linker search only as a fallback (like in rust-lang#123436). This will allow addressing issues like rust-lang#132394 once and for all on all targets. r? ``@bjorn3``
Rollup merge of rust-lang#138273 - petrochenkov:nonatroot, r=bjorn3 metadata: Ignore sysroot when doing the manual native lib search in rustc This is the opposite alternative to rust-lang#138170 and another way to make native library search consistent between rustc and linker. This way the directory list searched by rustc is still a prefix of the directory list considered by linker, but it's a shorter prefix than in rust-lang#138170. We can include the sysroot directories into rustc's search again later if the issues with rust-lang#138170 are resolved, it will be a backward compatible change. This may break some code doing weird things on unstable rustc, or tier 2-3 targets, like bundling `libunwind.a` or sanitizers into something. Note that this doesn't affect shipped `libc.a`, because it lives in `self-contained` directories in sysroot, and `self-contained` sysroot is already not included into the rustc's search. All libunwind and sanitizer libs should be moved to `self-contained` sysroot too eventually. With the consistent search directory list between rustc and linker we can make rustc own the native library search (at least for static libs) and use linker search only as a fallback (like in rust-lang#123436). This will allow addressing issues like rust-lang#132394 once and for all on all targets. r? ``@bjorn3``
Hi all,
This PR implements support for
MsvcLinkerto use import libraries following Meson and the MinGW toolchain's naming convention. Meson follows thelibfoo.dll.anaming convention to disambiguate between static and import libraries.This support already existed for static libraries (see #100101), but not for dynamic libraries. The latter case was added by duplicating the logic in
native_libs::find_native_static_library, but a separate case was added inlink_dylib_by_namefor the Windows CRT libraries which must be handled by the linker itself.See for prerequisites #129366, #126094, and #128370.
All feedback is appreciated!
Fixes #122455
cc @sdroege @nirbheek