Use CMAKE_MSVC_RUNTIME_LIBRARY to choose MSVC CRT

[dschuff]

For some reason CMake is now using the clang-style version of the flag (i.e. -MD
instead of /MD) to choose the runtime so this manual flag scrubbing no longer
works. Now that we are depending on CMake newer than version 3.15 we can use
its builtin support for choosing the MSVC runtime.

[dschuff]

I'm not actually sure this will fix the problem. I had a hunch that this was a dll issue, and this makes the build correctly static for me locally; but recent working builds (https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8724914020095768369/+/u/Build_Binaryen/stdout) include both /MT and -MD suggesting that this isn't really the problem. This is still a good change though.

[dschuff]

Sorry no; the above is a non-working build. The working builds have only /MT. Maybe what's actually happening is that when you separated the flag handling to apply our flags to only 1P code, we actually need /MT to apply to the 3P code as well.

[tlively]

Maybe what's actually happening is that when you separated the flag handling to apply our flags to only 1P code, we actually need /MT to apply to the 3P code as well.

That makes sense to me! I wasn't very careful about categorizing flags I didn't understand into those that could affect the ABI and those that only controlled diagnostics.

[tlively]

Do you know why our CI here passed even though the rollers failed? Should we be testing an additional or different Windows configuration here?

[dschuff]

This CI uses MSVC and MSBuild, whereas we use Chrome's build of clang and Ninja on the emscripten-releases CI. But even if we were to switch, the actual issue might be that the MSVCRT runtime dlls are installed on the OS image for the GitHub runners and not the ones that Chromium CI uses. I think maybe we'd need some kind of custom Windows image to get it to match exactly; I'm not sure if that's possible on Github CI.

[dschuff]

If the windows dumpbin utility is installed on the runners, we could maybe use that to write a test ensuring there are no dependencies on any dlls other than kernel32.dll and shell32.dll?
Although I guess it wouldn't necessarily catch differences in the way CMake handles real MSVC vs clang-cl...

[dschuff]

Do you want to LGTM this change, or does it need something else?

Going back to the "do we want to be testing differently" bit, I actually kind of like that our CI here covers real MSVC while our other CI covers clang-cl. I guess we could add clang-cl here, but I'm not sure it would be worth the effort given how rarely there have been any issues, and the fairly low cost of catching errors in the autorollers compared to here.