Bump protoc-bridge to 0.9.8 for Scala >= 2.12

scala_proto/scala_proto_toolchain.bzl

@@ -80,6 +80,13 @@ scala_proto_toolchain = rule(
             default = Label("//src/scala/scripts:scalapb_worker"),
             allow_files = True,
         ),
+        # `scripts.ScalaPbCodeGenerator` and `_main_generator_dep` are currently
+        # necessary to support protoc-bridge < 0.9.8, specifically 0.7.14
+        # required by Scala 2.11. See #1647 and scalapb/ScalaPB#1771.
+        #
+        # If we drop 2.11 support, restore `scalapb.ScalaPbCodeGenerator` here,
+        # remove `_main_generator_dep`, and delete
+        # `//src/scala/scripts:scalapb_codegenerator_wrapper` and its files.
         "main_generator": attr.string(
             default = "scripts.ScalaPbCodeGenerator",
         ),

scripts/create_repository.py

@@ -34,12 +34,14 @@
 PROTOBUF_JAVA_VERSION = "4.29.0"
 JLINE_VERSION = '3.27.1'
 SCALAPB_VERSION = '0.11.17'
-PROTOC_BRIDGE_VERSION = '0.9.7'
+PROTOC_BRIDGE_VERSION = '0.9.8'
 GRPC_VERSION = '1.68.1'
 GRPC_COMMON_PROTOS_VERSION = '2.48.0'
 GRPC_LIBS = ['netty', 'protobuf', 'stub']
 GUAVA_VERSION = '33.3.1-jre'
 
+# This should include values corresponding to `MavenCoordinates.artifact_name`,
+# i.e., group:artifact after stripping any Scala version suffix from artifact.
 EXCLUDED_ARTIFACTS = set(["com.google.guava:listenablefuture"])
 
 THIS_FILE = Path(__file__)
@@ -73,15 +75,25 @@ def select_root_artifacts(scala_version, scala_major, is_scala_3) -> List[str]:
         v for v in ROOT_SCALA_VERSIONS if v.startswith('2.')
     )
     max_scala_2_major = '.'.join(max_scala_2_version.split('.')[:2])
+    minor_version = int(scala_version.split('.')[1])
 
     scala_2_version = scala_version
     scala_2_major = scala_major
-    scalatest_major = scala_major
+    scalapb_major = scala_2_major
 
     if is_scala_3:
         scala_2_version = max_scala_2_version
         scala_2_major = max_scala_2_major
-        scalatest_major = '3'
+        scala_major = '3'
+        scalapb_major = scala_2_major if minor_version < 3 else scala_major
+
+    # For some reason, com.thesamet.scalapb:compilerplugin_3:0.11.17 depends on
+    # com.thesamet.scalapb:protoc-gen_2.13:0.9.7. Trying to use
+    # com.thesamet.scalapb:protoc-gen_3:0.9.8 causes a crash, saying:
+    # `java.lang.NoClassDefFoundError: Could not initialize class
+    # scalapb.ScalaPbCodeGenerator$`, even though that class is definitely in
+    # that jar. So we stick with protoc-gen_2.13 for now.
+    protoc_bridge_major = scala_2_major
 
     scalafmt_version = SCALAFMT_VERSION
     scalapb_version = SCALAPB_VERSION
@@ -97,13 +109,13 @@ def select_root_artifacts(scala_version, scala_major, is_scala_3) -> List[str]:
             GRPC_COMMON_PROTOS_VERSION,
         f'com.google.guava:guava:{GUAVA_VERSION}',
         f'com.google.protobuf:protobuf-java:{PROTOBUF_JAVA_VERSION}',
-        f'com.thesamet.scalapb:compilerplugin_{scala_2_major}:' +
+        f'com.thesamet.scalapb:compilerplugin_{scalapb_major}:' +
             scalapb_version,
-        f'com.thesamet.scalapb:protoc-bridge_{scala_2_major}:' +
+        f'com.thesamet.scalapb:protoc-bridge_{protoc_bridge_major}:' +
             protoc_bridge_version,
-        f'com.thesamet.scalapb:scalapb-runtime_{scala_2_major}:' +
+        f'com.thesamet.scalapb:scalapb-runtime_{scalapb_major}:' +
             scalapb_version,
-        f'com.thesamet.scalapb:scalapb-runtime-grpc_{scala_2_major}:' +
+        f'com.thesamet.scalapb:scalapb-runtime-grpc_{scalapb_major}:' +
             scalapb_version,
         f'org.scala-lang.modules:scala-parser-combinators_{scala_2_major}:' +
             PARSER_COMBINATORS_VERSION,
