From 6f702a6ac676014f48ace026a2d34e00fe215c42 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Sat, 9 Nov 2024 12:40:53 -0500 Subject: [PATCH] Wrap ScalaPbCodeGenerator, expose Scala 2.11 error 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.(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 ``` --- scala_proto/BUILD | 19 +++++------- scala_proto/default/default_deps.bzl | 22 ++++++++++---- scala_proto/scala_proto_toolchain.bzl | 29 ++++++++++++++----- src/scala/scripts/BUILD | 13 +++++++++ .../scripts/ScalaPbCodeGeneratorWrapper.scala | 18 ++++++++++++ .../ScalaPbCodeGeneratorWrapper_2_11.scala | 20 +++++++++++++ test/proto/custom_generator/BUILD.bazel | 25 +++++++--------- 7 files changed, 107 insertions(+), 39 deletions(-) create mode 100644 src/scala/scripts/ScalaPbCodeGeneratorWrapper.scala create mode 100644 src/scala/scripts/ScalaPbCodeGeneratorWrapper_2_11.scala diff --git a/scala_proto/BUILD b/scala_proto/BUILD index e924b1290..d97b07e76 100644 --- a/scala_proto/BUILD +++ b/scala_proto/BUILD @@ -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", @@ -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( diff --git a/scala_proto/default/default_deps.bzl b/scala_proto/default/default_deps.bzl index 2a6a95d29..db82f2869 100644 --- a/scala_proto/default/default_deps.bzl +++ b/scala_proto/default/default_deps.bzl @@ -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", @@ -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"], +) diff --git a/scala_proto/scala_proto_toolchain.bzl b/scala_proto/scala_proto_toolchain.bzl index b546ffbb0..40f198ea3 100644 --- a/scala_proto/scala_proto_toolchain.bzl +++ b/scala_proto/scala_proto_toolchain.bzl @@ -1,4 +1,4 @@ -load("@io_bazel_rules_scala//scala:providers.bzl", "DepsInfo") +load("//scala:providers.bzl", "DepsInfo") def _generators(ctx): return dict( @@ -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): @@ -74,10 +77,12 @@ 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], @@ -85,7 +90,7 @@ scala_proto_toolchain = rule( "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( @@ -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", + ), }, ) @@ -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], diff --git a/src/scala/scripts/BUILD b/src/scala/scripts/BUILD index cd1643579..0e813301a 100644 --- a/src/scala/scripts/BUILD +++ b/src/scala/scripts/BUILD @@ -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", @@ -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"], diff --git a/src/scala/scripts/ScalaPbCodeGeneratorWrapper.scala b/src/scala/scripts/ScalaPbCodeGeneratorWrapper.scala new file mode 100644 index 000000000..c133512b8 --- /dev/null +++ b/src/scala/scripts/ScalaPbCodeGeneratorWrapper.scala @@ -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()) + } + } +} diff --git a/src/scala/scripts/ScalaPbCodeGeneratorWrapper_2_11.scala b/src/scala/scripts/ScalaPbCodeGeneratorWrapper_2_11.scala new file mode 100644 index 000000000..1e4e2c8b1 --- /dev/null +++ b/src/scala/scripts/ScalaPbCodeGeneratorWrapper_2_11.scala @@ -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 + } + } +} diff --git a/test/proto/custom_generator/BUILD.bazel b/test/proto/custom_generator/BUILD.bazel index 213c1b557..0254afe4e 100644 --- a/test/proto/custom_generator/BUILD.bazel +++ b/test/proto/custom_generator/BUILD.bazel @@ -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: # @@ -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( @@ -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(