From 7095307027f53c30c28d911efe40aada056103c6 Mon Sep 17 00:00:00 2001 From: James Roper Date: Mon, 23 Mar 2015 14:28:45 +1100 Subject: [PATCH] Sorted out conf directory * conf directory in dist artifact is now added to the classpath by default * Added externalizeResources SBT setting to turn off including the conf directory as an external (as in, not in the applications jar) directory and including it in the classpath * Added --no-exit-sbt flag to Play start and stop commands to help with scripted tests * Updated distribution scripted test, which now tests running Play in prod mode, as well as testing if the conf directory is on the classpath. Fixes #3473 --- documentation/manual/releases/Migration24.md | 7 +++ .../play/sbt/ApplicationSecretGenerator.scala | 2 +- .../src/main/scala/play/sbt/PlayImport.scala | 2 +- .../main/scala/play/sbt/PlaySettings.scala | 31 +++++----- .../src/main/scala/play/sbt/run/PlayRun.scala | 25 ++++++-- .../distribution/app/Global.scala | 24 ++++++++ .../app/controllers/Application.scala | 4 ++ .../play-sbt-plugin/distribution/build.sbt | 56 +++++++++++++----- .../distribution/conf/alternate.conf | 3 + .../distribution/conf/application.conf | 57 +------------------ .../play-sbt-plugin/distribution/conf/routes | 6 +- .../play-sbt-plugin/distribution/test | 31 +++++++--- 12 files changed, 150 insertions(+), 98 deletions(-) create mode 100644 framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/app/Global.scala create mode 100644 framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/alternate.conf diff --git a/documentation/manual/releases/Migration24.md b/documentation/manual/releases/Migration24.md index a3faefb6e48..f73787e759c 100644 --- a/documentation/manual/releases/Migration24.md +++ b/documentation/manual/releases/Migration24.md @@ -390,3 +390,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. + diff --git a/framework/src/sbt-plugin/src/main/scala/play/sbt/ApplicationSecretGenerator.scala b/framework/src/sbt-plugin/src/main/scala/play/sbt/ApplicationSecretGenerator.scala index e34939c31fe..04db6c2af4d 100644 --- a/framework/src/sbt-plugin/src/main/scala/play/sbt/ApplicationSecretGenerator.scala +++ b/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()) { diff --git a/framework/src/sbt-plugin/src/main/scala/play/sbt/PlayImport.scala b/framework/src/sbt-plugin/src/main/scala/play/sbt/PlayImport.scala index 2323d725367..19e56280f64 100644 --- a/framework/src/sbt-plugin/src/main/scala/play/sbt/PlayImport.scala +++ b/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") diff --git a/framework/src/sbt-plugin/src/main/scala/play/sbt/PlaySettings.scala b/framework/src/sbt-plugin/src/main/scala/play/sbt/PlaySettings.scala index 4eebb572485..37235de7eb0 100644 --- a/framework/src/sbt-plugin/src/main/scala/play/sbt/PlaySettings.scala +++ b/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 ++= { diff --git a/framework/src/sbt-plugin/src/main/scala/play/sbt/run/PlayRun.scala b/framework/src/sbt-plugin/src/main/scala/play/sbt/run/PlayRun.scala index 5b63fe2adfd..563a390f1e1 100644 --- a/framework/src/sbt-plugin/src/main/scala/play/sbt/run/PlayRun.scala +++ b/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) + } } } diff --git a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/app/Global.scala b/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/app/Global.scala new file mode 100644 index 00000000000..c213e93fe3b --- /dev/null +++ b/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() + } + +} \ No newline at end of file diff --git a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/app/controllers/Application.scala b/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/app/controllers/Application.scala index 97d070b3cee..a800ff77789 100644 --- a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/app/controllers/Application.scala +++ b/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/app/controllers/Application.scala @@ -5,6 +5,7 @@ package controllers import play.api._ import play.api.mvc._ +import play.api.Play.current object Application extends Controller { @@ -12,4 +13,7 @@ object Application extends Controller { Ok(views.html.index("Your new application is ready.")) } + def config = Action { + Ok(Play.configuration.underlying.getString("some.config")) + } } diff --git a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/build.sbt b/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/build.sbt index 692a022f762..03944501fdf 100644 --- a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/build.sbt +++ b/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") } } \ No newline at end of file diff --git a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/alternate.conf b/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/alternate.conf new file mode 100644 index 00000000000..96000345e9e --- /dev/null +++ b/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" \ No newline at end of file diff --git a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/application.conf b/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/application.conf index 7b82e539296..6ba8327be07 100644 --- a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/application.conf +++ b/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" \ No newline at end of file diff --git a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/routes b/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/routes index 20fd042f342..a82a41b20e5 100644 --- a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/conf/routes +++ b/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) diff --git a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/test b/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/test index 91eafc3bbd3..b04911a8a53 100644 --- a/framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/distribution/test +++ b/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