@@ -112,11 +124,17 @@ def select_root_artifacts(scala_version, scala_major, is_scala_3) -> List[str]:
         f'org.scala-lang:scala-reflect:{scala_2_version}',
         f'org.scala-lang:scalap:{scala_2_version}',
         f'org.scalameta:scalafmt-core_{scala_2_major}:{scalafmt_version}',
-        f'org.scalatest:scalatest_{scalatest_major}:{SCALATEST_VERSION}',
+        f'org.scalatest:scalatest_{scala_major}:{SCALATEST_VERSION}',
         f'org.typelevel:kind-projector_{scala_2_version}:' +
             KIND_PROJECTOR_VERSION,
     ] + [f'io.grpc:grpc-{lib}:{GRPC_VERSION}' for lib in GRPC_LIBS]
 
+    if scala_major != '2.11':
+        root_artifacts.append(
+            f'com.thesamet.scalapb:protoc-gen_{protoc_bridge_major}:' +
+            protoc_bridge_version,
+        )
+
     if scala_version == max_scala_2_version or is_scala_3:
         # Since the Scala 2.13 compiler is included in Scala 3 deps.
         root_artifacts.append('org.jline:jline:' + JLINE_VERSION)
@@ -156,18 +174,62 @@ class MavenCoordinates:
     version: str
     coordinate: str
 
+    # The `artifact` with the Scala version suffix stripped
+    unversioned_artifact: str
+
+    # The Scala version suffix stripped from `unversioned_artifact`
+    scala_version: str
+
+    # Canonical name for comparing new and existing artifacts
+    artifact_name: str
+
     @staticmethod
-    def new(artifact) -> Self:
+    def new(coords) -> Self:
         """Creates a new MavenCoordinates from a Maven coordinate string."""
         # There are Maven artifacts that contain extra components like `:jar` in
         # their coordinates. However, the groupId and artifactId are always the
         # first two components, and the version is the last.
-        parts = artifact.split(':')
-        return MavenCoordinates(parts[0], parts[1], parts[-1], artifact)
-
-    def artifact_name(self):
-        """Returns the name to use as a hash key for existing artifacts."""
-        return f'{self.group}:{self.artifact}'
+        parts = coords.split(':')
+        group, artifact, vers = parts[0], parts[1], parts[-1]
+
+        # Remove any Scala version suffix from what will become the
+        # `artifact_name`. This is to avoid consecutive runs of the script
+        # flipping between the `_2.x` and `_3` versions of some artifacts.
+        #
+        # Specifically, there are ScalaPB root artifacts specified by this
+        # script that end in `_3` yet still transitively depend on artifacts
+        # ending in `_2.13`. However, some of these transitive dependencies are
+        # also specified as root artifacts ending in `_3`.
+        #
+        # Without trimming the version suffix, the script would see the `_3`
+        # root artifacts and the `_2.13` transitive dependency artifacts as
+        # entirely different. However, their computed repository labels would be
+        # the same, causing one version to replace the other on consecutive
+        # runs.
+        artifact_parts = artifact.rsplit('_', 1)
+        scala_version = ''
+
+        if len(artifact_parts) != 1:
+            version_suffix = artifact_parts[-1]
+
+            # "Why does `'2.13'.isdecimal()` return `False`, sir?"
+            # "Nobody knows."
+            # See: https://youtu.be/JYqfVE-fykk (couldn't resist!)
+            if version_suffix.split('.')[0].isdigit():
+                scala_version = version_suffix
+                del artifact_parts[-1]
+
+        unversioned_artifact = '_'.join(artifact_parts)
+        artifact_name = f'{group}:{unversioned_artifact}'
+        return MavenCoordinates(
+            group,
+            artifact,
+            vers,
+            coords,
+            unversioned_artifact,
+            scala_version,
+            artifact_name,
+        )
 
     def is_newer_than(self, other):
         """Determines if this artifact is newer than the other.
@@ -186,23 +248,28 @@ def is_newer_than(self, other):
             CreateRepositoryError if other doesn't match self.group and
                 self.artifact
         """
-        if (self.group != other.group) or (self.artifact != other.artifact):
+        if self.artifact_name != other.artifact_name:
             raise CreateRepositoryError(
                 f'Expected {self.group}:{self.artifact}, ' +
                 f'got {other.group}:{other.artifact}'
             )
+        return (
+            self.__compare_versions(other.scala_version, self.scala_version) or
+            self.__compare_versions(other.version, self.version)
+        )
 
-        lhs_parts = self.version.split(".")
-        rhs_parts = other.version.split(".")
+    def __compare_versions(self, lhs, rhs):
+        lhs_parts = lhs.split('.')
+        rhs_parts = rhs.split('.')
 
         for lhs_part, rhs_part in zip(lhs_parts, rhs_parts):
             if lhs_part == rhs_part:
                 continue
             if lhs_part.isdecimal() and rhs_part.isdecimal():
