Use an Enum for Process stdio redirections (#4445)

crystal-lang · Aug 19, 2017 · 4bec310 · 4bec310
1 parent 7f38887
commit 4bec310
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 29 deletions.
diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr
@@ -37,7 +37,7 @@ describe Process do
 
   it "redirects output to /dev/null" do
     # This doesn't test anything but no output should be seen while running tests
-    Process.run("/bin/ls", output: false).exit_code.should eq(0)
+    Process.run("/bin/ls", output: Process::Redirect::Close).exit_code.should eq(0)
   end
 
   it "gets output" do
@@ -83,7 +83,7 @@ describe Process do
 
   it "sets working directory" do
     parent = File.dirname(Dir.current)
-    value = Process.run("pwd", shell: true, chdir: parent, output: nil) do |proc|
+    value = Process.run("pwd", shell: true, chdir: parent, output: Process::Redirect::Pipe) do |proc|
       proc.output.gets_to_end
     end
     value.should eq "#{parent}\n"
@@ -96,19 +96,19 @@ describe Process do
   end
 
   it "looks up programs in the $PATH with a shell" do
-    proc = Process.run("uname", {"-a"}, shell: true, output: false)
+    proc = Process.run("uname", {"-a"}, shell: true, output: Process::Redirect::Close)
     proc.exit_code.should eq(0)
   end
 
   it "allows passing huge argument lists to a shell" do
-    proc = Process.new(%(echo "${@}"), {"a", "b"}, shell: true, output: nil)
+    proc = Process.new(%(echo "${@}"), {"a", "b"}, shell: true, output: Process::Redirect::Pipe)
     output = proc.output.gets_to_end
     proc.wait
     output.should eq "a b\n"
   end
 
   it "does not run shell code in the argument list" do
-    proc = Process.new("echo", {"`echo hi`"}, shell: true, output: nil)
+    proc = Process.new("echo", {"`echo hi`"}, shell: true, output: Process::Redirect::Pipe)
     output = proc.output.gets_to_end
     proc.wait
     output.should eq "`echo hi`\n"

diff --git a/src/compiler/crystal/codegen/link.cr b/src/compiler/crystal/codegen/link.cr
@@ -111,7 +111,7 @@ module Crystal
 
           if libname = attr.lib
             if has_pkg_config.nil?
-              has_pkg_config = Process.run("which", {"pkg-config"}, output: false).success?
+              has_pkg_config = Process.run("which", {"pkg-config"}, output: Process::Redirect::Close).success?
             end
 
             if has_pkg_config && (libflags = pkg_config_flags(libname, attr.static?, library_path))

diff --git a/src/compiler/crystal/command.cr b/src/compiler/crystal/command.cr
@@ -216,7 +216,7 @@ class Crystal::Command
     status, elapsed_time = @progress_tracker.stage("Execute") do
       begin
         start_time = Time.now
-        Process.run(output_filename, args: run_args, input: true, output: true, error: true) do |process|
+        Process.run(output_filename, args: run_args, input: Process::Redirect::Inherit, output: Process::Redirect::Inherit, error: Process::Redirect::Inherit) do |process|
           # Ignore the signal so we don't exit the running process
           # (the running process can still handle this signal)
           Signal::INT.ignore # do

diff --git a/src/compiler/crystal/command/deps.cr b/src/compiler/crystal/command/deps.cr
@@ -8,7 +8,7 @@ class Crystal::Command
       error "`shards` executable is missing. Please install shards: https://github.com/crystal-lang/shards"
     end
 
-    status = Process.run(path_to_shards, args: options, output: true, error: true)
+    status = Process.run(path_to_shards, args: options, output: Process::Redirect::Inherit, error: Process::Redirect::Inherit)
     exit status.exit_code unless status.success?
   end
 end
diff --git a/src/compiler/crystal/tools/playground/server.cr b/src/compiler/crystal/tools/playground/server.cr
@@ -150,7 +150,7 @@ module Crystal::Playground
       stop_process
 
       @logger.info "Code execution started (session=#{@session_key}, tag=#{tag}, filename=#{output_filename})."
-      process = @process = Process.new(output_filename, args: [] of String, input: nil, output: nil, error: nil)
+      process = @process = Process.new(output_filename, args: [] of String, input: Process::Redirect::Pipe, output: Process::Redirect::Pipe, error: Process::Redirect::Pipe)
       @running_process_filename = output_filename
 
       spawn do

diff --git a/src/process.cr b/src/process.cr
@@ -129,19 +129,28 @@ class Process
     pid
   end
 
-  # The standard `IO` configuration of a process:
-  #
-  # * `nil`: use a pipe
-  # * `false`: no `IO` (`/dev/null`)
-  # * `true`: inherit from parent
-  # * `IO`: use the given `IO`
-  alias Stdio = Nil | Bool | IO
+  # How to redirect the standard input, output and error IO of a process.
+  enum Redirect
+    # Pipe the IO so the parent process can read (or write) to the process IO
+    # throught `#input`, `#output` or `#error`.
+    Pipe
+
+    # Discards the IO.
+    Close
+
+    # Use the IO of the parent process.
+    Inherit
+  end
+
+  # The standard `IO` configuration of a process.
+  alias Stdio = Redirect | IO
   alias Env = Nil | Hash(String, Nil) | Hash(String, String?) | Hash(String, String)
 
   # Executes a process and waits for it to complete.
   #
   # By default the process is configured without input, output or error.
-  def self.run(command : String, args = nil, env : Env = nil, clear_env : Bool = false, shell : Bool = false, input : Stdio = false, output : Stdio = false, error : Stdio = false, chdir : String? = nil) : Process::Status
+  def self.run(command : String, args = nil, env : Env = nil, clear_env : Bool = false, shell : Bool = false,
+               input : Stdio = Redirect::Close, output : Stdio = Redirect::Close, error : Stdio = Redirect::Close, chdir : String? = nil) : Process::Status
     status = new(command, args, env, clear_env, shell, input, output, error, chdir).wait
     $? = status
     status
@@ -153,7 +162,8 @@ class Process
   # will be closed automatically at the end of the block.
   #
   # Returns the block's value.
-  def self.run(command : String, args = nil, env : Env = nil, clear_env : Bool = false, shell : Bool = false, input : Stdio = nil, output : Stdio = nil, error : Stdio = nil, chdir : String? = nil)
+  def self.run(command : String, args = nil, env : Env = nil, clear_env : Bool = false, shell : Bool = false,
+               input : Stdio = Redirect::Pipe, output : Stdio = Redirect::Pipe, error : Stdio = Redirect::Pipe, chdir : String? = nil)
     process = new(command, args, env, clear_env, shell, input, output, error, chdir)
     begin
       value = yield process
@@ -171,7 +181,8 @@ class Process
   # * `false`: no `IO` (`/dev/null`)
   # * `true`: inherit from parent
   # * `IO`: use the given `IO`
-  def self.exec(command : String, args = nil, env : Env = nil, clear_env : Bool = false, shell : Bool = false, input : Bool | IO::FileDescriptor = true, output : Bool | IO::FileDescriptor = true, error : Bool | IO::FileDescriptor = true, chdir : String? = nil)
+  def self.exec(command : String, args = nil, env : Env = nil, clear_env : Bool = false, shell : Bool = false,
+                input : Stdio = Redirect::Inherit, output : Stdio = Redirect::Inherit, error : Stdio = Redirect::Inherit, chdir : String? = nil)
     command, argv = prepare_argv(command, args, shell)
     exec_internal(command, argv, env, clear_env, input, output, error, chdir)
   end
@@ -194,14 +205,15 @@ class Process
   # To wait for it to finish, invoke `wait`.
   #
   # By default the process is configured without input, output or error.
-  def initialize(command : String, args = nil, env : Env = nil, clear_env : Bool = false, shell : Bool = false, input : Stdio = false, output : Stdio = false, error : Stdio = false, chdir : String? = nil)
+  def initialize(command : String, args = nil, env : Env = nil, clear_env : Bool = false, shell : Bool = false,
+                 input : Stdio = Redirect::Close, output : Stdio = Redirect::Close, error : Stdio = Redirect::Close, chdir : String? = nil)
     command, argv = Process.prepare_argv(command, args, shell)
 
     @wait_count = 0
 
     if needs_pipe?(input)
       fork_input, process_input = IO.pipe(read_blocking: true)
-      if input
+      if input.is_a?(IO)
         @wait_count += 1
         spawn { copy_io(input, process_input, channel, close_dst: true) }
       else
@@ -211,7 +223,7 @@ class Process
 
     if needs_pipe?(output)
       process_output, fork_output = IO.pipe(write_blocking: true)
-      if output
+      if output.is_a?(IO)
         @wait_count += 1
         spawn { copy_io(process_output, output, channel, close_src: true) }
       else
@@ -221,7 +233,7 @@ class Process
 
     if needs_pipe?(error)
       process_error, fork_error = IO.pipe(write_blocking: true)
-      if error
+      if error.is_a?(IO)
         @wait_count += 1
         spawn { copy_io(process_error, error, channel, close_src: true) }
       else
@@ -334,7 +346,7 @@ class Process
   end
 
   private def needs_pipe?(io)
-    io.nil? || (io.is_a?(IO) && !io.is_a?(IO::FileDescriptor))
+    (io == Redirect::Pipe) || (io.is_a?(IO) && !io.is_a?(IO::FileDescriptor))
   end
 
   private def copy_io(src, dst, channel, close_src = false, close_dst = false)
@@ -387,10 +399,9 @@ class Process
     when IO::FileDescriptor
       src_io.blocking = true
       dst_io.reopen(src_io)
-    when true
-      # use same io as parent
+    when Redirect::Inherit
       dst_io.blocking = true
-    when false
+    when Redirect::Close
       File.open("/dev/null", mode) do |file|
         dst_io.reopen(file)
       end
@@ -431,7 +442,7 @@ end
 # LICENSE shard.yml Readme.md spec src
 # ```
 def system(command : String, args = nil) : Bool
-  status = Process.run(command, args, shell: true, input: true, output: true, error: true)
+  status = Process.run(command, args, shell: true, input: Process::Redirect::Inherit, output: Process::Redirect::Inherit, error: Process::Redirect::Inherit)
   $? = status
   status.success?
 end
@@ -446,7 +457,7 @@ end
 # `echo hi` # => "hi\n"
 # ```
 def `(command) : String
-  process = Process.new(command, shell: true, input: true, output: nil, error: true)
+  process = Process.new(command, shell: true, input: Process::Redirect::Inherit, output: Process::Redirect::Pipe, error: Process::Redirect::Inherit)
   output = process.output.gets_to_end
   status = process.wait
   $? = status