This means that the runtime should restore the code page when it shuts down - this is done in a737357.

How bad is it not to do it? Even with best efforts, there are several situations where the clean shutdown path is not used. I can think for instance, of: (i) fatal errors such as not-properly detected stack overflows or other segfaults (not necessarily in OCaml code); (ii) direct exit from the main program when the OCaml runtime is embedded.

Yes, please split the PR into independent PRs. It will make it much easier to test and discuss the changes separately.

The support for the Windows Console is uncovering a lot of bugs that show up very late in the release cycle. We could make another choice, which is to revert the SetConsoleOutputCP-related changes from 4.06 (and Windows 10 detection), and advertise that unicode input/output from the Windows Console is not supported in this release. Are there people out there that think this would be a reasonable idea?

If I understand correctly, not doing any of the SetConsoleOutputCP stuff means user of the Console may get segfaults -and we cannot easily detect whether someone is running the Console or not. I wonder whether this can be used as a security flaw.

@shindere That would be a reasonable approach. Since as I understand it enabling this feature doesn't make ocaml unstable for majority of the cases.

We could make another choice, which is to revert the SetConsoleOutputCP-related changes from 4.06 (and Windows 10 detection), and advertise that unicode input/output from the Windows Console is not supported in this release. Are there people out there that think this would be a reasonable idea?

I still don't have a clear mental picture of the situation; looking forward to @dra27's blog post! Still, I can see two scenarios:

• The SetConsoleOutputCP business is a partial, Windows 10-only solution that would be subsumed by the solution based on ReadConsole/WriteConsole described in Windows Console Unicode Support #1408. In this case we should revert the SetConsoleOutputCP changes in 4.06 and get the Windows Console Unicode Support #1408 solution ready for 4.07.
• The SetConsoleOutputCP business is a prerequisite to other solutions such as Windows Console Unicode Support #1408. In this case we should keep the 4.06 code as it is today, even if it's not functional for the reasons listed in the present PR. Or if we want to make sure the code is innocuous, we comment out the call to SetConsoleOutputCP but leave all the other code around in preparation for improvement in 4.07.

The console changes in #1416 will only activate if the compiled executable is updated to have a Windows 10/Server 2016 manifest

Will this cause trouble running OCaml executables on older versions of Windows? What are all the effects of such a change in the manifest?

Rebased to switch to the released FlexDLL 0.37 - I'll let AppVeyor confirm the changes (they're fine in my private fork, but you never know) and then answer these (very helpful) comments.

OK, here goes:

@alainfrisch - it's not dreadful not to reset it, it just may not match a user's expectation. I discovered it because I was comparing an intentionally not-working ocamlc with a patched one, and discovered that the output of the broken one was correct as long as I had run the patched one first! What we're doing here is the "standard" way.

@gasche - the problem here, as with other things, is that these end up stacked, so I was hoping that we could discuss whether the changes listed are wanted in principle and then carve them off. The error in #1416 is partly an artefact of carving it from its parent #1408. At this stage, I would not bother with the risk of a revert - the code will simply not activate without doing something quite advanced to the executables produced by ocamlc/ocamlopt after they've been built. From a security perspective, we've introduced nothing new here, whether that code is left in or reverted (you could segfault programs before; you still can).

@shindere, @bikalgurung - I think this would boil down to shipping with WINDOWS_UNICODE=0 in the template config/Makefile.m*. I can get on board with that - if we did so, then I'd hope that ca9f26d can be merged as, by inspection, that code only does anything if WINDOWS_UNICODE=1.

@xavierleroy - the manifest indicates support for a given version of Windows, not a requirement, so manifesting an application for Windows 10 does not prevent (or change) its operation on previous versions. However, that's not how I propose fixing it (I'd intentionally avoided using manifests in #1416) and we should definitely not change something like that at this stage of the release cycle. For the SetConsoleOutputCP business, it's not quite as you say for the first scenario. The Windows 10 solution will not be subsumed by #1408 - the output-related stuff in #1408 allows previous versions to operate correctly, but on Windows 10 that code is not activated. We could use #1408 everywhere (I believe @nojb found that this is what Git-for-Windows does), but my personal feeling is that we should workaround old broken behaviour and not keep workarounds in place when the underlying function is fixed (as in the floating point special value hacks and VS2015/2017). For the second scenario, it's not so much that #1408 depends on #1416, the patch would just alter slightly if we'd reverted #1416 - #1408 assumes that Unicode is always enabled, but engages a different function to write to the console when running prior to Windows 10.

