Avoid wrapping shell pipes in writers, fixes technomancy#1857

The fix is actually very easy: Instead of piping stuff into a reader,
then feed it to *out*/*err* via a writer, we directly hook them to
System.out and System.err.

The problem is that it kills many tests that rely on `with-out-str`, so
we have to include `with-system-err/out-str` macros to catch
sysout/syserr data, and replace usage where tests fail.

leiningen-core/src/leiningen/core/eval.clj

@@ -167,11 +167,11 @@
         proc (.exec (Runtime/getRuntime) (into-array String cmd) env (io/file *dir*))]
     (.addShutdownHook (Runtime/getRuntime)
                       (Thread. (fn [] (.destroy proc))))
-    (with-open [out (io/reader (.getInputStream proc))
-                err (io/reader (.getErrorStream proc))
+    (with-open [out (.getInputStream proc)
+                err (.getErrorStream proc)
                 in (.getOutputStream proc)]
-      (let [pump-out (doto (Pipe. out *out*) .start)
-            pump-err (doto (Pipe. err *err*) .start)
+      (let [pump-out (doto (Pipe. out System/out) .start)
+            pump-err (doto (Pipe. err System/err) .start)
             ;; TODO: this prevents nrepl need-input msgs from being propagated
             ;; in the case of connecting to Leiningen over nREPL.
             pump-in (ClosingPipe. System/in in)]

test/leiningen/test/compile.clj

@@ -7,7 +7,8 @@
         [leiningen.test.helper :only [sample-project delete-file-recursively
                                       sample-failing-project
                                       tricky-name-project
-                                      more-gen-classes-project]])
+                                      more-gen-classes-project
+                                      with-system-err-str]])
   (:require [leiningen.core.eval :as eval]
             [leiningen.core.main :as main]))
 
@@ -86,12 +87,11 @@
 
 (deftest bad-aot-test
   (is (re-find #"does\.not\.exist|does\/not\/exist"
-               (with-out-str
-                 (binding [*err* *out*]
-                   (try
-                     (compile (assoc sample-project
-                                :aot '[does.not.exist]))
-                     (catch clojure.lang.ExceptionInfo _)))))))
+               (with-system-err-str
+                 (try
+                   (compile (assoc sample-project
+                                   :aot '[does.not.exist]))
+                   (catch clojure.lang.ExceptionInfo _))))))
 
 (deftest compilation-specs-tests
   (is (= '[foo bar] (compilation-specs ["foo" "bar"])))

test/leiningen/test/helper.clj

@@ -2,7 +2,9 @@
   (:require [leiningen.core.project :as project]
             [leiningen.core.user :as user]
             [leiningen.core.test.helper :as helper]
-            [clojure.java.io :as io]))
+            [clojure.java.io :as io])
+  (:import (java.io ByteArrayOutputStream PrintStream FileDescriptor
+                    FileOutputStream)))
 
 ;; TODO: fix
 (def local-repo (io/file (System/getProperty "user.home") ".m2" "repository"))
@@ -110,3 +112,25 @@
 (defn noerr-fixture [f]
   (binding [*err* (java.io.StringWriter.)]
     (f)))
+
+(defmacro with-system-out-str
+  "Like with-out-str, but for System/out."
+  [& body]
+  `(try (let [o# (ByteArrayOutputStream.)]
+          (System/setOut (PrintStream. o#))
+          ~@body
+          (.toString o#))
+     (finally
+       (System/setOut
+        (-> FileDescriptor/out FileOutputStream. PrintStream.)))))
+
+(defmacro with-system-err-str
+  "Like with-out-str, but for System/err."
+  [& body]
+  `(try (let [o# (ByteArrayOutputStream.)]
+          (System/setErr (PrintStream. o#))
+          ~@body
+          (.toString o#))
+     (finally
+       (System/setErr
+        (-> FileDescriptor/err FileOutputStream. PrintStream.)))))

test/leiningen/test/jvm_opts.clj

@@ -1,13 +1,14 @@
 (ns leiningen.test.jvm-opts
   (:require [leiningen.with-profile :refer [with-profile]]
-            [leiningen.test.helper :refer [jvm-opts-project]])
+            [leiningen.test.helper :refer [jvm-opts-project
+                                           with-system-out-str]])
   (:use clojure.test))
 
 ;; This is a regression test for technomancy/leiningen#1676 (make sure
 ;; that file.encoding can be overriden by profiles.)
 (deftest file-encoding-conveyed
   (let [exec '(println "system encoding" (System/getProperty "file.encoding"))
-        run-with #(with-out-str
+        run-with #(with-system-out-str
                     (with-profile jvm-opts-project % "run"
                       "-m" "clojure.main"
                       "-e" (pr-str exec)))]

test/leiningen/test/run.clj

@@ -4,7 +4,8 @@
             [clojure.java.io :as io]
             [leiningen.test.helper :as helper
              :refer [bad-require-project tmp-dir tricky-name-project
-                     java-main-project file-not-found-thrower-project]])
+                     java-main-project file-not-found-thrower-project
+                     with-system-out-str with-system-err-str]])
   (:use [clojure.test]
         [leiningen.run]))
 
@@ -45,10 +46,9 @@
 
 (deftest test-nonexistant-ns-error-message
   (is (re-find #"Can't find 'nonexistant.ns' as \.class or \.clj for lein run"
-               (with-out-str
-                 (binding [*err* *out*]
-                   (try (run tricky-name-project "-m" "nonexistant.ns")
-                        (catch Exception _)))))))
+               (with-system-err-str
+                 (try (run tricky-name-project "-m" "nonexistant.ns")
+                      (catch Exception _))))))
 
 (deftest test-escape-args
   (run tricky-name-project "--" ":bbb")
@@ -57,31 +57,29 @@
   (is (= "nom:-m" (slurp out-file))))
 
 (deftest test-bad-require-error-msg
-  (let [sw (java.io.StringWriter.)]
-    (binding [*err* sw]
-      (try (run bad-require-project)
-           (catch clojure.lang.ExceptionInfo e nil)))
-    (let [e-msg (str sw)]
-      ;; Don't throw the old ClassNotFoundException
-      (is (not (re-find #"ClassNotFoundException: bad-require.core" e-msg)))
-      ;; Do show a relevant error message
-      (is (re-find #"FileNotFoundException" e-msg))
-      (is (re-find #"this/namespace/does/not/exist.clj" e-msg)))))
+  (let [e-msg (with-system-err-str
+                (try (run bad-require-project)
+                     (catch clojure.lang.ExceptionInfo e nil)))]
+    ;; Don't throw the old ClassNotFoundException
+    (is (not (re-find #"ClassNotFoundException: bad-require.core" e-msg)))
+    ;; Do show a relevant error message
+    (is (re-find #"FileNotFoundException" e-msg))
+    (is (re-find #"this/namespace/does/not/exist.clj" e-msg))))
 
 (deftest test-run-java-main
   (leiningen.javac/javac java-main-project)
-  (let [out-result (with-out-str (run java-main-project))]
+  (let [out-result (with-system-out-str (run java-main-project))]
     (is (= (.trim out-result) ;; To avoid os-specific newline handling
             "Hello from Java!"))))
 
 ;; Regression test for https://github.com/technomancy/leiningen/issues/1469
 (deftest file-not-found-exception-test
-  (let [sw (java.io.StringWriter.)]
-    (binding [*err* sw]
-      (try (run file-not-found-thrower-project "-m" "file-not-found-thrower.core")
-           (catch clojure.lang.ExceptionInfo e nil))
-      ;; testing that the true exception is printed immediately and
-      ;; the inappropriate error message "Can't find
-      ;; 'file-not-found-thrower.core' as .class or .clj for lein run:
-      ;; please check the spelling." is not
-      (is (.startsWith (str sw) "Exception in thread \"main\" java.io.FileNotFoundException")))))
+  (let [s (with-system-err-str
+            (try (run file-not-found-thrower-project
+                   "-m" "file-not-found-thrower.core")
+                 (catch clojure.lang.ExceptionInfo e nil)))]
+    ;; testing that the true exception is printed immediately and
+    ;; the inappropriate error message "Can't find
+    ;; 'file-not-found-thrower.core' as .class or .clj for lein run:
+    ;; please check the spelling." is not
+    (is (.startsWith s "Exception in thread \"main\" java.io.FileNotFoundException"))))

test/leiningen/test/test.clj

@@ -3,7 +3,8 @@
   (:require [clojure.test :refer :all]
             [leiningen.test :refer :all]
             [leiningen.test.helper :refer [tmp-dir sample-no-aot-project
-                                           sample-failing-project abort-msg]]
+                                           sample-failing-project
+                                           with-system-err-str]]
             [clojure.java.io :as io]
             [leiningen.core.main :as main]
             [leiningen.core.project :as project]))
@@ -69,8 +70,12 @@
 
 (deftest test-invalid-namespace-argument
   (is (.contains
-       (abort-msg
-        test sample-no-aot-project "boom")
+       (with-system-err-str
+         (try
+           (test sample-no-aot-project "boom")
+           (catch clojure.lang.ExceptionInfo e
+             (when-not (:exit-code (ex-data e))
+               (throw e)))))
        "java.io.FileNotFoundException: Could not locate")))
 
 (deftest test-file-argument