Skip to content

Commit

Permalink
Fix redirection operators
Browse files Browse the repository at this point in the history 
Fixes #2259
Fixes #2355
Fixes #3347
  • Loading branch information
@Keno
Keno committed Jun 17, 2013
1 parent a651fb4 commit 377de75
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 126 deletions.
6 changes: 6 additions & 0 deletions base/deprecated.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,12 @@ export PipeString
@deprecate -(a::IntSet, b::IntSet) setdiff(a,b)
@deprecate ~(s::IntSet) complement(s)

# Redirection Operators
@deprecate |(a::AbstractCmd,b::AbstractCmd) (a|>b)
@deprecate >(a::Redirectable,b::AbstractCmd) (a|>b)
@deprecate >(a::ASCIIString,b::AbstractCmd) (a|>b)
@deprecate >(a::AbstractCmd,b::Redirectable) (a|>b)
@deprecate >(a::AbstractCmd,b::ASCIIString) (a|>b)

# note removed macros: str, B_str, I_str, E_str, L_str, L_mstr, I_mstr, E_mstr

Expand Down
6 changes: 3 additions & 3 deletions base/fs.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export File,
S_IROTH, S_IWOTH, S_IXOTH, S_IRWXO

#import Base.show, Base.open, Base.close, Base.write
import Base: uvtype, uvhandle, eventloop, fd, position, stat
import Base: uvtype, uvhandle, eventloop, fd, position, stat, close

include("file_constants.jl")

Expand Down Expand Up @@ -81,7 +81,7 @@ function close(f::File)
req = Intrinsics.box(Ptr{Void},Intrinsics.jl_alloca(Intrinsics.unbox(Int32,_sizeof_uv_fs_t)))
err = ccall(:uv_fs_close,Int32,(Ptr{Void},Ptr{Void},Int32,Ptr{Void}),
eventloop(),req,f.handle,C_NULL)
uv_error(err)
uv_error("close",err != 0)
f.handle = -1
f.open = false
ccall(:uv_fs_req_cleanup,Void,(Ptr{Void},),req)
Expand All @@ -92,7 +92,7 @@ function unlink(p::String)
req = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_fs_t)))
err = ccall(:uv_fs_unlink,Int32,(Ptr{Void},Ptr{Void},Ptr{Uint8},Ptr{Void}),
eventloop(),req,bytestring(p),C_NULL)
uv_error(err)
uv_error("unlink",err != 0)
end
function unlink(f::File)
if f.open
Expand Down
274 changes: 151 additions & 123 deletions base/process.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,36 +44,36 @@ function show(io::IO, cmd::Cmd)
end

function show(io::IO, cmds::OrCmds)
if isa(cmds.a, AndCmds)
print("(")
if isa(cmds.a, AndCmds) || isa(cmds.a, CmdRedirect)
print(io,"(")
show(io, cmds.a)
print(")")
print(io,")")
else
show(io, cmds.a)
end
print(" | ")
if isa(cmds.b, AndCmds)
print("(")
print(" |> ")
if isa(cmds.b, AndCmds) || isa(cmds.b, CmdRedirect)
print(io,"(")
show(io, cmds.b)
print(")")
print(io,")")
else
show(io, cmds.b)
end
end

function show(io::IO, cmds::AndCmds)
if isa(cmds.a, OrCmds)
print("(")
if isa(cmds.a, OrCmds) || isa(cmds.a, CmdRedirect)
print(io,"(")
show(io, cmds.a)
print(")")
print(io,")")
else
show(io, cmds.a)
end
print(" & ")
if isa(cmds.b, OrCmds)
print("(")
print(io," & ")
if isa(cmds.b, OrCmds) || isa(cmds.b, CmdRedirect)
print(io,"(")
show(io, cmds.b)
print(")")
print(io,")")
else
show(io, cmds.b)
end
Expand All @@ -83,29 +83,57 @@ const STDIN_NO = 0
const STDOUT_NO = 1
const STDERR_NO = 2

typealias Redirectable Union(UVStream,FS.File)
immutable FileRedirect
filename::ASCIIString

