better error messages when you run interactive commands without -i

9 files changed, 67 insertions, 49 deletions

diff --git a/build.sc b/build.sc
index 32c77310..c3aa017c 100755
--- a/build.sc
+++ b/build.sc
@@ -209,7 +209,8 @@ def launcherScript(jvmArgs: Seq[String],
      |
      |case "$$1" in
      |  -i | --interactive )
-     |    exec java $jvmArgsStr $$JAVA_OPTS -cp "$classPathStr" mill.Main "$${@:2}"
+     |    shift;
+     |    exec java $jvmArgsStr $$JAVA_OPTS -cp "$classPathStr" mill.Main "$$@"
      |    ;;
      |  *)
      |    exec java $jvmArgsStr $$JAVA_OPTS -cp "$classPathStr" mill.ClientMain "$$@"
diff --git a/ci/test-mill-built.sh b/ci/test-mill-built.sh
index e2015237..85879106 100755
--- a/ci/test-mill-built.sh
+++ b/ci/test-mill-built.sh
@@ -13,6 +13,6 @@ target/bin/mill dev.assembly
 # Second build & run tests using Mill
 
 
-out/dev/assembly/dest/out.jar all {clientserver,main,scalalib,scalajslib}.test
-out/dev/assembly/dest/out.jar integration.test "mill.integration.forked.{AmmoniteTests,BetterFilesTests}"
-out/dev/assembly/dest/out.jar dev.assembly
+out/dev/assembly/dest/out.jar -i all {clientserver,main,scalalib,scalajslib}.test
+out/dev/assembly/dest/out.jar -i integration.test "mill.integration.forked.{AmmoniteTests,BetterFilesTests}"
+out/dev/assembly/dest/out.jar -i dev.assembly
diff --git a/clientserver/src/mill/clientserver/ClientServer.scala b/clientserver/src/mill/clientserver/ClientServer.scala
index 84ea2e00..f775f8d6 100644
--- a/clientserver/src/mill/clientserver/ClientServer.scala
+++ b/clientserver/src/mill/clientserver/ClientServer.scala
@@ -48,14 +48,14 @@ object ClientServer{
     }
   }
 
