Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

+pro #15870 OSGi headers for experimental Streams and HTTP modules #17979

Merged
merged 2 commits into from Jul 14, 2015
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 42 additions & 6 deletions project/AkkaBuild.scala
Expand Up @@ -432,10 +432,12 @@ object AkkaBuild extends Build {
lazy val httpXml =
httpMarshallersScalaSubproject("xml")
.settings(Dependencies.httpXml)
.settings(OSGi.httpXml: _*)

lazy val httpSprayJson =
httpMarshallersScalaSubproject("spray-json")
.settings(Dependencies.httpSprayJson)
.settings(OSGi.httpSprayJson: _*)

def httpMarshallersScalaSubproject(name: String) =
Project(
Expand All @@ -459,6 +461,7 @@ object AkkaBuild extends Build {
lazy val httpJackson =
httpMarshallersJavaSubproject("jackson")
.settings(Dependencies.httpJackson)
.settings(OSGi.httpJackson: _*)

def httpMarshallersJavaSubproject(name: String) =
Project(
Expand Down Expand Up @@ -546,7 +549,7 @@ object AkkaBuild extends Build {
id = "akka-stream-testkit-experimental",
base = file("akka-stream-testkit"),
dependencies = Seq(stream),
settings = defaultSettings ++ formatSettings ++ scaladocSettings ++ experimentalSettings ++ javadocSettings ++ Seq(
settings = defaultSettings ++ formatSettings ++ scaladocSettings ++ experimentalSettings ++ javadocSettings ++ OSGi.streamTestkit ++ Seq(
version := streamAndHttpVersion,
libraryDependencies ++= Dependencies.streamTestkit,
previousArtifact := None
Expand Down Expand Up @@ -1382,16 +1385,43 @@ object AkkaBuild extends Build {

val cluster = exports(Seq("akka.cluster.*"), imports = Seq(protobufImport()))

val parsing = exports(Seq("akka.parboiled2.*", "akka.shapeless.*"))
val parsing = exports(Seq("akka.parboiled2.*", "akka.shapeless.*"),
imports = Seq(optionalResolution("scala.quasiquotes")))

val httpCore = exports(Seq("akka.http.*"),
imports = Seq(streamAndHttpImport("akka.stream.*"),
streamAndHttpImport("akka.parboiled2.*"),
streamAndHttpImport("akka.shapeless.*")))

val http = exports(Seq("akka.http.impl.server",
"akka.http.scaladsl.server.*", "akka.http.javadsl.server.*",
"akka.http.scaladsl.client", "akka.http.scaladsl.coding", "akka.http.scaladsl.common",
"akka.http.scaladsl.marshalling", "akka.http.scaladsl.unmarshalling"),
imports = Seq(streamAndHttpImport("akka.stream.*"),
streamAndHttpImport("akka.http.*"),
streamAndHttpImport("akka.parboiled2.*")))

val httpCore = Nil // FIXME #15689
val httpTestKit = exports(Seq("akka.http.scaladsl.testkit.*", "akka.http.javaadsl.testkit.*"),
Copy link
Member

Choose a reason for hiding this comment

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

excellent, thanks @rkrzewski

Copy link
Member

Choose a reason for hiding this comment

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

Bam! javaadsl spoiled the show. Hard to spot, though...

Copy link
Member

Choose a reason for hiding this comment

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

aaaaargh :<

Copy link
Author

Choose a reason for hiding this comment

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

sorry about that :(

Copy link
Member

Choose a reason for hiding this comment

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

No worries about that, this is so easy to get wrong. The problem is that there are no safeguards that would catch a problem like this.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, no worries - I mean, I reviewed and looked at this exact line and still missed the typo.
Will be fixed soon in 1.1 tho

Copy link
Author

Choose a reason for hiding this comment

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

Hmm, shouldn't the script written by @2m have detected this problem?

Also, the verification script should check that the generated jar does not contain more classes than target/classes folder, because it's easy to accidentally re-export packages from an upstream dependency if a too broad Export declaration is used (which is completely valid, and occasionally useful in OSGi environment)

imports = Seq(streamAndHttpImport("akka.stream.*"),
streamAndHttpImport("akka.http.*")))

val http = Nil // FIXME #15689
val httpSprayJson = exports(Seq("akka.http.scaladsl.marshallers.sprayjson"),
imports = Seq(streamAndHttpImport("akka.stream.*"),
streamAndHttpImport("akka.http.*")))

val httpTestKit = Nil
val httpXml = exports(Seq("akka.http.scaladsl.marshallers.xml"),
imports = Seq(streamAndHttpImport("akka.stream.*"),
streamAndHttpImport("akka.http.*")))

val httpJackson = exports(Seq("akka.http.javadsl.marshallers.jackson"),
imports = Seq(streamAndHttpImport("akka.stream.*"),
streamAndHttpImport("akka.http.*")))

val stream = exports(Seq("akka.stream.*"))

val streamTestkit = exports(Seq("akka.stream.testkit.*"),
imports = Seq(streamAndHttpImport("akka.stream.*")))

val fileMailbox = exports(Seq("akka.actor.mailbox.filebased.*"))

val mailboxesCommon = exports(Seq("akka.actor.mailbox.*"), imports = Seq(protobufImport()))
Expand Down Expand Up @@ -1428,11 +1458,17 @@ object AkkaBuild extends Build {
"com.google.protobuf")

def exports(packages: Seq[String] = Seq(), imports: Seq[String] = Nil) = osgiSettings ++ Seq(
OsgiKeys.importPackage := imports ++ defaultImports,
OsgiKeys.importPackage := imports,
OsgiKeys.importPackage <++= scalaVersion { v =>
Copy link
Member

Choose a reason for hiding this comment

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

"2.10" is scalaBinaryVersion :)
Then matching on it is OK

Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this change is at the right place? This will add scala.xml to any package that uses exports, right?

Copy link
Author

Choose a reason for hiding this comment

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

@jrudolph no worries, BND is smart and will add it to Import-Package header only when actually needed. I do this in this place, because it needs to be in front of scala.* to be effective.

@ktoso should I change the condition to scalaBinaryVersion { v => if(v == "2.10") ... or should it stay as is? I actually nicked that from cpsPlugin settings declaration at line 170

if (v.startsWith("2.10.")) Nil
else Seq(versionedImport("scala.xml.*", "1.0", "1.1"))
},
OsgiKeys.importPackage ++= defaultImports,
OsgiKeys.exportPackage := packages
)
def defaultImports = Seq("!sun.misc", akkaImport(), configImport(), scalaImport(), "*")
def akkaImport(packageName: String = "akka.*") = versionedImport(packageName, "2.3", "2.4")
def streamAndHttpImport(packageName: String) = versionedImport(packageName, "1.0", "1.1")
def configImport(packageName: String = "com.typesafe.config.*") = versionedImport(packageName, "1.2.0", "1.3.0")
def protobufImport(packageName: String = "com.google.protobuf.*") = versionedImport(packageName, "2.5.0", "2.6.0")
def scalaImport(packageName: String = "scala.*") = versionedImport(packageName, s"$scalaEpoch.$scalaMajor", s"$scalaEpoch.${scalaMajor+1}")
Expand Down