axodotdev / cargo-dist

📦 shippable application packaging
https://axodotdev.github.io/cargo-dist/
Apache License 2.0
1.55k stars 73 forks source link

Forcing +crt-static on windows: problematic with DLLs? #496

Open Gankra opened 1 year ago

Gankra commented 1 year ago

https://github.com/diesel-rs/diesel/actions/runs/6572883796/job/17854873471?pr=3833#step:9:271

= note: Creating library D:\a\diesel\diesel\target\x86_64-pc-windows-msvc\dist\deps\diesel.lib and object D:\a\diesel\diesel\target\x86_64-pc-windows-msvc\dist\deps\diesel.exp

LINK : warning LNK4098: defaultlib 'MSVCRT' conflicts with use of other libs; use /NODEFAULTLIB:library

https://stackoverflow.com/questions/3007312/resolving-lnk4098-defaultlib-msvcrt-conflicts-with

You'll see the error message you quoted when the linker is told both to link to msvcrt.lib (static libc) and libcmt.lib (dynamic libc). Which will happen if you link code that was compiled with /MT with code that was linked with /MD. There can be only one version of the CRT.

You can see libcmt.lib getting linked in, while cargo-dist turns on +crt-static on windows, as recommended by the Rust RFC:

Furthermore, it would have arguably been a "more correct" choice for Rust to by default statically link to the CRT on MSVC rather than dynamically. While this would be a breaking change today due to how C components are compiled, if this RFC is implemented it should not be a breaking change to switch the defaults in the future, after a reasonable transition period.

The support in this RFC implies that the exact artifacts that we're shipping will be usable for both dynamically and statically linking the CRT. Unfortunately, however, on MSVC code is compiled differently if it's linking to a dynamic library or not. The standard library uses very little of the MSVCRT, so this won't be a problem in practice for now, but runs the risk of binding our hands in the future. It's intended, though, that Cargo will eventually support custom compilation of the stdlib. The crt-static feature would simply be another input to this logic, so Cargo would custom-compile the standard library if it differed from the upstream artifacts, solving this problem.

It's possible we should adjust our rule from "always +crt-static" to "+crt-static if there's no system dependencies" or "there's a config flag to turn of this behaviour" (I would have liked to emit this into a persistent/normal rust build config, but +crt-static is in a weird place where there's no right place to put it, really, especially with dynamic-vs-static musl sharing the same target triple while you could reasonably want to support both).

Gankra commented 1 year ago

Currently looking into:

