Windows: correctly redirect stderr to stdout of called ppx

[MisterDA]

There's no need for quoting the files as we're not calling the ppx
through a shell.

Redirect the stderr of the child process to which of the parent (as
posix system(3)), and the stdout of the child to the stderr of the
child (as merlin's ppx_commandline).

Setting up the redirection in the parent process has the downside that
we must set the bInheritHandles flag of CreateProcess, which means
that if merlin leaks a handle (not setting it non-inheritable /
cloexec), the ppx process will have access to it.

cc @let-def @nojb

[MisterDA]

I think with the latest push the code should be right: we still need to quote the arguments given to the program, we add 1>&2 when we're not running on Windows. For the process creation, we disable stdin of the child. We get the stderr of the parent, check whether it is inheritable (keep-on-exec) (it should be). If not, we duplicate it and mark it inheritable. We set the child's stderr and stdout to that handle.

[nojb]

Looks good, but I think there are a couple of points to correct (and a stylistic suggestion).

[MisterDA]

Thanks for the thorough review! A lot of omissions on my end...

[nojb]

LGTM. I can give this a try on my setup a bit later today and confirm that it works.

[nojb]

I tried it very briefly and it seems to work. LGTM.

[let-def]

Thanks @MisterDA and @nojb.

@MisterDA: can you update the changelog (CHANGES.md)?
And I will merge.

[MisterDA]

@MisterDA: can you update the changelog (CHANGES.md)? And I will merge.

Done.

[let-def]

Thank you ;).

[voodoos]

@MisterDA should we backport this patch to Merlin for OCaml 411 and/or 412 in your opinion ?

[MisterDA]

Yes. Merlin is still usable, but without the patch is broken when using ppx's. It cannot do anything when the code is in extension points (so that breaks for instance lwt_ppx or ppx_expect) or cannot show the type of functions generated by derivers (for instance ppx_sexp or yojson). I ignored errors on derivers, but on extension points it's more annoying not to be able to use Merlin on bigger chunks of code.
I don't think the patch would be difficult to backport.