This comment has been minimized.

Sign in to view

Copy link
@JeffBezanson

JeffBezanson Jun 18, 2013

Member

What about UTF8 filenames?
append::Bool
end

typealias Redirectable Union(UVStream,FS.File,FileRedirect)

type CmdRedirect <: AbstractCmd
cmd::AbstractCmd
handle::Redirectable
stream_no::Int
end

function show(io::IO, cr::CmdRedirect)
if cr.stream_no == STDOUT_NO
show(io,cr.cmd)
print(io," |> ")
show(io,cr.handle)
elseif cr.stream_no == STDERR_NO
show(io,cr.cmd)
print(io," .> ")
show(io,cr.handle)
elseif cr.stream_no == STDIN_NO
show(io,cr.handle)
print(io," |> ")
show(io,cr.cmd)
end
end


ignorestatus(cmd::Cmd) = (cmd.ignorestatus=true; cmd)
ignorestatus(cmd::Union(OrCmds,AndCmds)) = (ignorestatus(cmd.a); ignorestatus(cmd.b); cmd)
detach(cmd::Cmd) = (cmd.detach=true; cmd)

(&)(left::AbstractCmd,right::AbstractCmd) = AndCmds(left,right)
(|)(src::AbstractCmd,dest::AbstractCmd) = OrCmds(src,dest)
(<)(left::AbstractCmd,right::Redirectable) = CmdRedirect(left,right,STDIN_NO)

This comment has been minimized.

Sign in to view

Copy link
@JeffBezanson

JeffBezanson Jun 18, 2013

Member

The < operators need deprecations or definitions.

This comment has been minimized.

Sign in to view

Copy link
@Keno

Keno Jun 18, 2013

Author Member

