Rust test

[filnet]

[filnet]

Posted a message about the breaking change in mingw-w64: https://sourceforge.net/p/mingw-w64/mailman/message/58760949/

For now the fix is only for MINGW64? Should it also target MINGW32 ?

I will share the run-make tests failure here.

[filnet]

Where do we pull in mingw-w64?

There is https://github.com/msys2/MSYS2-packages/blob/master/mingw-w64-cross-headers/PKGBUILD#L19 but it uses version 11.0.1 which does not contain the breaking change.

[filnet]

More questions...

Cheks are disabled in CI : https://github.com/msys2/MINGW-packages/blob/master/.ci/ci-build.sh#L136

What is the reason and would it make sense to enable them for specific packages like Rust ?

[ognevny]

More questions...

Cheks are disabled in CI : https://github.com/msys2/MINGW-packages/blob/master/.ci/ci-build.sh#L136

What is the reason and would it make sense to enable them for specific packages like Rust ?

they are disable as they can run very long or unexpectedly fail (which stops the whole build)

[ognevny]

Where do we pull in mingw-w64?

There is https://github.com/msys2/MSYS2-packages/blob/master/mingw-w64-cross-headers/PKGBUILD#L19 but it uses version 11.0.1 which does not contain the breaking change.

wrong package, actually https://github.com/msys2/MINGW-packages/tree/master/mingw-w64-headers-git is used for non-msys environments

[jeremyd2019]

I'm not sure about bothering with .gitignore - I don't remember seeing other packages doing that. Also, it might be a good idea to switch to the official upstream commits now that they work (https://github.com/rust-lang/compiler-builtins/commit/d8ab794ed61e2c7c0750f57332a680d5aa8db48c.patch and https://github.com/rust-lang/compiler-builtins/commit/3f47913bc6414bab4254d49f9f6e7238fecace69.patch)

[filnet]

I also had errors in run-make/doctests-keep-binaries but after enabling the doc and compiler doc they worked... Will report it upstream if confirmed.

[filnet]

I also had errors in run-make/doctests-keep-binaries but after enabling the doc and compiler doc they worked... Will report it upstream if confirmed.

I can't reproduce anymore...

So I am all set with this PR.

[filnet]

Would be great to run the check in CI. I am only testing MINGW64 atm.

[ognevny]

Would be great to run the check in CI. I am only testing MINGW64 atm.

maybe a separate job for build+check?

[jeremyd2019]

Would be great to run the check in CI. I am only testing MINGW64 atm.

maybe a separate job for build+check?

Might be a worthwhile "discussion" or separate issue, but in the past having checks that failed but were 'expected' to fail (even if not configured as required for merging) would confuse contributors, and setting up the job so that the job itself would not fail when the step failed would make it too hard for people who cared to notice that it had failed.

[ognevny]

Would be great to run the check in CI. I am only testing MINGW64 atm.

maybe a separate job for build+check?

Might be a worthwhile "discussion" or separate issue, but in the past having checks that failed but were 'expected' to fail (even if not configured as required for merging) would confuse contributors, and setting up the job so that the job itself would not fail when the step failed would make it too hard for people who cared to notice that it had failed.

can this job be run by manual requests only?

[jeremyd2019]

can this job be run by manual requests only?

We could have a workflow_dispatch triggered workflow, but a) only users with rights to the repository could run it (others could enable and run it in their forks, but they could do that now by just taking --nocheck out of the script in their fork). and more importantly, b) it wouldn't be associated with a pull request, so the git magic that determines what packages were modified so it knows what to build wouldn't be able to work. packages to check would have to be specified manually on the workflow_dispatch input.

I was just thinking, maybe some sort of hack such as some magic name of the head branch of the pull request could be a way for the creator of the pull request to signal that they want check to run? I don't know if there is any other convenient user-controlled flag that would be available to filter on. Maybe a label on the pull request, but I don't know that non-privileged users can label their own pull requests.

[filnet]

I was not suggesting to check all packages but only selected ones.

We could do something similar to what is done for installing or not. See https://github.com/msys2/MINGW-packages/blob/master/.ci/ci-build.sh#L147. Note that ci-dont-install-list.txt is currently empty. So this feature is not used.

Alternatively we could move the rust tests to the build phase.

[jeremyd2019]

I did a CI run on my fork with --nocheck removed. MINGW64 and UCRT64 ~~are still running~~ seem to have succeeded, but everything else failed: https://github.com/jeremyd2019/MINGW-packages/actions/runs/8728491385