-                return int(rhs_part) < int(lhs_part)
-            return rhs_part < lhs_part
+                return int(lhs_part) < int(rhs_part)
+            return lhs_part < rhs_part
 
-        return len(rhs_parts) < len(lhs_parts)
+        return len(lhs_parts) < len(rhs_parts)
 
 
 @dataclass
@@ -232,8 +299,7 @@ def get_label(self, coordinates) -> str:
     def _get_label_impl(self, coordinates) -> str:
         group = coordinates.group
         group_label = self._labelize(group)
-        artifact = self._remove_scala_version_suffix(coordinates.artifact)
-        artifact_label = self._labelize(artifact)
+        artifact_label = self._labelize(coordinates.unversioned_artifact)
 
         if group in self._SCALA_LANG_GROUPS:
             return self._get_scala_lang_label(artifact_label, coordinates)
@@ -244,7 +310,7 @@ def _get_label_impl(self, coordinates) -> str:
         if group in self._SCALA_PROTO_RULES_GROUPS:
             return self._get_scala_proto_label(artifact_label, coordinates)
 
-        artifact_name = f'{group}:{artifact}'
+        artifact_name = coordinates.artifact_name
 
         if artifact_name in self._SPECIAL_CASE_ARTIFACT_LABELS:
             return self._SPECIAL_CASE_ARTIFACT_LABELS[artifact_name]
@@ -254,14 +320,6 @@ def _get_label_impl(self, coordinates) -> str:
     def _labelize(s):
         return s.replace('.', '_').replace('-', '_')
 
