C stubs for native Windows support #3242

dra27 · 2018-03-04T16:51:19Z

This PR starts the process of merging some of the larger patches needed for proper native Windows support. My main focus here is to try to get things which will hugely affect both the API and build process in before the release of 2.0.0.

Windows support will necessarily require C stubs. The C code is provided in src/stubs but the jbuild is only symlinked/copied on Windows, so this library is never built on Unix. opam-core is then altered to select between either a dummy implementation of the stubs for Unix or the stubs library for Windows. This may be replaced in the future as and when Dune gets better support for conditional compilation for os-specific things like this.

Prior to Windows 10 1511, colour support is only available via specific Win32 API calls. Tempting as it may be to abandon pre-Windows 10 users, it's a bit mean as Windows 7 and 8 are still supported for quite a few more years. However, in order to translate VT100 sequences into the appropriate calls, it means that anything which may include an escape sequence must go via an OpamConsole function. This necessitates a few refactorings:

There's a little hook added to allow OpamStd.Config.env to access OpamConsole.warning which means that warning messages from that function display in colour everywhere.
I moved OpamStd.Format.print_table to OpamConsole.print_table. Morally, this function doesn't really have a home in either module - it could be implemented similarly to the trick in OpamStd.Config.env with the circular dependency closed at runtime?
Tricks with flushing handles for status lines have to go, as clearing the status line requires an API call.

A problem with re-routing all the functions is that %a has a different signature for printf vs sprintf which has affected the public API for some of these changes. It is possible to hook this while maintaining printf but it's a lot hairier.

UTF-8 support on Windows is quietly improving - OCaml 4.06 added a lot and there's an open PR to push this support back to Windows 8 and earlier. The code here predates that work, but lots of it is still applicable, even with better support in OCaml. A key problem is that the default console fonts on Windows don't include the symbols required (I haven't yet found a good alternative font which actually includes them all - it's possible if we ask Microsoft nicely they might update the Consolas with a few of the missing ones in future...). The code is refactored to allow substitution with different symbols and there's quite a lot of C code for determining what the console font is and what glyphs it contains.

In fixing the compatibility layer so that Uchar and Buffer.add_utf_8_uchar were available for 4.02.3, I noticed that module type of wasn't being "correctly", so that's fixed too.

The following more minor things are also implemented in C:

Unix.getpid does not return a unique value or the normal notion of "process ID" on Windows (this is known: see Mantis #4034). This affects opam's attempts to generate unique log and temporary filenames.
Unix.wait isn't available on Windows, but opam knows the list of child processes its launched, so it's possible to implement OpamProcess.wait_one using the Win32 WaitForMultipleObjects call.
opam init has cause to write to the Windows Registry, so there's a stub for doing that.
opam init also has cause to offer to update the HOME environment variable (Windows users won't have this set by default, but it's a good idea - other software packages pick up on it too). Changing this involves several API calls - one to retrieve the correct value to put in HOME and another to broadcast to other Windows processes that the persisted environment has changed.

Finally, there is a huge piece of machinery for the Windows equivalent of eval $(opam env) which is one area in which Windows is vastly superior to Unix, since it's never necessary for the user to run it. It is possible on Windows to run putenv in the context of your parent (i.e. the shell) using process injection (which is an entirely standard thing to do on Windows - I got the idea from another tool I've used for years doing exactly the same thing). The only non-standard bit is that I've done the injection directly rather than the more conventional use of a DLL, but that's because calling SetEnvironmentVariable is not a lot of code, so there's no need for the build system complexity of two DLLs. However, it's necessary to worry about whether you're a 32-bit opam running in a 64-bit shell and vice versa. On Windows, this means that an auxiliary C program is compiled called opam-putenv.exe which will always be for the opposite architecture from opam.exe. If there's a mismatch between opam and the shell, then it uses the auxiliary (which won't have a mismatch) to do the code injection and update the parent.

It's really neat, and it's been happily working since 2015 when I wrote it!

There's then a refactoring commit for a lovely piece of software called Clink which adds GNU readline support to the Windows Command Processor and also allows prompt customisation and tab completion (both of which I've implemented, but which aren't in this PR). The refactoring is necessary because of how Clink works: cmd.exe starts which launches Clink and then clink has the equivalent of .bash_profile scripts (the Windows Command Processor doesn't really have these itself) so this is the perfect place to ensure that opam config env is run every time you open a new command prompt (that's the only time it is necessary to run opam config env, as at the moment I've chosen not to persist opam's environment changes... I may change my mind on that though!). At this point, when opam is setting the environment variables, it needs to run putenv not in the parent process, but in the parent of its parent!

This PR is probably best reviewed commit-by-commit rather than as one huge diff.

avsm · 2018-03-04T17:44:13Z

configure

@@ -3099,12 +3102,26 @@ EOF
 $as_echo "$OCAML_OS_TYPE" >&6; }


