Fixed all the forms of process input/output red...

Fixed all the forms of process input/output redirection so the exit code
which makes it out is the exit code of the process. Also changing names
to be internally consistent and trying to prune pieces which don't make
so much sense without sbt around. Started on documentation. No review.

6 files changed, 85 insertions, 23 deletions

diff --git a/src/library/scala/sys/process/BasicIO.scala b/src/library/scala/sys/process/BasicIO.scala
index b4105086e4..88e66afc91 100644
--- a/src/library/scala/sys/process/BasicIO.scala
+++ b/src/library/scala/sys/process/BasicIO.scala
@@ -35,11 +35,14 @@ object BasicIO {
     }
   }
 
+  private[process] trait Uncloseable extends Closeable {
+    final override def close() { }
+  }
   private[process] object Uncloseable {
-    def apply(in: InputStream): InputStream      = new FilterInputStream(in) { override def close() { } }
-    def apply(out: OutputStream): OutputStream   = new FilterOutputStream(out) { override def close() { } }
-    def protect(in: InputStream): InputStream    = if (in eq System.in) Uncloseable(in) else in
-    def protect(out: OutputStream): OutputStream = if ((out eq System.out) || (out eq System.err)) Uncloseable(out) else out
+    def apply(in: InputStream): InputStream      = new FilterInputStream(in) with Uncloseable { }
+    def apply(out: OutputStream): OutputStream   = new FilterOutputStream(out) with Uncloseable { }
+    def protect(in: InputStream): InputStream    = if (in eq stdin) Uncloseable(in) else in
+    def protect(out: OutputStream): OutputStream = if ((out eq stdout) || (out eq stderr)) Uncloseable(out) else out
   }
 
   def apply(withIn: Boolean, output: String => Unit, log: Option[ProcessLogger]) =
@@ -49,17 +52,15 @@ object BasicIO {
     new ProcessIO(input(withIn), processFully(buffer), getErr(log))
 
   def apply(withIn: Boolean, log: ProcessLogger) =
-    new ProcessIO(input(withIn), processInfoFully(log), processErrFully(log))
+    new ProcessIO(input(withIn), processOutFully(log), processErrFully(log))
 
   def getErr(log: Option[ProcessLogger]) = log match {
     case Some(lg) => processErrFully(lg)
     case None     => toStdErr
   }
 
-  private def processErrFully(log: ProcessLogger)  = processFully(log error _)
-  private def processInfoFully(log: ProcessLogger) = processFully(log info _)
-
-  def ignoreOut = (i: OutputStream) => ()
+  private def processErrFully(log: ProcessLogger) = processFully(log err _)
+  private def processOutFully(log: ProcessLogger) = processFully(log out _)
 
   def close(c: Closeable) = try c.close() catch { case _: IOException => () }
   def processFully(buffer: Appendable): InputStream => Unit = processFully(appendLine(buffer))
@@ -78,13 +79,13 @@ object BasicIO {
     }
     readFully()
   }
-  def connectToIn(o: OutputStream): Unit = transferFully(System.in, o)
-  def input(connect: Boolean): OutputStream => Unit = if (connect) connectToIn else ignoreOut
+  def connectToIn(o: OutputStream): Unit = transferFully(stdin, o)
+  def input(connect: Boolean): OutputStream => Unit = if (connect) connectToIn else _ => ()
   def standard(connectInput: Boolean): ProcessIO = standard(input(connectInput))
   def standard(in: OutputStream => Unit): ProcessIO = new ProcessIO(in, toStdOut, toStdErr)
 
-  def toStdErr = (in: InputStream) => transferFully(in, System.err)
-  def toStdOut = (in: InputStream) => transferFully(in, System.out)
+  def toStdErr = (in: InputStream) => transferFully(in, stderr)
+  def toStdOut = (in: InputStream) => transferFully(in, stdout)
 
   def transferFully(in: InputStream, out: OutputStream): Unit =
     try transferFullyImpl(in, out)
