Dune: Support spaces in ocamllex path

[Chris00]

When looking for ocamllex using Windows PATH, one may well end up with a name such as C:\Program Files/OCaml\bin\ocamllex.opt.exe (with spaces). Thus the line exec "%s -q %s" ocamllex src fails as the executable name is misunderstood. Strangely using Filename.quote ocamllex does not work. (@dra27 any idea why?)

[fdopen]

You must add extra quotes around the whole command 🙃

(* windows code path *)
 let n = exec "\"%s -q %s\"" (Filename.quote ocamllex) (Filename.quote src) in

See https://www.borngeek.com/2011/03/22/calls-to-system-in-windows/ or https://stackoverflow.com/a/9965141

[dra27]

That's the one, thanks @fdopen! Fortunately (here), we can get away without having to put Windows-specific code in the bootstrap, as the only thing which needs to be quoted is the command, as the filenames and switches are known not to contain spaces.

[Chris00]

@dra27 Given that this behavior is not intuitive at all, what do you think about proposing a Filename.quote_command : string list -> string (or maybe with the program name separated to imitate the unix tools) to be added to the standard library?

[Chris00]

Or simply Sys.command should always wrap the command in quotes on Windows?

[dra27]

For opam, I "solved" it by moving away from using Sys.command, but that's of course a problem when you have a cmd command you want to execute, rather than a program!

It would have to be more complicated than just always adding two quote characters, because you could introduce double-quote preserving behaviour in a string which otherwise wouldn't have had it. There's also the problem of any existing code which is expecting the broken version... but I agree that it's worth some further consideration!

[Chris00]

Opened https://caml.inria.fr/mantis/view.php?id=7672 so as not to forget!

[fdopen]

Fortunately (here), we can get away without having to put Windows-specific code in the bootstrap, as the only thing which needs to be quoted is the command,

That's enough. Already the command (ocamllex, ocamlopt,...) could contain a wrong character, so additional quotes around the whole command line are necessary. Test it yourself:

let cygwin_dir = "C:\\cygwin" in
let dubious_name = "test(a)" in
let (//) = Filename.concat in
let test_dir = cygwin_dir // "tmp" // dubious_name in
Unix.mkdir test_dir 0o755;
let echo = Filename.concat test_dir "echo.exe" in
let copy_cmd = Printf.sprintf "copy %s %s" (Filename.quote (cygwin_dir // "bin" // "echo.exe")) (Filename.quote echo) in
assert (Sys.command copy_cmd = 0);
assert (Sys.file_exists echo);
assert (0 = Sys.command (Printf.sprintf "\"%s Hello\"" (Filename.quote echo))); (* ok! *)
assert (0 = Sys.command (Printf.sprintf "%s Hello" (Filename.quote echo))); (* not ok! *)

[dra27]

I see what you mean - the special characters are the problem. In fact, it wouldn't even need to be dubious - OCaml installed to C:\Program Files (x86)\OCaml would do it. I guess the patch is unavoidable.