[MLton] Re: [MLton-commit] r6699

[Wesley W. Terpstra]

On Tue, Jun 16, 2009 at 5:36 PM, Matthew Fluet <fluet at tti-c.org> wrote:
>
> O.k., then what is the rationale for the Basis Library implementation to add another level of munging on top of that done by Cygwin or MinGW?

The extra munging for spawn() is to work around the windows CRT bug
for MinGW. This way you get the usual unix behaviour where the
arguments you pass come back unmolested as CommandLine.arguments ().

The CreateProcess munging serves a similar purpose.

>> I wouldn't be surprised if the spawn() function on cygwin1.dll
>> requires no escaping at all. They're in a position to fix this bug for
>> their applications.
>
> My testing seems to support this claim.  The <src>/regression/args-spawn.sml test (renamed from spawn.sml and using CommandLine.name() to fetch the executable name) works fine, including when run from a path with spaces and when invoked with a full path (with spaces).

Excellent, so the cygwin1.dll spawn() and exec() work. Just remove the
munging of arguements to spawn/exec for cygwin. The MinGW munging will
need to stay, of course.

>
> Of course, this is using a Cygwin program to spawn a Cygwin program (that happens to be itself); I'm not sure whether this configuration is meant to support a Cygwin program spawning a Windows CRT program?  It seems to

That suggests that both the windows CRT and cygwin1.dll parse their
single argument string the same way.

>
> Ok, because the CreateProcess function takes a single string for the arguments, so the munging needs to be done on the ML side.  And, the current claim is that the "default" munging for Cygwin, MinGW, and (normal) Windows CRT are all different? Or, rather, that there is no "standard" munging?

The current claim is that they *might* be different, but are probably
the same. Your testing of spawn() on finger certainly points to this.

>
> In any case, there seems to be a problem with CreateProcess, possibly independent of the argument munging.  The child process seems to to be created:
>
> [fluet at winxp-cygwin tmp]$ cat args-create.sml
> val cmd = CommandLine.name ()
> val _ = print (concat ["cmd: ", cmd, "\n"])
> val args = CommandLine.arguments ()
> val _ = foldl (fn (arg,()) => print (concat ["arg: ", arg, "\n"])) () args
>
> open MLton.Process
> val () =
>   let
>      val pid =
>         create {args = tl args,
>                 env = NONE,
>                 path = hd args,
>                 stderr = Param.self,
>                 stdin = Param.self,
>                 stdout = Param.self}
>      val status = reap pid
>   in
>      ()
>   end
> [fluet at winxp-cygwin tmp]$ ./args-create 'C:\WINDOWS\system32\finger.exe'
> cmd: ./args-create
> arg: C:\WINDOWS\system32\finger.exe
> unhandled exception: SysErr: No child processes [child]

I suspect this is why:

> /* 20070822, fluet: The following 'pure win32' implementation of cwait
>  * no longer works on recent Cygwin versions.  It always takes the
>  * {errno = ECHILD; return -1} branch, even when the child process
>  * exists.
>  */
>
> /* Cygwin replaces cwait with a call to waitpid.
>  * waitpid only works when the process was created by cygwin and there
>  * is a secret magical pipe for sending signals and exit statuses over.
>  * Screw that. We implement our own cwait using pure win32.
>  */
> /* C_Errno_t(C_PId_t) MLton_Process_cwait(C_PId_t pid, Ref(C_Status_t) status) {

The code you removed was designed to work when reaping child cygwin
processes in addition to native applications (like finger). Perhaps
restore this function?