-  def interruptWith[T](millis: Int, close: => Unit)(t: => T): Option[T] = {
+  def interruptWith[T](millis: Int, close: => Unit, t: => T): Option[T] = {
     @volatile var interrupt = true
     @volatile var interrupted = false
     new Thread(() => {
       Thread.sleep(millis)
       if (interrupt) {
-        close
         interrupted = true
+        close
       }
     }).start()
 
diff --git a/clientserver/src/mill/clientserver/Server.scala b/clientserver/src/mill/clientserver/Server.scala
index a40c7cbe..52549f69 100644
--- a/clientserver/src/mill/clientserver/Server.scala
+++ b/clientserver/src/mill/clientserver/Server.scala
@@ -41,13 +41,10 @@ class Server[T](lockBase: String,
         val ioSocket = new UnixDomainServerSocket(ioPath)
         val sockOpt = ClientServer.interruptWith(
           acceptTimeout,
-          {
-            try new UnixDomainSocket(ioPath).close()
-            catch{case e: Throwable => }
-          }
-        ){
+          new UnixDomainSocket(ioPath).close(),
           ioSocket.accept()
-        }
+        )
+
         sockOpt match{
           case None => running = false
           case Some(sock) =>
diff --git a/clientserver/test/src/mill/clientserver/ClientServerTests.scala b/clientserver/test/src/mill/clientserver/ClientServerTests.scala
index 55453c4f..db5c6396 100644
--- a/clientserver/test/src/mill/clientserver/ClientServerTests.scala
+++ b/clientserver/test/src/mill/clientserver/ClientServerTests.scala
@@ -38,10 +38,9 @@ object ClientServerTests extends TestSuite{
 
   def tests = Tests{
     'hello - {
-      var currentTimeMillis = 0
       val (tmpDir, locks) = init()
 
-      def spawnEchoServer() = {
+      def spawnEchoServer(): Unit = {
         new Thread(() => new Server(
           tmpDir.toString,
           new EchoServer(),
@@ -55,7 +54,6 @@ object ClientServerTests extends TestSuite{
       def runClient(arg: String) = {
         val (in, out, err) = initStreams()
         locks.clientLock.lockBlock{
-          println("Client Lock")
           val c = new Client(
             tmpDir.toString,
             () => spawnEchoServer(),
@@ -65,7 +63,6 @@ object ClientServerTests extends TestSuite{
             err
           )
           c.run(Array(arg))
-          println("Client unlock")
           (new String(out.toByteArray), new String(err.toByteArray))
         }
       }
@@ -101,8 +98,6 @@ object ClientServerTests extends TestSuite{
       )
 
       // Make sure the server times out of not used for a while
-      println("Sleep 2000")
-      currentTimeMillis += 60001
       Thread.sleep(2000)
       assert(
         locks.clientLock.probe(),
diff --git a/core/src/mill/util/DummyInputStream.scala b/core/src/mill/util/DummyInputStream.scala
new file mode 100644
index 00000000..310b358b
--- /dev/null
+++ b/core/src/mill/util/DummyInputStream.scala
@@ -0,0 +1,5 @@
+package mill.util
+
+import java.io.ByteArrayInputStream
+
+object DummyInputStream extends ByteArrayInputStream(Array())
\ No newline at end of file
diff --git a/main/src/mill/Main.scala b/main/src/mill/Main.scala
index 4adb8d9c..482de0bf 100644
--- a/main/src/mill/Main.scala
+++ b/main/src/mill/Main.scala
@@ -1,12 +1,13 @@
 package mill
 
-import java.io.{InputStream, OutputStream, PrintStream}
+import java.io.{InputStream, PrintStream}
 
 import ammonite.main.Cli._
 import ammonite.ops._
 import ammonite.util.Util
 import mill.clientserver.{Client, FileLocks}
 import mill.eval.Evaluator
+import mill.util.DummyInputStream
 
 
 object ClientMain {
@@ -73,7 +74,7 @@ object ServerMain extends mill.clientserver.ServerMain[Evaluator.State]{
     stateCache,
     mainInteractive,
     watchInterrupted,
-    stdin,
+    DummyInputStream,
     stdout,
     stderr
   )
diff --git a/main/src/mill/modules/Jvm.scala b/main/src/mill/modules/Jvm.scala
index fb2661e1..5e297072 100644
--- a/main/src/mill/modules/Jvm.scala
+++ b/main/src/mill/modules/Jvm.scala
@@ -1,6 +1,6 @@
 package mill.modules
 
-import java.io.FileOutputStream
+import java.io.{ByteArrayInputStream, FileOutputStream}
 import java.lang.reflect.Modifier
 import java.net.URLClassLoader
 import java.nio.file.attribute.PosixFilePermission
@@ -28,39 +28,48 @@ object Jvm {
                             envArgs: Map[String, String] = Map.empty,
                             mainArgs: Seq[String] = Seq.empty,
                             workingDir: Path = null): Unit = {
-
-    import ammonite.ops.ImplicitWd._
-    val commandArgs =
+    baseInteractiveSubprocess(
       Vector("java") ++
         jvmArgs ++
         Vector("-cp", classPath.mkString(":"), mainClass) ++
-        mainArgs
-    baseInteractiveSubprocess(commandArgs, envArgs, workingDir)
+        mainArgs,
+      envArgs,
+      workingDir
+    )
   }
+
   def baseInteractiveSubprocess(commandArgs: Seq[String],
                                 envArgs: Map[String, String],
                                 workingDir: Path) = {
     val builder = new java.lang.ProcessBuilder()
-    import collection.JavaConverters._
+
     for ((k, v) <- envArgs){
       if (v != null) builder.environment().put(k, v)
       else builder.environment().remove(k)
     }
     builder.directory(workingDir.toIO)
 
-    val process =
-      builder
+    val process = if (System.in.isInstanceOf[ByteArrayInputStream]){
+
+      val process = builder
         .command(commandArgs:_*)
         .start()
 
-    val sources = Seq(
-      process.getInputStream -> System.out,
-      process.getErrorStream -> System.err,
-      System.in -> process.getOutputStream
-    )
+      val sources = Seq(
+        process.getInputStream -> System.out,
+        process.getErrorStream -> System.err,
+        System.in -> process.getOutputStream
+      )
 
-    for((std, dest) <- sources){
-      new Thread(new ClientInputPumper(std, dest)).start()
+      for((std, dest) <- sources){
+        new Thread(new ClientInputPumper(std, dest)).start()
+      }
+      process
+    }else{
+      builder
+        .command(commandArgs:_*)
+        .inheritIO()
+        .start()
     }
 
     val exitCode = process.waitFor()
diff --git a/scalalib/src/mill/scalalib/ScalaModule.scala b/scalalib/src/mill/scalalib/ScalaModule.scala
index a2888048..f92d4d49 100644
--- a/scalalib/src/mill/scalalib/ScalaModule.scala
+++ b/scalalib/src/mill/scalalib/ScalaModule.scala
@@ -11,7 +11,7 @@ import mill.modules.Jvm.{createAssembly, createJar, interactiveSubprocess, runLo
 import Lib._
 import mill.define.Cross.Resolver
 import mill.util.Loose.Agg
-import mill.util.Strict
+import mill.util.{DummyInputStream, Strict}
 
 /**
   * Core configuration required to compile a single Scala compilation target
@@ -268,25 +268,35 @@ trait ScalaModule extends mill.Module with TaskModule { outer =>
   }
 
   def console() = T.command{
-    Jvm.interactiveSubprocess(
-      mainClass = "scala.tools.nsc.MainGenericRunner",
-      classPath = runClasspath().map(_.path) ++ scalaCompilerClasspath().map(_.path),
-      mainArgs = Seq("-usejavacp"),
-      workingDir = pwd
-    )
+    if (T.ctx().log.inStream == DummyInputStream){
+      Result.Failure("repl needs to be run with the -i/--interactive flag")
+    }else{
+      Jvm.interactiveSubprocess(
+        mainClass = "scala.tools.nsc.MainGenericRunner",
+        classPath = runClasspath().map(_.path) ++ scalaCompilerClasspath().map(_.path),
+        mainArgs = Seq("-usejavacp"),
+        workingDir = pwd
+      )
+      Result.Success()
+    }
   }
 
   def ammoniteReplClasspath = T{
     resolveDeps(T.task{Agg(ivy"com.lihaoyi:::ammonite:1.0.3")})()
   }
   def repl() = T.command{
+    if (T.ctx().log.inStream == DummyInputStream){
+      Result.Failure("repl needs to be run with the -i/--interactive flag")
+    }else{
+      Jvm.interactiveSubprocess(
+        mainClass = "ammonite.Main",
+        classPath = runClasspath().map(_.path) ++ ammoniteReplClasspath().map(_.path),
+        mainArgs = Nil,
+        workingDir = pwd
+      )
+      Result.Success()
+    }
 
-    Jvm.interactiveSubprocess(
-      mainClass = "ammonite.Main",
-      classPath = runClasspath().map(_.path) ++ ammoniteReplClasspath().map(_.path),
-      mainArgs = Nil,
-      workingDir = pwd
-    )
   }
 
   // publish artifact with name "mill_2.12.4" instead of "mill_2.12"