rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.27k stars 12.71k forks source link

run-make: audit the `ignore-{windows,msvc,windows-msvc}` tests #128602

Open jieyouxu opened 3 months ago

jieyouxu commented 3 months ago

There are several ignore-msvc/ignore-windows/ignore-windows-msvc Makefile and rmake.rs (tests that are ported to pure Rust files) run-make tests that have those ignore-*s because of several common challenges:

  1. Can't figure out how to build the proper e.g. import lib for dynamic lib to satisfy link.exe.
  2. Can't figure out the msvc build invocations to generate the desired output artifacts.
  3. Debug info or object symbol differences versus mingw or linux or apple.
  4. Weird appearance of unexpected symbols or lack of expected symbols.

It would be great and super helpful if Windows experts could take a look at them, and see if some of the ignore-*s can be resolved or otherwise how we can expand the tests to cover msvc as well, or have proper reasons why they must be ignore-*'d. The test suite is quite tricky to run on Windows, see https://rustc-dev-guide.rust-lang.org/tests/running.html?highlight=run-make#windows, seems to require msys2 + make, binutils, diffutils for the remaining Makefiles.

ignore-msvc:

ignore-windows-msvc:

ignore-windows:

rustbot commented 3 months ago

Error: Parsing ping command in comment failed: ...'ng windows' | error: expected end of command at >| ' (if our W'...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

jieyouxu commented 3 months ago

If the Windows experts could take a look when they have time, it would be very helpful :3 @rustbot ping windows

rustbot commented 3 months ago

Hey Windows Group! This bug has been identified as a good "Windows candidate". In case it's useful, here are some instructions for tackling these sorts of bugs. Maybe take a look? Thanks! <3

cc @albertlarsan68 @arlosi @ChrisDenton @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @nico-abram @retep998 @sivadeilra @wesleywiser

jieyouxu commented 3 months ago

You can find the current run-make tests that have ignore-msvc with https://github.com/search?q=repo%3Arust-lang%2Frust%20ignore-msvc%20path%3Atests%2Frun-make&type=code as well as ignore-windows-msvc with https://github.com/search?q=repo%3Arust-lang%2Frust+ignore-windows-msvc+path%3Atests%2Frun-make&type=code.

I'm happy to help answer questions about the rmake.rs setup :3

ChrisDenton commented 3 months ago

I very quickly did one in #128603. It uses a library function rather than nm (using llvm-nm would also have been an option).

ChrisDenton commented 3 months ago

I don't have time to fix it atm but zero-extend-abi-param-passing looks like another easy one. It just needs two things.

A version of build_native_static_lib that optimizes the output. Maybe called build_native_static_lib_optimized or something. This would be exactly the same as build_native_static_lib but would also set -O3 for non-msvc and /O2 for msvc.

param_passing.rs should also be changed to use #[link(name = "bad", kind = "static")]: https://github.com/rust-lang/rust/blob/ad0a2b7180c06514370c4c7a7a73ee75158e88fa/tests/run-make/zero-extend-abi-param-passing/param_passing.rs#L5

Or the #[link] line could be removed entirely as it's set in the call to rustc.

ChrisDenton commented 3 months ago

Ok I've done the above now as well as the other non-Makefile tests. I've not looked at the Makefile tests yet since I'd prefer to wait until they've been ported to pure Rust as I find it much easier to work on Rust tests without the added layer of emulation.

mati865 commented 1 week ago

Somewhat related, do you have an idea how to make test run on windows-gnullvm and msvc only? If I write:

//@ only-windows
//@ ignore-windows-gnu // or just ignore-gnu

Then gnullvm will be also excluded. There are few things (like CFGuad) that will probsbly never work with mingw-w64+GNU targets but are supported on mingw-w64+LLVM (not yet within Rust).

jieyouxu commented 1 week ago

@mati865 I think this is a problem with how the windows-gnu matcher in compiletest works, do you mind filing a new issue (i.e. windows-gnu also means ignore gnullvm and there's no way to specifiy running windows-gnu but skipping gnu-llvm)?

mati865 commented 1 week ago

Sure, I'll do it later today.