Was about to add that.
(>)(left::AbstractCmd,right::Redirectable) = CmdRedirect(left,right,STDOUT_NO)
(<)(left::AbstractCmd,right::String) = left < FS.open(right,JL_O_RDONLY)
(>)(left::AbstractCmd,right::String) = left > FS.open(right,JL_O_WRONLY|JL_O_CREAT|JL_O_TRUNC,S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
(>>)(left::AbstractCmd,right::String) = left > FS.open(right,JL_O_WRONLY|JL_O_APPEND|JL_O_CREAT,S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
(>)(left::Any,right::AbstractCmd) = right < left
(.>)(left::AbstractCmd,right::Redirectable) = CmdRedirect(left,right,STDERR_NO)

typealias RawOrBoxedHandle Union(UVHandle,UVStream,FS.File)
(|>)(src::AbstractCmd,dest::AbstractCmd) = OrCmds(src,dest)

# Stream Redirects
(|>)(dest::Redirectable,src::AbstractCmd) = CmdRedirect(src,dest,STDIN_NO)
(|>)(src::AbstractCmd,dest::Redirectable) = CmdRedirect(src,dest,STDOUT_NO)
(.>)(src::AbstractCmd,dest::Redirectable) = CmdRedirect(src,dest,STDERR_NO)

# File redirects
(|>)(src::AbstractCmd,dest::ASCIIString) = CmdRedirect(src,FileRedirect(dest,false),STDOUT_NO)
(|>)(dest::Redirectable,src::AbstractCmd) = CmdRedirect(src,dest,STDIN_NO)
(.>)(src::AbstractCmd,dest::Redirectable) = CmdRedirect(src,FileRedirect(dest,false),STDERR_NO)

This comment has been minimized.

Sign in to view

Copy link
@JeffBezanson

JeffBezanson Jun 18, 2013

Member

Won't this in some cases try to construct a FileRedirect from a dest that is not a string?

This comment has been minimized.

Sign in to view

Copy link
@Keno

Keno Jun 18, 2013

Author Member

Yes, you're right. That should be String (I will change the filename above from ASCIIString to String as well)
(>>)(src::AbstractCmd,dest::ASCIIString) = CmdRedirect(src,FileRedirect(dest,true),STDOUT_NO)
(.>>)(src::AbstractCmd,dest::Redirectable) = CmdRedirect(src,FileRedirect(dest,true),STDERR_NO)

This comment has been minimized.

Sign in to view

Copy link
@StefanKarpinski

StefanKarpinski Jun 17, 2013

Member

Using .>, >> and .>> seems kind of bad now that we use |> for piping. Not sure what to do instead though.

This comment has been minimized.

Sign in to view

Copy link
@carlobaldassi

carlobaldassi Jun 17, 2013

Member

|>> seems more consistent than >>.
I wouldn't know about .> and .>>.

BTW is <| missing? At least there should be a deprecation, if possible.

This comment has been minimized.

Sign in to view

Copy link
@carlobaldassi

carlobaldassi Jun 18, 2013

Member

Maybe |.> or |!> or something along those lines?

This comment has been minimized.

Sign in to view

Copy link
@Keno

Keno Jun 18, 2013

Author Member

Yes I decided to not support <| because it just makes things confusing and I couldn't come up with a case in which it would be useful. This way data always flows from left to right. I'm open for suggestions as to the redirect syntax. I just left it like this for now pending further discussion. I don't find >> terribly bad for append, but maybe &> and &>> would be better for STDERR?

This comment has been minimized.

Sign in to view

Copy link
@JeffBezanson

JeffBezanson Jun 18, 2013

Member

I'm fine with using |> for pipes and > etc. for file redirects.

This comment has been minimized.

Sign in to view

Copy link
@StefanKarpinski

StefanKarpinski Jun 18, 2013

Member

The whole point of this operator change was to avoid syntax punning that muddles the meaning of generic operators. Does >> mean "right shift" or "append"? One of the great things about generic functions is how they capture a single concept as an object. If we're not going to be disciplined about this, why bother with this change in the first place?

This comment has been minimized.

Sign in to view

Copy link
@JeffBezanson

JeffBezanson Jun 20, 2013

Member

We could add |>> and <<|. I could also live without special syntax for stderr. It could be something like stderr(cmd, file) |> "out".

This comment has been minimized.

Sign in to view

Copy link
@StefanKarpinski

StefanKarpinski Jun 20, 2013

Member

Yes, this:

We could add |>> and <<|. I could also live without special syntax for stderr. It could be something like stderr(cmd, file) |> "out".

I'm not certain why we need <<| though.


typealias RawOrBoxedHandle Union(UVHandle,UVStream,FS.File,FileRedirect)
typealias StdIOSet (RawOrBoxedHandle, RawOrBoxedHandle, RawOrBoxedHandle)

type Process
Expand Down Expand Up @@ -136,9 +164,9 @@ end

type ProcessChain
processes::Vector{Process}
in::UVStream
out::UVStream
err::UVStream
in::Redirectable
out::Redirectable
err::Redirectable
ProcessChain(stdios::StdIOSet) = new(Process[],stdios[1],stdios[2],stdios[3])
end
typealias ProcessChainOrNot Union(Bool,ProcessChain)
Expand Down Expand Up @@ -184,52 +212,6 @@ function _uv_hook_close(proc::Process)
notify(proc.closenotify)
end

function spawn(pc::ProcessChainOrNot,cmd::Cmd,stdios::StdIOSet,exitcb::Callback,closecb::Callback)
loop = eventloop()
close_in,close_out,close_err = false,false,false
pp = Process(cmd,C_NULL,stdios[1],stdios[2],stdios[3]);
in,out,err=stdios
if(isa(stdios[1],NamedPipe)&&stdios[1].handle==C_NULL)
in = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_named_pipe)))
#in = c_malloc(_sizeof_uv_named_pipe)
link_pipe(in,false,stdios[1],true)
close_in = true
end
if(isa(stdios[2],NamedPipe)&&stdios[2].handle==C_NULL)
out = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_named_pipe)))
#out = c_malloc(_sizeof_uv_named_pipe)
link_pipe(stdios[2],false,out,true)
close_out = true
end
if(isa(stdios[3],NamedPipe)&&stdios[3].handle==C_NULL)
err = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_named_pipe)))
#err = c_malloc(_sizeof_uv_named_pipe)
link_pipe(stdios[3],false,err,true)
close_err = true
end
ptrs = _jl_pre_exec(cmd.exec)
pp.exitcb = exitcb
pp.closecb = closecb
pp.handle = _jl_spawn(ptrs[1], convert(Ptr{Ptr{Uint8}}, ptrs), loop, pp,
in,out,err)
if pc != false
push!(pc.processes, pp)
end
if(close_in)
close_pipe_sync(in)
#c_free(in)
end
if(close_out)
close_pipe_sync(out)
#c_free(out)
end
if(close_err)
close_pipe_sync(err)
#c_free(err)
end
pp
end