MINGW32 seems to have failed because of the -lmingwex issue. CLANG64 failed on

  failures:
      [ui] tests\ui\rfcs\rfc-2627-raw-dylib\dlltool-failed.rs
      [ui] tests\ui\rfcs\rfc-2627-raw-dylib\invalid-dlltool.rs

CLANG32 failed with fatal runtime error: failed to initiate panic, error 5 I don't know what that's about.

It wouldn't surprise me a bit if some tests are screwed on CLANG32/CLANG64 due to the clang subsystem patch (that is, lying about the target being -gnu instead of -gnullvm)

[mati865]

CLANG64 failed on

  failures:
      [ui] tests\ui\rfcs\rfc-2627-raw-dylib\dlltool-failed.rs
      [ui] tests\ui\rfcs\rfc-2627-raw-dylib\invalid-dlltool.rs

IIRC that's indeed because of using gnu triple.

CLANG32 failed with fatal runtime error: failed to initiate panic, error 5 I don't know what that's about.

Hard to tell, but likely caused by the same reason.

[jeremyd2019]

Same failure on CLANG64 with gnullvm host/target

failures:
    [ui] tests\ui\rfcs\rfc-2627-raw-dylib\dlltool-failed.rs
    [ui] tests\ui\rfcs\rfc-2627-raw-dylib\invalid-dlltool.rs

test result: FAILED. 15875 passed; 2 failed; 194 ignored; 0 measured; 0 filtered out; finished in 225.75s

Some tests failed in compiletest suite=ui mode=ui host=x86_64-pc-windows-gnullvm target=x86_64-pc-windows-gnullvm

[filnet]

About the -lmingwex issue, Martin Storsjö said:

We have a fair bit of these kinds of circular dependencies between libmingwex.a and libmsvcr*.a. And we'll probably get more of them, not less, as we specialize some of our helper routines into different versions for different CRTs (like this change).

So therefore, breakage of this kind is indeed not surprising, when linking with ld.bfd, if the libs aren't specified repeatedly.

When driving linking with GCC and Clang, the linker is already invoked with multiple "-lmsvcrt -lmingwex [..] -lmsvcrt -lmingwex". There are also other similar issues that we've had to fix by adding more of these duplicate linker options, see e.g. https://github.com/gcc-mirror/gcc/commit/850533ab160ef40eccfd039e1e3b138cf26e76b8.

This is not an issue with lld, because it scans for symbols in all loaded libraries, not in the strict sequential manner that ld.bfd does.

Alternatively, instead of just repeating these linker options over and over, we could consider to add --start-group --end-group around these linker options, in GCC, Clang and Rust, which would allow resolving any of the dependencies in these libraries among themselves. Clang already passes this, see e.g. https://github.com/llvm/llvm-project/blob/llvmorg-18.1.4/clang/lib/Driver/ToolChains/MinGW.cpp#L277-L348, but it only seems to do this when intending to link statically. Perhaps this should be done unconditionally - this is already done for some targets, see e.g. https://github.com/llvm/llvm-project/blob/llvmorg-18.1.4/clang/lib/Driver/ToolChains/NaCl.cpp#L146-L147.

// Martin

The change that is mentioned is this one : https://github.com/mingw-w64/mingw-w64/commit/a64c1f4b969cff5f9e81c9a760117dc1b92d6ee1

[Kreijstal]

Would be great to run the check in CI. I am only testing MINGW64 atm.

maybe a separate job for build+check?

Might be a worthwhile "discussion" or separate issue, but in the past having checks that failed but were 'expected' to fail (even if not configured as required for merging) would confuse contributors, and setting up the job so that the job itself would not fail when the step failed would make it too hard for people who cared to notice that it had failed.

if the tests are expected to fail, it should pass, not fail, should it not?

[jeremyd2019]

Regarding the dlltool tests on CLANG64: I suspect upstream they should have // ignore-llvm added to ignore the llvm abi (yeah, that's kind of an odd thing to write, but the syntax is ignore-<abi> and the abi is llvm). What they have now is only-gnu (which both gnu and gnullvm satisfy the env being gnu) and needs-dlltool (which appears to mean: has dlltool on the PATH, which we do both in /clang64/bin/ and in /usr/bin because msys binutils is installed too IIRC).

Unfortunately that doesn't help in our case where the target is a lie. I'm not sure how to work that out for this package - is there some way to specify a list of specific tests to ignore?

[filnet]

Unfortunately that doesn't help in our case where the target is a lie. I'm not sure how to work that out for this package - is there some way to specify a list of specific tests to ignore?

There is documentation about testing : https://rustc-dev-guide.rust-lang.org/tests/running.html It has examples on how to run specific tests but it does not discuss --skip. I tried to skip specific tests but failed to do so. You might be luckier.

[filnet]

