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

RFC: deprecate readsfrom/writesto in favor of simply using open on AbstractCmd #6948

Merged
merged 3 commits into from
May 31, 2014
Merged
Show file tree
Hide file tree
Changes from 1 commit
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: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ Deprecated or removed

* `bitmix` is replaced by a 2-argument form of `hash`.

* `readsfrom` and `writesto` are replaced by `open` ([#6948]).

[#4042]: https://github.com/JuliaLang/julia/issues/4042
[#5164]: https://github.com/JuliaLang/julia/issues/5164
[#4026]: https://github.com/JuliaLang/julia/issues/4026
Expand Down
3 changes: 3 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,9 @@ Set{T<:Number}(xs::T...) = Set{T}(xs)
@deprecate bitmix(x, y::Union(Uint32, Int32)) convert(Uint32, hash(x, uint(y)))
@deprecate bitmix(x, y::Union(Uint64, Int64)) convert(Uint64, hash(x, hash(y)))

@deprecate readsfrom(cmd, args...) open(cmd, "r", args...)
@deprecate writesto(cmd, args...) open(cmd, "w", args...)

# 0.3 discontinued functions

function nnz(X)
Expand Down
2 changes: 0 additions & 2 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1258,12 +1258,10 @@ export
process_exited,
process_running,
readandwrite,
readsfrom,
run,
setenv,
spawn,
success,
writesto,

# C interface
c_free,
Expand Down
14 changes: 6 additions & 8 deletions base/interactiveutil.jl
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,9 @@ end

@osx_only begin
function clipboard(x)
w,p = writesto(`pbcopy`)
print(w,x)
close(w)
wait(p)
open(`pbcopy`, "w") do io
print(io, x)
end
end
clipboard() = readall(`pbpaste`)
end
Expand All @@ -89,10 +88,9 @@ end
cmd = c == :xsel ? `xsel --nodetach --input --clipboard` :
c == :xclip ? `xclip -quiet -in -selection clipboard` :
error("unexpected clipboard command: $c")
w,p = writesto(cmd)
print(w,x)
close(w)
wait(p)
open(cmd, "w") do io
print(io, x)
end
end
function clipboard()
c = clipboardcmd()
Expand Down
4 changes: 2 additions & 2 deletions base/multi.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,7 @@ function launch_local_workers(cman::LocalManager, np::Integer, config::Dict)

# start the processes first...
for i in 1:np
io, pobj = readsfrom(detach(`$(dir)/$(exename) --bind-to $(LPROC.bind_addr) $exeflags`))
io, pobj = open(detach(`$(dir)/$(exename) --bind-to $(LPROC.bind_addr) $exeflags`), "r")
io_objs[i] = io
configs[i] = merge(config, {:process => pobj})
end
Expand Down Expand Up @@ -1175,7 +1175,7 @@ function launch_ssh_workers(cman::SSHManager, np::Integer, config::Dict)
cmd = `sh -l -c $(shell_escape(cmd))` # shell to launch under
cmd = `ssh -n $sshflags $host $(shell_escape(cmd))` # use ssh to remote launch

io, pobj = readsfrom(detach(cmd))
io, pobj = open(detach(cmd), "r")
io_objs[i] = io
configs[i] = merge(config, {:machine => cman.machines[i]})
end
Expand Down
53 changes: 30 additions & 23 deletions base/process.jl
Original file line number Diff line number Diff line change
Expand Up @@ -418,30 +418,42 @@ function eachline(cmd::AbstractCmd,stdin)
end
eachline(cmd::AbstractCmd) = eachline(cmd,DevNull)

#returns a pipe to read from the last command in the pipelines
readsfrom(cmds::AbstractCmd) = readsfrom(cmds, DevNull)
function readsfrom(cmds::AbstractCmd, stdin::AsyncStream)
processes = @tmp_rpipe out tmp spawn(false, cmds, (stdin,tmp,STDERR))
start_reading(out)
(out, processes)
# return a (Pipe,Process) pair to write/read to/from the pipeline
function open(cmds::AbstractCmd, mode::String="r", stdio::AsyncStream=DevNull)
if mode == "r"
processes = @tmp_rpipe out tmp spawn(false, cmds, (stdio,tmp,STDERR))
start_reading(out)
(out, processes)
elseif mode == "w"
processes = @tmp_wpipe tmp inpipe spawn(false,cmds, (tmp,stdio,STDERR))
(inpipe, processes)
else
throw(ArgumentError("mode must be \"r\" or \"w\", not \"$mode\""))
end
end

function writesto(cmds::AbstractCmd, stdout::UVStream)
processes = @tmp_wpipe tmp inpipe spawn(false, cmds, (tmp,stdout,STDERR))
(inpipe, processes)
function open(f::Function, cmds::AbstractCmd, args...)
try
io, P = open(cmds, args...)
ret = f(io)
close(io)
!success(P) && pipeline_error(P)
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
success || die
might read better than the negative form

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay.

return ret
catch
kill(P)
Copy link
Member

Choose a reason for hiding this comment

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

Does this need close(io) here

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 was thinking that killing P would be enough, but you're probably right that I should close(io) too. However, this may some rearrangement to avoid calling close(io) twice, avoid killing P if we throw from pipeline_error(P), and so on.

rethrow()
end
end
writesto(cmds::AbstractCmd) = writesto(cmds, DevNull)

# TODO: convert this to use open(cmd, "r+"), with a single read/write pipe
function readandwrite(cmds::AbstractCmd)
(out, processes) = @tmp_wpipe tmp inpipe readsfrom(cmds, tmp)
(out, processes) = @tmp_wpipe tmp inpipe open(cmds, "r", tmp)
(out, inpipe, processes)
end

function readbytes(cmd::AbstractCmd, stdin::AsyncStream=DevNull)
(out,pc) = readsfrom(cmd, stdin)
if !success(pc)
pipeline_error(pc)
end
(out,pc) = open(cmd, "r", stdin)
!success(pc) && pipeline_error(P)
wait_close(out)
return takebuf_array(out.buffer)
end
Expand All @@ -450,15 +462,10 @@ function readall(cmd::AbstractCmd, stdin::AsyncStream=DevNull)
return bytestring(readbytes(cmd, stdin))
end

writeall(cmd::AbstractCmd, stdin::String) = writeall(cmd, stdin, DevNull)
function writeall(cmd::AbstractCmd, stdin::String, stdout::AsyncStream)
(in,pc) = writesto(cmd, stdout)
write(in, stdin)
close(in)
if !success(pc)
pipeline_error(pc)
function writeall(cmd::AbstractCmd, stdin::String, stdout::AsyncStream=DevNull)
open(cmd, "w", stdout) do io
write(io, stdin)
end
return true
end

function run(cmds::AbstractCmd,args...)
Expand Down
6 changes: 3 additions & 3 deletions base/sharedarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,9 @@ function print_shmem_limits(slen)
@linux_only pfx = "kernel"
@osx_only pfx = "kern.sysv"

shmmax_MB = div(int(split(readall(readsfrom(`sysctl $(pfx).shmmax`)[1]))[end]), 1024*1024)
page_size = int(split(readall(readsfrom(`getconf PAGE_SIZE`)[1]))[end])
shmall_MB = div(int(split(readall(readsfrom(`sysctl $(pfx).shmall`)[1]))[end]) * page_size, 1024*1024)
shmmax_MB = div(int(split(readall(`sysctl $(pfx).shmmax`))[end]), 1024*1024)
page_size = int(split(readall(`getconf PAGE_SIZE`))[end])
shmall_MB = div(int(split(readall(`sysctl $(pfx).shmall`))[end]) * page_size, 1024*1024)

println("System max size of single shmem segment(MB) : ", shmmax_MB,
"\nSystem max size of all shmem segments(MB) : ", shmall_MB,
Expand Down
9 changes: 9 additions & 0 deletions doc/manual/running-external-programs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ can be used instead::
julia> (chomp(a)) == "hello"
true

More generally, you can use ``open`` to read from or write to an external
command. For example:

julia> open(`less`, "w", STDOUT) do io
for i = 1:1000
println(io, i)
end
end

.. _man-command-interpolation:

Interpolation
Expand Down
19 changes: 16 additions & 3 deletions doc/stdlib/base.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4949,9 +4949,22 @@ System

Send a signal to a process. The default is to terminate the process.

.. function:: readsfrom(command)

Starts running a command asynchronously, and returns a tuple (stream,process). The first value is a stream reading from the process' standard output.
.. function:: open(command, mode::String="r", stdio=DevNull)

Start running ``command`` asynchronously, and return a tuple
``(stream,process)``. If ``mode`` is ``"r"``, then ``stream``
reads from the process's standard output and ``stdio`` optionally
specifies the process's standard input stream. If ``mode`` is
``"w"``, then ``stream`` writes to the process's standard input
and ``stdio`` optionally specifies the process's standard output
stream.

.. function:: open(f::Function, command, mode::String="r", stdio=DevNull)

Similar to ``open(command, mode, stdio)``, but calls ``f(stream)``
on the resulting read or write stream, then closes the stream
and waits for the process to complete. Returns the value returned
by ``f``.

.. function:: writesto(command)
Copy link
Member

Choose a reason for hiding this comment

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

Also needs to be removed.


Expand Down
7 changes: 4 additions & 3 deletions test/spawn.jl
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ end
file = tempname()
run(`echo hello world` |> file)
@test readall(file |> `cat`) == "hello world\n"
@test open(readall, file |> `cat`, "r") == "hello world\n"
rm(file)

# Stream Redirection
Expand Down Expand Up @@ -99,9 +100,9 @@ str2 = readall(stdout)

# This test hangs if the end of run walk across uv streams calls shutdown on a stream that is shutting down.
file = tempname()
stdin, proc = writesto(`cat -` |> file)
write(stdin, str)
close(stdin)
open(`cat -` |> file, "w") do io
write(io, str)
end
rm(file)

# issue #3373
Expand Down