function spawn(pc::ProcessChainOrNot,redirect::CmdRedirect,stdios::StdIOSet,exitcb::Callback,closecb::Callback)
spawn(pc,redirect.cmd,(redirect.stream_no==STDIN_NO ?redirect.handle:stdios[1],
redirect.stream_no==STDOUT_NO?redirect.handle:stdios[2],
Expand Down Expand Up @@ -258,45 +240,95 @@ function spawn(pc::ProcessChainOrNot,cmds::OrCmds,stdios::StdIOSet,exitcb::Callb
pc
end

macro setup_stdio()
esc(
quote
close_in,close_out,close_err = false,false,false
in,out,err = stdios
if isa(stdios[1],NamedPipe)
if stdios[1].handle==C_NULL
in = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_named_pipe)))
#in = c_malloc(_sizeof_uv_named_pipe)
link_pipe(in,false,stdios[1],true)
close_in = true
end
elseif isa(stdios[1],FileRedirect)
in = FS.open(stdios[1].filename,O_RDONLY)

This comment has been minimized.

Sign in to view

Copy link
@JeffBezanson

JeffBezanson Jun 18, 2013

Member

Should this be JL_O_RDONLY?

This comment has been minimized.

Sign in to view

Copy link
@Keno

Keno Jun 18, 2013

Author Member

Yep, I should obviously add more tests for this stuff.
close_in = true
end
if isa(stdios[2],NamedPipe)
if stdios[2].handle==C_NULL
out = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_named_pipe)))
#out = c_malloc(_sizeof_uv_named_pipe)
link_pipe(stdios[2],false,out,true)
close_out = true
end
elseif isa(stdios[2],FileRedirect)
out = FS.open(stdios[2].filename,JL_O_WRONLY|JL_O_CREAT|(stdios[2].append?JL_O_APPEND:JL_O_TRUNC),S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
close_out = true
end
if isa(stdios[3],NamedPipe)
if stdios[3].handle==C_NULL
err = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_named_pipe)))
#err = c_malloc(_sizeof_uv_named_pipe)
link_pipe(stdios[3],false,err,true)
close_err = true
end
elseif isa(stdios[3],FileRedirect)
err = FS.open(stdios[3].filename,JL_O_WRONLY|JL_O_CREAT|(stdios[3].append?JL_O_APPEND:JL_O_TRUNC),S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
close_err = true
end
end)
end

macro cleanup_stdio()
esc(
quote
if close_in
if isa(in,Ptr)
close_pipe_sync(in)
else
close(in)
end
end
if(close_out)
if isa(out,Ptr)
close_pipe_sync(out)
else
close(out)
end
end
if(close_err)
if isa(err,Ptr)
close_pipe_sync(err)
else
close(err)
end
end
end)
end

function spawn(pc::ProcessChainOrNot,cmd::Cmd,stdios::StdIOSet,exitcb::Callback,closecb::Callback)
loop = eventloop()
pp = Process(cmd,C_NULL,stdios[1],stdios[2],stdios[3]);
@setup_stdio
ptrs = _jl_pre_exec(cmd.exec)
pp.exitcb = exitcb
pp.closecb = closecb
pp.handle = _jl_spawn(ptrs[1], convert(Ptr{Ptr{Uint8}}, ptrs), loop, pp,
in,out,err)
@cleanup_stdio
pp
end

