Skip to content

Commit

Permalink
Wrap ScalaPbCodeGenerator, expose Scala 2.11 error
Browse files Browse the repository at this point in the history
Wraps `scalapb.ScalaPbCodeGenerator` to catch and return all errors.
Without this, any errors escaping from `scalapb.ScalaPbCodeGenerator`
will cause the `scala_proto` aspect workers to hang.

Also deletes some `@io_bazel_rules_scala` references, replacing them
with `Label` instances as appropriate.

The `test_scala_version 2.11.12` case from `test_version.sh` would
previously hang given the combination of ScalaPB 0.9.8 (the newest
available for Scala 2.11) and Protobuf v28.3. Running the following in a
`test_version/test_scala_version_*` repo generated by `test_version.sh`
reproduced the hang:

```txt
$ bazel build --repo_env=SCALA_VERSION=2.11.12 //proto:test_proto

[ ...wait 10 seconds, then CTRL-C... ]

INFO: Analyzed target //proto:test_proto (2 packages loaded, 22 targets configured).
INFO: Found 1 target...
[1,038 / 1,047] 4 actions running
    ProtoScalaPBRule proto/test_service_scala_scalapb.srcjar; 10s worker
    ProtoScalaPBRule proto/test2_scala_scalapb.srcjar; 10s worker
Target //proto:test_proto failed to build
```

With this change, the command now exposes the exact incompatibility
between these versions:

```txt
$ bazel build --repo_env=SCALA_VERSION=2.11.12 //proto:test_proto

ERROR: .../test_version/test_scala_version_1731169107/proto/BUILD:14:14:
  ProtoScalaPBRule proto/test3_scala_scalapb.srcjar failed: (Exit 1):
  scalapb_worker failed: error executing command
  (from target //proto:test3)

bazel-out/.../bin/external/io_bazel_rules_scala/src/scala/scripts/scalapb_worker
  ... (remaining 2 arguments skipped)

--scala_out: java.lang.NoSuchMethodError:
  'void com.google.protobuf.Descriptors$FileDescriptor.internalBuildGeneratedFileFrom(java.lang.String[],
    com.google.protobuf.Descriptors$FileDescriptor[],
    com.google.protobuf.Descriptors$FileDescriptor$InternalDescriptorAssigner)'
      at scalapb.options.compiler.Scalapb.<clinit>(Scalapb.java:10592)
      at scalapb.ScalaPbCodeGenerator$.run(ScalaPbCodeGenerator.scala:14)
      at scalapb.ScalaPbCodeGenerator$.run(ScalaPbCodeGenerator.scala:10)
      at scripts.ScalaPbCodeGenerator$.run(ScalaPbCodeGeneratorWrapper_2_11.scala:8)
      at protocbridge.frontend.PluginFrontend$$anonfun$runWithBytes$2.apply(PluginFrontend.scala:52)
      at protocbridge.frontend.PluginFrontend$$anonfun$runWithBytes$2.apply(PluginFrontend.scala:52)
      at scala.util.Try$.apply(Try.scala:192)
      at protocbridge.frontend.PluginFrontend$.runWithBytes(PluginFrontend.scala:51)
      at protocbridge.frontend.PluginFrontend$.runWithInputStream(PluginFrontend.scala:103)
      at protocbridge.frontend.PosixPluginFrontend$$anonfun$prepare$1.apply$mcV$sp(PosixPluginFrontend.scala:33)
      at protocbridge.frontend.PosixPluginFrontend$$anonfun$prepare$1.apply(PosixPluginFrontend.scala:31)
      at protocbridge.frontend.PosixPluginFrontend$$anonfun$prepare$1.apply(PosixPluginFrontend.scala:31)
      at scala.concurrent.impl.Future$PromiseCompletingRunnable.liftedTree1$1(Future.scala:24)
      at scala.concurrent.impl.Future$PromiseCompletingRunnable.run(Future.scala:24)
      at scala.concurrent.impl.ExecutionContextImpl$AdaptedForkJoinTask.exec(ExecutionContextImpl.scala:121)
      at scala.concurrent.forkjoin.ForkJoinTask.doExec(ForkJoinTask.java:260)
      at scala.concurrent.forkjoin.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1339)
      at scala.concurrent.forkjoin.ForkJoinPool.runWorker(ForkJoinPool.java:1979)
      at scala.concurrent.forkjoin.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:107)

java.lang.RuntimeException: Exit with code 1
      at scala.sys.package$.error(package.scala:27)
      at scripts.ScalaPBWorker$.work(ScalaPBWorker.scala:44)
      at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:96)
      at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49)
      at scripts.ScalaPBWorker$.main(ScalaPBWorker.scala:39)
      at scripts.ScalaPBWorker.main(ScalaPBWorker.scala)

Target //proto:test_proto failed to build
```
  • Loading branch information
