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.

Sign up for GitHub

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

Make it possible again to read STDERR without calling cat #11919

Closed
wants to merge 4 commits into from
+126 −121
Closed

Make it possible again to read STDERR without calling cat #11919

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
14bdff3
Make it possible again to read STDERR without calling `cat`
Keno Jun 28, 2015
a201a5b
Rename pipe -> pipeline
Keno Jul 17, 2015
ac569bd
Remove a test that no longer tests what it thinks it tests
Keno Aug 16, 2015
10966d7
fix spawn test to call readall before waiting for the process to exit…
vtjnash Aug 20, 2015
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
21 changes: 11 additions & 10 deletions base/deprecated.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,16 +207,17 @@ const MemoryError = OutOfMemoryError
#9295
@deprecate push!(t::Associative, key, v) setindex!(t, v, key)

@deprecate (|>)(src::AbstractCmd, dest::AbstractCmd) pipe(src, dest)
@deprecate (.>)(src::AbstractCmd, dest::AbstractCmd) pipe(src, stderr=dest)
@deprecate (|>)(src::Redirectable, dest::AbstractCmd) pipe(src, dest)
@deprecate (|>)(src::AbstractCmd, dest::Redirectable) pipe(src, dest)
@deprecate (.>)(src::AbstractCmd, dest::Redirectable) pipe(src, stderr=dest)
@deprecate (|>)(src::AbstractCmd, dest::AbstractString) pipe(src, dest)
@deprecate (|>)(src::AbstractString, dest::AbstractCmd) pipe(src, dest)
@deprecate (.>)(src::AbstractCmd, dest::AbstractString) pipe(src, stderr=dest)
@deprecate (>>)(src::AbstractCmd, dest::AbstractString) pipe(src, stdout=dest, append=true)
@deprecate (.>>)(src::AbstractCmd, dest::AbstractString) pipe(src, stderr=dest, append=true)
@deprecate (|>)(src::AbstractCmd, dest::AbstractCmd) pipeline(src, dest)
@deprecate (.>)(src::AbstractCmd, dest::AbstractCmd) pipeline(src, stderr=dest)
@deprecate (|>)(src::Redirectable, dest::AbstractCmd) pipeline(src, dest)
@deprecate (|>)(src::AbstractCmd, dest::Redirectable) pipeline(src, dest)
@deprecate (.>)(src::AbstractCmd, dest::Redirectable) pipeline(src, stderr=dest)
@deprecate (|>)(src::AbstractCmd, dest::AbstractString) pipeline(src, dest)
@deprecate (|>)(src::AbstractString, dest::AbstractCmd) pipeline(src, dest)
@deprecate (.>)(src::AbstractCmd, dest::AbstractString) pipeline(src, stderr=dest)
@deprecate (>>)(src::AbstractCmd, dest::AbstractString) pipeline(src, stdout=dest, append=true)
@deprecate (.>>)(src::AbstractCmd, dest::AbstractString) pipeline(src, stderr=dest, append=true)
@deprecate pipe pipeline

# 10314
@deprecate filter!(r::Regex, d::Dict) filter!((k,v)->ismatch(r,k), d)
Expand Down
3 changes: 2 additions & 1 deletion base/exports.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,8 @@ export
nb_available,
ntoh,
open,
pipe,
pipeline,
Pipe,
Copy link
Contributor

Choose a reason for hiding this comment

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

kinda confusing to export this when it's a rather different thing than the pipe function

Copy link
Member Author

Choose a reason for hiding this comment

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

I think once processes are readable/writable, this can just be pipe() (although @vtjnash wants that to return a pair). I think maybe the right design is instead of have another method that creates the other pipe end.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe have a real API design discussion about this?

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 I'd be ok with calling this pipe() and adding pipepair() in the future if necessary. Names that differ only in case are really annoying.

Copy link
Member

Choose a reason for hiding this comment

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

So what does the pipe function do now then? Either connect two commands via a pipe or construct a pipe object?

Copy link
Member

Choose a reason for hiding this comment

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

Keno and I both suggested that at the beginning of this thread 17 days ago.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem with that is that pipe with more than one argument is a lazy constructor for a command object, while Pipe() creates a pipe object that run modifies.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it might just have to be Pipe and pipeline, though I sorta dread having to tell people they have to go from one character to 8 for this functionality.

Of course, I also suggested const 🚰= Pipe

Copy link
Member

Choose a reason for hiding this comment

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

Hmm. Maybe having pipe and Pipe isn't so bad :-\