-    @staticmethod
-    def _remove_scala_version_suffix(artifact):
-        """Removes the Scala version suffix from artifact, e.g., scopt_2.13."""
-        parts = artifact.split('_')
-        if len(parts) != 1 and parts[-1][0].isdigit():
-            return '_'.join(parts[:-1])
-        return artifact
-
     _ARTIFACT_LABEL_ONLY_GROUPS = set([
         "com.google.guava",
         "com.twitter",
@@ -359,9 +417,9 @@ def resolve_artifacts(
 
         for artifact in artifacts_data['dependencies']:
             coords = MavenCoordinates.new(artifact['coord'])
-            current = current_artifacts_map.get(coords.artifact_name())
+            current = current_artifacts_map.get(coords.artifact_name)
 
-            if coords.artifact_name() in EXCLUDED_ARTIFACTS:
+            if coords.artifact_name in EXCLUDED_ARTIFACTS:
                 continue
 
             if current is None or coords.is_newer_than(current.coordinates):
@@ -393,7 +451,7 @@ def _create_current_artifacts_map(original_artifacts):
 
         for metadata in original_artifacts.values():
             coordinates = MavenCoordinates.new(metadata['artifact'])
-            name = coordinates.artifact_name()
+            name = coordinates.artifact_name
 
             if name not in result and metadata.get('testonly') is not True:
                 result[name] = ResolvedArtifact(
@@ -410,7 +468,7 @@ def _get_artifact_metadata(self, artifact) -> str:
                 MavenCoordinates.new(d) for d in artifact['directDependencies']
             ]
             metadata['deps'] = [
-                d for d in deps if d.artifact_name() not in EXCLUDED_ARTIFACTS
+                d for d in deps if d.artifact_name not in EXCLUDED_ARTIFACTS
             ]
             with open(artifact['file'], 'rb') as f:
                 metadata['checksum'] = hashlib.sha256(f.read()).hexdigest()
@@ -531,7 +589,7 @@ def _update_artifact_labels(artifacts, labeler):
         for existing_label, metadata in artifacts.items():
             coords = MavenCoordinates.new(metadata['artifact'])
 
-            if coords.artifact_name() in EXCLUDED_ARTIFACTS:
+            if coords.artifact_name in EXCLUDED_ARTIFACTS:
                 continue
 
             label = (

src/scala/scripts/BUILD

@@ -23,6 +23,13 @@ scala_binary(
     ],
 )
 
+# Used by `scala_proto_toolchain()` to support protoc-bridge < 0.9.8,
+# specifically 0.7.14 required by Scala 2.11. See #1647 and
+# scalapb/ScalaPB#1771.
+#
+# If we drop 2.11 support, delete this target and its files, and update
+# `scala_proto_toolchain()` (in `scala_proto/scala_proto_toolchain.bzl`) to use
+# `scalapb.ScalaPbCodeGenerator` as its `main_generator`.
 scala_library(
     name = "scalapb_codegenerator_wrapper",
     srcs = select_for_scala_version(

src/scala/scripts/ScalaPbCodeGeneratorWrapper.scala

@@ -3,15 +3,6 @@ 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())
-        }
-    }
+    def process(request: CodeGenRequest): CodeGenResponse =
+        scalapb.ScalaPbCodeGenerator.process(request)
 }

test/src/main/scala/scalarules/test/extra_protobuf_generator/ExtraProtobufGenerator.scala

@@ -43,7 +43,6 @@ class CustomProtobufGenerator(
 
 }
 
-
 object ExtraProtobufGenerator extends ProtocCodeGenerator {
    override def run(req: Array[Byte]): Array[Byte] = {
     val b = CodeGeneratorResponse.newBuilder
@@ -58,7 +57,12 @@ object ExtraProtobufGenerator extends ProtocCodeGenerator {
       case e: Throwable =>
         // Yes, we want to catch _all_ errors and send them back to the
         // requestor. Otherwise uncaught errors will cause the generator to
-        // die and the worker invoking it to hang.
+        // die and the worker invoking it to hang under protoc-bridge < 0.9.8.
+        // See #1647 and scalapb/ScalaPB#1771.
+        //
+        // Scala 2.11 is stuck at protoc-bridge 0.7.14. If/when we drop
+        // Scala 2.11 support, we can remove this `catch` block (and elide the
+        // `ProtobufAdapter` implementation and delete its files).
         val stackStream = new java.io.ByteArrayOutputStream
         e.printStackTrace(new java.io.PrintStream(stackStream))
         b.setError(stackStream.toString())

third_party/repositories/scala_2_12.bzl

@@ -815,16 +815,16 @@ artifacts = {
         ],
     },
     "scala_proto_rules_scalapb_protoc_bridge": {
-        "artifact": "com.thesamet.scalapb:protoc-bridge_2.12:0.9.7",
-        "sha256": "6d039a28d29253ac78aec0e3102f6423d269e65203c114a17f0d52a91d4876f4",
+        "artifact": "com.thesamet.scalapb:protoc-bridge_2.12:0.9.8",
+        "sha256": "4af997be5176753aa480ce40cbe9aab89ba659740a1ca6dae660afffb7bb343a",
         "deps": [
             "@dev_dirs_directories",
             "@io_bazel_rules_scala_scala_library",
         ],
     },
     "scala_proto_rules_scalapb_protoc_gen": {
-        "artifact": "com.thesamet.scalapb:protoc-gen_2.12:0.9.7",
-        "sha256": "81df11e24e52887515dff20eb4d1a050fd58e078200291c3c87fd04218abe53b",
+        "artifact": "com.thesamet.scalapb:protoc-gen_2.12:0.9.8",
+        "sha256": "65391bf190ac9cab45674dcd8063893e4bffd4f7289742cad145962b42928648",
         "deps": [
             "@io_bazel_rules_scala_scala_library",
             "@scala_proto_rules_scalapb_protoc_bridge",

third_party/repositories/scala_2_13.bzl

@@ -837,16 +837,16 @@ artifacts = {
         ],
     },
     "scala_proto_rules_scalapb_protoc_bridge": {
-        "artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.7",
-        "sha256": "403f0e7223c8fd052cff0fbf977f3696c387a696a3a12d7b031d95660c7552f5",
+        "artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.8",
+        "sha256": "0b3827da2cd9bca867d6963c2a821e7eaff41f5ac3babf671c4c00408bd14a9b",
         "deps": [
             "@dev_dirs_directories",
             "@io_bazel_rules_scala_scala_library",
         ],
     },
     "scala_proto_rules_scalapb_protoc_gen": {
-        "artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
-        "sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
+        "artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.8",
+        "sha256": "cf2b50721952cb4f10ca05a0ed36d7b01b88eb6505a9478556ee5a7af1a21775",
         "deps": [
             "@io_bazel_rules_scala_scala_library",
             "@scala_proto_rules_scalapb_protoc_bridge",

third_party/repositories/scala_3_1.bzl

@@ -887,16 +887,16 @@ artifacts = {
         ],
     },
     "scala_proto_rules_scalapb_protoc_bridge": {
-        "artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.7",
-        "sha256": "403f0e7223c8fd052cff0fbf977f3696c387a696a3a12d7b031d95660c7552f5",
+        "artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.8",
+        "sha256": "0b3827da2cd9bca867d6963c2a821e7eaff41f5ac3babf671c4c00408bd14a9b",
         "deps": [
             "@dev_dirs_directories",
             "@io_bazel_rules_scala_scala_library_2",
         ],
     },
     "scala_proto_rules_scalapb_protoc_gen": {
-        "artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
-        "sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
+        "artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.8",
+        "sha256": "cf2b50721952cb4f10ca05a0ed36d7b01b88eb6505a9478556ee5a7af1a21775",
         "deps": [
             "@io_bazel_rules_scala_scala_library_2",
             "@scala_proto_rules_scalapb_protoc_bridge",