test suite: Output UTF-8 from cmd.exe on Windows

[petrochenkov]

Fixes #25268 and a couple of similar test errors

r? @alexcrichton

[alexcrichton]

Oh interesting! If this setting is applied, what happens when the program does indeed print non-utf8 contents? Does it die with an error or does Windows auto-translate it?

[retep998]

I'm somewhat confused how SetConsoleOutputCP works here since we're piping stdout and not working with the actual console, and the docs state it's just used to map codepoints to font glyphs.
Looks like I've got more research to do.

[petrochenkov]

@retep998

we're piping stdout and not working with the actual console

There is a console, that exact console where you type make check. This console instance is inherited (and shared) by all the child processes, including cmd.exe process spawned from tests.

what happens when the program does indeed print non-utf8 contents?

cmd.exe is special in the regard that it encodes its output according to the console code page, I don't think this tweak will affect any test code written in Rust.

I've added a test for this issue which will fail if the proposed tweak is incorrect and the testing machine have English (US) system locale.
I also improved the fix - now it restores the old code page after running the tests.

[alexcrichton]

This console instance is inherited (and shared) by all the child processes, including cmd.exe process spawned from tests

We override the standard console handles, however, when spawning a child process. Are you sure that this test fails on Windows before this change?

I also improved the fix

I'm somewhat wary about changing the global state of the Windows console here (e.g. if you ctrl-c then the old state is lost), so I'd personally be more in favor of just not assuming the output is UTF-8 and pass along Vec<u8> elsewhere and only convert to a str when necessary (printing errors as appropriate).

[petrochenkov]

We override the standard console handles, however, when spawning a child process. Are you sure that this test fails on Windows before this change?

Yes.
(I haven't checked yet what is overriden when a process is spawned, but associated console and stdout/stderr handles may be unrelated things.)

not assuming the output is UTF-8 and pass along Vec elsewhere and only convert to a str when necessary (printing errors as appropriate)

It doesn't help other failing tests like test_inherit_env in process which assume that environment variable list goes unharmed through cmd.exe

Ideally I'd moved the fix from libtest to compiletest to localize it, but some tests are run through librustdoc and not compiletest.

[petrochenkov]

Or ConsoleOutputCPGuard can be used locally around every invocation of cmd.exe, there are not so many of them.

[petrochenkov]

Oh wait, tests can run in parallel. That's bad :D
Probably a documentation solution could be the simplest one here, something like "Dear Windows developer, please execute cmd /c "chcp 65001" in your console before running the test suite, otherwise some tests may fail. You can restore you favourite non-UTF-8 code page later if you wish"

[alexcrichton]

Whoa there are multiple tests failing if the code page for cmd.exe is not UTF-8? In that case this does sound like a "let's provide a warning when make check is run" kind of situation.

[retep998]

I guess setting the code page of the console affects what encoding certain processes use when writing to a piped stdout/stderr even though a pipe isn't a console.

[petrochenkov]

So, I removed all the changes and left only a warning.
I put the check into check-sanitycheck.py, it looks like a proper place for such things. I also made some more test targets other than check (check-notidy, check-lite etc) dependent on check-sanitycheck because it seems equally applicable to them.

[alexcrichton]

@bors: r+ 8c86f8f

Thanks!

[bors]

⌛ Testing commit 8c86f8f with merge affcc24...

[bors]

💔 Test failed - auto-mac-64-opt

[retep998]

It seems your @only_on(('windows')) didn't work on mac.

[petrochenkov]

Yeah, one forgotten comma and one platform turns into seven :)

[alexcrichton]

@bors: r+ a40bca2

[bors]

⌛ Testing commit a40bca2 with merge 2bc526a...

[bors]

💔 Test failed - auto-linux-32-nopt-t

[petrochenkov]

The failure seems completely unrelated.

[alexcrichton]

@bors: retry

[bors]

⌛ Testing commit a40bca2 with merge f813f97...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt