[MLton] Finished (?) MLton.Child

[Stephen Weeks]

> CreateProcess does not work with cygwin; I said so in the comments.
> That codepath is completely untested by me. I was using fork().

True.  My thinking was to allow MLton.Process.create to work on Cygwin
with both mmap and VirtualAlloc.  When using mmap,
MLton.Process.create uses fork (as you did) and behaves just like
other Unixen.  When using VirtualAlloc, then MLton.Process.create
behaves as on MinGW.

> However, the problem here is probably that you are using cygwin's waitpid.
> You will notice that in platform/mingw.c waitpid calls _cwait. Maybe if
> cygwin were also changed to use _cwait during reap this would work.
> 
> There is a cwait in process.h; it might do the same thing... no _ though.
> 
> The same goes for kill(); it's not a process id and this will fail.
> You need to use TerminateProcess (ppsee kill from mingw.c).

You're right.  My approach definitely confuses HANDLEs with file
descriptors and pids.  If one were to try to make this work, the right
approach might be to get into the SML as quickly as possible, making
these three types distinct, and then have the type system help out.
But, I'm not convinced it's worth pursuing.  One can use
Cygwin+fork/mmap or MinGW+CreateProcess/VirtualAlloc.  I don't see the
need for pushing hard to make Cygwin+CreateProcess/VirtualAlloc work.

> The main reason I used fork() on cygwin is that I don't know how to
> turn a HANDLE into a cygwin file descriptor; there is no
> _open_osfhandle. On the other hand, I suppose right now the code is
> using cygwin's pipe(), so maybe things will just magically work
> out. Did you see valid output prior to reap?

Yes.  You are probably right about the confusion between waitpid on
Cygwin and on MinGW (using _cwait).

> get_osfhandle in io.h may or may not be the same as _get_osfhandle.
> This may get you the HANDLE back, but the important direction is to
> turn the HANDLE into a read()able fd. I will need to look in the
> (ick) cygwin source.
...
> You earlier comment about a 'reap' problem with CP has also given me hope 
> for that method. :) If VirtualAlloc and CreateProcess can coexist: great!

Don't do this on our account just to make Cygwin+CreateProcess work.
I'd say to wait until someone really needs it ... but, if you want,
feel free to investigate.

> > * It seems wrong that on Cygwin MLton.Process.create expects
> >   Windows-style paths, whereas the rest of Cygwin expects Unix-style
> >   paths.
> 
> It's a very windowsy system call...
> I am surprised that it's available at all.

There's no problem with Cygwin+fork/mmap since that ends up using
Cygwin's exec.  But this problem needs to be resolved if you get
Cygwin+CreateProcess to work.

> > datatype z = datatype PosixPrimitive.file_desc
> 
> What is this for?

The idiom "datatype z = datatype t" introduces the constructors
associated with t into scope.  In this case, just FD.

> >        if (useWindows)
> >                _setmode(fd, _O_BINARY);
> >        else
> >                /* cygwin has a different method for working with its fds */
> >                setmode(fd, O_BINARY);
> 
> I am not sure this is a good idea.
> Are both of these even defined on both platforms?

I guess they're defined, or it wouldn't have compiled.  But you're
right.  I moved this code into cygwin.c and took out the useWindows
stuff.

> > static HANDLE dupHandle (int fd) {
> >        HANDLE raw, dupd;
> >
> >        raw = (HANDLE)_get_osfhandle (fd);
> 
> cygwin.c:
> > #include "create.c"
> 
> !? How does this compile? I see no definition for _get_osfhandle in cygwin.
> There's a get_osfhandle in io.h, but where is _get_osfhandle?

Dunno.  This was part of my attempt to make Cygwin+CreateProcess
work.  I figured since _get_osfhandle worked on MinGW, it would work
here.  I did have to add a prototype in Cygwin.h though.  In any case,
in an attempt to get back closer to your patch and to at least make
stuff work with Cygwin+fork/mmap, I've removed this.

> I will try cvs/HEAD tomorrow to see this hang.
> I definitely did not experience this in my cygwin->cygwin (+fork) builds.

With my recent checkin to remove my Cygwin+CreateProcess attempt, I
now (as expected) get an immediate error if I try to use
Process.create

  unhandled exception: SysErr: Function not implemented [nosys]

I've also now fixed the (stupid) bug with use-mmap.  The test now
works perfectly.  SML test code below.

----------------------------------------------------------------------

open MLton.Process

val (path1, path2) =
   let
      open MLton.Platform.OS
      val bsd = ("/bin/ls", "/usr/bin/grep")
   in
      case host of
	 Cygwin => ("/bin/ls", "/bin/grep")
	    (* ("c:\\cygwin\\bin\\ls.exe", "c:\\cygwin\\bin\\grep.exe") *)
       | Darwin => bsd
       | FreeBSD => bsd
       | Linux => ("/bin/ls", "/bin/grep")
       | MinGW => ("c:\\cygwin\\bin\\ls.exe", "c:\\msys\\1.0\\bin\\grep.exe")
       | NetBSD => bsd
       | OpenBSD => bsd
       | Solaris => ("/bin/ls", "/bin/grep")
   end

val p = create {args = ["-l", "/tmp"],
		env = NONE,
		path = path1,
		stderr = Param.self,
		stdin = Param.null,
		stdout = Param.pipe}

val q = create {args = ["--", "2"],
		env = NONE,
		path = path2,
		stderr = Param.self,
		stdin = Param.child (getStdout p),
		stdout = Param.pipe}

val _ = print "Sucking stream\n"
val () = print (TextIO.inputAll (Child.textIn (getStdout q)))
val _ = print "Done sucking\n"
val _ = reap p (* Posix.Signal.term *)
val _ = print "Reaped\n"
val _ = reap q (* Posix.Signal.term *)
val _ = print "Reaped\n"