function spawn(pc::ProcessChainOrNot,cmds::AndCmds,stdios::StdIOSet,exitcb::Callback,closecb::Callback)
if pc == false
pc = ProcessChain(stdios)
end
close_in,close_out,close_err = false,false,false
in,out,err = stdios
if(isa(stdios[1],NamedPipe)&&stdios[1].handle==C_NULL)
in = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_named_pipe)))
#in = c_malloc(_sizeof_uv_named_pipe)
link_pipe(in,false,stdios[1],true)
close_in = true
end
if(isa(stdios[2],NamedPipe)&&stdios[2].handle==C_NULL)
out = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_named_pipe)))
#out = c_malloc(_sizeof_uv_named_pipe)
link_pipe(stdios[2],false,out,true)
close_out = true
end
if(isa(stdios[3],NamedPipe)&&stdios[3].handle==C_NULL)
err = box(Ptr{Void},Intrinsics.jl_alloca(unbox(Int32,_sizeof_uv_named_pipe)))
#err = c_malloc(_sizeof_uv_named_pipe)
link_pipe(stdios[3],false,err,true)
close_err = true
end
@setup_stdio
spawn(pc, cmds.a, (in,out,err), exitcb, closecb)
spawn(pc, cmds.b, (in,out,err), exitcb, closecb)
if(close_in)
close_pipe_sync(in)
#c_free(in)
end
if(close_out)
close_pipe_sync(out)
#c_free(out)
end
if(close_err)
close_pipe_sync(err)
#c_free(err)
end
pp
@cleanup_stdio
pc
end

Expand All @@ -319,22 +351,18 @@ end
# | \ The function to be called once the process exits
# \ A set of up to 256 stdio instructions, where each entry can be either:
# | - An AsyncStream to be passed to the child
# | - true: This will let the child inherit the parents' io (only valid for 0-2)
# \ - false: None (for 3-255) or /dev/null (for 0-2)


for (sym, stdin, stdout, stderr) in {(:spawn_opts_inherit, STDIN,STDOUT,STDERR),
(:spawn_opts_swallow, null_handle,null_handle,null_handle)}
@eval begin
($sym)(stdios::StdIOSet,exitcb::Callback,closecb::Callback) = (stdios,exitcb,closecb)
($sym)(stdios::StdIOSet,exitcb::Callback) = (stdios,exitcb,false)
($sym)(stdios::StdIOSet) = (stdios,false,false)
($sym)() = (($stdin,$stdout,$stderr),false,false)
($sym)(in::UVStream) = ((isa(in,AsyncStream)?in:$stdin,$stdout,$stderr),false,false)
($sym)(in::UVStream,out::UVStream) = ((isa(in,AsyncStream)?in:$stdin,isa(out,AsyncStream)?out:$stdout,$stderr),false,false)
($sym)(in::UVStream,out::UVStream,err::UVStream) = ((isa(in,AsyncStream)?in:$stdin,isa(out,AsyncStream)?out:$stdout,isa(err,AsyncStream)?err:$stderr),false,false)
end
end
# | - null_handle to pass /dev/null
# | - An FS.File object to redirect the output to
# \ - An ASCIIString specifying a filename to be opened

spawn_opts_swallow(stdios::StdIOSet,exitcb::Callback=false,closecb::Callback=false) =
(stdios,exitcb,closecb)
spawn_opts_swallow(in::Redirectable=null_handle,out::Redirectable=null_handle,err::Redirectable=null_handle,args...) =
(tuple(in,out,err,args...),false,false)
spawn_opts_inherit(stdios::StdIOSet,exitcb::Callback=false,closecb::Callback=false) =
(stdios,exitcb,closecb)
spawn_opts_inherit(in::Redirectable=STDIN,out::Redirectable=STDOUT,err::Redirectable=STDERR,args...) =
(tuple(in,out,err,args...),false,false)

spawn(pc::ProcessChainOrNot,cmds::AbstractCmd,args...) = spawn(pc,cmds,spawn_opts_swallow(args...)...)
spawn(cmds::AbstractCmd,args...) = spawn(false,cmds,spawn_opts_swallow(args...)...)
Expand Down

0 comments on commit 377de75

Please sign in to comment.