Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REPL shell mode for Windows #33474

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ include("iobuffer.jl")
include("intfuncs.jl")
include("strings/strings.jl")
include("parse.jl")
include("shell.jl")
include("regex.jl")
include("shell.jl")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is needed since we use substitution string in the shell.jl file.

include("show.jl")
include("arrayshow.jl")
include("methodshow.jl")
Expand Down
52 changes: 45 additions & 7 deletions base/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,20 @@ stackframe_lineinfo_color() = repl_color("JULIA_STACKFRAME_LINEINFO_COLOR", :bol
stackframe_function_color() = repl_color("JULIA_STACKFRAME_FUNCTION_COLOR", :bold)

function repl_cmd(cmd, out)
shell = shell_split(get(ENV, "JULIA_SHELL", get(ENV, "SHELL", "/bin/sh")))
shell_env = get(ENV, "JULIA_SHELL", nothing)
if shell_env === nothing || isempty(shell_env)
shell_env = Sys.iswindows() ? "cmd" : get(ENV, "SHELL", "/bin/sh")
end
shell = shell_split(shell_env)
shell_name = Base.basename(shell[1])
Sys.iswindows() && (shell_name = lowercase(splitext(shell_name)[1])) # canonicalize for comparisons below

# Immediately expand all arguments, so that typing e.g. ~/bin/foo works.
cmd.exec .= expanduser.(cmd.exec)

isempty(cmd.exec) && throw(ArgumentError("no cmd to execute"))

if isempty(cmd.exec)
throw(ArgumentError("no cmd to execute"))
elseif cmd.exec[1] == "cd"
if cmd.exec[1] == "cd"
new_oldpwd = pwd()
if length(cmd.exec) > 2
throw(ArgumentError("cd method only takes one argument"))
Expand All @@ -58,19 +63,52 @@ function repl_cmd(cmd, out)
ENV["OLDPWD"] = new_oldpwd
println(out, pwd())
else
@static if !Sys.iswindows()
local command
if Sys.iswindows()
if shell_name == "cmd"
command = _CMD_execute(shell, cmd)
elseif shell_name in ("powershell", "pwsh")
command = _powershell_execute(shell, cmd)
elseif shell_name == "busybox"
Copy link
Contributor Author

@musm musm Oct 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really necessary ? We don't even ship with busy box anymore...

I think this is from older julia code, so my suggestion is to just remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arg .. looks like it is needed for testing on windows... maybe we should look into WSL for testing on windows instead of downloading busybox?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still something about this rubs me the wrong way. We should still try to remove this and try to work around them in the tests. Thoughts welcome

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make this the else block

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly do you mean by "making this the else block"?
Do you mean :

           if shell_name == "cmd"
                command = _CMD_execute(shell, cmd)
            elseif shell_name in ("powershell", "pwsh")
                command = _powershell_execute(shell, cmd)
            else
                  command  =  `$shell sh -c $(shell_escape_posixly(cmd))`  

Is the assumption here that if you don't want cmd or powershell then you probably want it to default to use a linux-based shell running in a windows environment?

command = `$shell sh -c $(shell_escape_posixly(cmd))`
else
command = `$shell $cmd`
end
else
if shell_name == "fish"
shell_escape_cmd = "begin; $(shell_escape_posixly(cmd)); and true; end"
command = `$shell -c $shell_escape_cmd`
elseif shell_name == "pwsh"
command = _powershell_execute(shell, cmd)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this do the wrong escaping on Unix?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm not sure it's worth the effort to support pwsh on Unix. This is only used for interactive mode, after all.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Powershell's handling of arguments is known (PowerShell/PowerShell#1995) to be buggy, so can't reliably be used for launching programs anyways.

Copy link
Member

@stevengj stevengj Nov 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vtjnash, that's argument handling on Windows. Presumably on Unix systems pwsh just uses *argv, no? I guess some amount of escaping is still needed for passing arguments through to subprocesses, but possibly the same problem does not appear on Unix because there is no CommandLineToArgvW layer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, no (e.g. PowerShell/PowerShell#10440)

no CommandLineToArgvW layer

Sadly, it is currently present (although both broken and undocumented—and eventually supposed to be unnecessary)

else
shell_escape_cmd = "($(shell_escape_posixly(cmd))) && true"
command = `$shell -c $shell_escape_cmd`
end
cmd = `$shell -c $shell_escape_cmd`
end
run(ignorestatus(cmd))
run(ignorestatus(command))
end
nothing
end

# process cmd's passed to CMD
_CMD_execute(shell, cmd) = Cmd(`$shell /c $(shell_escape_CMDly(shell_escape_winsomely(cmd)))`, windows_verbatim=true)

function _powershell_execute(shell, cmd)
# process cmd's passed to powershell
CommandType = nothing
try
CommandType = readchomp(`$shell -Command "Get-Command -- $(shell_escape_PWSH_cmdlet_ly(cmd.exec[1])) | Select-Object -ExpandProperty CommandType"`)
catch
end
# TODO: while CommandType == "Alias"; CommandType = ...; end
if CommandType == "Application"
command = Cmd(`$shell -Command "& $(shell_escape_PWSHly(shell_escape_winsomely(cmd)))"`)
else # handle Function and Cmdlet # TODO: what is the proper handling for the other types (ExternalScript, Script, Workflow, Configuration, and Filter)
command = Cmd(`$shell -Command "& $(shell_escape_PWSH_cmdlet_ly(cmd))"`)
end
return command
end

# deprecated function--preserved for DocTests.jl
function ip_matches_func(ip, func::Symbol)
for fr in StackTraces.lookup(ip)
Expand Down
10 changes: 4 additions & 6 deletions base/cmd.jl
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,10 @@ end
hash(x::AndCmds, h::UInt) = hash(x.a, hash(x.b, h))
==(x::AndCmds, y::AndCmds) = x.a == y.a && x.b == y.b

shell_escape(cmd::Cmd; special::AbstractString="") =
shell_escape(cmd.exec..., special=special)
shell_escape_posixly(cmd::Cmd) =
shell_escape_posixly(cmd.exec...)
shell_escape_winsomely(cmd::Cmd) =
shell_escape_winsomely(cmd.exec...)
shell_escape(cmd::Cmd; special::AbstractString="") = shell_escape(cmd.exec..., special=special)
shell_escape_posixly(cmd::Cmd) = shell_escape_posixly(cmd.exec...)
shell_escape_winsomely(cmd::Cmd) = shell_escape_winsomely(cmd.exec...)
shell_escape_PWSH_cmdlet_ly(cmd::Cmd) = shell_escape_PWSH_cmdlet_ly(cmd.exec...)

function show(io::IO, cmd::Cmd)
print_env = cmd.env !== nothing
Expand Down
68 changes: 67 additions & 1 deletion base/shell.jl
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ end


"""
shell_escaped_winsomely(args::Union{Cmd,AbstractString...})::String
shell_escaped_winsomely(args::Union{Cmd,AbstractString...}) -> String

Convert the collection of strings `args` into single string suitable for passing as the argument
string for a Windows command line. Windows passes the entire command line as a single string to
Expand All @@ -312,3 +312,69 @@ julia> println(shell_escaped_winsomely("A B\\", "C"))
"""
shell_escape_winsomely(args::AbstractString...) =
sprint(print_shell_escaped_winsomely, args..., sizehint=(sum(length, args)) + 3*length(args))

function print_shell_escaped_CMDly(io::IO, arg::AbstractString)
any(c -> c in ('\r', '\n'), arg) && throw(ArgumentError("Encountered unsupported character by CMD."))
# include " so to avoid toggling behavior of ^
arg = replace(arg, r"[%!^\"<>&|]" => s"^\0")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
arg = replace(arg, r"[%!^\"<>&|]" => s"^\0")
arg = replace(arg, r"[%)!^\"<>&|]" => s"^\0")

Upon closer inspection of https://stackoverflow.com/questions/4094699/how-does-the-windows-command-interpreter-cmd-exe-parse-scripts/4095133#4095133, I realized that ) is a metacharacter even though ( is not. (or we can include both here for symmetry)

print(io, arg)
end

"""
shell_escape_CMDly(arg::AbstractString) -> String

The unexported `shell_escape_CMDly` function takes a string and escapes any special characters
in such a way that it is safe to pass it as an argument to some `CMD.exe`. This may be useful
in concert with the `windows_verbatim` flag to [`Cmd`](@ref) when constructing process
pipelines.

See also [`shell_escape_PWSHly`](@ref).

# Example
```jldoctest
julia> println(shell_escape_CMDly("\"A B\\\" & C"))
^"A B\\^" ^& C

!important
Due to a peculiar behavior of the CMD, each command after a literal `|` character
(indicating a command pipeline) must have `shell_escape_CMDly` applied twice. For example:
```
to_print = "All for 1 & 1 for all!"
run(Cmd(Cmd(["cmd /c \"break | echo \$(shell_escape_CMDly(shell_escape_CMDly(to_print)))"]), windows_verbatim=true))
```
"""
shell_escape_CMDly(arg::AbstractString) = sprint(print_shell_escaped_CMDly, arg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to need tests. Also, can you split this aspect into a separate commit?


function print_shell_escaped_PWSHly(io::IO, arg::AbstractString)
# escape several characters that usually have special meaning
arg = replace(arg, r"[`\"\$#;|><&(){}=]" => s"`\0")
# escape special control chars
arg = replace(replace(replace(arg, '\r' => "`r"), '\t' => "`t"), '\t' => "`t")
print(io, arg)
end

"""
shell_escape_PWSHly(arg::AbstractString) -> String

Escapes special characters so they can be appropriately used with PowerShell.

See also [`shell_escape_CMDly`](@ref).
"""
shell_escape_PWSHly(arg::AbstractString) = sprint(print_shell_escaped_PWSHly, arg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's my suggested source for the PWSH_cmdlet_ly function I mention above:

Suggested change
shell_escape_PWSHly(arg::AbstractString) = sprint(print_shell_escaped_PWSHly, arg)
shell_escape_PWSHly(arg::AbstractString) = sprint(print_shell_escaped_PWSHly, arg)
function print_shell_escaped_PWSH_cmdlet_ly(io::IO, args::AbstractString...)
# often the shortest way to escape a powershell string is to double any single quotes and then wrap the whole thing in single quotes (alternatively, we could prefix all the non-word characters with a back-tick and replace newlines with \`r and \`n)
# but skip the escaping for common cases we always know are safe (e.g. so that named parameters are typically still interpreted correctly)
isword(c::AbstractChar) = '0' <= c <= '9' || 'a' <= c <= 'z' || 'A' <= c <= 'Z' || c == '_' || c == '\\' || c == ':' || c == '/' || c == '-'
join(io, (all(isword, arg) ? arg : "'" * replace(arg, "'" => "''") * "'" for arg in args), " ")
end
"""
shell_escape_PWSH_cmdlet_ly(args::AbstractString...) -> String
Escapes special characters so they can be appropriately used with a PowerShell cmdlet (such as `echo`).
See also [`shell_escape_PWSHly`](@ref) and [`shell_escape_winsomely`](@ref).
"""
print_shell_escaped_PWSH_cmdlet_ly(io::IO, args::AbstractString...)


function print_shell_escaped_PWSH_cmdlet_ly(io::IO, args::AbstractString...)
# often the shortest way to escape a powershell string is to double any single quotes and then wrap the whole thing in single quotes
# (alternatively, we could prefix all the non-word characters with a back-tick and replace newlines with `r and `n)
# but skip the escaping for common cases we always know are safe (e.g. so that named parameters are typically still interpreted correctly)
isword(c::AbstractChar) = '0' <= c <= '9' || 'a' <= c <= 'z' || 'A' <= c <= 'Z' || c == '_' || c == '\\' || c == ':' || c == '/' || c == '-'
join(io, (all(isword, arg) ? arg : string("'", replace(arg, "'" => "''"), "'") for arg in args), " ")
end

"""
shell_escape_PWSH_cmdlet_ly(args::AbstractString...) -> String

Escapes special characters so they can be appropriately used with a PowerShell cmdlet (such as `echo`).

See also [`shell_escape_PWSHly`](@ref) and [`shell_escape_winsomely`](@ref).
"""
shell_escape_PWSH_cmdlet_ly(args::AbstractString...) = sprint(print_shell_escaped_PWSH_cmdlet_ly, args...)
7 changes: 3 additions & 4 deletions doc/src/manual/environment-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,9 @@ The absolute path of the shell with which Julia should execute external commands
(via `Base.repl_cmd()`). Defaults to the environment variable `$SHELL`, and
falls back to `/bin/sh` if `$SHELL` is unset.

!!! note

On Windows, this environment variable is ignored, and external commands are
executed directly.
On Windows, `$JULIA_SHELL` can be set to `cmd`, `powershell`, `busybox` or `""`.
If set to `""` external commands are executed directly. Defaults to `cmd` if
`$JULIA_SHELL` is not set.

### `JULIA_EDITOR`

Expand Down
1 change: 1 addition & 0 deletions stdlib/REPL/docs/src/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ julia> ; # upon typing ;, the prompt changes (in place) to: shell>
shell> echo hello
hello
```
See `JULIA_SHELL` in the Environment Variables section of the Julia manual.

### Search modes

Expand Down