Copy link
Member Author

Choose a reason for hiding this comment

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

One suggestion that I forgot to mention here was pipe and PipeEnd. I also considered Piping (as in to get data out of your pipeline, just add some Piping).

PipeBuffer,
poll_fd,
poll_file,
Expand Down
6 changes: 3 additions & 3 deletions base/interactiveutil.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ end
global _clipboardcmd
_clipboardcmd !== nothing && return _clipboardcmd
for cmd in (:xclip, :xsel)
success(pipe(`which $cmd`, DevNull)) && return _clipboardcmd = cmd
success(pipeline(`which $cmd`, DevNull)) && return _clipboardcmd = cmd
end
error("no clipboard command found, please install xsel or xclip")
end
Expand Down Expand Up @@ -165,7 +165,7 @@ function versioninfo(io::IO=STDOUT, verbose::Bool=false)
println(io, " WORD_SIZE: ", Sys.WORD_SIZE)
if verbose
lsb = ""
@linux_only try lsb = readchomp(pipe(`lsb_release -ds`, stderr=DevNull)) end
@linux_only try lsb = readchomp(pipeline(`lsb_release -ds`, stderr=DevNull)) end
@windows_only try lsb = strip(readall(`$(ENV["COMSPEC"]) /c ver`)) end
if lsb != ""
println(io, " ", lsb)
Expand Down Expand Up @@ -343,7 +343,7 @@ downloadcmd = nothing
global downloadcmd
if downloadcmd === nothing
for checkcmd in (:curl, :wget, :fetch)
if success(pipe(`which $checkcmd`, DevNull))
if success(pipeline(`which $checkcmd`, DevNull))
downloadcmd = checkcmd
break
end
Expand Down
2 changes: 1 addition & 1 deletion base/pkg/entry.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function add(pkg::AbstractString, vers::VersionSet)
outdated = :yes
else
try
run(pipe(Git.cmd(`fetch -q --all`, dir="METADATA"),stdout=DevNull,stderr=DevNull))
run(pipeline(Git.cmd(`fetch -q --all`, dir="METADATA"),stdout=DevNull,stderr=DevNull))
outdated = Git.success(`diff --quiet origin/$branch`, dir="METADATA") ?
(:no) : (:yes)
end
Expand Down
2 changes: 1 addition & 1 deletion base/pkg/generate.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ github_user() = readchomp(ignorestatus(`git config --global --get github.user`))
function git_contributors(dir::AbstractString, n::Int=typemax(Int))
contrib = Dict()
tty = @windows? "CON:" : "/dev/tty"
for line in eachline(pipe(tty, Git.cmd(`shortlog -nes`, dir=dir)))
for line in eachline(pipeline(tty, Git.cmd(`shortlog -nes`, dir=dir)))
m = match(r"\s*(\d+)\s+(.+?)\s+\<(.+?)\>\s*$", line)
m === nothing && continue
commits, name, email = m.captures
Expand Down
2 changes: 1 addition & 1 deletion base/pkg/git.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function git(d)
end

cmd(args::Cmd; dir="") = `$(git(dir)) $args`
run(args::Cmd; dir="", out=STDOUT) = Base.run(pipe(cmd(args,dir=dir), out))
run(args::Cmd; dir="", out=STDOUT) = Base.run(pipeline(cmd(args,dir=dir), out))
readall(args::Cmd; dir="") = Base.readall(cmd(args,dir=dir))
readchomp(args::Cmd; dir="") = Base.readchomp(cmd(args,dir=dir))

Expand Down
30 changes: 18 additions & 12 deletions base/process.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function show(io::IO, cmd::Cmd)
end

function show(io::IO, cmds::Union{OrCmds,ErrOrCmds})
print(io, "pipe(")
print(io, "pipeline(")
show(io, cmds.a)
print(io, ", ")
print(io, isa(cmds, ErrOrCmds) ? "stderr=" : "stdout=")
Expand Down Expand Up @@ -100,7 +100,7 @@ type CmdRedirect <: AbstractCmd
end