mbland committed Nov 10, 2024
1 parent e55faba commit 6f702a6
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 39 deletions.
19 changes: 8 additions & 11 deletions scala_proto/BUILD
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
load(
"//scala_proto:scala_proto_toolchain.bzl",
"scala_proto_deps_toolchain",
"scala_proto_toolchain",
)
load("//scala:providers.bzl", "declare_deps_provider")
load(
"//scala_proto/default:default_deps.bzl",
"DEFAULT_SCALAPB_COMPILE_DEPS",
"DEFAULT_SCALAPB_GRPC_DEPS",
"DEFAULT_SCALAPB_WORKER_DEPS",
)
load("//scala:providers.bzl", "declare_deps_provider")
load("//scala_proto/private:toolchain_deps.bzl", "export_scalapb_toolchain_deps")
load(
"//scala_proto:scala_proto_toolchain.bzl",
"scala_proto_deps_toolchain",
"scala_proto_toolchain",
)

toolchain_type(
name = "toolchain_type",
Expand Down Expand Up @@ -80,11 +81,7 @@ declare_deps_provider(
name = "scalapb_worker_deps_provider",
deps_id = "scalapb_worker_deps",
visibility = ["//visibility:public"],
deps = [
"@com_google_protobuf//:protobuf_java",
"@scala_proto_rules_scalapb_compilerplugin",
"@scala_proto_rules_scalapb_protoc_bridge",
],
deps = DEFAULT_SCALAPB_WORKER_DEPS,
)

export_scalapb_toolchain_deps(
Expand Down
22 changes: 17 additions & 5 deletions scala_proto/default/default_deps.bzl
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
# These are the compile/runtime dependencies needed for scalapb compilation
# and grpc compile/runtime.
#
# In a complex environment you may want to update the toolchain to not refer to these anymore
# If you are using a resolver (like bazel-deps) that can export compile + runtime jar paths
# for you, then you should only need much shorter dependency lists. This needs to be the unrolled
# transitive path to be used without such a facility.
#
# In a complex environment you may want to update the toolchain to not refer to
# these anymore If you are using a resolver (like bazel-deps) that can export
# compile + runtime jar paths for you, then you should only need much shorter
# dependency lists. This needs to be the unrolled transitive path to be used
# without such a facility.

load("//scala:scala_cross_version_select.bzl", "select_for_scala_version")

DEFAULT_SCALAPB_COMPILE_DEPS = [
"//scala/private/toolchain_deps:scala_library_classpath",
"@com_google_protobuf//:protobuf_java",
Expand Down Expand Up @@ -41,3 +44,12 @@ DEFAULT_SCALAPB_GRPC_DEPS = [
"@scala_proto_rules_perfmark_api",
"@scala_proto_rules_scalapb_runtime_grpc",
]

DEFAULT_SCALAPB_WORKER_DEPS = [
"@com_google_protobuf//:protobuf_java",
"@scala_proto_rules_scalapb_compilerplugin",
"@scala_proto_rules_scalapb_protoc_bridge",
] + select_for_scala_version(
any_2_11 = [],
since_2_12 = ["@scala_proto_rules_scalapb_protoc_gen"],
)
29 changes: 21 additions & 8 deletions scala_proto/scala_proto_toolchain.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@io_bazel_rules_scala//scala:providers.bzl", "DepsInfo")
load("//scala:providers.bzl", "DepsInfo")

def _generators(ctx):
return dict(
Expand All @@ -7,9 +7,12 @@ def _generators(ctx):
)

def _generators_jars(ctx):
generator_deps = ctx.attr.extra_generator_dependencies + [
ctx.attr._main_generator_dep,
]
return depset(transitive = [
dep[JavaInfo].transitive_runtime_jars
for dep in ctx.attr.extra_generator_dependencies
for dep in generator_deps
])

def _generators_opts(ctx):
Expand Down Expand Up @@ -74,18 +77,20 @@ scala_proto_toolchain = rule(
"code_generator": attr.label(
executable = True,
cfg = "exec",
default = Label("@io_bazel_rules_scala//src/scala/scripts:scalapb_worker"),
default = Label("//src/scala/scripts:scalapb_worker"),
allow_files = True,
),
"main_generator": attr.string(default = "scalapb.ScalaPbCodeGenerator"),
"main_generator": attr.string(
default = "scripts.ScalaPbCodeGenerator",
),
"named_generators": attr.string_dict(),
"extra_generator_dependencies": attr.label_list(
providers = [JavaInfo],
),
"scalac": attr.label(
executable = True,
cfg = "exec",
default = Label("@io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac"),
default = Label("//src/java/io/bazel/rulesscala/scalac"),
allow_files = True,
),
"protoc": attr.label(
Expand All @@ -105,6 +110,14 @@ scala_proto_toolchain = rule(
[proto rules documentation](https://docs.bazel.build/versions/master/be/protocol-buffer.html#proto_library)
""",
),
"_main_generator_dep": attr.label(
default = Label(
"//src/scala/scripts:scalapb_codegenerator_wrapper",
),
allow_single_file = True,
executable = False,
cfg = "exec",
),
},
)

Expand All @@ -119,9 +132,9 @@ scala_proto_deps_toolchain = rule(
attrs = {
"dep_providers": attr.label_list(
default = [
"@io_bazel_rules_scala//scala_proto:scalapb_compile_deps_provider",
"@io_bazel_rules_scala//scala_proto:scalapb_grpc_deps_provider",
"@io_bazel_rules_scala//scala_proto:scalapb_worker_deps_provider",
Label("//scala_proto:scalapb_compile_deps_provider"),
Label("//scala_proto:scalapb_grpc_deps_provider"),
Label("//scala_proto:scalapb_worker_deps_provider"),
],
cfg = "target",
providers = [DepsInfo],
Expand Down
13 changes: 13 additions & 0 deletions src/scala/scripts/BUILD
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
load("//scala:scala.bzl", "scala_binary", "scala_library")
load("//scala:scala_cross_version_select.bzl", "select_for_scala_version")

scala_library(
name = "scrooge_worker_lib",
Expand All @@ -22,6 +23,18 @@ scala_binary(
],
)

scala_library(
name = "scalapb_codegenerator_wrapper",
srcs = select_for_scala_version(
any_2_11 = ["ScalaPbCodeGeneratorWrapper_2_11.scala"],
since_2_12 = ["ScalaPbCodeGeneratorWrapper.scala"],
),
visibility = ["//visibility:public"],
deps = [
"//scala_proto:scalapb_worker_deps",
],
)

scala_library(
name = "scalapb_worker_lib",
srcs = ["ScalaPBWorker.scala"],
Expand Down
18 changes: 18 additions & 0 deletions src/scala/scripts/ScalaPbCodeGeneratorWrapper.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package scripts

import protocgen.{CodeGenApp,CodeGenRequest,CodeGenResponse}

object ScalaPbCodeGenerator extends CodeGenApp {
def process(request: CodeGenRequest): CodeGenResponse = {

try {
scalapb.ScalaPbCodeGenerator.process(request)

} catch {
case e: Throwable =>
val stackStream = new java.io.ByteArrayOutputStream
e.printStackTrace(new java.io.PrintStream(stackStream))
CodeGenResponse.fail(stackStream.toString())
}
}
}
20 changes: 20 additions & 0 deletions src/scala/scripts/ScalaPbCodeGeneratorWrapper_2_11.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package scripts
import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse

object ScalaPbCodeGenerator extends protocbridge.ProtocCodeGenerator {
override def run(req: Array[Byte]): Array[Byte] = {

try {
scalapb.ScalaPbCodeGenerator.run(req)

} catch {
case e: Throwable =>
val b = CodeGeneratorResponse.newBuilder
val stackStream = new java.io.ByteArrayOutputStream

e.printStackTrace(new java.io.PrintStream(stackStream))
b.setError(stackStream.toString())
b.build.toByteArray
}
}
}
25 changes: 10 additions & 15 deletions test/proto/custom_generator/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
load("//scala_proto:scala_proto.bzl", "scala_proto_library")
load("//scala:scala.bzl", "scala_library", "scala_test")
load("//scala:providers.bzl", "declare_deps_provider")
load("//scala_proto:scala_proto_toolchain.bzl", "scala_proto_deps_toolchain", "scala_proto_toolchain")
load("//scala:scala.bzl", "scala_library", "scala_test")
load("//scala_proto/default:default_deps.bzl", "DEFAULT_SCALAPB_WORKER_DEPS")
load("//scala_proto:scala_proto.bzl", "scala_proto_library")
load(
"//scala_proto:scala_proto_toolchain.bzl",
"scala_proto_deps_toolchain",
"scala_proto_toolchain",
)

# This package contains manual tests for custom generators:
#
Expand Down Expand Up @@ -38,12 +43,7 @@ scala_library(
declare_deps_provider(
name = "scalapb_worker_deps_provider",
deps_id = "scalapb_worker_deps",
deps = [
":DummyGenerator",
"@com_google_protobuf//:protobuf_java",
"@scala_proto_rules_scalapb_compilerplugin",
"@scala_proto_rules_scalapb_protoc_bridge",
],
deps = [":DummyGenerator"] + DEFAULT_SCALAPB_WORKER_DEPS,
)

scala_proto_deps_toolchain(
Expand Down Expand Up @@ -83,12 +83,7 @@ scala_library(
declare_deps_provider(
name = "failing_scalapb_worker_deps_provider",
deps_id = "scalapb_worker_deps",
deps = [
":FailingGenerator",
"@com_google_protobuf//:protobuf_java",
"@scala_proto_rules_scalapb_compilerplugin",
"@scala_proto_rules_scalapb_protoc_bridge",
],
deps = [":FailingGenerator"] + DEFAULT_SCALAPB_WORKER_DEPS,
)

scala_proto_deps_toolchain(
Expand Down

0 comments on commit 6f702a6

Please sign in to comment.