Skip to content
This repository has been archived by the owner on May 25, 2023. It is now read-only.

Prevent native packager bin-compat issues #169

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ lazy val root = (project in file("."))
IO.write(versionFile, versionSource)

Seq(versionFile)

}

addSbtPlugin("com.typesafe.sbt" % "sbt-native-packager" % Versions.nativePackager)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.lightbend.rp.sbtreactiveapp
import com.typesafe.sbt.packager.Keys._
import com.typesafe.sbt.packager.archetypes.scripts.BashStartScriptPlugin
import com.typesafe.sbt.packager.docker
import com.typesafe.sbt.packager.docker.DockerKeys
import sbt.{ Def, _ }
import sbt.Keys._
import sbt.plugins.JvmPlugin
Expand Down Expand Up @@ -92,7 +93,7 @@ object SbtReactiveAppPlugin extends AutoPlugin {
}
}

object localImport extends docker.DockerKeys
val localImport: DockerKeys = docker.DockerPlugin.autoImport
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the original change. The breaking changes in docker.DockerKeys prevents user sbt's from running. This PR adds a scripted test to assert this fix kicks in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


override def requires = SbtReactiveAppPluginAll && docker.DockerPlugin && BashStartScriptPlugin && SbtReactiveAppPluginAll

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package controllers

import javax.inject._
import play.api._
import play.api.mvc._

@Singleton
class HelloController @Inject() (cc: ControllerComponents) extends AbstractController(cc) {
def index() = Action { _ =>
Ok("Hello, World")
}

def hello(id: String) = Action { _ =>
Ok(s"Hello, $id")
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This scripted test is a copy/paste/trim version of play-endpoints with a specific version of Play. I could have trimmed it more or even created a very small sbt project just mixing sbt-native-packager versions but I wanted to reproduce the issue detected on Play as closely as possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just update play-endpoints to Play 2.6.21 and keep this one minimal to the plugin conflict? Also, maybe rename this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept play-endpoints as originally implemented (depending on play's sbt-plugin 2.6.7 on purpose when I upgraded sbt-reactive-app's dependency to sbt-native-packager from 1.3.2 to 1.3.15.
But we could tell Play users that in order to use newer versions of sbt-reactive-app they have to upgrade Play too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah users should be able to always upgrade to latest bugfix version, in this case of Play and sbt-reactive-app.

37 changes: 37 additions & 0 deletions src/sbt-test/sbt-reactive-app/play-2.6.21-endpoints/build.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
name := "hello-play"
scalaVersion := "2.11.12"

libraryDependencies += guice

lazy val root = (project in file("."))
.enablePlugins(PlayScala, SbtReactiveAppPlugin)
.settings(
packageName in Docker := "hello-play",
httpIngressPorts := Seq(9000),
httpIngressPaths := Seq("/")
)

TaskKey[Unit]("check") := {
val outputDir = (stage in Docker).value
val contents = IO.read(outputDir / "Dockerfile")
val lines = Seq(
"""com.lightbend.rp.endpoints.0.protocol="http"""",
"""com.lightbend.rp.endpoints.0.ingress.0.ingress-ports.0="9000"""",
"""com.lightbend.rp.endpoints.0.ingress.0.type="http"""",
"""com.lightbend.rp.endpoints.0.ingress.0.paths.0="/"""",
"""com.lightbend.rp.endpoints.0.name="http"""",
"""com.lightbend.rp.modules.akka-cluster-bootstrapping.enabled="false"""",
"""com.lightbend.rp.modules.play-http-binding.enabled="true"""",
"""com.lightbend.rp.app-type="play"""",
"""com.lightbend.rp.app-name="hello-play"""",
"""com.lightbend.rp.modules.common.enabled="true"""",
"""com.lightbend.rp.modules.secrets.enabled="false"""",
"""com.lightbend.rp.modules.service-discovery.enabled="false""""
)

lines.foreach { line =>
if (!contents.contains(line)) {
sys.error(s"""Dockerfile is missing line "$line"""")
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
play.crypto.secret = whatever
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GET / controllers.HelloController.index
GET /hello/:id controllers.HelloController.hello(id: String)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
sys.props.get("plugin.version") match {
case Some(x) => addSbtPlugin("com.lightbend.rp" % "sbt-reactive-app" % x)
case _ => sys.error("""|The system property 'plugin.version' is not defined.
|Specify this property using the scriptedLaunchOpts -D.""".stripMargin)
}

addSbtPlugin("com.typesafe.play" % "sbt-plugin" % "2.6.21")

2 changes: 2 additions & 0 deletions src/sbt-test/sbt-reactive-app/play-2.6.21-endpoints/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
> docker:stage
> check