So, I think we have:

• 8187684 is a build system change (which is tested fully in this PR using AppVeyor and can be verified also on precheck). It's not a regression, as you couldn't compile Windows OCaml in a directory including UTF-8 sequences before, so it's up to you whether that's just for trunk.
• 06775b3, thanks to @nojb spotting my hideous diff artefact, can now be split off to another GPR for both 4.06 and trunk, which I'm happy to do (the reason it's here is that the issue affects ocamldebug which breaks the testsuite when loading files from within a Unicode directory, that's how I found it).
• fdb1255 updates FlexDLL to 0.37 and can also be split off to another GPR for both 4.06 and trunk. This new version uses the new Buffer.add_utf_16le_uchar which fixes the encoding of UTF-16 surrogate pairs.
• the last four commits are all AppVeyor-related and non-critical therefore and are dependent on the previous two items being merged.

Finally, there is a choice:

• Definitively disable Switch Windows 10 Console to UTF-8 Encoding #1416 by commenting out the call to caml_setup_win32_terminal in byterun/sys.c (the call to caml_probe_win32_version is safe, though that could be commented out too)
• Fix Switch Windows 10 Console to UTF-8 Encoding #1416 by merging 5c16a6d so that OS detection returns caml_win32_major == 10 on Windows 10 instead of caml_win32_major == 6 and cope with the fact that this leaves the console in UTF-8 mode after any OCaml-compiled process terminates
• Do the previous and also merge ca9f26d so that under normal circumstances we restore the codepage on exit.

I would obviously quite like to merge the lot in 4.06 and I'd be happy to add @shindere's suggestion of disabling the new Unicode support by default for 4.06.0, but encouraging users to enable it.

I think that 8187684 (why is the a change to dynlink.c in a "build system change"?), 06775b3 and fdb1255 must all be sent to separate PRs, and I would also send the AppVeyor fixes away on their own (referring to the dependency). People can keep discussing what to do for the Windows Console here, but if you keep all changes bundled the probability that they all get reviewed properly and we can check that they were all reviewed properly by tomorrow seems close to zero.

Split off to #1446, #1447, #1448 and #1449... it will take Travis/AppVeyor a little while to catch up.

Thanks for the explanations concerning SetConsoleOutputCP and alternatives. I can agree that SetConsoleOutputCP is the way of the future, but I find it worrying (bordering on not acceptable) that OCaml is supposed to restore the CP to its initial value before exiting, as this looks very hard to guarantee.

The alternative to restoring before exiting is setting and restoring around each call that outputs to the console, right? Does'nt look like a good option.

@alainfrisch - yes, we could - I agree that it doesn't feel like a good option.

@xavierleroy - there are other things which can cause trouble - for example, if I press CTRL+C at the wrong time when Git is displaying output, I can end up with a green/red screen font left over because Git terminates without restoring the previous console colour settings (I don't think that's a thing when using ANSI escape sequences instead, although maybe?). I don't think that's a counter-argument to your worry, but it's possibly worth remembering that all a user has to do if the console is left in an "unacceptable" state is re-issue the chcp command or reload the console. If the application has been started directly, then the console is "ours" anyway, so we can do what we like - if the application has been spawned by another process, then the parent application had the option of controlling access to its console and is supposed to allow for the fact that it may be returned in a different state (arguably, Cmd is misbehaving in not restoring the code page itself and certainly misbehaving in reporting that the wrong one is active).

I still don't know whether we should merge this in 4.06 or not.

My favorite solution would be to remove all the SetConsoleOutputCp code and Windows-version-detection from 4.06. If we are going to keep the code that is already in there, then what is proposed in (what remains of) this PR seems not-worse to me, so it might make sense to merge it.

My favorite solution would be to remove all the SetConsoleOutputCp code and Windows-version-detection from 4.06.

You're the boss. Go ahead.

What about @shindere's idea to make WINDOWS_UNICODE=0 by default and therefore make the whole of this a beta "opt-in" for 4.06?

To my non-expert eyes, the "Unicode in file names" part of the work looks pretty solid and well worth exposing by default in 4.06. It's the "Unicode on the console" part that is currently in flux.

@xavierleroy fair point, which could be addressed by introducing WINDOWS_UNICODE_CONSOLE and using it in the one relevant place in byterun/sys.c?

Cherry-picked in 4.06 : 7103830.