function show(io::IO, cr::CmdRedirect)
print(io, "pipe(")
print(io, "pipeline(")
show(io, cr.cmd)
print(io, ", ")
if cr.stream_no == STDOUT_NO
Expand Down Expand Up @@ -149,7 +149,7 @@ redir_err(src::AbstractCmd, dest::AbstractString) = CmdRedirect(src, FileRedirec
redir_out_append(src::AbstractCmd, dest::AbstractString) = CmdRedirect(src, FileRedirect(dest, true), STDOUT_NO)
redir_err_append(src::AbstractCmd, dest::AbstractString) = CmdRedirect(src, FileRedirect(dest, true), STDERR_NO)

function pipe(cmd::AbstractCmd; stdin=nothing, stdout=nothing, stderr=nothing, append::Bool=false)
function pipeline(cmd::AbstractCmd; stdin=nothing, stdout=nothing, stderr=nothing, append::Bool=false)
if append && stdout === nothing && stderr === nothing
error("append set to true, but no output redirections specified")
end
Expand All @@ -165,10 +165,10 @@ function pipe(cmd::AbstractCmd; stdin=nothing, stdout=nothing, stderr=nothing, a
return cmd
end

pipe(cmd::AbstractCmd, dest) = pipe(cmd, stdout=dest)
pipe(src::Union{Redirectable,AbstractString}, cmd::AbstractCmd) = pipe(cmd, stdin=src)
pipeline(cmd::AbstractCmd, dest) = pipeline(cmd, stdout=dest)
pipeline(src::Union{Redirectable,AbstractString}, cmd::AbstractCmd) = pipeline(cmd, stdin=src)

pipe(a, b, c, d...) = pipe(pipe(a,b), c, d...)
pipeline(a, b, c, d...) = pipeline(pipeline(a,b), c, d...)

typealias RawOrBoxedHandle Union{UVHandle,AsyncStream,Redirectable,IOStream}
typealias StdIOSet NTuple{3,RawOrBoxedHandle}
Expand Down Expand Up @@ -311,7 +311,9 @@ macro setup_stdio()
in,out,err = stdios
if isa(stdios[1], Pipe)
if stdios[1].handle == C_NULL
Copy link
Member

Choose a reason for hiding this comment

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

ref #10469 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

reminder (this should be using the test isopen(stdios[1]), now that we have that field)

Copy link
Member Author

Choose a reason for hiding this comment

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

As per our discussion, the C_NULL here should be fine as this way you get an error in the use-after-close case whereas otherwise it would silently pass /dev/null.

error("pipes passed to spawn must be initialized")
in = box(Ptr{Void},Intrinsics.jl_alloca(_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, JL_O_RDONLY)
Expand All @@ -321,7 +323,9 @@ macro setup_stdio()
end
if isa(stdios[2], Pipe)
if stdios[2].handle == C_NULL
error("pipes passed to spawn must be initialized")
out = box(Ptr{Void},Intrinsics.jl_alloca(_sizeof_uv_named_pipe))
link_pipe(stdios[2],true,out,false)
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)
Expand All @@ -331,7 +335,9 @@ macro setup_stdio()
end
if isa(stdios[3], Pipe)
if stdios[3].handle == C_NULL
error("pipes passed to spawn must be initialized")
err = box(Ptr{Void},Intrinsics.jl_alloca(_sizeof_uv_named_pipe))
link_pipe(stdios[3],true,err,false)
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)
Expand All @@ -345,9 +351,9 @@ end
macro cleanup_stdio()
esc(
quote
close_in && close(in)
close_out && close(out)
close_err && close(err)
close_in && (isa(in,Ptr) ? close_pipe_sync(in) : close(in))
close_out && (isa(out,Ptr) ? close_pipe_sync(out) : close(out))
close_err && (isa(err,Ptr) ? close_pipe_sync(err) : close(err))
end)
end

Expand Down
2 changes: 1 addition & 1 deletion base/random.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function make_seed()
seed = reinterpret(UInt64, time())
seed = hash(seed, UInt64(getpid()))
try
seed = hash(seed, parse(UInt64, readall(pipe(`ifconfig`, `sha1sum`))[1:40], 16))
seed = hash(seed, parse(UInt64, readall(pipeline(`ifconfig`, `sha1sum`))[1:40], 16))
end
return make_seed(seed)
end
Expand Down
7 changes: 6 additions & 1 deletion base/socket.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,12 @@ _jl_sockaddr_set_port(ptr::Ptr{Void},port::UInt16) =
ccall(:jl_sockaddr_set_port,Void,(Ptr{Void},UInt16),ptr,port)

accept(server::TCPServer) = accept(server, TCPSocket())
accept(server::PipeServer) = accept(server, Pipe())

# Libuv will internally reset the readable and writable flags on
# this pipe after it has successfully accepted the connection, to
# remember that before that this is an invalid pipe
accept(server::PipeServer) = accept(server, init_pipe!(Pipe();
readable=false, writable=false, julia_only=true))

##

Expand Down
34 changes: 13 additions & 21 deletions base/stream.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,18 +128,7 @@ type Pipe <: AsyncStream
nothing, ReentrantLock(),
DEFAULT_READ_BUFFER_SZ)
end
function Pipe()
handle = Libc.malloc(_sizeof_uv_named_pipe)
try
ret = Pipe(handle)
associate_julia_struct(ret.handle,ret)
finalizer(ret,uvfinalize)
return init_pipe!(ret;readable=true)
catch
Libc.free(handle)
rethrow()
end
end
Pipe() = Pipe(C_NULL)