-if test "${OCAML_OS_TYPE}" = "Win32" ; then
+if  test "${OCAML_OS_TYPE}" = "Win32"; then :


Out of curiosity, is this : at the end for portability on Windows?

That's autoconf, not me 😄 That's coming from AS_IF (I try to follow m4sh coding standards, although m4sh, coding and standards I find to be co-oxymoronic terms!). I think it's a cheap way of ensuring that you never get an empty then branch, as that would be a syntax error. There's similar things in the Makefiles of the OCaml testsuite, IIRC.

AltGr · 2018-03-13T12:38:26Z

Great work! I'll review ASAP. Thanks!

dra27 · 2018-03-13T12:52:06Z

I've revised the Console stubs in the last few days (it's all pushed, so as here!). It should now be the case that the Unix and Windows 10 behaviour for the console status line is identical - so it does still use unflushed \r\027K instructions to ensure that the status is erased on next output. The clear_status call is now just a flush. However, to maintain correct console operation for before Windows 10 1511, all output after a status_line must be via an OpamConsole function or the status line won't be erased. Obviously, it's just a display fault if that gets forgotten, but my change means it's now only a display fault on old versions of Windows, not everywhere!

dra27 · 2018-03-19T14:04:52Z

I had made a fundamental and stupid mistake with the console function refactoring - %! of course makes no sense with ksprintf! Three commits added: two of which are patches to the previous work converting erroneous %! to actual flush calls and the third goes through the entire codebase eliminating unnecessary calls to %! (before, these calls were unnecessary because they were passed to functions which flushed anyway; now they're incorrect since %! is a no-op).

AltGr

Thanks for this huge work.
Apart from a few separate nitpicking comments, the only part I am not totally convinced about is the UTF8 support, which adds quite a lot of complexity while the textual fallback was already working OK. In comparison, the color support is complex too, but more straightforward, and adds much more to the UI.

For the other parts, the W32 support for waitpid (and consequently, full parallelism) is really awesome, putenv is really neat, and better analysis for the os, os-version variables is great. Thanks!

AltGr · 2018-03-28T12:19:44Z

src/core/opamStd.ml

@@ -500,7 +500,7 @@ module OpamString = struct

 end

-
+let is_windows = Sys.os_type = "Win32"


As was pointed to me earlier, since we now require 4.01.0, we can equivalently (but more cleanly) use Sys.win32. See the (conflicting — sorry) patch 04bfd5d

That's true - good opportunity to change it!

(I'd already resolved the conflict!)

AltGr · 2018-03-28T12:55:20Z

src/core/opamConsole.ml

+      output_string str;
+  in
+  List.iter print_line table
+


I made some fixes to print_table in the meantime, did you make sure to port them here too ?

I will double-check that's the case, but I think so.

This is definitely OK (I diff'd the two code blocks on the last rebase to be sure)

AltGr · 2018-03-28T12:59:39Z

src/core/opamConsole.ml

@@ -186,7 +190,12 @@ let status_line fmt =
      (fun s -> if s <> !last_status then (last_status := s; print_endline s))
      fmt
  else
-    Printf.ksprintf print_string ("\r\027[K" ^^ fmt ^^ "%!\r\027[K")
+    let print_string s =


I am nitpicking ;)
but according to /.ocp-indent, which has strict_else=auto, this shouldn't be indented.

AltGr · 2018-03-28T14:03:14Z

src/solver/opamActionGraph.ml

  | `Remove _ ->
-      OpamConsole.utf8_symbol OpamConsole.Symbols.circled_division_slash ""
+      utf8_symbol Symbols.circled_division_slash
+                  ~alternates:[Symbols.greek_small_letter_lambda] "X"


We already have a fallback to more explicit strings (just above) in case UTF8 is not available. Are the single-letter fallbacks really needed ?
I figure the intent is to allow partial fallback if only some of the symbols are not available ?

Indeed - the alternates list is based on looking at the console fonts in previous versions of Windows and then having the single letter for complete failure. FWIW it was only for completeness - Consolas has something useful on all support versions of Windows.

AltGr · 2018-03-28T14:16:41Z

src/tools/jbuild

@@ -17,6 +17,8 @@
   (flags (:standard (:include ../ocaml-flags-standard.sexp) (:include ../ocaml-context-flags.sexp)))
   (libraries (opam-client))))

+(include opam-putenv.inc)


Not really an issue, but we should be aware that we lose on Windows the property that opam can be used from a single installed binary (well, given that all its dependencies are there on course)
In any case, for Windows, we'll need some kind of installer, so that property isn't as valuable that it is on Linux. And it's in any case not a requirement.

It's not completely lost - it's just that the user would otherwise have to ensure that they are using the correct opam.exe. You really don't want to see the gymnastics required to eliminate opam-putenv.exe completely 😉

AltGr · 2018-03-28T16:03:34Z

src/core/opamStubs.mli

@@ -0,0 +1,130 @@
+(**************************************************************************)


For this (and other added modules) could you also add a line in /doc/index.html ? Thanks

dra27 · 2018-04-04T15:22:57Z

This is rebased and review comments addressed. Notes:

I've changed the "Refactor OpamStd.Sys.is_windows" commit to be a replacement of OpamStd.Sys.is_windows with Sys.win32 and updated all the other commits to use that instead.
I can confirm that the version of print_table transferred is identical, except for the obvious translations of module names from moving it out of opamStd.ml and the refactoring of output_string (which, apart from calling the formatting functions directly, also automatically adds a newline character, as it was only used in that context). Are you definitely OK with this function being in OpamConsole, rather than using a hack as with OpamStd.Sys.set_warning_printer? The moral home of print_table is not entirely clear to me, so I'm not sure which is better - housed in OpamConsole with no dirty ref, or in OpamStd.Format but with an under-the-hood hack to break the cyclic dependency.
The ocp-indent point is no longer relevant because that branch disappeared when I fixed the buffering code, but is the rest of the PR OK? At some point in the coming weeks, I should have a stable enough Windows opam that I'll actually develop on a machine with opam installed, which is the reason I don't tend to have tools like ocp-indent around, just because I'm lazy about maintaining my manually-installed system compiler! Merlin I can't live without, of course...
I've updated the documentation indexes with the new modules

For the UTF-8 part: I'd anticipate that mucking around with console fonts on Unix is less common than on Windows. The problem with the UTF8 bit is that it assumes that either all the characters are there, or none of them - the Windows fallback otherwise is just to display a square, which is pretty hideous for things like the line-art. It's not crucial, but I think converting it to have both alternate code-points which may have glyphs and being able to use ASCII for specific missing chars only is better. I'll obviously keep on eye on new glyphs which appear in Consolas, but we can't easily legislate for other console fonts.

dra27 · 2018-04-04T15:23:24Z

I've checked that this still builds on my machine, but the CI needs to run too.

dra27 · 2018-04-04T18:51:34Z

Right, fingers crossed that should pass on macOS as well. Since Dune beta18, the UChar module in extlib (which is compiled when extlib-compat, rather than extlib, is installed) conflicts with the standard library's Uchar module on case-preserving, rather then case-sensitive, file systems (so macOS and Windows). This won't be seen on Windows because lib-pkg compiles extlib with minimal=1 (as the opam extlib package does) but was seen on Travis macOS here. I've fixed it by changing the CI to specify extlib explicitly and also altering the opam-core package to conflict with extlib-compat.

Sys.win32 was added in OCaml 4.01.0, so it's available on all supported versions.

Unix.getpid doesn't return the actual Windows Process ID (see http://caml.inria.fr/mantis/view.php?id=4034). What's worse is that it can return the same process ID for two separate running instances, which creates problems for OPAM's logging code which, reasonably, assumes unique process IDs. Patch calls the Windows API function GetCurrentProcessID. This introduces C stubs to opam for the first time, but the C stubs are included in a library which is only generated on Windows. Signed-off-by: David Allsopp <david.allsopp@metastack.com>

OpamConsole uses OpamStd, preventing OpamStd.Config.env from printing coloured warning messages. This little hack breaks the dependency between the two, allowing OpamConsole.warning to be passed to OpamStd after load.

OpamConsole.msg, etc. already flush and don't support %!.

Prior to Windows 10 1511, the Windows Console is controlled using API functions, rather than escape sequences embedded in the text. This commit introduces three changes to faciliate emulation of VT100 sequences on the Windows Console: - All output to the Console containing coloured text must go through one of OpamConsole's functions (in particular, print_string, Printf.printf, output_string stdout/stderr must not be used). Using ksprintf instead of printf changes the type of functions used with %a. - OpamStd.Format.print_table is moved to OpamConsole.print_table, since it must now use OpamConsole output functions. - OpamConsole.carriage_delete replaces manually writing [K control sequence. This is isn't strictly necessary, but it's cleaner than having hard-coded control sequences. Signed-off-by: David Allsopp <david.allsopp@metastack.com>

%! doesn't work with ksprintf - fix the functions to call flush where appropriate.

The translation of VT100 sequences to Windows API calls for the Console mean that tricks relying on channel buffering no longer work (indeed, they can't be implemented without a change in OCaml). This necessitates a change in the way OpamConsole.status_line works, as it leaves the a carriage_delete operation pending on the channel such that flushing the channel will erase the status line. OpamConsole.clear_status must now be called when OpamConsole.status_line is no longer needed on-screen. It's not necessary to pair calls to status_line and clear_status. Signed-off-by: David Allsopp <david.allsopp@metastack.com>

Prior to Windows 10 1511, the Windows Console didn't natively support ANSI escape sequences, but it does include API functions providing most of the required support. In order to maintain the option of compiling with MSVC, a custom wrapper is implemented for the small number of required escape sequences, rather than using Cygwin or MSYS's ANSI layer. Particular care has been taken to ensure that OpamConsole.log and OpamConsole.slog behave in the same way; specifically this means that strings logged should never include ANSI escape sequences because these cannot be translated. Signed-off-by: David Allsopp <david.allsopp@metastack.com>

%! doesn't work with ksprintf - fix the functions to call flush where appropriate.

The with_process_in auxiliary in OpamSys is Unix-only. However, it's used for processing uname doesn't apply on Windows. Add an exception to guard against accidentally calling with_process_in on Windows. Neater alternatives for uname -s and uname -m added. Signed-off-by: David Allsopp <david.allsopp@metastack.com>

Unix.wait (and its equivalent Unix.waitpid [] -1) isn't available under Windows. Added a new stub OPAMW_waitpids which instead explicitly waits for one of the given processes to terminate using WaitForMultipleObjects. Signed-off-by: David Allsopp <david.allsopp@metastack.com>

Only REG_SZ implemented (for now) Signed-off-by: David Allsopp <david.allsopp@metastack.com>

Move handling of UTF-8 to OpamConsole, instead of having hard-coded UTF-8 sequences arbitrarily in code. Using Uchar from the stdlib requires extlib-compat rather than extlib (UChar.cmi mustn't be compiled on macOS or Windows). Signed-off-by: David Allsopp <david.allsopp@metastack.com>

The Windows Console Host supports UTF-8 as long as a TrueType font has been selected (two are installed by default). Console logic updated to detect this font support and utilise it, if UTF-8 support has been enabled (either by the user, or by OCaml 4.06.0+). At present, the Console does not support Unicode surrogate pairs, so sadly there will be no Camel symbol for OPAM-on-Windows for the foreseeable future :'( (additionally, the only common font containing it is Segoe UI Symbol which is not usable as a Console font). Although Windows has two suitable fonts (Lucida Console and Consolas) containing Unicode glyphs, neither of these fonts contain the glyphs used by OPAM's solver. Unsurprisingly, Windows font mapping for symbols is also vastly inferior compared with Linux, so instead of requiring the user to find a suitable console font, OPAM detects the glyphs supported by the font and falls back on some suitable alternatives if the defaults are not available. Implementation uses the Windows Unicode Runtime, and so requires OCaml 4.06.0 or later. Signed-off-by: David Allsopp <david.allsopp@metastack.com>

Ensure that type equations are also propagated.

The Windows Command prompt doesn't have an obvious equivalent to sh's eval, beyond the hideously ugly generation of a temporary .cmd script without setlocal at the start. Windows, however, has some moderately devious mechanisms to use an alternative. OpamStd.Win32.parent_putenv is the same as Unix.putenv, except that it manipulates the parent-process's environment (which, for opam.exe, will be the Command Prompt). The implementation relies on process injection which and takes into account Windows-on-Windows (i.e. 64/32-bit issues). In 64-bit Windows, a injecting 32-bit code into a 64-bit process will silently fail and injecting 64-bit code into a 32-bit process will kill the process. It is relatively easy for a 64-bit process to inject 32-bit code into its parent but it's incredibly hard to do the reverse and both mechanisms are fairly brittle (the curious reader is invited to search online for Heaven's Gate). In order to get around this problem, Windows builds of OPAM compile and install an additional small utility called opam-putenv. This utility is a 64-bit executable if OPAM is 32-bit and vice versa. An warning is displayed if the complementary compiler is not found - in this instance, OPAM must be run in a shell using the same architecture (meaning on 64-bit Windows, a 32-bit build of OPAM must be run from %windir%\SysWOW64\cmd.exe or it will display an error message). This implementation has the benefit, for Windows users, that unlike in other shells, it's not necessary to run opam config env at all after opam init, opam switch, etc. because OPAM can configure the environment instantly. This may prove to be the only place where Windows/CMD has the upper hand... This technique was inspired by Bill Stewart's editvar (see http://www.westmesatech.com/editv.html), a handy little utility primarily designed for prompting for passwords in Windows CMD scripts, which outputs the password entered by manipulating the parent's environment. Signed-off-by: David Allsopp <david.allsopp@metastack.com>

HOME is not commonly set in Windows installations. Wrap SHGetFolderPath to allow the user's Documents directory to be located and add a utility function for updating the HOME environment variable and persisting the change. Signed-off-by: David Allsopp <david.allsopp@metastack.com>

Clink (http://mridgers.github.io/clink) adds GNU readline support to the Windows Command Processor and exposes programmable completion via the Lua programming language. Refactor the support for OpamStd.Win32.parent_putenv to support invocation via Clink. The changes are: 1. OpamStd.Win32.set_parent_pid allows the pid written to by parent_putenv to be changed. This allows parent_putenv to behave us parent_parent_putenv when invoked by the Clink processor (the changes need to take place in the Windows Command Processor which is the parent our parent) 2. Generalising some of the C stubs so that parent_putenv becomes process_putenv and the machinery for detecting WoW64 and parent process is fully exposed as OpamStubs.getParentProcessID and OpamStubs.isWoW64Process (which replaces OpamStubs.get_mismatched_WoW64_ppid). 3. Adding a C stub for GetConsoleAlias (necessary for detecting where clink scripts are installed).

AltGr · 2018-04-05T10:37:15Z

Great, thanks!

avsm reviewed Mar 4, 2018

View reviewed changes

dra27 force-pushed the windows-stubs branch 3 times, most recently from a9d9e19 to b2ef05c Compare March 10, 2018 21:16

dra27 mentioned this pull request Mar 14, 2018

Pass fd to functions in OpamFilename.with_flock* #3248

Merged

dra27 added a commit to dra27/opam that referenced this pull request Mar 17, 2018

PR ocaml#3242, ocaml#3248 and ocaml#3255 squashed.

c583fd0

dra27 mentioned this pull request Mar 17, 2018

Fully test native Windows in the testsuite #3260

Merged

dra27 force-pushed the windows-stubs branch 3 times, most recently from 264de8d to 33ec40f Compare March 19, 2018 13:54

AltGr self-requested a review March 28, 2018 12:15

AltGr approved these changes Mar 28, 2018

View reviewed changes

AltGr reviewed Mar 28, 2018

View reviewed changes

dra27 force-pushed the windows-stubs branch from 33ec40f to 47ef451 Compare April 4, 2018 15:19

dra27 added a commit to dra27/opam that referenced this pull request Apr 4, 2018

PR ocaml#3242, ocaml#3248 and ocaml#3255 squashed.

c23080e

dra27 force-pushed the windows-stubs branch from 47ef451 to 0c59396 Compare April 4, 2018 18:48

dra27 added 8 commits April 4, 2018 21:44

Replace OpamStd.Sys.is_windows with Sys.win32

63dbd10

Sys.win32 was added in OCaml 4.01.0, so it's available on all supported versions.

Allow OpamStd access to OpamConsole

0fd45b2

OpamConsole uses OpamStd, preventing OpamStd.Config.env from printing coloured warning messages. This little hack breaks the dependency between the two, allowing OpamConsole.warning to be passed to OpamStd after load.

Remove unnecessary %! from OpamConsole calls

0e93622

OpamConsole.msg, etc. already flush and don't support %!.

Fix uses of ksprintf

b0abcdb

%! doesn't work with ksprintf - fix the functions to call flush where appropriate.

dra27 added 11 commits April 4, 2018 21:44

Fix uses of ksprintf

b566506

%! doesn't work with ksprintf - fix the functions to call flush where appropriate.

Add function for writing to the Windows Registry

494f63c

Only REG_SZ implemented (for now) Signed-off-by: David Allsopp <david.allsopp@metastack.com>

Add Buffer and Uchar compat layers

6863809

Fix uses of include module type of

e239395

Ensure that type equations are also propagated.

dra27 force-pushed the windows-stubs branch from 0c59396 to 1ceffbf Compare April 4, 2018 19:45

AltGr merged commit c70661e into ocaml:master Apr 5, 2018

dra27 deleted the windows-stubs branch May 11, 2018 15:11

dra27 mentioned this pull request Jun 13, 2024

Does OPAM work on windows? #246

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C stubs for native Windows support #3242

C stubs for native Windows support #3242

dra27 commented Mar 4, 2018

avsm Mar 4, 2018

dra27 Mar 4, 2018

AltGr commented Mar 13, 2018

dra27 commented Mar 13, 2018

dra27 commented Mar 19, 2018

AltGr left a comment

AltGr Mar 28, 2018

dra27 Mar 28, 2018

dra27 Mar 28, 2018

AltGr Mar 28, 2018

dra27 Mar 28, 2018

dra27 Apr 4, 2018

AltGr Mar 28, 2018

AltGr Mar 28, 2018

dra27 Mar 28, 2018

AltGr Mar 28, 2018

dra27 Mar 28, 2018

AltGr Mar 28, 2018

dra27 Apr 4, 2018

dra27 commented Apr 4, 2018

dra27 commented Apr 4, 2018

dra27 commented Apr 4, 2018

AltGr commented Apr 5, 2018

		@@ -500,7 +500,7 @@ module OpamString = struct

		end


		let is_windows = Sys.os_type = "Win32"

		@@ -0,0 +1,130 @@
		(**************************************************************************)

C stubs for native Windows support #3242

C stubs for native Windows support #3242

Conversation

dra27 commented Mar 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AltGr commented Mar 13, 2018

dra27 commented Mar 13, 2018

dra27 commented Mar 19, 2018

AltGr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dra27 commented Apr 4, 2018

dra27 commented Apr 4, 2018

dra27 commented Apr 4, 2018

AltGr commented Apr 5, 2018