diff --git a/src/library/scala/sys/process/ProcessBuilder.scala b/src/library/scala/sys/process/ProcessBuilder.scala
index bdd13f11af..0e34e7be1b 100644
--- a/src/library/scala/sys/process/ProcessBuilder.scala
+++ b/src/library/scala/sys/process/ProcessBuilder.scala
@@ -71,7 +71,16 @@ trait ProcessBuilder extends Source with Sink {
   /** Constructs a command that will run this command and then `other`.  The exit code will be the exit code of `other`.*/
   def ### (other: ProcessBuilder): ProcessBuilder
 
+  /** True if this command can be the target of a pipe.
+   */
   def canPipeTo: Boolean
+
+  /** True if this command has an exit code which should be propagated to the user.
+   *  Given a pipe between A and B, if B.hasExitValue is true then the exit code will
+   *  be the one from B; if it is false, the one from A.  This exists to prevent output
+   *  redirections (implemented as pipes) from masking useful process error codes.
+   */
+  def hasExitValue: Boolean
 }
 
 object ProcessBuilder extends ProcessBuilderImpl {
diff --git a/src/library/scala/sys/process/ProcessBuilderImpl.scala b/src/library/scala/sys/process/ProcessBuilderImpl.scala
index 0c0a93b661..adca575d4d 100644
--- a/src/library/scala/sys/process/ProcessBuilderImpl.scala
+++ b/src/library/scala/sys/process/ProcessBuilderImpl.scala
@@ -30,12 +30,16 @@ private[process] trait ProcessBuilderImpl {
   private[process] class OStreamBuilder(
     stream: => OutputStream,
     label: String
-  ) extends ThreadBuilder(label, _ writeInput protect(stream)) { }
+  ) extends ThreadBuilder(label, _ writeInput protect(stream)) {
+    override def hasExitValue = false
+  }
 
   private[process] class IStreamBuilder(
     stream: => InputStream,
     label: String
-  ) extends ThreadBuilder(label, _ processOutput protect(stream)) { }
+  ) extends ThreadBuilder(label, _ processOutput protect(stream)) {
+    override def hasExitValue = false
+  }
 
   private[process] abstract class ThreadBuilder(
     override val toString: String,
@@ -130,6 +134,7 @@ private[process] trait ProcessBuilderImpl {
       log buffer run(log, connectInput).exitValue()
 
     def canPipeTo = false
+    def hasExitValue = true
   }
 
   private[process] class URLImpl(url: URL) extends URLBuilder with Source {
diff --git a/src/library/scala/sys/process/ProcessImpl.scala b/src/library/scala/sys/process/ProcessImpl.scala
index b4f1146a9f..857c1af15e 100644
--- a/src/library/scala/sys/process/ProcessImpl.scala
+++ b/src/library/scala/sys/process/ProcessImpl.scala
@@ -132,10 +132,13 @@ private[process] trait ProcessImpl {
       val first = a.run(firstIO)
       try {
         runInterruptible {
-          first.exitValue
+          val exit1 = first.exitValue
           currentSource put None
           currentSink put None
-          second.exitValue
+          val exit2 = second.exitValue
+          // Since file redirection (e.g. #>) is implemented as a piped process,
+          // we ignore its exit value so cmd #> file doesn't always return 0.
+          if (b.hasExitValue) exit2 else exit1
         } {
           first.destroy()
           second.destroy()
diff --git a/src/library/scala/sys/process/ProcessLogger.scala b/src/library/scala/sys/process/ProcessLogger.scala
index 309c9ef2b5..1ce77c9196 100644
--- a/src/library/scala/sys/process/ProcessLogger.scala
+++ b/src/library/scala/sys/process/ProcessLogger.scala
@@ -9,16 +9,58 @@
 package scala.sys
 package process
 
+import java.io._
+
+/** Encapsulates the output and error streams of a running process.
+ *  Many of the methods of ProcessBuilder accept a ProcessLogger as
+ *  an argument.
+ *
+ *  @see      ProcessBuilder
+ */
 trait ProcessLogger {
-  def info(s: => String): Unit
-  def error(s: => String): Unit
+  /** Will be called with each line read from the process output stream.
+   */
+  def out(s: => String): Unit
+
+  /** Will be called with each line read from the process error stream.
+   */
+  def err(s: => String): Unit
+
+  /** If a process is begun with one of these ProcessBuilder methods:
+   *
+   *    def !(log: ProcessLogger): Int
+   *    def !<(log: ProcessLogger): Int
+   *
+   *  The run will be wrapped in a call to buffer.  This gives the logger
+   *  an opportunity to set up and tear down buffering.  At present the
+   *  library implementations of ProcessLogger simply execute the body unbuffered.
+   */
   def buffer[T](f: => T): T
 }
 
+class FileProcessLogger(file: File) extends ProcessLogger with Closeable with Flushable {
+  private val writer = (
+    new PrintWriter(
+      new BufferedWriter(
+        new OutputStreamWriter(
+          new FileOutputStream(file, true)
+        )
+      )
+    )
+  )
+  def out(s: => String): Unit = writer println s
+  def err(s: => String): Unit = writer println s
+  def buffer[T](f: => T): T = f
+  def close(): Unit = writer.close()
+  def flush(): Unit = writer.flush()
+}
+
 object ProcessLogger {
-  def apply(fn: String => Unit): ProcessLogger = new ProcessLogger {
-    def info(s: => String): Unit = fn(s)
-    def error(s: => String): Unit = fn(s)
+  def apply(file: File): FileProcessLogger = new FileProcessLogger(file)
+  def apply(fn: String => Unit): ProcessLogger = apply(fn, fn)
+  def apply(fout: String => Unit, ferr: String => Unit): ProcessLogger = new ProcessLogger {
+    def out(s: => String): Unit = fout(s)
+    def err(s: => String): Unit = ferr(s)
     def buffer[T](f: => T): T = f
   }
 }
diff --git a/src/library/scala/sys/process/package.scala b/src/library/scala/sys/process/package.scala
index 9421bff88d..51368e8e4c 100644
--- a/src/library/scala/sys/process/package.scala
+++ b/src/library/scala/sys/process/package.scala
@@ -17,6 +17,9 @@ package scala.sys {
 
       java.lang.management.ManagementFactory.getRuntimeMXBean().getInputArguments().toList
     }
+    def stdin  = java.lang.System.in
+    def stdout = java.lang.System.out
+    def stderr = java.lang.System.err
   }
   // private val shell: String => Array[String] =
   //   if (isWin) Array("cmd.exe", "/C", _)
@@ -27,7 +30,6 @@ package scala.sys {
     // due to the issues described in tickets #3160 and #3836.
     private[process] object processInternal {
       final val processDebug = props contains "scala.process.debug"
-      // final val processDebug = true
       dbg("Initializing process package.")
 
       type =?>[-A, +B]     = PartialFunction[A, B]