Suggestion: Windows CI
Partially related to #28.
Would you be receptive to a PR adding Windows CI? I'm in the process of submitting a PR to FlatBuffers and I'm seeing what looks like invalid generated cmake commands on their AppVeyor CI jobs:
--- stdout
running: "cmake" "C:\\projects\\flatbuffers\\rust\\flatc\\..\\.." "-Thost=x64" "-DCMAKE_GENERATOR_PLATFORM=x64" "-G" "Visual Studio 10 2010" "-DCMAKE_BUILD_TYPE=Release" "-DCMAKE_INSTALL_BINDIR=bin" "-DFLATBUFFERS_BUILD_TESTS=OFF" "-DFLATBUFFERS_BUILD_FLATLIB=OFF" "-DFLATBUFFERS_BUILD_FLATHASH=OFF" "-DCMAKE_INSTALL_PREFIX=C:\\projects\\flatbuffers\\tests\\rust_usage_test\\target\\debug\\build\\flatc-a64531bc5efc6cfc\\out" "-DCMAKE_C_FLAGS= /nologo /MD" "-DCMAKE_CXX_FLAGS= /nologo /MD"
-- Found Windows SDK v7.1: C:\Program Files\Microsoft SDKs\Windows\v7.1\
-- Configuring incomplete, errors occurred!
See also "C:/projects/flatbuffers/tests/rust_usage_test/target/debug/build/flatc-a64531bc5efc6cfc/out/build/CMakeFiles/CMakeOutput.log".
--- stderr
CMake Error at CMakeLists.txt:6 (project):
Generator
Visual Studio 10 2010
given toolset specification
host=x64
that contains invalid field 'host=x64'.
Seems to me like it should generate cmake -G "Visual Studio 10 2010 Win64" without -Thost=x64 as pointed out in the prior issue.
Thanks for the report! Can you see what version of CMake was running in that build? The relevant PR claimed that the change was supported in most recent versions of CMake
CMake 3.13.3. I think the error is being caused by the change in this commit.
Having said that the only VS version this seems to be an issue with is 10 2010.
Ah ok interesting! Maybe that patch should be backed out for those versions of visual studio? Would you be up for testing a patch that does that?
Sure, that's fine.
Do you think it's worth extending the CI for this repository to also cover Windows? Does the crate have a minimum required CMake version or minimum required VS version?
Yes extending to include Windows seems fine by me, if you're interested in adding that I can switch this repository to azure pipelines so it can be more easily added.
There's no currently documented minimum CMake/VS version, it's generally "if you need it and it doesn't work we should try to add support for it, and then try to not break support for older versions"
That sounds great. I don't have a personal Windows machine so a Windows based pipeline would be really useful for verification.
I've just taken a look at the current Travis setup and I see it's only running doc tests. Is there a particular reason no integration tests with CMake have been added? There are so many combinations of OSs and generators I'm not sure where I would start! Tier 1 platforms and unix makefiles/VS project files would be nice but... it's a pretty big job.
Yes exhaustively testing this crate isn't really within the purview of the CI, I don't really personally want to maintain that and try to keep it running all the time. In general I try to just be judicious and careful about changing things, but it obviously doesn't work all the time.
I'm fine for adding some smoke tests, but I don't think there's really any need to be exhaustive on CI. It's basically impossible to be exhaustive in this case I think.
Yeah I would agree with you there; I don't think exhausting the combinations is right, but I do think we need to (somehow) verify that the patch both works and doesn't break existing functionality.
I'd definitely like to add some tests for VS 10 2010 on i686/x86_64, but that's mainly because it's what I'm currently blocked on. I'm more than happy to add other generator targets while I'm at it, if there's any that stand out to you.
Nah that sounds like a great start!
Wasmtime building on Windows also panic at cmake 0.1.38 too:
error: failed to run custom build command for `wasmtime-wasi v0.0.0 (E:\learning-rust\wasmtime\wasmtime-wasi)`
process didn't exit successfully: `E:\learning-rust\wasmtime\target\debug\build\wasmtime-wasi-9cd4d5eba15448e8\build-script-build` (exit code: 101)
--- stderr
thread 'main' panicked at '
command did not execute successfully, got: exit code: 1
build script failed, must exit now', C:\Users\34937\.cargo\registry\src\mirrors.ustc.edu.cn-61ef6e0cd06fb9b8\cmake-0.1.38\src\lib.rs:813:5
stack backtrace:
0: std::sys::windows::backtrace::set_frames
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\sys\windows\backtrace\mod.rs:95
1: std::sys::windows::backtrace::unwind_backtrace
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\sys\windows\backtrace\mod.rs:82
2: std::sys_common::backtrace::_print
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\sys_common\backtrace.rs:71
3: std::sys_common::backtrace::print
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\sys_common\backtrace.rs:59
4: std::panicking::default_hook::{{closure}}
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\panicking.rs:197
5: std::panicking::default_hook
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\panicking.rs:211
6: std::panicking::rust_panic_with_hook
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\panicking.rs:474
7: std::panicking::continue_panic_fmt
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\panicking.rs:381
8: std::panicking::begin_panic_fmt
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\panicking.rs:336
9: cmake::fail
at C:\Users\34937\.cargo\registry\src\mirrors.ustc.edu.cn-61ef6e0cd06fb9b8\cmake-0.1.38\src\lib.rs:813
10: cmake::run
at C:\Users\34937\.cargo\registry\src\mirrors.ustc.edu.cn-61ef6e0cd06fb9b8\cmake-0.1.38\src\lib.rs:791
11: cmake::Config::build
at C:\Users\34937\.cargo\registry\src\mirrors.ustc.edu.cn-61ef6e0cd06fb9b8\cmake-0.1.38\src\lib.rs:700
12: build_script_build::main
at .\build.rs:9
13: std::rt::lang_start::{{closure}}<()>
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\src\libstd\rt.rs:64
14: std::rt::lang_start_internal::{{closure}}
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\rt.rs:49
15: std::panicking::try::do_call<closure,i32>
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\panicking.rs:293
16: panic_unwind::__rust_maybe_catch_panic
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libpanic_unwind\lib.rs:87
17: std::panicking::try
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\panicking.rs:272
18: std::panic::catch_unwind
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\panic.rs:388
19: std::rt::lang_start_internal
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\/src\libstd\rt.rs:48
20: std::rt::lang_start<()>
at /rustc/3de0106789468b211bcc3a25c09c0cf07119186d\src\libstd\rt.rs:64
21: main
22: invoke_main
at f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:78
23: __scrt_common_main_seh
at f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283
24: BaseThreadInitThunk
25: RtlUserThreadStart
please add the Windows CI to make sure that cmake-rs is cross-platform compatible I was using 3.11.2
@alexcrichton It seems the oldest version of VS with direct support via Azure pipelines is 2015 via the vs2015-win2012r2 image.
The CI job for 2010 might be possible by using our own Dockerfile launched in the win1803 image, but I've never actually tried to install VS build tools in a Docker image, and after a quick search the outlook for doing that with such an old version doesn't look great.
We can do the best we can to add CI, but I wouldn't want to bend over backwards too much to test every version of CMake/VS that we can
No. But it seems a shame I can't easily test the target version of VS that I'm having an issue with!