Merge pull request #4116 from jroper/3473-conf-dir

Sorted out conf directory

documentation/manual/releases/Migration24.md

@@ -392,3 +392,10 @@ The API should be backward compatible with your code using Play 2.3 so there is
 ## IntelliJ IDEA
 
 Play no longer includes the sbt idea plugin.  IntelliJ is now able to import sbt projects natively, so we recommend using that instead.  Alternatively, the sbt idea plugin can be manually installed and used, instructions can be found [here](https://github.com/mpeltonen/sbt-idea).
+
+## Distribution
+
+Previously, Play added all the resources to the the `conf` directory in the distribution, but didn't add the `conf` directory to the classpath.  Now Play adds the `conf` directory to the classpath by default.
+
+This can be turned off by setting `PlayKeys.externalizeResources := false`, which will cause no `conf` directory to be created in the distribution, and it will not be on the classpath.  The contents of the applications `conf` directory will still be on the classpath by virtue of the fact that it's included in the applications jar file.
+

framework/src/sbt-plugin/src/main/scala/play/sbt/ApplicationSecretGenerator.scala

@@ -37,7 +37,7 @@ object ApplicationSecretGenerator {
 
     val appConfFile = Option(System.getProperty("config.file")) match {
       case Some(applicationConf) => new File(baseDir, applicationConf)
-      case None => confDirectory.value / "application.conf"
+      case None => (Keys.resourceDirectory in Compile).value / "application.conf"
     }
 
     if (appConfFile.exists()) {

framework/src/sbt-plugin/src/main/scala/play/sbt/PlayImport.scala

@@ -94,7 +94,7 @@ object PlayImport {
     /** A hook to configure how play blocks on user input while running. */
     val playInteractionMode = SettingKey[PlayInteractionMode]("play-interaction-mode", "Hook to configure how Play blocks when running")
 
-    val confDirectory = SettingKey[File]("play-conf", "Where the Play conf directory lives")
+    val externalizeResources = SettingKey[Boolean]("playExternalizeResources", "Whether resources should be externalized into the conf directory when Play is packaged as a distribution.")
 
     val playOmnidoc = SettingKey[Boolean]("play-omnidoc", "Determines whether to use the aggregated Play documentation")
     val playDocsName = SettingKey[String]("play-docs-name", "Artifact name of the Play documentation")

framework/src/sbt-plugin/src/main/scala/play/sbt/PlaySettings.scala

@@ -52,7 +52,7 @@ object PlaySettings {
       "Typesafe Releases Repository" at "https://repo.typesafe.com/typesafe/releases/"
     ),
 
-    confDirectory <<= resourceDirectory in Compile,
+    externalizeResources := true,
 
     javacOptions in (Compile, doc) := List("-encoding", "utf8"),
 
@@ -82,11 +82,6 @@ object PlaySettings {
 
     testOptions in Test += Tests.Argument(TestFrameworks.JUnit, "--ignore-runners=org.specs2.runner.JUnitRunner"),
 
-    // Adds config directory's source files to continuous hot reloading
-    watchSources <+= confDirectory map {
-      all => all
-    },
-
     // Adds app directory's source files to continuous hot reloading
     watchSources <++= (sourceDirectory in Compile, sourceDirectory in Assets) map { (sources, assets) =>
       (sources ** "*" --- assets ** "*").get
@@ -136,7 +131,10 @@ object PlaySettings {
 
     routesImport ++= Seq("controllers.Assets.Asset"),
 
-    routesFiles in Compile ++= ((confDirectory.value * "routes").get ++ (confDirectory.value * "*.routes").get),
+    routesFiles in Compile ++= {
+      val dirs = (unmanagedResourceDirectories in Compile).value
+      (dirs * "routes").get ++ (dirs * "*.routes").get
+    },
 
     playMonitoredFiles <<= PlayCommands.playMonitoredFilesTask,
 
@@ -190,13 +188,20 @@ object PlaySettings {
 
     mainClass in Compile <<= mainClass in (Compile, Keys.run),
 
+    // Support for externalising resources
     mappings in Universal ++= {
-      val confDirectoryLen = confDirectory.value.getCanonicalPath.length
-      val pathFinder = confDirectory.value ** ("*" -- "routes")
-      pathFinder.get map {
-        confFile: File =>
-          confFile -> ("conf/" + confFile.getCanonicalPath.substring(confDirectoryLen))
-      }
+      if (externalizeResources.value) {
+        val rdirs = (unmanagedResourceDirectories in Compile).value
+        val resourceMappings = ((unmanagedResources in Compile).value --- rdirs) pair (relativeTo(rdirs) | flat)
+        resourceMappings.map {
+          case (resource, path) => resource -> ("conf/" + path)
+        }
+      } else Nil
+    },
+    scriptClasspath := {
+      if (externalizeResources.value) {
+        "../conf" +: scriptClasspath.value
+      } else scriptClasspath.value
     },
 
     mappings in Universal ++= {

framework/src/sbt-plugin/src/main/scala/play/sbt/run/PlayRun.scala

@@ -190,8 +190,13 @@ object PlayRun {
     val extracted = Project.extract(state)
 
     val interaction = extracted.get(playInteractionMode)
+    val noExitSbt = args.contains("--no-exit-sbt")
+
+    val filter = Set("--no-exit-sbt")
+    val filtered = args.filterNot(filter)
+
     // Parse HTTP port argument
-    val (properties, httpPort, httpsPort, httpAddress) = Reloader.filterArgs(args, extracted.get(playDefaultPort), extracted.get(playDefaultAddress))
+    val (properties, httpPort, httpsPort, httpAddress) = Reloader.filterArgs(filtered, extracted.get(playDefaultPort), extracted.get(playDefaultAddress))
     require(httpPort.isDefined || httpsPort.isDefined, "You have to specify https.port when http.port is disabled")
 
     Project.runTask(stage, state).get._2.toEither match {
@@ -220,7 +225,11 @@ object PlayRun {
         val builder = new java.lang.ProcessBuilder(args.asJava)
         new Thread {
           override def run() {
-            System.exit(Process(builder).!)
+            if (noExitSbt) {
+              Process(builder).!
+            } else {
+              System.exit(Process(builder).!)
+            }
           }
         }.start()
 
@@ -233,7 +242,11 @@ object PlayRun {
 
         println()
 
-        state.copy(remainingCommands = Seq.empty)
+        if (noExitSbt) {
+          state
+        } else {
+          state.copy(remainingCommands = Seq.empty)
+        }
     }
 
   }
@@ -253,6 +266,10 @@ object PlayRun {
     }
     println()
 
-    state.copy(remainingCommands = Seq.empty)
+    if (args.contains("--no-exit-sbt")) {
+      state
+    } else {
+      state.copy(remainingCommands = Seq.empty)
+    }
   }
 }

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/app/Global.scala

@@ -0,0 +1,24 @@
+import play.api._
+
+object Global extends GlobalSettings {
+
+  override def onStart(app: Application): Unit = {
+    // If the app lasts for more than 10 seconds, shut it down, so that the scripted tests don't leak,
+    // but be sure that we don't call System.exit while shutting down, as this will create a deadlock
+    val thread = new Thread() {
+      override def run() = {
+        try {
+          Thread.sleep(10000)
+          // We won't reach here if the app shuts down normally, because an exception will be thrown
+          println("Forcibly terminating JVM that hasn't been shut down")
+          System.exit(1)
+        } catch {
+          case _: Throwable =>
+        }
+      }
+    }
+    thread.setDaemon(true)
+    thread.start()
+  }
+
+}

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/app/controllers/Application.scala

@@ -5,11 +5,15 @@ package controllers
 
 import play.api._
 import play.api.mvc._
+import play.api.Play.current
 
 object Application extends Controller {
 
   def index = Action {
     Ok(views.html.index("Your new application is ready."))
   }
 
+  def config = Action {
+    Ok(Play.configuration.underlying.getString("some.config"))
+  }
 }

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/build.sbt

@@ -6,22 +6,52 @@ lazy val root = (project in file(".")).enablePlugins(PlayScala)
 
 scalaVersion := Option(System.getProperty("scala.version")).getOrElse("2.10.4")
 
-val distAndUnzip = TaskKey[File]("dist-and-unzip")
+val checkStartScript = InputKey[Unit]("check-start-script")
 
-distAndUnzip := {
-  val unzippedFiles = IO.unzip(dist.value, target.value)
-  val unzippedFilePath = unzippedFiles.head.getCanonicalPath.stripPrefix(target.value.getCanonicalPath)
-  val unzippedFolder = target.value / unzippedFilePath.split('/')(1)
-  val targetUnzippedFolder = target.value / "dist"
-  IO.move(Seq(unzippedFolder -> targetUnzippedFolder))
-  targetUnzippedFolder
+checkStartScript := {
+  val args = Def.spaceDelimited().parsed
+  val startScript = target.value / "universal/stage/bin/dist-sample"
+  def startScriptError(contents: String, msg: String) = {
+    println("Error in start script, dumping contents:")
+    println(contents)
+    sys.error(msg)
+  }
+  val contents = IO.read(startScript)
+  if (!contents.contains( """app_mainclass="play.core.server.NettyServer"""")) {
+    startScriptError(contents, "Cannot find the declaration of the main class in the script")
+  }
+  if (args.contains("no-conf")) {
+    if (contents.contains("../conf")) {
+      startScriptError(contents, "Start script is adding conf directory to the classpath when it shouldn't be")
+    }
+  } else {
+    if (!contents.contains("../conf")) {
+      startScriptError(contents, "Start script is not adding conf directory to the classpath when it should be")
+    }
+  }
 }
 
-val checkStartScript = TaskKey[Unit]("check-start-script")
+def retry[B](max: Int = 10, sleep: Long = 500, current: Int = 1)(block: => B): B = {
+  try {
+    block
+  } catch {
+    case scala.util.control.NonFatal(e) =>
+      if (current == max) {
+        throw e
+      } else {
+        Thread.sleep(sleep)
+        retry(max, sleep, current + 1)(block)
+      }
+  }
+}
 
-checkStartScript := {
-  val startScript = distAndUnzip.value / "bin/dist-sample"
-  if (!IO.read(startScript).contains( """app_mainclass="play.core.server.NettyServer"""")) {
-    error("Cannot find the declaration of the main class in the script")
+InputKey[Unit]("check-config") := {
+  val expected = Def.spaceDelimited().parsed.head
+  import java.net.URL
+  val config = retry() {
+    IO.readLinesURL(new URL("http://localhost:9000/config")).mkString("\n")
+  }
+  if (expected != config) {
+    sys.error(s"Expected config $expected but got $config")
   }
 }

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/alternate.conf

@@ -0,0 +1,3 @@
+play.crypto.secret=";1[WE]JmK;XMCxV=S2P6kYl?A<^YcKYW3aui[SmusaQlkjq97A`M8l_S:iV?OmDh"
+play.i18n.langs = [ "en" ]
+some.config="bar"

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/application.conf

@@ -1,58 +1,3 @@
-# This is the main configuration file for the application.
-# ~~~~~
-
-# Secret key
-# ~~~~~
-# The secret key is used to secure cryptographics functions.
-# If you deploy your application to several instances be sure to use the same key!
 play.crypto.secret=";1[WE]JmK;XMCxV=S2P6kYl?A<^YcKYW3aui[SmusaQlkjq97A`M8l_S:iV?OmDh"
-
-# The application languages
-# ~~~~~
 play.i18n.langs = [ "en" ]
-
-# Global object class
-# ~~~~~
-# Define the Global object class for this application.
-# Default to Global in the root package.
-# application.global=Global
-
-# Router
-# ~~~~~
-# Define the Router object to use for this application.
-# This router will be looked up first when the application is starting up,
-# so make sure this is the entry point.
-# Furthermore, it's assumed your route file is named properly.
-# So for an application router like `my.application.Router`,
-# you may need to define a router file `conf/my.application.routes`.
-# Default to Routes in the root package (and conf/routes)
-# application.router=my.application.Routes
-
-# Database configuration
-# ~~~~~
-# You can declare as many datasources as you want.
-# By convention, the default datasource is named `default`
-#
-# db.default.driver=org.h2.Driver
-# db.default.url="jdbc:h2:mem:play"
-# db.default.user=sa
-# db.default.password=""
-
-# Evolutions
-# ~~~~~
-# You can disable evolutions if needed
-# play.modules.evolutions.enabled=false
-
-# Logger
-# ~~~~~
-# You can also configure logback (http://logback.qos.ch/), by providing a logger.xml file in the conf directory .
-
-# Root logger:
-logger.root=ERROR
-
-# Logger used by the framework:
-logger.play=INFO
-
-# Logger provided to your application:
-logger.application=DEBUG
-
+some.config="foo"

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/routes

@@ -3,7 +3,9 @@
 # ~~~~
 
 # Home page
-GET     /                           controllers.Application.index
+GET        /                    controllers.Application.index
+
+GET        /config              controllers.Application.config
 
 # Map static resources from the /public folder to the /assets URL path
-GET     /assets/*file               controllers.Assets.at(path="/public", file)
+GET        /assets/*file        controllers.Assets.at(path="/public", file)

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/test

@@ -1,12 +1,27 @@
 # Build the distribution and ensure that the files we expect are indeed there
-> dist-and-unzip
-$ exists target/dist/README
-$ exists target/dist/SomeFile.txt
-$ exists target/dist/SomeFolder/SomeOtherFile.txt
+> stage
+$ exists target/universal/stage/README
+$ exists target/universal/stage/SomeFile.txt
+$ exists target/universal/stage/SomeFolder/SomeOtherFile.txt
 
 > check-start-script
-$ exists target/dist/conf/application.conf
--$ exists target/dist/conf/routes
-$ exists target/dist/lib
-$ exists target/dist/share/doc/api
+$ exists target/universal/stage/conf/application.conf
+$ exists target/universal/stage/lib
+$ exists target/universal/stage/share/doc/api
 
+# Run it to make sure everything works
+> start --no-exit-sbt
+> check-config foo
+> stop --no-exit-sbt
+
+# Change the configuration in the conf directory, make sure it takes effect
+$ copy-file conf/alternate.conf target/universal/stage/conf/application.conf
+> start --no-exit-sbt
+> check-config bar
+> stop --no-exit-sbt
+
+# Turn off externalize resources, rebuild, make sure the conf directory is not created or on the classpath
+> set PlayKeys.externalizeResources := false
+> stage
+-$ exists target/dist/conf/application.conf
+> check-start-script no-conf