Skipping tests is dicussed here : https://github.com/rust-lang/rust/pull/123342#pullrequestreview-1976122348 I did not try with the extra --.

[jeremyd2019]

Interesting, I'll try that next

[filnet]

And https://github.com/rust-lang/rust/issues/111894

[jeremyd2019]

in the past having checks that failed but were 'expected' to fail (even if not configured as required for merging) would confuse contributors if the tests are expected to fail, it should pass, not fail, should it not?

Sorry, I meant tests that may pass or fail, we don't know, but won't generally hold up merging either way. But I think this would be better in a separate discussion.

[mati865]

Regarding the dlltool tests on CLANG64: I suspect upstream they should have // ignore-llvm added to ignore the llvm abi (yeah, that's kind of an odd thing to write, but the syntax is ignore-<abi> and the abi is llvm). What they have now is only-gnu (which both gnu and gnullvm satisfy the env being gnu) and needs-dlltool (which appears to mean: has dlltool on the PATH, which we do both in /clang64/bin/ and in /usr/bin because msys binutils is installed too IIRC).

Unfortunately that doesn't help in our case where the target is a lie. I'm not sure how to work that out for this package - is there some way to specify a list of specific tests to ignore?

I was looking into failure just like that several months ago but I cannot recall how I solved it locally and why I haven't opened PR.

// ignore-llvm won't work so easily because every // ignore-XYZ requires manual implementation in compiletest.

[jeremyd2019]

// ignore-llvm won't work so easily because every // ignore-XYZ requires manual implementation in compiletest.

Huh it did locally for me

[jeremyd2019]

I did not try with the extra --.

That seems to have done the trick - CLANG64 tests moved on, and now fails:

failures:

---- spec::tests::i686_pc_windows_gnu stdout ----
thread 'spec::tests::i686_pc_windows_gnu' panicked at compiler\rustc_target\src\spec\tests\tests_impl.rs:41:17:
assertion failed: !flavor_args.is_empty()
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic
   3: <rustc_target::spec::Target>::check_consistency
   4: rustc_target::spec::tests::tests_impl::test_target
   5: <rustc_target::spec::tests::i686_pc_windows_gnu::{closure#0} as core::ops::function::FnOnce<()>>::call_once
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

---- spec::tests::x86_64_pc_windows_gnu stdout ----
thread 'spec::tests::x86_64_pc_windows_gnu' panicked at compiler\rustc_target\src\spec\tests\tests_impl.rs:41:17:
assertion failed: !flavor_args.is_empty()
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic
   3: <rustc_target::spec::Target>::check_consistency
   4: rustc_target::spec::tests::tests_impl::test_target
   5: <rustc_target::spec::tests::x86_64_pc_windows_gnu::{closure#0} as core::ops::function::FnOnce<()>>::call_once
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    spec::tests::i686_pc_windows_gnu
    spec::tests::x86_64_pc_windows_gnu

test result: FAILED. 238 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 15.36ms

Those probably have to be skipped too.

[mati865]

Yeah

spec::tests::i686_pc_windows_gnu
spec::tests::x86_64_pc_windows_gnu

are caused by our hack to use gnu triple with CLANG* envs.

[jeremyd2019]

OK, gcc-based MSYSTEMs all pass now https://github.com/jeremyd2019/MINGW-packages/actions/runs/8741870251. I've got another run started now with my latest commit to skip some tests on CLANG*, will see how that turns out.

Locally it fails:

failures:

---- core::config::tests::detect_src_and_out stdout ----
thread 'core::config::tests::detect_src_and_out' panicked at src\core\config\tests.rs:67:13:
assertion `left == right` failed
  left: "D:\\tmp"
 right: "C:\\tmp"
stack backtrace:
   0: rust_begin_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library\std\src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library\core\src/panicking.rs:72:14
   2: core::panicking::assert_failed_inner
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library\core\src/panicking.rs:342:17
   3: core::panicking::assert_failed
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce\library\core\src/panicking.rs:297:5
   4: bootstrap::core::config::tests::detect_src_and_out::test
             at .\src\core\config\tests.rs:67:13
   5: bootstrap::core::config::tests::detect_src_and_out
             at .\src\core\config\tests.rs:98:9
   6: bootstrap::core::config::tests::detect_src_and_out::{{closure}}
             at .\src\core\config\tests.rs:51:24
   7: core::ops::function::FnOnce::call_once
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce\library\core\src\ops/function.rs:250:5
   8: core::ops::function::FnOnce::call_once
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library\core\src\ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    core::config::tests::detect_src_and_out

But that may just be because I'm building on D: rather than C:. Pretty strange anyway