Slightly complicated by the fact that, as a build flag, we need this to interact with precise-builds inference (can't build --workspace if different packages in the workspace disagree on whether they want +crt-static), so now we would have inference-on-inference...

Gankra commented 1 year ago

Got some good clarification from windows devs over at https://toot.cat/@Gankra/111263387382919461

TL;DR -- it is still the case that if you want to dynamically link the windows crt, you really need to ship a redistributable to ensure everything's installed on the user's system. That said, if the user is a developer they probably already have one, so devtools like diesel's cli might be "kind of ok".

Not a terribly compelling user story to "roll the dice". We should explore options for detecting the need for redistributables (whether +crt-static should be set), what version they used (with the post-build linkage checker), and then providing them as part of our installers.

For the powershell installer we can probably fetch it from microsoft's servers at install-time?

MSIs on the other hand probably want to vendor the redistributable installer, but I don't think cargo-wix knows how to do that, so we'll also want to teach it.


if you link the CRT dynamically, either:

  • your user must have the redist installed (so, in other words, you should make sure your installer also installs the redist). this is a global thing, but we're backwards compatible so updating the redist is totes fine (using a binary compiled against the new redist with the old one is not fine)
  • you must ship the redist DLLs next to your exe (called app-local deployment). i do not recommend this.

dynamic linking of the CRT is required if you want multiple DLLs to be able to communicate with the CRT, since otherwise global data will be duplicated (I'm not sure how much of a problem this is in practice for C library usage, but it's extremely problematic for C++.)

There are two DLLs involved here. The ucrt is distributed with windows (since Windows 7). The vcruntime is installed separately. Rust mostly only uses the latter for panic handling, but also it has some critical functions (like memcpy, memcmp et al).

yeah, if you're shipping to devs they'll probably have it, but otherwise you should ship the redist

There was also a note of:

Secret tip doctors don't want you to know about: you can dynamically link the ucrt but statically link the vcruntime. Link libcmt (static), libvcruntime (static) and ucrt (dynamic). Best done in combination with +crt-static. Also remember to use /Wx linker argument because the linker will warn about mismatched CRTs but rust hides linker warnings.

See also https://learn.microsoft.com/en-us/cpp/

Which I don't fully comprehend the implications of yet (most magical interpretation is that we do not in fact ever need to ship redistributable installers, but that sounds too good to be true...).

strega-nil commented 1 year ago

I would personally argue that a packaging tool should probably default to dynamic and ship .msis, rather than default to static and ship portable binaries. Once you're using other DLLs, it's fairly rare that you don't want everyone using the same CRT instance.

As the owner of the redist, I also cannot recommend dynamic-UCRT-static-redist.

For Rust dev-tools, if someone has installed rustc, you can assume they've installed the C++ toolset which gets you the redist (note: this was true when i last did rust, may not still be true)

Gankra commented 1 year ago

Just checking: is there weird licensing/legal implications about a powershell installer (equivalent of curl | sh) dynamically fetching and installing a microsoft redistributable?

Gankra commented 1 year ago

Additonal info:

Much like on macOS, it is apparently just Cool And Normal to gobble up every dll you depend on and embed it in your application, see vcpkg (official microsoft tool) doing this: https://github.com/microsoft/vcpkg/blob/master/scripts/buildsystems/msbuild/applocal.ps1

This strategy bypasses the need to do things like "figure out what choco packages are needed at install-time", by basically just vendoring the DLLs those choco packages produced.

You can even technically do this for crt stuff, although this is Poor Form, so there is an extremely cursed theoretical world where we ship .zips that contain a vendored crt dlls (so the zip on its own works), but then the ps1 installer deletes those and runs the proper redist installer.

strega-nil commented 1 year ago

@Gankra

Just checking: is there weird licensing/legal implications about a powershell installer (equivalent of curl | sh) dynamically fetching and installing a microsoft redistributable?

https://learn.microsoft.com/en-us/cpp/windows/redistributing-visual-cpp-files?view=msvc-170 is where all that information is.

ChrisDenton commented 1 year ago

but that sounds too good to be true

Yeah, basically. I don't think a general build system should try it. It kinda works for pure Rust because we don't use much of vcruntime. We do use some exception stuff but panicking between Rust cdylibs is already maybe a bit sketchy (e.g. if built with different Rust versions or even different compile) and throwing/catching C++ exceptions was right out (officially at least). Though I think the C-unwind work has changed things here.

It's a bigger problem for mixed Rust/C++ code tho.

Gankra commented 1 year ago

Talked myself into starting with a very simplistic flag to opt back into the default rust behaviour of msvc==dynamic-libc.

https://github.com/axodotdev/cargo-dist/pull/507

There's similar questions to answer around static vs dynamic musl, and we should Just Do the redist stuff if we're going to put much effort into dynamic linking on windows. So start simple, iterate from there.

CAD97 commented 1 year ago

More details on the Hybrid CRT technique; basically statically linking vcruntime, but dynamically linking the UCRT. Doing hybrid CRT is theoretically as simple as using +crt-static (/MT for MSVC) and adding linker arguments of /NODEFAULTLIB:libucrt.lib /DEFAULTLIB:ucrt.lib to switch UCRT back to dynamic. This is supposedly preferable to full static linking when only targeting in-support Windows (which provides UCRT as an OS component); see the linked explainer from the WindowsAppSDK for more details.

As of versions 1.0.3 and 1.1 Preview 2, all Windows App SDK DLLs and EXEs containing C/C++ code are built using the 'Hybrid CRT' technique. This frees developers from the burden of installing the CRT redistributables on end user devices

There shouldn't be any issues with Hybrid CRT if DLLs always do the "proper" ABI marshalling, but it's extremely tempting for C++ libraries to skip doing that and just put STL types in their DLL interface. (I wonder how possible it is to do dynamic UCRT, static vcruntime, dynamic STL... probably not very.)

Fun aside: The UCRT does cool PE tricks to version imports, working much like glibc symbol versioning, to back its compatibility.