corrosion
corrosion copied to clipboard
Fix windows-latest CI issues
- Replace undocumented
cmake -H
option withcmake -S
, since-H
is aliased with--help
on more recent cmake versions. - Use the default shell in CI where possible, which means powershell on windows. This is required because git bash does not work with the
msvc-dev-cmd
action we need for msvc. - Use Yaml
>
syntax to replace line continuations with spaces for long commands. Powershell line continuation is space + backtick, so we can't just use\
.
Thanks for looking into that. I really didn't want to investigate what needed to be fixed there and this is why i pinned the previous version.
@ogoffart Could you fork https://github.com/AndrewGaspar/install-cmake into the corrosion-rs organization? I need to update the action, since the artifact paths for cmake have changed for more recent versions of cmake. To test windows-2022 with MSVC we need at least Cmake 3.21, and we can't install that without updating the action.
Regarding this pull-request - The scope has somewhat changed - Since upgrading to windows-2022 raises the minimum supported cmake version for windows, this change does not make much sense by itself.
I would just test that windows-2022 works in general and in the same scope try to reduce the amount of combinations we are testing in the matrix.
Okay, this PR is kind of stuck now, since I can't reproduce why cross-compiling with windows and msvc is failing in CI.
Observations:
- On both windows-2022 and windows-2019 with the msvc toolchain, the cross-compiling test fails if Rust < 1.54. Judging from the logs it seems as if a build script is erroneously linked with cross-compiled libraries.
- I can't reproduce the failures on my local machine with windows 11 and msvc 2022 17.1 (and powershell 5/7).
So currently going forward with this PR does not make much sense, unless we would want to increase the MSRV anyway.
Yes, we can increase the msrv for newer msvc. If someone can increase the msvc version, they certainly can upgrade rustc
Okay, I more or less figured out what is going on here.
-
I was not able to reproduce because I was using PowerShell and running
vsvarsall.bat
has no effect there, because it is meant for the windows command prompt and not powershell. After figuring that out and setting the required env variables with the help of StackOverflow , I could reproduce the linking issue locally. This would have been much faster if I had better knowledge about shells on windows :D. -
This is not really a corrosion or powershell issue, but a cargo issue (that presumably got fixed in 1.54). Basically what happens is that
vcvarsall.bat
sets environment variable (e.g.VSCMD_ARG_HOST_ARCH
andVSCMD_ARG_TGT_ARCH
). Before 1.54 this has the effect, that a build script build which invokes the linker would fail, since the build script msvc linker would assume (based on the environment variables) that it should cross-link. Of course the build script should be compiled and linked for the host platform, so linking fails.
This can be reproduced without corrosion with a minimal example:
Cargo.toml:
[package]
name = "vs_cc_bs_test"
version = "0.1.0"
edition = "2018"
[dependencies]
[build-dependencies]
proc-macro2 = "1.0.36"
build.rs:
// Add dependency
#[allow(unused_imports)]
use proc_macro2;
fn main() {
for var in std::env::vars() {
println!("{:?}", var);
}
}
If you then setup the environment correctly for msvc cross-compiling via vsvarsXXX.bat
and set CC, CXX to cl
, and then run cargo +1.45 build --target=aarch64-pc-windows-msvc
, linking of the build script will fail. With more recent versions of cargo this will not fail anymore.
- As to why our cross-compile tests work with git bash: If you use git bash, then I guess the environment is not "properly initialized", so msvc does not know that it should cross-compile. I'm not sure what further implications this has on what is actually compiled and linked in this case.
So to summarize:
Due to an issue in cargo before 1.54, cross-compiling with msvc may not work if you have a build script. If you happen to have cl
in path already, and don`t setup the msvc environment variables, the it may work, but I'm not sure what other implications this has, since msvc may not work correctly if the environment is not correctly initialized.
So for corrosion this should probably mean that we:
- Increase the official MSRV for windows-msvc to 1.54 if cross-compilation is selected.
If someone can increase the msvc version, they certainly can upgrade rustc
For clarification, this is not related to the MSVC version and seems to only be related to cargo. The issue came to light, because cl
was not in path anymore with git bash
on windows-2022
, which caused me to use powershell, where the environment is correctly initialized for VS, which made the problem appear.