type PipeServer <: UVServer
handle::Ptr{Void}
Expand All @@ -157,8 +146,9 @@ end

function init_pipe!(pipe::Union{Pipe,PipeServer};readable::Bool=false,writable=false,julia_only=true)
if pipe.handle == C_NULL
error("failed to initialize pipe")
elseif pipe.status != StatusUninit
malloc_julia_pipe!(pipe)
end
if pipe.status != StatusUninit
error("pipe is already initialized")
end
uv_error("init_pipe",ccall(:jl_init_pipe, Cint, (Ptr{Void},Int32,Int32,Int32), pipe.handle, writable,readable,julia_only))
Expand Down Expand Up @@ -652,7 +642,7 @@ function process_events(block::Bool)
end

## pipe functions ##
function malloc_julia_pipe(x)
function malloc_julia_pipe!(x)
x.handle = Libc.malloc(_sizeof_uv_named_pipe)
associate_julia_struct(x.handle,x)
finalizer(x,uvfinalize)
Expand All @@ -676,7 +666,7 @@ end

function link_pipe(read_end::Pipe,readable_julia_only::Bool,write_end::Ptr{Void},writable_julia_only::Bool)
if read_end.handle == C_NULL
malloc_julia_pipe(read_end)
malloc_julia_pipe!(read_end)
end
init_pipe!(read_end; readable = true, writable = false, julia_only = readable_julia_only)
uv_error("init_pipe",ccall(:jl_init_pipe, Cint, (Ptr{Void},Int32,Int32,Int32), write_end, 1, 0, writable_julia_only))
Expand All @@ -685,7 +675,7 @@ function link_pipe(read_end::Pipe,readable_julia_only::Bool,write_end::Ptr{Void}
end
function link_pipe(read_end::Ptr{Void},readable_julia_only::Bool,write_end::Pipe,writable_julia_only::Bool)
if write_end.handle == C_NULL
malloc_julia_pipe(write_end)
malloc_julia_pipe!(write_end)
end
uv_error("init_pipe",ccall(:jl_init_pipe, Cint, (Ptr{Void},Int32,Int32,Int32), read_end, 0, 1, readable_julia_only))
init_pipe!(write_end; readable = false, writable = true, julia_only = writable_julia_only)
Expand All @@ -694,10 +684,10 @@ function link_pipe(read_end::Ptr{Void},readable_julia_only::Bool,write_end::Pipe
end
function link_pipe(read_end::Pipe,readable_julia_only::Bool,write_end::Pipe,writable_julia_only::Bool)
if write_end.handle == C_NULL
malloc_julia_pipe(write_end)
malloc_julia_pipe!(write_end)
end
if read_end.handle == C_NULL
malloc_julia_pipe(read_end)
malloc_julia_pipe!(read_end)
end
init_pipe!(read_end; readable = true, writable = false, julia_only = readable_julia_only)
init_pipe!(write_end; readable = false, writable = true, julia_only = writable_julia_only)
Expand Down Expand Up @@ -961,7 +951,7 @@ function accept_nonblock(server::PipeServer,client::Pipe)
err
end
function accept_nonblock(server::PipeServer)
client = Pipe()
client = init_pipe!(Pipe(); readable=true, writable=true, julia_only=true)
uv_error("accept", accept_nonblock(server,client) != 0)
client
end
Expand Down Expand Up @@ -1031,7 +1021,9 @@ function connect(sock::AsyncStream, args...)
sock
end

connect(path::AbstractString) = connect(Pipe(),path)
# Libuv will internally reset read/writability, which is uses to
# mark that this is an invalid pipe.
connect(path::AbstractString) = connect(init_pipe!(Pipe(); readable=false, writable=false, julia_only=true),path)

_fd(x::IOStream) = RawFD(fd(x))
@unix_only _fd(x::AsyncStream) = RawFD(ccall(:jl_uv_handle,Int32,(Ptr{Void},),x.handle))
Expand Down
Loading