New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Implement `:handle-win-shell` arg to `run`, `shell`, and `Proc::Async.new` #2005

Open
zoffixznet opened this Issue Jul 1, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@zoffixznet
Contributor

zoffixznet commented Jul 1, 2018

N.B.: I started on implementation already as part of fixing roast on Windows effort, so unless there's any 馃憥 on this, it will go in soonish.

The API is a bit warty, hoping there are better ideas.

The Problem

  • Window's shell (cmd.exe) requires special quoting of command line arguments that conflict with "normal" quoting.
  • This is responsible for large portion of current [prop/spec]test failures on Windows, because is_run helper test routine fails execution if arguments are involved (more of them were passing in the past because we passed arguments verbatim on windows, which used to cause a different set of issues)
  • This means some commands are impossible to shell on Windows, because even if you quote stuff for cmd.exe yourself, the "normal" quoting MoarVM then does on top of that creates conflicts. E.g. this doesn't run: shell 锝erl6 -e "say \"@*ARGS foo\"" foo bar ber锝;
  • The reasons run and Proc::Async.new are included as well is because they can be used to execute a "batch" file (a shell script basically), so that option will be used with them as well. See RT#132258 run "perl6" ... can be made to execute shell commands

Proposed Solution

  • Make shell, run, and Proc::Async.new accept :$handle-win-shell arg
    • It will have no effect on OSes that aren't Windows
    • It will currently take values True and False; in the future, it'll also take Nil for behaviour where it retries with alternative quoting if the first execution fails (I believe Perl 5's qx// or system do this by default).
  • on shell, this argument will default to True and will mean to pass the given string to cmd.exe without applying any quoting. On implementation side of things, this will set UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS libuv flag, which is how we had things until about a year ago, and that's it. We won't do any quoting at all.
    • Setting it to False would disable that libuv flag
  • on run and Proc::Async.new, default will be False and setting it to True will apply cmd.exe's quoting to all the args, instead of the "normal" quoting. On impl. side, this will set UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS flag and will use our hand-rolled arg quoter, based on this info: https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/

Warts

I don't like that the default value is not the same across all three routines. Perhaps, this option shouldn't be available on shell at all and should just be enabled by default at all times. Is there any good reason to have the option off?

If the arg is to not be available on shell, then I'd rather name it :quote-win-shell

@zoffixznet zoffixznet self-assigned this Jul 1, 2018

@ugexe

This comment has been minimized.

Show comment
Hide comment
@ugexe

ugexe Jul 7, 2018

Member

Other languages using libuv just expose theUV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS flag:

JuliaLang/julia#13780
nodejs/node#5060

Of course such a flag is only enabling/disabling behavior that doesn't make much sense in the context of other back ends ( except the nodejs one which is backed by libuv, but that is besides the point ) since the mangling/escaping doesn't happen in rakudo itself. This makes me think the best solution would be to implement UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS in rakudo, and add a flag to toggle its use.

Member

ugexe commented Jul 7, 2018

Other languages using libuv just expose theUV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS flag:

JuliaLang/julia#13780
nodejs/node#5060

Of course such a flag is only enabling/disabling behavior that doesn't make much sense in the context of other back ends ( except the nodejs one which is backed by libuv, but that is besides the point ) since the mangling/escaping doesn't happen in rakudo itself. This makes me think the best solution would be to implement UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS in rakudo, and add a flag to toggle its use.

@zoffixznet zoffixznet removed their assignment Jul 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment