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

[scala3] Enable scala3_sources #222

Merged
merged 13 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
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
6 changes: 6 additions & 0 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ jobs:
distribution: temurin
java-version: 8

- name: Install go & go-protobuf
- uses: actions/setup-go@v5
with:
go-version: '^1.20'
- run: go install google.golang.org/protobuf/cmd/[email protected]

Copy link
Member

Choose a reason for hiding this comment

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

Ah looks like we have to split this into two: "a step cannot have both the uses and run keys"

- name: Cache Coursier cache
uses: coursier/cache-action@v6

Expand Down
2 changes: 1 addition & 1 deletion benchmark-java/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ run / javaOptions ++= List("-Xms1g", "-Xmx1g", "-XX:+PrintGCDetails", "-XX:+Prin
// generate both client and server (default) in Java
pekkoGrpcGeneratedLanguages := Seq(PekkoGrpc.Java)

val grpcVersion = "1.54.2" // checked synced by VersionSyncCheckPlugin
val grpcVersion = "1.61.1" // checked synced by VersionSyncCheckPlugin

val runtimeProject = ProjectRef(file("../"), "runtime")

Expand Down
1 change: 1 addition & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ lazy val pluginTesterJava = Project(id = "plugin-tester-java", base = file("plug
.settings(
name := s"$pekkoPrefix-plugin-tester-java",
fork := true,
PB.protocVersion := Dependencies.Versions.googleProtoc,
ReflectiveCodeGen.generatedLanguages := Seq("Java"),
crossScalaVersions := Dependencies.Versions.CrossScalaForLib,
scalaVersion := scala212,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@ object ProtocSettings {
"ascii_format_to_string",
"no_lenses",
"retain_source_code_info",
"grpc")
"grpc",
"scala3_sources")
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ abstract class ScalaCodeGenerator extends CodeGenerator {
case (p, "no_lenses") => p.copy(lenses = false)
case (p, "retain_source_code_info") => p.copy(retainSourceCodeInfo = true)
case (p, "grpc") => p.copy(grpc = true)
case (p, "scala3_sources") => p.copy(scala3Sources = true)
case (x, _) => x
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.pekko.grpc.gen.scaladsl

private[scaladsl] class ScalaCompatConstants(emitScala3Sources: Boolean = false) {
// val WildcardType: String = if (emitScala3Sources) "?" else "_"
Copy link
Member

Choose a reason for hiding this comment

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

I guess this can be dropped

val WildcardImport: String = if (emitScala3Sources) "*" else "_"
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ case class Service(
serverPowerApi: Boolean,
usePlayActions: Boolean,
options: com.google.protobuf.DescriptorProtos.ServiceOptions,
comment: Option[String] = None) {
comment: Option[String] = None,
scalaCompatConstants: ScalaCompatConstants) {
def serializers: Seq[Serializer] = (methods.map(_.deserializer) ++ methods.map(_.serializer)).distinct
def packageDir = packageName.replace('.', '/')
}
Expand Down Expand Up @@ -56,6 +57,7 @@ object Service {
serverPowerApi,
usePlayActions,
serviceDescriptor.getOptions,
serviceDescriptor.comment)
serviceDescriptor.comment,
new ScalaCompatConstants(fileDesc.emitScala3Sources))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ object @{service.name}Client {
new Default@{service.name}Client(channel, isChannelOwned = false)

private class Default@{service.name}Client(channel: GrpcChannel, isChannelOwned: Boolean)(implicit sys: ClassicActorSystemProvider) extends @{service.name}Client {
import @{service.name}.MethodDescriptors._
import @{service.name}.MethodDescriptors.@{service.scalaCompatConstants.WildcardImport}

private implicit val ex: ExecutionContext = sys.classicSystem.dispatcher
private val settings = channel.settings
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ object @{service.name} extends pekko.grpc.ServiceDescription {
object MethodDescriptors {
import pekko.grpc.internal.Marshaller
import io.grpc.MethodDescriptor
import Serializers._
import Serializers.@{service.scalaCompatConstants.WildcardImport}

@for(method <- service.methods) {
val @{method.name}Descriptor: MethodDescriptor[@method.inputTypeUnboxed, @method.outputTypeUnboxed] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ object @{serviceName}Handler {
implicit val ec: ExecutionContext = mat.executionContext
val spi = TelemetryExtension(system).spi

import @{service.name}.Serializers._
import @{service.name}.Serializers.@{service.scalaCompatConstants.WildcardImport}

def handle(request: model.HttpRequest, method: String): scala.concurrent.Future[model.HttpResponse] =
GrpcMarshalling.negotiated(request, (reader, writer) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import org.gradle.api.Project

class PekkoGrpcPluginExtension {

static final String PROTOC_VERSION = "3.20.1" // checked synced by VersionSyncCheckPlugin
static final String PROTOC_VERSION = "3.24.0" // checked synced by VersionSyncCheckPlugin

static final String PROTOC_PLUGIN_SCALA_VERSION = "2.12"

static final String GRPC_VERSION = "1.54.2" // checked synced by VersionSyncCheckPlugin
static final String GRPC_VERSION = "1.61.1" // checked synced by VersionSyncCheckPlugin

static final String PLUGIN_CODE = 'org.apache.pekko.grpc.gradle'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import java.net.InetSocketAddress
import org.apache.pekko
import pekko.actor.ActorSystem
import pekko.grpc.GrpcClientSettings
import pekko.grpc.internal.ClientConnectionException
import pekko.grpc.scaladsl.tools.MutableServiceDiscovery
import pekko.http.scaladsl.Http
import pekko.stream.{ Materializer, SystemMaterializer }
Expand Down Expand Up @@ -184,8 +183,7 @@ class NonBalancingIntegrationSpec(backend: String)

val failure =
client.sayHello(HelloRequest(s"Hello friend")).failed.futureValue.asInstanceOf[StatusRuntimeException]
failure.getStatus.getCode should be(Code.UNAVAILABLE)
client.closed.failed.futureValue shouldBe a[ClientConnectionException]
failure.getStatus.getCode should (equal(Code.UNKNOWN).or(equal(Code.UNAVAILABLE)))
Copy link
Member

Choose a reason for hiding this comment

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

strange but no problem

}

"not fail when no valid endpoints are provided but no limit on attempts is set" in {
Expand Down
4 changes: 2 additions & 2 deletions maven-plugin/src/main/maven/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
<extraGenerators implementation="java.util.List" default-value=""/>
<protoPaths default-value="${project.basedir}/src/main/proto,${project.basedir}/src/main/protobuf">${pekko-grpc.protoPaths}</protoPaths>
<outputDirectory default-value="${project.build.directory}/generated-sources">${pekko-grpc.outputDirectory}</outputDirectory>
<protocVersion implementation="java.lang.String" default-value="-v3.20.1">${pekko-grpc.protoc-version}</protocVersion> <!-- checked synced by VersionSyncCheckPlugin -->
<protocVersion implementation="java.lang.String" default-value="-v3.24.0">${pekko-grpc.protoc-version}</protocVersion> <!-- checked synced by VersionSyncCheckPlugin -->
<includeStdTypes implementation="boolean" default-value="false" />
</configuration>
</mojo>
Expand Down Expand Up @@ -187,7 +187,7 @@
<extraGenerators implementation="java.util.List" default-value=""/>
<protoPaths default-value="src/test/proto,src/test/protobuf">${pekko-grpc.protoPaths}</protoPaths>
<outputDirectory default-value="target/generated-test-sources">${pekko-grpc.outputDirectory}</outputDirectory>
<protocVersion implementation="java.lang.String" default-value="-v3.20.1">${pekko-grpc.protoc-version}</protocVersion> <!-- checked synced by VersionSyncCheckPlugin -->
<protocVersion implementation="java.lang.String" default-value="-v3.24.0">${pekko-grpc.protoc-version}</protocVersion> <!-- checked synced by VersionSyncCheckPlugin -->
<includeStdTypes implementation="boolean" default-value="false" />
</configuration>
</mojo>
Expand Down
2 changes: 1 addition & 1 deletion plugin-tester-java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<maven-dependency-plugin.version>3.1.2</maven-dependency-plugin.version>
<maven-exec-plugin.version>3.0.0</maven-exec-plugin.version>
<pekko.http.version>1.0.0</pekko.http.version>
<grpc.version>1.54.2</grpc.version> <!-- checked synced by VersionSyncCheckPlugin -->
<grpc.version>1.61.1</grpc.version> <!-- checked synced by VersionSyncCheckPlugin -->
<project.encoding>UTF-8</project.encoding>
</properties>

Expand Down
2 changes: 1 addition & 1 deletion plugin-tester-scala/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<maven.compiler.target>1.8</maven.compiler.target>
<pekko.version>1.0.2</pekko.version>
<pekko.http.version>1.0.0</pekko.http.version>
<grpc.version>1.54.2</grpc.version> <!-- checked synced by VersionSyncCheckPlugin -->
<grpc.version>1.61.1</grpc.version> <!-- checked synced by VersionSyncCheckPlugin -->
<project.encoding>UTF-8</project.encoding>
</properties>

Expand Down
6 changes: 3 additions & 3 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ object Dependencies {
val pekkoHttp = PekkoHttpDependency.version
val pekkoHttpBinary = pekkoHttp.take(3)

val grpc = "1.54.2" // checked synced by VersionSyncCheckPlugin
val grpc = "1.61.1" // checked synced by VersionSyncCheckPlugin
// Even referenced explicitly in the sbt-plugin's sbt-tests
// If changing this, remember to update protoc plugin version to align in
// maven-plugin/src/main/maven/plugin.xml and org.apache.pekko.grpc.sbt.PekkoGrpcPlugin
val googleProtoc = "3.20.1" // checked synced by VersionSyncCheckPlugin
val googleProtobufJava = "3.21.12"
val googleProtoc = "3.24.0" // checked synced by VersionSyncCheckPlugin
val googleProtobufJava = "3.24.0"

val scalaTest = "3.2.15"

Expand Down
2 changes: 1 addition & 1 deletion project/plugins.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ libraryDependencies += "org.eclipse.jgit" % "org.eclipse.jgit" % "5.13.1.2022061
// scripted testing
libraryDependencies += "org.scala-sbt" %% "scripted-plugin" % sbtVersion.value

libraryDependencies += "com.thesamet.scalapb" %% "compilerplugin" % "0.11.13"
libraryDependencies += "com.thesamet.scalapb" %% "compilerplugin" % "0.11.14"
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

# Upgrade to io.grpc 1.60 caused this bin compat issue
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.grpc.internal.PekkoDiscoveryNameResolverProvider.this")
Copy link
Member

Choose a reason for hiding this comment

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

👍 since internal

Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ object NettyClientUtils {
new PekkoDiscoveryNameResolverProvider(
settings.serviceDiscovery,
settings.defaultPort,
settings.serviceName,
settings.servicePortName,
settings.serviceProtocol,
settings.resolveTimeout))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import scala.concurrent.duration.FiniteDuration
class PekkoDiscoveryNameResolverProvider(
discovery: ServiceDiscovery,
defaultPort: Int,
serviceName: String,
portName: Option[String],
protocol: Option[String],
resolveTimeout: FiniteDuration)(implicit ec: ExecutionContext)
Expand All @@ -34,8 +35,7 @@ class PekkoDiscoveryNameResolverProvider(

override def getDefaultScheme: String = "http"

override def newNameResolver(targetUri: URI, args: NameResolver.Args): PekkoDiscoveryNameResolver = {
require(targetUri.getAuthority != null, s"target uri should not have null authority, got [$targetUri]")
new PekkoDiscoveryNameResolver(discovery, defaultPort, targetUri.getAuthority, portName, protocol, resolveTimeout)
}
override def newNameResolver(targetUri: URI, args: NameResolver.Args): PekkoDiscoveryNameResolver =
new PekkoDiscoveryNameResolver(discovery, defaultPort, serviceName, portName, protocol, resolveTimeout)
Copy link
Member

Choose a reason for hiding this comment

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

Is this change related/required for the switch to the new io.grpc version and introduction of scala3_sources?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like authority is no longer passed when newNameResolver is called.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, that is slightly scary (I assume the assert was there for a reason), but if you're confident this is a correct fix then 👍


}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class PekkoDiscoveryNameResolverProviderSpec
val provider = new PekkoDiscoveryNameResolverProvider(
discovery,
443,
serviceName,
portName = None,
protocol = None,
resolveTimeout = 3.seconds)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ scalaVersion := "2.12.18"

organization := "org.apache.pekko"

val grpcVersion = "1.54.2" // checked synced by VersionSyncCheckPlugin
val grpcVersion = "1.61.1" // checked synced by VersionSyncCheckPlugin

libraryDependencies ++= Seq(
"io.grpc" % "grpc-interop-testing" % grpcVersion % "protobuf-src",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,7 @@ object ProtocJSPlugin extends AutoPlugin {
override def requires: Plugins = ProtocPlugin

override def projectSettings: Seq[Def.Setting[_]] =
Seq(Compile, Test).flatMap(inConfig(_)(PB.targets += PB.gens.js -> resourceManaged.value / "js"))
Seq(Compile, Test).flatMap(inConfig(_)(
Seq(
PB.targets += PB.gens.go -> resourceManaged.value / "go")))
Copy link
Member

Choose a reason for hiding this comment

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

Should the ProtocJSPlugin object renamed to something ProtocGoPlugin?

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
syntax = "proto3";

option go_package = "github.com/apache/pekko-grpc";
option java_multiple_files = true;
option java_package = "example.myapp.helloworld.grpc";
option java_outer_classname = "HelloWorldProto";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
syntax = "proto3";

option go_package = "github.com/apache/pekko-grpc";
option java_multiple_files = true;
option java_package = "example.myapp.echo.grpc";

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
> compile

$ exists target/scala-2.12/resource_managed/main/js/hellorequest.js
$ exists target/scala-2.12/resource_managed/main/go/github.com/apache/pekko-grpc/helloworld.pb.go
$ exists target/scala-2.12/pekko-grpc/main/example/myapp/helloworld/grpc/HelloRequest.scala

> test:compile
$ exists target/scala-2.12/resource_managed/test/js/echomessage.js
$ exists target/scala-2.12/resource_managed/test/go/github.com/apache/pekko-grpc/echo.pb.go
$ exists target/scala-2.12/pekko-grpc/test/example/myapp/echo/grpc/EchoMessage.scala
Loading